-
-
Notifications
You must be signed in to change notification settings - Fork 411
[Php82] Add ReadOnlyClassRector #2296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9621a2e
9aedda7
458e0ab
d695419
fd38da9
ea45914
e7ff154
ddaec2f
9d62247
c3bab83
8892d68
bb3e69b
ec62cff
d110c9d
f4cae77
2252bb1
c718d39
01f8f5d
857bf25
1dc02f0
1e57d14
f01e401
ff9935a
43f765a
2636f90
31c4464
e063a52
4c6db88
605a1be
a13123f
6ff7f94
322c13d
a9de1ae
5d36a73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
use Rector\Config\RectorConfig; | ||
use Rector\Core\ValueObject\PhpVersion; | ||
use Rector\Set\ValueObject\LevelSetList; | ||
use Rector\Set\ValueObject\SetList; | ||
|
||
return static function (RectorConfig $rectorConfig): void { | ||
$rectorConfig->sets([SetList::PHP_82, LevelSetList::UP_TO_PHP_81]); | ||
|
||
// parameter must be defined after import, to override imported param version | ||
$rectorConfig->phpVersion(PhpVersion::PHP_82); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
use Rector\Config\RectorConfig; | ||
use Rector\Php82\Rector\Class_\ReadOnlyClassRector; | ||
|
||
return static function (RectorConfig $rectorConfig): void { | ||
$rectorConfig->rule(ReadOnlyClassRector::class); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final class OnlyReadonlyProperty | ||
{ | ||
private readonly string $property; | ||
} | ||
|
||
?> | ||
----- | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final readonly class OnlyReadonlyProperty | ||
{ | ||
private string $property; | ||
} | ||
|
||
?> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final class OnlyReadonlyProperty2 | ||
{ | ||
public function __construct(private readonly string $property) | ||
{ | ||
} | ||
} | ||
|
||
?> | ||
----- | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final readonly class OnlyReadonlyProperty2 | ||
{ | ||
public function __construct(private string $property) | ||
{ | ||
} | ||
} | ||
|
||
?> | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
#[\AllowDynamicProperties] | ||
final class SkipAllowDynamic | ||
{ | ||
private readonly string $property; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
class SkipAnonymousClass | ||
{ | ||
public function run() | ||
{ | ||
new class { | ||
private readonly string $foo; | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final class SkipHasWritableProperty | ||
{ | ||
private string $property; | ||
} | ||
Comment on lines
+1
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That willl be skipped for writable+readonly, i will add more fixture There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final class SkipNoProperties | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final class SkipNoProperties2 | ||
{ | ||
public function __construct() | ||
{ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
class SkipNonFinalClass | ||
{ | ||
private readonly string $property; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?php | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\Fixture; | ||
|
||
final class SkipPropertyPromotionWritable | ||
{ | ||
public function __construct(private string $data) | ||
{ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector; | ||
|
||
use Iterator; | ||
use Rector\Testing\PHPUnit\AbstractRectorTestCase; | ||
use Symplify\SmartFileSystem\SmartFileInfo; | ||
|
||
final class ReadOnlyClassRectorTest extends AbstractRectorTestCase | ||
{ | ||
/** | ||
* @dataProvider provideData() | ||
*/ | ||
public function test(SmartFileInfo $fileInfo): void | ||
{ | ||
$this->doTestFileInfo($fileInfo); | ||
} | ||
|
||
/** | ||
* @return Iterator<SmartFileInfo> | ||
*/ | ||
public function provideData(): Iterator | ||
{ | ||
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); | ||
} | ||
|
||
public function provideConfigFilePath(): string | ||
{ | ||
return __DIR__ . '/config/configured_rule.php'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
use Rector\Config\RectorConfig; | ||
use Rector\Php82\Rector\Class_\ReadOnlyClassRector; | ||
|
||
return static function (RectorConfig $rectorConfig): void { | ||
$rectorConfig->rule(ReadOnlyClassRector::class); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Rector\Php82\Rector\Class_; | ||
|
||
use PhpParser\Node; | ||
use PhpParser\Node\Stmt\Class_; | ||
use PhpParser\Node\Stmt\ClassMethod; | ||
use PhpParser\Node\Stmt\Property; | ||
use Rector\Core\NodeAnalyzer\ClassAnalyzer; | ||
use Rector\Core\Rector\AbstractRector; | ||
use Rector\Core\ValueObject\MethodName; | ||
use Rector\Core\ValueObject\PhpVersionFeature; | ||
use Rector\Core\ValueObject\Visibility; | ||
use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer; | ||
use Rector\Privatization\NodeManipulator\VisibilityManipulator; | ||
use Rector\VersionBonding\Contract\MinPhpVersionInterface; | ||
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; | ||
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; | ||
|
||
/** | ||
* @changelog https://wiki.php.net/rfc/readonly_classes | ||
* | ||
* @see \Rector\Tests\Php82\Rector\Class_\ReadOnlyClassRector\ReadOnlyClassRectorTest | ||
*/ | ||
final class ReadOnlyClassRector extends AbstractRector implements MinPhpVersionInterface | ||
{ | ||
/** | ||
* @var string | ||
*/ | ||
private const ATTRIBUTE = 'AllowDynamicProperties'; | ||
|
||
public function __construct( | ||
private readonly ClassAnalyzer $classAnalyzer, | ||
private readonly VisibilityManipulator $visibilityManipulator, | ||
private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer | ||
) { | ||
} | ||
|
||
public function getRuleDefinition(): RuleDefinition | ||
{ | ||
return new RuleDefinition('Decorate read-only class with `readonly` attribute', [ | ||
new CodeSample( | ||
<<<'CODE_SAMPLE' | ||
final class SomeClass | ||
{ | ||
public function __construct( | ||
private readonly string $name | ||
) { | ||
} | ||
} | ||
CODE_SAMPLE | ||
|
||
, | ||
<<<'CODE_SAMPLE' | ||
final readonly class SomeClass | ||
{ | ||
public function __construct( | ||
private string $name | ||
) { | ||
} | ||
} | ||
CODE_SAMPLE | ||
), | ||
]); | ||
} | ||
|
||
/** | ||
* @return array<class-string<Node>> | ||
*/ | ||
public function getNodeTypes(): array | ||
{ | ||
return [Class_::class]; | ||
} | ||
|
||
/** | ||
* @param Class_ $node | ||
*/ | ||
public function refactor(Node $node): ?Node | ||
{ | ||
if ($this->shouldSkip($node)) { | ||
return null; | ||
} | ||
|
||
$this->visibilityManipulator->makeReadonly($node); | ||
|
||
$constructClassMethod = $node->getMethod(MethodName::CONSTRUCT); | ||
if ($constructClassMethod instanceof ClassMethod) { | ||
foreach ($constructClassMethod->getParams() as $param) { | ||
$this->visibilityManipulator->removeReadonly($param); | ||
} | ||
} | ||
|
||
foreach ($node->getProperties() as $property) { | ||
$this->visibilityManipulator->removeReadonly($property); | ||
} | ||
|
||
return $node; | ||
} | ||
|
||
public function provideMinPhpVersion(): int | ||
{ | ||
return PhpVersionFeature::READONLY_CLASS; | ||
} | ||
|
||
private function shouldSkip(Class_ $class): bool | ||
{ | ||
// need to have test fixture once feature added to nikic/PHP-Parser | ||
if ($this->visibilityManipulator->hasVisibility($class, Visibility::READONLY)) { | ||
return true; | ||
} | ||
|
||
if ($this->classAnalyzer->isAnonymousClass($class)) { | ||
return true; | ||
} | ||
|
||
if (! $class->isFinal()) { | ||
return true; | ||
} | ||
|
||
if ($this->phpAttributeAnalyzer->hasPhpAttribute($class, self::ATTRIBUTE)) { | ||
return true; | ||
} | ||
|
||
$properties = $class->getProperties(); | ||
if ($this->hasWritableProperty($properties)) { | ||
return true; | ||
} | ||
|
||
$constructClassMethod = $class->getMethod(MethodName::CONSTRUCT); | ||
if (! $constructClassMethod instanceof ClassMethod) { | ||
// no __construct means no property promotion, skip if class has no property defined | ||
return $properties === []; | ||
} | ||
|
||
$params = $constructClassMethod->getParams(); | ||
if ($params === []) { | ||
// no params means no property promotion, skip if class has no property defined | ||
return $properties === []; | ||
} | ||
|
||
foreach ($params as $param) { | ||
// has non-property promotion, skip | ||
if (! $this->visibilityManipulator->hasVisibility($param, Visibility::READONLY)) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* @param Property[] $properties | ||
*/ | ||
private function hasWritableProperty(array $properties): bool | ||
{ | ||
foreach ($properties as $property) { | ||
if (! $property->isReadonly()) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be one more fixture with one
readonly
property and one normal oneUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That willl be skipped, i will add more fixture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2304