From c7ff21825ec2c480bba1f09f09d2d650b37de5b4 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 17 Apr 2022 22:17:24 +0700 Subject: [PATCH] [Php81] Skip multiple assign on __construct on ReadOnlyPropertyRector (#2088) * [Php81] Skip multiple assign on __construct on ReadOnlyPropertyRector * Fixed :tada: * final touch: check multi assign on last check * final touch: eol * final touch: stop traversal when count already = 2 * final touch: check only in __construct * [ci-review] Rector Rectify * [ci-review] Rector Rectify * really final touch: ensure parent ClassLike is equals with the property ClassLike * really final touch: ensure parent ClassLike is equals with the property ClassLike Co-authored-by: GitHub Action --- .../Fixture/skip_multiple_assign.php.inc | 21 +++++++ .../Property/ReadOnlyPropertyRector.php | 6 ++ .../PropertyFetchAssignManipulator.php | 55 ++++++++++++++++++- 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 rules-tests/Php81/Rector/Property/ReadOnlyPropertyRector/Fixture/skip_multiple_assign.php.inc diff --git a/rules-tests/Php81/Rector/Property/ReadOnlyPropertyRector/Fixture/skip_multiple_assign.php.inc b/rules-tests/Php81/Rector/Property/ReadOnlyPropertyRector/Fixture/skip_multiple_assign.php.inc new file mode 100644 index 00000000000..33136ccb98d --- /dev/null +++ b/rules-tests/Php81/Rector/Property/ReadOnlyPropertyRector/Fixture/skip_multiple_assign.php.inc @@ -0,0 +1,21 @@ +name = $name; + + if ($flag) { + $this->name = 'change'; + } + + if ($flag) { + $this->name = 'change again'; + } + } +} diff --git a/rules/Php81/Rector/Property/ReadOnlyPropertyRector.php b/rules/Php81/Rector/Property/ReadOnlyPropertyRector.php index 7eb4281e5b5..e10da090cab 100644 --- a/rules/Php81/Rector/Property/ReadOnlyPropertyRector.php +++ b/rules/Php81/Rector/Property/ReadOnlyPropertyRector.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr; use PhpParser\Node\Param; use PhpParser\Node\Stmt\Property; +use Rector\Core\NodeManipulator\PropertyFetchAssignManipulator; use Rector\Core\NodeManipulator\PropertyManipulator; use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\PhpVersionFeature; @@ -27,6 +28,7 @@ final class ReadOnlyPropertyRector extends AbstractRector implements MinPhpVersi { public function __construct( private readonly PropertyManipulator $propertyManipulator, + private readonly PropertyFetchAssignManipulator $propertyFetchAssignManipulator, private readonly VisibilityManipulator $visibilityManipulator, ) { } @@ -121,6 +123,10 @@ private function refactorProperty(Property $property): ?Property return null; } + if ($this->propertyFetchAssignManipulator->isAssignedMultipleTimesInConstructor($property)) { + return null; + } + $this->visibilityManipulator->makeReadonly($property); $attributeGroups = $property->attrGroups; diff --git a/src/NodeManipulator/PropertyFetchAssignManipulator.php b/src/NodeManipulator/PropertyFetchAssignManipulator.php index 291060408dc..12767c569d5 100644 --- a/src/NodeManipulator/PropertyFetchAssignManipulator.php +++ b/src/NodeManipulator/PropertyFetchAssignManipulator.php @@ -8,6 +8,13 @@ use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Stmt\ClassLike; +use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Property; +use PhpParser\NodeTraverser; +use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer; +use Rector\Core\PhpParser\Node\BetterNodeFinder; +use Rector\Core\ValueObject\MethodName; use Rector\NodeNameResolver\NodeNameResolver; use Symplify\Astral\NodeTraverser\SimpleCallableNodeTraverser; @@ -15,10 +22,56 @@ final class PropertyFetchAssignManipulator { public function __construct( private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser, - private readonly NodeNameResolver $nodeNameResolver + private readonly NodeNameResolver $nodeNameResolver, + private readonly BetterNodeFinder $betterNodeFinder, + private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer ) { } + public function isAssignedMultipleTimesInConstructor(Property $property): bool + { + $classLike = $this->betterNodeFinder->findParentType($property, ClassLike::class); + if (! $classLike instanceof ClassLike) { + return false; + } + + $classMethod = $classLike->getMethod(MethodName::CONSTRUCT); + if (! $classMethod instanceof ClassMethod) { + return false; + } + + $count = 0; + $propertyName = $this->nodeNameResolver->getName($property); + + $this->simpleCallableNodeTraverser->traverseNodesWithCallable( + (array) $classMethod->getStmts(), + function (Node $node) use ($propertyName, $classLike, &$count): ?int { + if (! $node instanceof Assign) { + return null; + } + + if (! $this->propertyFetchAnalyzer->isLocalPropertyFetchName($node->var, $propertyName)) { + return null; + } + + $parentClassLike = $this->betterNodeFinder->findParentType($node, ClassLike::class); + if ($parentClassLike !== $classLike) { + return null; + } + + ++$count; + + if ($count === 2) { + return NodeTraverser::STOP_TRAVERSAL; + } + + return null; + } + ); + + return $count === 2; + } + /** * @return string[] */