diff --git a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php index eddf812f10d..6486d358327 100644 --- a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php +++ b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php @@ -97,7 +97,6 @@ PHP return null; } - if ($this->isSelfReferencing($node)) { return null; } @@ -160,7 +159,7 @@ PHP private function isSelfReferencing(Assign $assign): bool { - return (bool) $this->betterNodeFinder->findFirst($assign->expr, function ($subNode) use ($assign) : bool { + return (bool) $this->betterNodeFinder->findFirst($assign->expr, function ($subNode) use ($assign): bool { return $this->areNodesEqual($assign->var, $subNode); }); } diff --git a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php index e82b5b45ad0..d5f039bb152 100644 --- a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php +++ b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php @@ -13,6 +13,7 @@ use PhpParser\Node\Stmt\Property; use Rector\DeadCode\Analyzer\SetterOnlyMethodAnalyzer; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PhpParser\Node\Manipulator\AssignManipulator; +use Rector\PhpParser\Node\Manipulator\PropertyFetchManipulator; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; @@ -34,12 +35,19 @@ final class RemoveSetterOnlyPropertyAndMethodCallRector extends AbstractRector */ private $assignManipulator; + /** + * @var PropertyFetchManipulator + */ + private $propertyFetchManipulator; + public function __construct( SetterOnlyMethodAnalyzer $setterOnlyMethodAnalyzer, - AssignManipulator $assignManipulator + AssignManipulator $assignManipulator, + PropertyFetchManipulator $propertyFetchManipulator ) { $this->setterOnlyMethodAnalyzer = $setterOnlyMethodAnalyzer; $this->assignManipulator = $assignManipulator; + $this->propertyFetchManipulator = $propertyFetchManipulator; } public function getDefinition(): RectorDefinition @@ -179,7 +187,11 @@ PHP } /** @var Assign $node */ - $propertyFetch = $node->var; + $propertyFetch = $this->propertyFetchManipulator->matchPropertyFetch($node->var); + if ($propertyFetch === null) { + return; + } + /** @var PropertyFetch $propertyFetch */ if ($this->isNames($propertyFetch->name, $propertyNames)) { $this->removeNode($node); diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/remove_dim_fetch.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/remove_dim_fetch.php.inc new file mode 100644 index 00000000000..ba1e8d3d03a --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/remove_dim_fetch.php.inc @@ -0,0 +1,25 @@ +name[] = $name; + } +} + +?> +----- + diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/remove_multiple_dim_fetch.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/remove_multiple_dim_fetch.php.inc new file mode 100644 index 00000000000..b6b17b77b15 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/remove_multiple_dim_fetch.php.inc @@ -0,0 +1,25 @@ +name[][][][][][][] = $name; + } +} + +?> +----- + diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php index 7101fb874da..3d9b7b590fa 100644 --- a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php @@ -22,6 +22,10 @@ final class RemoveSetterOnlyPropertyAndMethodCallRectorTest extends AbstractRect { yield [__DIR__ . '/Fixture/fixture.php.inc']; yield [__DIR__ . '/Fixture/in_constructor.php.inc']; + + yield [__DIR__ . '/Fixture/remove_dim_fetch.php.inc']; + yield [__DIR__ . '/Fixture/remove_multiple_dim_fetch.php.inc']; + yield [__DIR__ . '/Fixture/keep_many_to_one.php.inc']; yield [__DIR__ . '/Fixture/keep_static_property.php.inc']; yield [__DIR__ . '/Fixture/keep_public_property.php.inc']; diff --git a/src/PhpParser/Node/Manipulator/AssignManipulator.php b/src/PhpParser/Node/Manipulator/AssignManipulator.php index 132ae6c2343..6e23b0016be 100644 --- a/src/PhpParser/Node/Manipulator/AssignManipulator.php +++ b/src/PhpParser/Node/Manipulator/AssignManipulator.php @@ -6,12 +6,9 @@ namespace Rector\PhpParser\Node\Manipulator; use PhpParser\Node; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\List_; -use PhpParser\Node\Expr\PropertyFetch; -use PhpParser\Node\Expr\StaticPropertyFetch; use Rector\PhpParser\Node\Resolver\NameResolver; final class AssignManipulator @@ -21,9 +18,15 @@ final class AssignManipulator */ private $nameResolver; - public function __construct(NameResolver $nameResolver) + /** + * @var PropertyFetchManipulator + */ + private $propertyFetchManipulator; + + public function __construct(NameResolver $nameResolver, PropertyFetchManipulator $propertyFetchManipulator) { $this->nameResolver = $nameResolver; + $this->propertyFetchManipulator = $propertyFetchManipulator; } /** @@ -37,9 +40,7 @@ final class AssignManipulator return false; } - $potentialPropertyFetch = $node->var instanceof ArrayDimFetch ? $node->var->var : $node->var; - - return $potentialPropertyFetch instanceof PropertyFetch || $potentialPropertyFetch instanceof StaticPropertyFetch; + return (bool) $this->propertyFetchManipulator->matchPropertyFetch($node->var); } /** @@ -53,7 +54,11 @@ final class AssignManipulator return false; } - $propertyFetch = $node->var instanceof ArrayDimFetch ? $node->var->var : $node->var; + /** @var Assign $node */ + $propertyFetch = $this->propertyFetchManipulator->matchPropertyFetch($node->var); + if ($propertyFetch === null) { + return false; + } return $this->nameResolver->isNames($propertyFetch, $propertyNames); } diff --git a/src/PhpParser/Node/Manipulator/ClassManipulator.php b/src/PhpParser/Node/Manipulator/ClassManipulator.php index 8b188a53ebe..c2a5552fd3e 100644 --- a/src/PhpParser/Node/Manipulator/ClassManipulator.php +++ b/src/PhpParser/Node/Manipulator/ClassManipulator.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Rector\PhpParser\Node\Manipulator; use PhpParser\Node; +use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\StaticCall; @@ -69,6 +70,11 @@ final class ClassManipulator */ private $betterStandardPrinter; + /** + * @var PropertyFetchManipulator + */ + private $propertyFetchManipulator; + public function __construct( NameResolver $nameResolver, NodeFactory $nodeFactory, @@ -76,7 +82,8 @@ final class ClassManipulator CallableNodeTraverser $callableNodeTraverser, NodeRemovingCommander $nodeRemovingCommander, DocBlockManipulator $docBlockManipulator, - BetterStandardPrinter $betterStandardPrinter + BetterStandardPrinter $betterStandardPrinter, + PropertyFetchManipulator $propertyFetchManipulator ) { $this->nodeFactory = $nodeFactory; $this->nameResolver = $nameResolver; @@ -85,6 +92,7 @@ final class ClassManipulator $this->nodeRemovingCommander = $nodeRemovingCommander; $this->docBlockManipulator = $docBlockManipulator; $this->betterStandardPrinter = $betterStandardPrinter; + $this->propertyFetchManipulator = $propertyFetchManipulator; } public function addConstructorDependency(Class_ $classNode, string $name, Type $type): void @@ -298,7 +306,6 @@ final class ClassManipulator /** @var string $methodName */ $methodName = $this->nameResolver->getName($method); - $publicMethodNames[] = $methodName; } @@ -317,15 +324,16 @@ final class ClassManipulator $this->callableNodeTraverser->traverseNodesWithCallable([$node], function (Node $node) use ( &$propertyNonAssignNames ): void { - if (! $node instanceof PropertyFetch && ! $node instanceof StaticPropertyFetch) { + $propertyFetch = $this->propertyFetchManipulator->matchPropertyFetch($node); + if ($propertyFetch === null) { return; } - if (! $this->isNonAssignPropertyFetch($node)) { + if (! $this->isNonAssignPropertyFetch($propertyFetch)) { return; } - $propertyNonAssignNames[] = $this->nameResolver->getName($node); + $propertyNonAssignNames[] = $this->nameResolver->getName($propertyFetch); }); // skip serializable properties, because they are probably used in serialization even though assign only @@ -473,11 +481,7 @@ final class ClassManipulator private function isNonAssignPropertyFetch(Node $node): bool { if ($node instanceof PropertyFetch) { - if (! $node->var instanceof Variable) { - return false; - } - - if (! $this->nameResolver->isName($node->var, 'this')) { + if (! $this->isLocalPropertyFetch($node)) { return false; } @@ -571,7 +575,32 @@ final class ClassManipulator private function isNodeLeftPartOfAssign(Node $node): bool { $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); + if ($parentNode instanceof Assign && $parentNode->var === $node) { + return true; + } - return $parentNode instanceof Assign && $parentNode->var === $node; + // traverse up to array dim fetches + if ($parentNode instanceof ArrayDimFetch) { + $previousParentNode = $parentNode; + while ($parentNode instanceof ArrayDimFetch) { + $previousParentNode = $parentNode; + $parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE); + } + + if ($parentNode instanceof Assign) { + return $parentNode->var === $previousParentNode; + } + } + + return false; + } + + private function isLocalPropertyFetch(PropertyFetch $propertyFetch): bool + { + if (! $propertyFetch->var instanceof Variable) { + return false; + } + + return $this->nameResolver->isName($propertyFetch->var, 'this'); } } diff --git a/src/PhpParser/Node/Manipulator/PropertyFetchManipulator.php b/src/PhpParser/Node/Manipulator/PropertyFetchManipulator.php index ac4c5e6fccb..b9759e4e52f 100644 --- a/src/PhpParser/Node/Manipulator/PropertyFetchManipulator.php +++ b/src/PhpParser/Node/Manipulator/PropertyFetchManipulator.php @@ -6,8 +6,10 @@ namespace Rector\PhpParser\Node\Manipulator; use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\StaticPropertyFetch; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Param; use PhpParser\Node\Stmt\Class_; @@ -60,13 +62,19 @@ final class PropertyFetchManipulator NodeTypeResolver $nodeTypeResolver, Broker $broker, NameResolver $nameResolver, - CallableNodeTraverser $callableNodeTraverser, - AssignManipulator $assignManipulator + CallableNodeTraverser $callableNodeTraverser ) { $this->nodeTypeResolver = $nodeTypeResolver; $this->broker = $broker; $this->nameResolver = $nameResolver; $this->callableNodeTraverser = $callableNodeTraverser; + } + + /** + * @required + */ + public function autowirePropertyFetchManipulator(AssignManipulator $assignManipulator): void + { $this->assignManipulator = $assignManipulator; } @@ -296,6 +304,33 @@ final class PropertyFetchManipulator return null; } + /** + * @param Node $node + * @return PropertyFetch|StaticPropertyFetch|null + */ + public function matchPropertyFetch(Node $node): ?Node + { + if ($node instanceof PropertyFetch) { + return $node; + } + + if ($node instanceof StaticPropertyFetch) { + return $node; + } + + if ($node instanceof ArrayDimFetch) { + $nestedNode = $node->var; + + while ($nestedNode instanceof ArrayDimFetch) { + $nestedNode = $nestedNode->var; + } + + return $this->matchPropertyFetch($nestedNode); + } + + return null; + } + private function hasPublicProperty(PropertyFetch $propertyFetch, string $propertyName): bool { $nodeScope = $propertyFetch->getAttribute(AttributeKey::SCOPE);