[perf] Skip format-preserving print for unchanged files

bench/skip_unchanged_file_print_bench.php

@@ -0,0 +1,92 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Benchmark for the "skip print on unchanged file" change in FileProcessor::processFile().
+ *
+ * Previously every file - including the majority that no rule touches - paid one full
+ * printFormatPreserving() pass just to detect "nothing changed". The change breaks out of
+ * the convergence loop before that print when the file is still pristine and the pass
+ * recorded no RectorWithLineChange.
+ *
+ * With a bare container (no rules registered) every file is unchanged, so this isolates
+ * exactly one skipped print per file as the only difference between the two branches.
+ *
+ * Run on both branches to compare:
+ *   php bench/skip_unchanged_file_print_bench.php
+ *
+ * Reports CPU time (user+sys); CPU is the reliable metric, wall is noisy under concurrent load.
+ */
+
+use Rector\Application\FileProcessor;
+use Rector\Application\Provider\CurrentFileProvider;
+use Rector\Configuration\ConfigurationFactory;
+use Rector\DependencyInjection\LazyContainerFactory;
+use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider;
+use Rector\ValueObject\Application\File;
+
+require __DIR__ . '/../vendor/autoload.php';
+
+const STATEMENTS = 300; // methods in the source file, sizes the print cost
+const REPEAT = 200;     // files processed
+
+$rectorConfig = (new LazyContainerFactory())->create();
+$rectorConfig->boot();
+
+$fileProcessor = $rectorConfig->make(FileProcessor::class);
+
+/** @var CurrentFileProvider $currentFileProvider */
+$currentFileProvider = $rectorConfig->make(CurrentFileProvider::class);
+
+/** @var DynamicSourceLocatorProvider $dynamicSourceLocatorProvider */
+$dynamicSourceLocatorProvider = $rectorConfig->make(DynamicSourceLocatorProvider::class);
+
+/** @var ConfigurationFactory $configurationFactory */
+$configurationFactory = $rectorConfig->make(ConfigurationFactory::class);
+
+// build a sizable, already-clean file; no rule is registered so it stays unchanged
+$source = "<?php\n\ndeclare(strict_types=1);\n\nnamespace Bench;\n\nclass SomeClass\n{\n";
+for ($i = 0; $i < STATEMENTS; ++$i) {
+    $source .= "    public function method{$i}(int \$value{$i}): int\n    {\n        return \$value{$i} + {$i};\n    }\n\n";
+}
+
+$source .= "}\n";
+
+$sourceFilePath = tempnam(sys_get_temp_dir(), 'bench_') . '.php';
+file_put_contents($sourceFilePath, $source);
+
+$dynamicSourceLocatorProvider->setFilePath($sourceFilePath);
+$configuration = $configurationFactory->createForTests([$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;
+};
+
+$run = static function () use ($fileProcessor, $currentFileProvider, $sourceFilePath, $configuration): void {
+    // File is single-use (hydrateStmtsAndTokens rejects a second hydrate), so make a fresh one
+    $file = new File($sourceFilePath, file_get_contents($sourceFilePath));
+    $currentFileProvider->setFile($file);
+    $fileProcessor->processFile($file, $configuration);
+};
+
+// warm up autoloading/reflection/scope caches once, excluded from timing
+$run();
+
+$wallStart = microtime(true);
+$cpuStart = $cpuSeconds();
+
+for ($repeat = 0; $repeat < REPEAT; ++$repeat) {
+    $run();
+}
+
+$cpu = $cpuSeconds() - $cpuStart;
+$wall = microtime(true) - $wallStart;
+
+@unlink($sourceFilePath);
+
+printf('statements: %d, files processed: %d%s', STATEMENTS, REPEAT, PHP_EOL);
+printf('CPU:  %.4f s  (%.3f ms/file)%s', $cpu, $cpu / REPEAT * 1_000, PHP_EOL);
+printf('wall: %.4f s%s', $wall, PHP_EOL);

rules/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector.php

@@ -101,9 +101,9 @@ public function refactor(Node $node): ?Node
         $hasParentConstructCall = false;
 
         foreach ($constructClassMethod->stmts as $key => $stmt) {
-            // code that is possibly breaking flow
+            // code that is possibly breaking flow; keep any inlining already done before it
             if ($stmt instanceof If_) {
-                return null;
+                break;
             }
 
             if (! $stmt instanceof Expression) {

rules/Removing/Rector/ClassMethod/ArgumentRemoverRector.php

@@ -115,9 +115,13 @@ private function processPosition(
     ): void {
         if ($argumentRemover->getValue() === null) {
             if ($node instanceof MethodCall || $node instanceof StaticCall) {
-                unset($node->args[$argumentRemover->getPosition()]);
-            } else {
+                if (isset($node->args[$argumentRemover->getPosition()])) {
+                    unset($node->args[$argumentRemover->getPosition()]);
+                    $this->hasChanged = true;
+                }
+            } elseif (isset($node->params[$argumentRemover->getPosition()])) {
                 unset($node->params[$argumentRemover->getPosition()]);
+                $this->hasChanged = true;
             }
 
             return;
@@ -149,6 +153,7 @@ private function removeByName(ClassMethod|StaticCall|MethodCall $node, int $posi
         if ($node instanceof MethodCall || $node instanceof StaticCall) {
             if (isset($node->args[$position]) && $this->isName($node->args[$position], $name)) {
                 unset($node->args[$position]);
+                $this->hasChanged = true;
             }
 
             return;
@@ -159,6 +164,7 @@ private function removeByName(ClassMethod|StaticCall|MethodCall $node, int $posi
         }
 
         unset($node->params[$position]);
+        $this->hasChanged = true;
     }
 
     /**

src/Application/FileProcessor.php

@@ -69,6 +69,9 @@ public function processFile(File $file, Configuration $configuration): FileProce
         do {
             $file->changeHasChanged(false);
 
+            // every rule and post rector records a line change when it mutates the AST
+            $changeCountBeforePass = count($file->getRectorWithLineChanges());
+
             // 1. change nodes with Rector Rules
             $newStmts = $this->rectorNodeTraverser->traverse($file->getNewStmts(), $file);
 
@@ -78,6 +81,14 @@ public function processFile(File $file, Configuration $configuration): FileProce
             // 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);