From 0307acf9dedb726776984de84b764971f37f85b1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 4 Dec 2025 23:57:08 +0100 Subject: [PATCH 1/3] avoid double checking node type --- composer.json | 4 ++-- .../AbstractImmutableNodeTraverser.php | 17 +++++++++++++++++ .../NodeTraverser/RectorNodeTraverser.php | 1 + src/Rector/AbstractRector.php | 16 ---------------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/composer.json b/composer.json index 8fca0284522..ce878e83d21 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "require": { "php": "^8.2", "clue/ndjson-react": "^1.3", - "composer/pcre": "^3.3.0", + "composer/pcre": "^3.3.2", "composer/semver": "^3.4", "composer/xdebug-handler": "^3.0.5", "doctrine/inflector": "^2.1", @@ -50,7 +50,7 @@ "phpstan/phpstan-phpunit": "^2.0", "phpstan/phpstan-webmozart-assert": "^2.0", "phpunit/phpunit": "^11.5", - "rector/jack": "^0.4.0", + "rector/jack": "^0.4", "rector/release-notes-generator": "^0.5", "rector/swiss-knife": "^2.3", "rector/type-perfect": "^2.1", diff --git a/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php b/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php index 8d9329bdbe6..e50b501ccf7 100644 --- a/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php @@ -105,9 +105,19 @@ protected function traverseNode(Node $node): void $return = $visitor->enterNode($subNode); if ($return !== null) { if ($return instanceof Node) { + $originalSubNodeClass = $subNode::class; + $this->ensureReplacementReasonable($subNode, $return); $subNode = $return; $node->{$name} = $return; + + if ($originalSubNodeClass !== $subNode::class) { + // stop traversing as node has changed its class and visitors do not work + $traverseChildren = false; + $this->stopTraversal = true; + break 2; + } + } elseif ($return === NodeVisitor::DONT_TRAVERSE_CHILDREN) { $traverseChildren = false; } elseif ($return === NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN) { @@ -181,8 +191,15 @@ protected function traverseArray(array $nodes): array $return = $visitor->enterNode($node); if ($return !== null) { if ($return instanceof Node) { + $originalNodeNodeClass = $node::class; $this->ensureReplacementReasonable($node, $return); $nodes[$i] = $node = $return; + + if ($originalNodeNodeClass !== $return::class) { + // stop traversing as node has changed its class and visitors do not work + $this->stopTraversal = true; + break 2; + } } elseif (\is_array($return)) { $doNodes[] = [$i, $return]; continue 2; diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index f80922a89bc..e221d7dba4b 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -67,6 +67,7 @@ public function getVisitorsForNode(Node $node): array if (! isset($this->visitorsPerNodeClass[$nodeClass])) { $this->visitorsPerNodeClass[$nodeClass] = []; + /** @var RectorInterface $visitor */ foreach ($this->visitors as $visitor) { foreach ($visitor->getNodeTypes() as $nodeType) { diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 44b38d41851..7e3c1c86c2e 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -132,10 +132,6 @@ public function beforeTraverse(array $nodes): ?array */ final public function enterNode(Node $node): int|Node|null { - if (! $this->isMatchingNodeType($node)) { - return null; - } - if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->file->containsHTML()) { return null; } @@ -317,16 +313,4 @@ private function refreshScopeNodes(array | Node $node, string $filePath, ?Mutati $this->changedNodeScopeRefresher->refresh($node, $filePath, $mutatingScope); } } - - private function isMatchingNodeType(Node $node): bool - { - $nodeClass = $node::class; - foreach ($this->getNodeTypes() as $nodeType) { - if (is_a($nodeClass, $nodeType, true)) { - return true; - } - } - - return false; - } } From 24821a6f7cf62a80107485bd0c137a7baef2d610 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 20 Dec 2025 12:24:45 +0100 Subject: [PATCH 2/3] init test --- .../AbstractImmutableNodeTraverser.php | 11 ++--- .../Class_/RuleChangingClassToTraitRector.php | 29 +++++++++++++ .../Class_/RuleCheckingClassRector.php | 33 +++++++++++++++ .../StopTraverseOnTypeChangeTest.php | 41 +++++++++++++++++++ 4 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php create mode 100644 tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleCheckingClassRector.php create mode 100644 tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/StopTraverseOnTypeChangeTest.php diff --git a/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php b/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php index e50b501ccf7..78f64369928 100644 --- a/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php @@ -112,10 +112,8 @@ protected function traverseNode(Node $node): void $node->{$name} = $return; if ($originalSubNodeClass !== $subNode::class) { - // stop traversing as node has changed its class and visitors do not work - $traverseChildren = false; - $this->stopTraversal = true; - break 2; + // stop traversing as node type changed and visitors won't work + return; } } elseif ($return === NodeVisitor::DONT_TRAVERSE_CHILDREN) { @@ -196,9 +194,8 @@ protected function traverseArray(array $nodes): array $nodes[$i] = $node = $return; if ($originalNodeNodeClass !== $return::class) { - // stop traversing as node has changed its class and visitors do not work - $this->stopTraversal = true; - break 2; + // stop traversing as node type changed and visitors won't work + return $nodes; } } elseif (\is_array($return)) { $doNodes[] = [$i, $return]; diff --git a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php new file mode 100644 index 00000000000..b49ad89112d --- /dev/null +++ b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php @@ -0,0 +1,29 @@ +rectorNodeTraverser = $this->make(RectorNodeTraverser::class); + + $this->rectorNodeTraverser->refreshPhpRectors([ + $this->make(RuleChangingClassToTraitRector::class), + $this->make(RuleCheckingClassRector::class) + ]); + + $currentFileProvider = $this->make(CurrentFileProvider::class); + + // @todo + // $currentFileProvider->setFile(); + } + + public function testGetVisitorsForNodeWhenNoVisitorsAvailable(): void + { + $class = new Class_('test'); + + $changedNodes = $this->rectorNodeTraverser->traverse([$class]); + $this->assertContainsOnlyInstancesOf(Trait_::class, $changedNodes); + } +} From 99998f1afb0a77ac32438c98f86213bf113328a6 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 20 Dec 2025 13:07:47 +0100 Subject: [PATCH 3/3] add simple test to allow node change type --- phpstan.neon | 5 ++++ .../AbstractImmutableNodeTraverser.php | 2 ++ .../NodeTraverser/RectorNodeTraverser.php | 1 + .../Class_/RuleChangingClassToTraitRector.php | 19 ++++++++----- .../Class_/RuleCheckingClassRector.php | 6 +++-- .../Fixture/SimpleClass.php | 9 +++++++ .../StopTraverseOnTypeChangeTest.php | 27 ++++++++++++------- 7 files changed, 52 insertions(+), 17 deletions(-) create mode 100644 tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Fixture/SimpleClass.php diff --git a/phpstan.neon b/phpstan.neon index 78c2471c135..cac98bc58f4 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -433,6 +433,11 @@ parameters: - rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php - rules/Php81/Enum/AttributeName.php + - + identifier: symplify.seeAnnotationToTest + paths: + - tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_ + # deprecated rule - '#Rule Rector\\Php81\\Rector\\Array_\\FirstClassCallableRector must implements Rector\\VersionBonding\\Contract\\MinPhpVersionInterface#' - '#Register "Rector\\Php81\\Rector\\Array_\\FirstClassCallableRector" service to "php81\.php" config set#' diff --git a/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php b/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php index 78f64369928..0015c3ba50a 100644 --- a/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php @@ -101,6 +101,7 @@ protected function traverseNode(Node $node): void $traverseChildren = true; $visitorIndex = -1; $currentNodeVisitors = $this->getVisitorsForNode($subNode); + foreach ($currentNodeVisitors as $visitorIndex => $visitor) { $return = $visitor->enterNode($subNode); if ($return !== null) { @@ -185,6 +186,7 @@ protected function traverseArray(array $nodes): array $traverseChildren = true; $visitorIndex = -1; $currentNodeVisitors = $this->getVisitorsForNode($node); + foreach ($currentNodeVisitors as $visitorIndex => $visitor) { $return = $visitor->enterNode($node); if ($return !== null) { diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index e221d7dba4b..b91a82bc225 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -95,6 +95,7 @@ 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); diff --git a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php index b49ad89112d..f85c4212ffa 100644 --- a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php +++ b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleChangingClassToTraitRector.php @@ -1,29 +1,36 @@ namespacedName = new Name('SomeNamespace\SomeTrait'); + + return $trait; } } diff --git a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleCheckingClassRector.php b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleCheckingClassRector.php index 83e209543f5..def96ad8c80 100644 --- a/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleCheckingClassRector.php +++ b/tests/PhpParser/NodeTraverser/StopTraverseOnTypeChange/Class_/RuleCheckingClassRector.php @@ -1,5 +1,7 @@ rectorNodeTraverser = $this->make(RectorNodeTraverser::class); $this->rectorNodeTraverser->refreshPhpRectors([ $this->make(RuleChangingClassToTraitRector::class), - $this->make(RuleCheckingClassRector::class) + $this->make(RuleCheckingClassRector::class), ]); - $currentFileProvider = $this->make(CurrentFileProvider::class); - - // @todo - // $currentFileProvider->setFile(); + $this->testingParser = $this->make(TestingParser::class); } public function testGetVisitorsForNodeWhenNoVisitorsAvailable(): void { - $class = new Class_('test'); + // must be cloned + Scope set to allow node replacement + $nodes = $this->testingParser->parseFileToDecoratedNodes(__DIR__ . '/Fixture/SimpleClass.php'); + + $changedNodes = $this->rectorNodeTraverser->traverse($nodes); + + $nodeFinder = new NodeFinder(); + $classes = $nodeFinder->findInstanceOf($changedNodes, Class_::class); + $this->assertCount(0, $classes); - $changedNodes = $this->rectorNodeTraverser->traverse([$class]); - $this->assertContainsOnlyInstancesOf(Trait_::class, $changedNodes); + $traits = $nodeFinder->findInstanceOf($changedNodes, Trait_::class); + $this->assertCount(1, $traits); } }