From a036740ed36d34a50e8a9ac6ff66a449c99cacb4 Mon Sep 17 00:00:00 2001 From: Jeroen Smit Date: Sun, 27 Oct 2019 21:37:33 +0100 Subject: [PATCH 1/2] Fixed removal of non expressions Fixed issue with property fetches --- ...eSetterOnlyPropertyAndMethodCallRector.php | 3 ++ .../deal_with_property_fetches.php.inc | 39 +++++++++++++++++++ .../node_removal_on_non_expression.php.inc | 26 +++++++++++++ ...terOnlyPropertyAndMethodCallRectorTest.php | 2 + .../Node/Commander/NodeRemovingCommander.php | 5 ++- 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/deal_with_property_fetches.php.inc create mode 100644 packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/node_removal_on_non_expression.php.inc diff --git a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php index 5be7dff3c562..edb3a416aacb 100644 --- a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php +++ b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php @@ -162,6 +162,9 @@ private function processClassStmts(Node $node, array $setterOnlyPropertiesAndMet if ($this->assignManipulator->isLocalPropertyAssign($node)) { /** @var Assign $node */ $propertyFetch = $node->var; + if ($propertyFetch instanceof Node\Expr\ArrayDimFetch) { + $propertyFetch = $propertyFetch->var; + } /** @var PropertyFetch $propertyFetch */ if ($this->isNames($propertyFetch->name, $propertyNames)) { $this->removeNode($node); diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/deal_with_property_fetches.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/deal_with_property_fetches.php.inc new file mode 100644 index 000000000000..08d91e5c2cdb --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/deal_with_property_fetches.php.inc @@ -0,0 +1,39 @@ +unused = $unused; + } + + public function foo() + { + $this->prop[] = 'foo'; + + } +} +?> +----- +prop[] = 'foo'; + + } +} +?> diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/node_removal_on_non_expression.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/node_removal_on_non_expression.php.inc new file mode 100644 index 000000000000..88c430ad7146 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/node_removal_on_non_expression.php.inc @@ -0,0 +1,26 @@ +foo = $foo; + } + +} +?> +----- + diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php index 7101fb874da3..0c31dc7f8f5c 100644 --- a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php @@ -26,6 +26,8 @@ public function provideDataForTest(): Iterator yield [__DIR__ . '/Fixture/keep_static_property.php.inc']; yield [__DIR__ . '/Fixture/keep_public_property.php.inc']; yield [__DIR__ . '/Fixture/keep_serializable_object.php.inc']; + yield [__DIR__ . '/Fixture/deal_with_property_fetches.php.inc']; + yield [__DIR__ . '/Fixture/node_removal_on_non_expression.php.inc']; } protected function getRectorClass(): string diff --git a/src/PhpParser/Node/Commander/NodeRemovingCommander.php b/src/PhpParser/Node/Commander/NodeRemovingCommander.php index c77fff19207e..21ea5ee82036 100644 --- a/src/PhpParser/Node/Commander/NodeRemovingCommander.php +++ b/src/PhpParser/Node/Commander/NodeRemovingCommander.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Expression; +use PhpParser\NodeDumper; use PhpParser\NodeTraverser; use Rector\Contract\PhpParser\Node\CommanderInterface; use Rector\Exception\ShouldNotHappenException; @@ -37,8 +38,8 @@ public function addNode(Node $node): void $this->ensureIsNotPartOfChainMethodCall($node); $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); - if (! $node instanceof Expression && $parentNode instanceof Expression) { - // only expressions can be removed + if (! $node instanceof Stmt && $parentNode instanceof Expression) { + // only stmts can be removed $node = $parentNode; } From 76f986543882684ffc4b9f3409078021542a513a Mon Sep 17 00:00:00 2001 From: Jeroen Smit Date: Sat, 2 Nov 2019 17:41:58 +0100 Subject: [PATCH 2/2] Run rector on self --- .../Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php index edb3a416aacb..f8ee20b4ade8 100644 --- a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php +++ b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php @@ -4,6 +4,7 @@ namespace Rector\DeadCode\Rector\Class_; +use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\MethodCall; @@ -162,7 +163,7 @@ private function processClassStmts(Node $node, array $setterOnlyPropertiesAndMet if ($this->assignManipulator->isLocalPropertyAssign($node)) { /** @var Assign $node */ $propertyFetch = $node->var; - if ($propertyFetch instanceof Node\Expr\ArrayDimFetch) { + if ($propertyFetch instanceof ArrayDimFetch) { $propertyFetch = $propertyFetch->var; } /** @var PropertyFetch $propertyFetch */