From 13f5981a8d7703bfe6b7a56dd4ce985a35725cad Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 25 Oct 2026 19:28:35 +0100 Subject: [PATCH 1/3] perf: drop do-while reprint loop from FileProcessor, single pass --- src/Application/FileProcessor.php | 75 ++++++++++------------------ src/ValueObject/Application/File.php | 5 -- 2 files changed, 26 insertions(+), 54 deletions(-) diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 2d65ae111ad..5aa1c2ec9e4 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -56,65 +56,42 @@ public function processFile(File $file, Configuration $configuration): FileProce return new FileProcessResult([$parsingSystemError], null, false); } - $fileHasChanged = false; $filePath = $file->getFilePath(); - do { - $file->changeHasChanged(false); + // 1. change nodes with Rector Rules + $newStmts = $this->rectorNodeTraverser->traverse($file->getNewStmts(), $file); - // every rule and post rector records a line change when it mutates the AST - $changeCountBeforePass = count($file->getRectorWithLineChanges()); + // 2. apply post rectors + $postNewStmts = $this->postFileProcessor->traverse($newStmts, $file); - // 1. change nodes with Rector Rules - $newStmts = $this->rectorNodeTraverser->traverse($file->getNewStmts(), $file); + // 3. this is needed for new tokens added in "afterTraverse()" + $file->changeNewStmts($postNewStmts); - // 2. apply post rectors - $postNewStmts = $this->postFileProcessor->traverse($newStmts, $file); - - // 3. this is needed for new tokens added in "afterTraverse()" - $file->changeNewStmts($postNewStmts); - - // file never changed and this pass changed nothing: the print would just reproduce the - // original content, so skip the expensive format-preserving print entirely. - // once the file has changed we keep printing every pass, as a reprint can still - // stabilize the output (e.g. drop a leftover trailing comma) - if (! $fileHasChanged && count($file->getRectorWithLineChanges()) === $changeCountBeforePass) { - break; - } - - // 4. print to file or string - // important to detect if file has changed - $this->printFile($file, $configuration, $filePath); - - // no change in current iteration, stop - if (! $file->hasChanged()) { - break; - } + // no rule recorded a change: the print would just reproduce the original content, + // so skip the expensive format-preserving print entirely and cache the file + if ($file->getRectorWithLineChanges() === []) { + $this->changedFilesDetector->addCacheableFile($filePath); + return new FileProcessResult([], null, false); + } - $fileHasChanged = true; - } while (true); + // 4. print to file or string; this updates the file content and resolves + // whether the file really changed (line changes can still print identically) + $this->printFile($file, $configuration, $filePath); - // 5. add as cacheable if not changed at all - if (! $fileHasChanged) { + // content is identical despite reported line changes, so it is cacheable + if (! $file->hasChanged()) { $this->changedFilesDetector->addCacheableFile($filePath); - } else { - // when changed, set final status changed to true - // to ensure it make sense to verify in next process when needed - $file->changeHasChanged(true); } - $rectorWithLineChanges = $file->getRectorWithLineChanges(); - if ($file->hasChanged() || $rectorWithLineChanges !== []) { - $currentFileDiff = $this->fileDiffFactory->createFileDiffWithLineChanges( - $configuration->shouldShowDiffs(), - $file, - $file->getOriginalFileContent(), - $file->getFileContent(), - $file->getRectorWithLineChanges() - ); - - $file->setFileDiff($currentFileDiff); - } + $currentFileDiff = $this->fileDiffFactory->createFileDiffWithLineChanges( + $configuration->shouldShowDiffs(), + $file, + $file->getOriginalFileContent(), + $file->getFileContent(), + $file->getRectorWithLineChanges() + ); + + $file->setFileDiff($currentFileDiff); return new FileProcessResult([], $file->getFileDiff(), $file->hasChanged()); } diff --git a/src/ValueObject/Application/File.php b/src/ValueObject/Application/File.php index 5190e72963b..2ab3f9a53c6 100644 --- a/src/ValueObject/Application/File.php +++ b/src/ValueObject/Application/File.php @@ -87,11 +87,6 @@ public function hasChanged(): bool return $this->hasChanged; } - public function changeHasChanged(bool $status): void - { - $this->hasChanged = $status; - } - public function setFileDiff(FileDiff $fileDiff): void { $this->fileDiff = $fileDiff; From 3df8c0af1166238aaff90a22542a9c6ace1c496d Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 25 Oct 2026 23:00:06 +0100 Subject: [PATCH 2/3] 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 64ea8bcf6ef72de20314424a159aa87c71d02b09 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sun, 25 Oct 2026 22:03:10 +0000 Subject: [PATCH 3/3] [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;