diff --git a/phpstan.neon b/phpstan.neon index 38f660b0e2e..c61f86ccad2 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -379,3 +379,5 @@ parameters: - message: '#Property Rector\\PhpParser\\NodeTraverser\\AbstractImmutableNodeTraverser\:\:\$visitors \(list\) does not accept array#' path: src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php + + - '#Method Rector\\Bridge\\PhpParser\\NodeClassFinder\:\:find\(\) should return array> but returns array#' diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodParameterRector/Fixture/remove_on_caller_via_self.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodParameterRector/Fixture/remove_on_caller_via_self.php new file mode 100644 index 00000000000..36c97246e28 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodParameterRector/Fixture/remove_on_caller_via_self.php @@ -0,0 +1,21 @@ + diff --git a/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php b/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php index 12d21ee35f6..e2b13ba9dc6 100644 --- a/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php +++ b/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php @@ -80,6 +80,10 @@ public function refactor(Node $node): ?Node continue; } + if ($key === 0) { + continue; + } + $previousStmt = $node->stmts[$key - 1] ?? null; if (! $previousStmt instanceof If_) { continue; diff --git a/src/Bridge/PhpParser/NodeClassFinder.php b/src/Bridge/PhpParser/NodeClassFinder.php new file mode 100644 index 00000000000..7e45c5ac208 --- /dev/null +++ b/src/Bridge/PhpParser/NodeClassFinder.php @@ -0,0 +1,49 @@ +> + */ + public static function find(): array + { + $robotLoader = new RobotLoader(); + $robotLoader->acceptFiles = ['*.php']; + + $phpParserNodeDirectory = __DIR__ . '/../../../vendor/nikic/php-parser/lib/PhpParser/Node/'; + $robotLoader->addDirectory($phpParserNodeDirectory); + + $robotLoader->setTempDirectory(sys_get_temp_dir() . '/node-classes'); + $robotLoader->refresh(); + + /** @var array $nodeClasses */ + $nodeClasses = array_keys($robotLoader->getIndexedClasses()); + + $instantiableNodeClasses = array_filter($nodeClasses, function (string $nodeClass): bool { + $reflectionClass = new ReflectionClass($nodeClass); + if ($reflectionClass->isAbstract()) { + return false; + } + + return ! $reflectionClass->isInterface(); + }); + + // this is needed, as PHPStan replaces the Class_ node of anonymous class completely + // @see https://github.com/phpstan/phpstan-src/blob/2.1.x/src/Parser/AnonymousClassVisitor.php + $specialPHPStanNodes = [AnonymousClassNode::class]; + + $specialRectorNodes = [FileWithoutNamespace::class]; + + return array_merge($instantiableNodeClasses, $specialPHPStanNodes, $specialRectorNodes); + } +} diff --git a/src/Configuration/ConfigurationRuleFilter.php b/src/Configuration/ConfigurationRuleFilter.php index 83e351ce575..352e5720777 100644 --- a/src/Configuration/ConfigurationRuleFilter.php +++ b/src/Configuration/ConfigurationRuleFilter.php @@ -31,7 +31,11 @@ public function filter(array $rectors): array $onlyRule = $this->configuration->getOnlyRule(); if ($onlyRule !== null) { - return $this->filterOnlyRule($rectors, $onlyRule); + foreach ($rectors as $rector) { + if ($rector instanceof $onlyRule) { + return [$rector]; + } + } } return $rectors; @@ -46,7 +50,7 @@ public function filterOnlyRule(array $rectors, string $onlyRule): array $activeRectors = []; foreach ($rectors as $rector) { if ($rector instanceof $onlyRule) { - $activeRectors[] = $rector; + return [$rector]; } } diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index f80922a89bc..7a11bd6bed7 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -4,6 +4,7 @@ namespace Rector\PhpParser\NodeTraverser; +use Rector\Bridge\PhpParser\NodeClassFinder; use PhpParser\Node; use PhpParser\Node\Stmt; use PhpParser\NodeVisitor; @@ -63,22 +64,12 @@ public function refreshPhpRectors(array $rectors): void */ public function getVisitorsForNode(Node $node): array { - $nodeClass = $node::class; - - if (! isset($this->visitorsPerNodeClass[$nodeClass])) { - $this->visitorsPerNodeClass[$nodeClass] = []; - /** @var RectorInterface $visitor */ - foreach ($this->visitors as $visitor) { - foreach ($visitor->getNodeTypes() as $nodeType) { - if (is_a($nodeClass, $nodeType, true)) { - $this->visitorsPerNodeClass[$nodeClass][] = $visitor; - continue 2; - } - } - } + // only in tests + if ($this->areNodeVisitorsPrepared === false) { + $this->prepareNodeVisitors(); } - return $this->visitorsPerNodeClass[$nodeClass]; + return $this->visitorsPerNodeClass[$node::class] ?? []; } /** @@ -94,9 +85,23 @@ private function prepareNodeVisitors(): void // filer out by version $this->visitors = $this->phpVersionedFilter->filter($this->rectors); + // filter by configuration $this->visitors = $this->configurationRuleFilter->filter($this->visitors); + // 1. get all node non-interface, non-abstract classes + // 2. iterate through them + /** @var RectorInterface $visitor */ + foreach ($this->visitors as $visitor) { + foreach (NodeClassFinder::find() as $nodeClass) { + foreach ($visitor->getNodeTypes() as $matchingNodeType) { + if (is_a($nodeClass, $matchingNodeType, true)) { + $this->visitorsPerNodeClass[$nodeClass][] = $visitor; + } + } + } + } + $this->areNodeVisitorsPrepared = true; } } diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 8ca3985f849..cebc53c28ee 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -191,7 +191,7 @@ final public function enterNode(Node $node): int|Node|null /** * Replacing nodes in leaveNode() method avoids infinite recursion - * see"infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown + * see "infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown */ final public function leaveNode(Node $node): array|int|Node|null { @@ -199,6 +199,11 @@ final public function leaveNode(Node $node): array|int|Node|null return null; } + // perf: hash node only if there is some object to be removed or replaced by array + if ($this->toBeRemovedNodeId === null && $this->nodesToReturn === []) { + return null; + } + $objectId = spl_object_id($node); if ($this->toBeRemovedNodeId === $objectId) { $this->toBeRemovedNodeId = null; diff --git a/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php b/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php index 25b2462dceb..c31a1cf245c 100644 --- a/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php +++ b/tests/PhpParser/NodeTraverser/RectorNodeTraverserTest.php @@ -39,57 +39,39 @@ protected function setUp(): void public function testGetVisitorsForNodeWhenNoVisitorsAvailable(): void { $class = new Class_('test'); + $visitorsForNode = $this->rectorNodeTraverser->getVisitorsForNode($class); - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertSame([], $visitors); + $this->assertSame([], $visitorsForNode); } public function testGetVisitorsForNodeWhenNoVisitorsMatch(): void { $class = new Class_('test'); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingFunctionRector); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); + $this->rectorNodeTraverser->refreshPhpRectors([$this->ruleUsingFunctionRector]); - $this->assertSame([], $visitors); + $visitorsForNode = $this->rectorNodeTraverser->getVisitorsForNode($class); + $this->assertSame([], $visitorsForNode); } public function testGetVisitorsForNodeWhenSomeVisitorsMatch(): void { $class = new Class_('test'); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingFunctionRector); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassRector); - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); + $this->rectorNodeTraverser->refreshPhpRectors([new RuleUsingFunctionRector(), new RuleUsingClassRector()]); - $this->assertEquals([$this->ruleUsingClassRector], $visitors); + $visitorsForNode = $this->rectorNodeTraverser->getVisitorsForNode($class); + $this->assertEquals([$this->ruleUsingClassRector], $visitorsForNode); } public function testGetVisitorsForNodeWhenAllVisitorsMatch(): void { $class = new Class_('test'); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassRector); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassLikeRector); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors); - } - - public function testGetVisitorsForNodeUsesCachedValue(): void - { - $class = new Class_('test'); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassRector); - $this->rectorNodeTraverser->addVisitor($this->ruleUsingClassLikeRector); - - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); - - $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors); - - $this->rectorNodeTraverser->removeVisitor($this->ruleUsingClassRector); - $visitors = $this->rectorNodeTraverser->getVisitorsForNode($class); + $this->rectorNodeTraverser->refreshPhpRectors([ + $this->ruleUsingClassRector, + $this->ruleUsingClassLikeRector, + ]); - $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitors); + $visitorsForNode = $this->rectorNodeTraverser->getVisitorsForNode($class); + $this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $visitorsForNode); } }