From 6e6fef6873236398f04d44e2ee4430e30c34e3ee Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 25 Oct 2026 23:00:06 +0100 Subject: [PATCH 1/2] fix: re-offer Class_ to rules after children traversed, single-pass convergence --- bench/rerun_class_rectors_bench.php | 91 +++++++++++++++++++ .../NodeTraverser/RectorNodeTraverser.php | 33 +++++++ 2 files changed, 124 insertions(+) create mode 100644 bench/rerun_class_rectors_bench.php diff --git a/bench/rerun_class_rectors_bench.php b/bench/rerun_class_rectors_bench.php new file mode 100644 index 00000000000..981e9f2d8ec --- /dev/null +++ b/bench/rerun_class_rectors_bench.php @@ -0,0 +1,91 @@ +create(); +$rectorConfig->boot(); + +/** @var RectorRegistry $rectorRegistry */ +$rectorRegistry = $rectorConfig->make(RectorRegistry::class); +$rectorRegistry->refreshRectors([$rectorConfig->make(RemoveEmptyClassMethodRector::class)]); + +/** @var RectorNodeTraverser $rectorNodeTraverser */ +$rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class); + +/** @var TestingParser $testingParser */ +$testingParser = $rectorConfig->make(TestingParser::class); + +// build a file of many classes, every method non-empty so no node is removed; +// the re-offer runs the rule once per class and returns null - pure added overhead +$source = "parseFileToDecoratedNodes($sourceFilePath); + +$cpuSeconds = static function (): float { + $usage = getrusage(); + return $usage['ru_utime.tv_sec'] + $usage['ru_utime.tv_usec'] / 1_000_000 + + $usage['ru_stime.tv_sec'] + $usage['ru_stime.tv_usec'] / 1_000_000; +}; + +// warm up caches once, excluded from timing +$rectorNodeTraverser->traverse($stmts, $file); + +$wallStart = microtime(true); +$cpuStart = $cpuSeconds(); + +for ($repeat = 0; $repeat < REPEAT; ++$repeat) { + $rectorNodeTraverser->traverse($stmts, $file); +} + +$cpu = $cpuSeconds() - $cpuStart; +$wall = microtime(true) - $wallStart; + +@unlink($sourceFilePath); + +printf('classes: %d, methods/class: %d, traversals: %d%s', CLASSES, METHODS, REPEAT, PHP_EOL); +printf('CPU: %.4f s (%.4f ms/traversal)%s', $cpu, $cpu / REPEAT * 1_000, PHP_EOL); +printf('wall: %.4f s%s', $wall, PHP_EOL); diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index f45faa2dcdf..f68906e4bc3 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -8,6 +8,7 @@ use PhpParser\Node; use PhpParser\Node\Expr; use PhpParser\Node\Stmt; +use PhpParser\Node\Stmt\Class_; use PhpParser\NodeVisitor; use Rector\Application\RectorRegistry; use Rector\Rector\RectorRunner; @@ -77,6 +78,11 @@ private function traverseNode(Node $node, File $file): void $this->traverseNode($subNode, $file); + // a child mutation (e.g. an emptied method body, or a rename that now duplicates) + // can make a Class_ rule newly apply; re-offer the class once children are settled. + // this substitutes for the former whole-tree do-while loop in FileProcessor + $node->{$name} = $subNode = $this->reRunClassRectors($subNode, $file); + if ($this->stopTraversal) { break; } @@ -126,6 +132,10 @@ private function traverseArray(array $nodes, File $file): array } $this->traverseNode($node, $file); + + // re-offer a Class_ after its children settle (see traverseNode) + $nodes[$i] = $node = $this->reRunClassRectors($node, $file); + if ($this->stopTraversal) { break; } @@ -140,6 +150,29 @@ private function traverseArray(array $nodes, File $file): array return $nodes; } + /** + * Re-run Class_ rules once, after its children are traversed, so a rule whose + * precondition is only met by a descendant removal still fires in a single pass. + */ + private function reRunClassRectors(Node $node, File $file): Node + { + if (! $node instanceof Class_) { + return $node; + } + + foreach ($this->rectorRegistry->forNodeAndFile($node, $file) as $currentNodeVisitor) { + $return = $this->rectorRunner->run($currentNodeVisitor, $node); + if (! $return instanceof Node) { + continue; + } + + $this->ensureReplacementReasonable($node, $return); + $node = $return; + } + + return $node; + } + private function ensureReplacementReasonable(Node $old, Node $new): void { if ($old instanceof Stmt) { From 8a4e786e05c9af8e21da8861584066515b602cf1 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sun, 25 Oct 2026 22:17:37 +0000 Subject: [PATCH 2/2] [ci-review] Rector Rectify --- src/PhpParser/NodeTraverser/RectorNodeTraverser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index f68906e4bc3..4e9ebfe18df 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -77,11 +77,11 @@ private function traverseNode(Node $node, File $file): void } $this->traverseNode($subNode, $file); - // a child mutation (e.g. an emptied method body, or a rename that now duplicates) // can make a Class_ rule newly apply; re-offer the class once children are settled. // this substitutes for the former whole-tree do-while loop in FileProcessor - $node->{$name} = $subNode = $this->reRunClassRectors($subNode, $file); + $node->{$name} = $this->reRunClassRectors($subNode, $file); + $subNode = $node->{$name}; if ($this->stopTraversal) { break;