diff --git a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php index 22076c9036f..6229134f03d 100644 --- a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php +++ b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php @@ -6,7 +6,7 @@ use PhpParser\Node; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\ClassLike; +use PhpParser\Node\Stmt\Interface_; use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\Trait_; use Rector\NodeTypeResolver\Node\AttributeKey; @@ -64,13 +64,13 @@ CODE_SAMPLE return null; } - /** @var ClassLike|null $classNode */ + /** @var Class_|Interface_|Trait_|null $classNode */ $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); - if ($classNode === null || $classNode instanceof Trait_) { + if ($classNode === null || $classNode instanceof Trait_ || $classNode instanceof Interface_) { return null; } - if ($classNode instanceof Class_ && $classNode->isAnonymous()) { + if ($classNode->isAnonymous()) { return null; } @@ -86,7 +86,6 @@ CODE_SAMPLE } $uselessAssigns = $this->resolveUselessAssignNode($propertyFetches); - if (count($uselessAssigns) > 0) { $this->removeNode($node); foreach ($uselessAssigns as $uselessAssign) { @@ -98,6 +97,9 @@ CODE_SAMPLE } /** + * Matches all-only: "$this->property = x" + * If these is ANY OTHER use of property, e.g. process($this->property), it returns [] + * * @param PropertyFetch[] $propertyFetches * @return Assign[] */ @@ -107,9 +109,11 @@ CODE_SAMPLE foreach ($propertyFetches as $propertyFetch) { $propertyFetchParentNode = $propertyFetch->getAttribute(AttributeKey::PARENT_NODE); + if ($propertyFetchParentNode instanceof Assign && $propertyFetchParentNode->var === $propertyFetch) { $uselessAssigns[] = $propertyFetchParentNode; } else { + // it is used another way as well → nothing to remove return []; } } diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_function.php.inc b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_function.php.inc index 43809d614d8..5bb7457db08 100644 --- a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_function.php.inc +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_function.php.inc @@ -2,10 +2,6 @@ namespace Rector\DeadCode\Tests\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture; -use PhpParser\Node; -use PhpParser\NodeVisitor; -use PhpParser\NodeVisitorAbstract; - class SkipAnonymousFunction { /** diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_nested_closure.php.inc b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_nested_closure.php.inc new file mode 100644 index 00000000000..3850c9a34cd --- /dev/null +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_nested_closure.php.inc @@ -0,0 +1,38 @@ +message = new stdClass(); + $this->mailer = new stdClass(); + } + + public function test() + { + $this->mailer->method() + ->with( + $this->anything(), + $this->callback( + function () { + $this->message + ->expects($this->once()) + ->method() + ->with($this->a(), $this->b()); + + $this->call(); + + return true; + } + ) + ); + } +} diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php index f2ca539b71b..a9ee020c9df 100644 --- a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php @@ -12,9 +12,10 @@ final class RemoveUnusedPrivatePropertyRectorTest extends AbstractRectorTestCase $this->doTestFiles([ __DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/property_assign.php.inc', + __DIR__ . '/Fixture/with_trait.php.inc', __DIR__ . '/Fixture/skip_anonymous_class.php.inc', __DIR__ . '/Fixture/skip_anonymous_function.php.inc', - __DIR__ . '/Fixture/with_trait.php.inc', + __DIR__ . '/Fixture/skip_nested_closure.php.inc', ]); } diff --git a/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/RemoveDeepChainMethodCallNodeVisitor.php b/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/RemoveDeepChainMethodCallNodeVisitor.php index 41a11fc35a1..5073bbfcbf2 100644 --- a/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/RemoveDeepChainMethodCallNodeVisitor.php +++ b/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/RemoveDeepChainMethodCallNodeVisitor.php @@ -16,9 +16,10 @@ use Rector\PhpParser\Node\BetterNodeFinder; final class RemoveDeepChainMethodCallNodeVisitor extends NodeVisitorAbstract { /** + * @warning might cause bugs with fluent interfaces like https://github.com/rectorphp/rector/issues/1646 * @var int */ - private const NESTED_CHAIN_METHOD_CALL_LIMIT = 10; + private const NESTED_CHAIN_METHOD_CALL_LIMIT = 15; /** * @var BetterNodeFinder diff --git a/rector.yaml b/rector.yaml index 399369c358b..5a4d0af1e24 100644 --- a/rector.yaml +++ b/rector.yaml @@ -17,4 +17,4 @@ parameters: php_version_features: '7.1' services: - Rector\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector: ~ +# Rector\CodingStyle\Rector\Namespace_\ImportFullyQualifiedNamesRector: ~ diff --git a/src/PhpParser/Node/Manipulator/PropertyManipulator.php b/src/PhpParser/Node/Manipulator/PropertyManipulator.php index 70b9337d942..5c32229abf5 100644 --- a/src/PhpParser/Node/Manipulator/PropertyManipulator.php +++ b/src/PhpParser/Node/Manipulator/PropertyManipulator.php @@ -63,13 +63,13 @@ final class PropertyManipulator $nodesToSearch[] = $classNode; return $this->betterNodeFinder->find($nodesToSearch, function (Node $node) use ($property) { - // itself - if ($this->betterStandardPrinter->areNodesEqual($node, $property)) { + // property + static fetch + if (! $node instanceof PropertyFetch && ! $node instanceof StaticPropertyFetch) { return null; } - // property + static fetch - if (! $node instanceof PropertyFetch && ! $node instanceof StaticPropertyFetch) { + // itself + if ($this->betterStandardPrinter->areNodesEqual($node, $property)) { return null; }