[perf] Enforce single-pass rule application in tests, make rules idempotent

phpunit.xml

@@ -20,5 +20,7 @@
 
     <php>
         <ini name="memory_limit" value="-1"/>
+        <!-- fail a rule that does not reach its final result in a single pass; only rector-src enforces this -->
+        <env name="RECTOR_ENFORCE_SINGLE_PASS" value="1"/>
     </php>
 </phpunit>

rules/CodeQuality/Rector/BooleanNot/ReplaceConstantBooleanNotRector.php

@@ -69,14 +69,27 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        if ($this->valueResolver->isFalse($node->expr)) {
-            return new ConstFetch(new Name('true'));
+        // unwrap the whole negation chain at once, so nested cases like !!false resolve in a single pass
+        $negationCount = 0;
+        $innerExpr = $node;
+        while ($innerExpr instanceof BooleanNot) {
+            ++$negationCount;
+            $innerExpr = $innerExpr->expr;
         }
 
-        if ($this->valueResolver->isTrue($node->expr)) {
-            return new ConstFetch(new Name('false'));
+        if ($this->valueResolver->isTrue($innerExpr)) {
+            $value = true;
+        } elseif ($this->valueResolver->isFalse($innerExpr)) {
+            $value = false;
+        } else {
+            return null;
         }
 
-        return null;
+        // every negation flips the literal value
+        if ($negationCount % 2 === 1) {
+            $value = ! $value;
+        }
+
+        return new ConstFetch(new Name($value ? 'true' : 'false'));
     }
 }

rules/DeadCode/Rector/Block/ReplaceBlockToItsStmtsRector.php

@@ -60,6 +60,31 @@ public function refactor(Node $node): int|array
             return NodeVisitor::REMOVE_NODE;
         }
 
-        return $node->stmts;
+        $stmts = $this->flattenBlocks($node->stmts);
+        if ($stmts === []) {
+            return NodeVisitor::REMOVE_NODE;
+        }
+
+        return $stmts;
+    }
+
+    /**
+     * Flatten nested blocks and drop empty ones in the same pass, as replaced stmts are not re-traversed
+     * @param Stmt[] $stmts
+     * @return Stmt[]
+     */
+    private function flattenBlocks(array $stmts): array
+    {
+        $flattenedStmts = [];
+        foreach ($stmts as $stmt) {
+            if ($stmt instanceof Block) {
+                $flattenedStmts = [...$flattenedStmts, ...$this->flattenBlocks($stmt->stmts)];
+                continue;
+            }
+
+            $flattenedStmts[] = $stmt;
+        }
+
+        return $flattenedStmts;
     }
 }

rules/DeadCode/Rector/Plus/RemoveDeadZeroAndOneOperationRector.php

@@ -89,7 +89,24 @@ public function refactor(Node $node): ?Node
         }
 
         // -, +
-        return $this->processBinaryOp($node);
+        $result = $this->processBinaryOp($node);
+        if (!$result instanceof Expr) {
+            return null;
+        }
+
+        // the chosen operand can be a same-class operation that still carries a dead 0/1 at its own
+        // top level (e.g. 0 + 5 + 0 returns 0 + 5); such a returned node is skip-marked for this rule,
+        // so reduce its chain here to finish in a single pass
+        while ($result instanceof Plus || $result instanceof Minus || $result instanceof Mul || $result instanceof Div) {
+            $reduced = $this->processBinaryOp($result);
+            if (!$reduced instanceof Expr || $reduced === $result) {
+                break;
+            }
+
+            $result = $reduced;
+        }
+
+        return $result;
     }
 
     private function processAssignOp(AssignOp $assignOp): ?Expr

rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php

@@ -4,6 +4,7 @@
 
 namespace Rector\EarlyReturn\Rector\If_;
 
+use PhpParser\Node\Stmt;
 use PhpParser\Node;
 use PhpParser\Node\Expr\Exit_;
 use PhpParser\Node\Expr\Throw_;
@@ -73,6 +74,20 @@ public function getNodeTypes(): array
      * @return Node[]|null
      */
     public function refactor(Node $node): ?array
+    {
+        $nodesToReturn = $this->splitAlwaysElse($node);
+        if ($nodesToReturn === null) {
+            return null;
+        }
+
+        // the returned stmts are not re-traversed in this pass, so split nested always-else here too
+        return $this->splitNestedAlwaysElse($nodesToReturn);
+    }
+
+    /**
+     * @return Node[]|null
+     */
+    private function splitAlwaysElse(If_ $node): ?array
     {
         if ($this->doesNotLastStatementBreakFlow($node)) {
             return null;
@@ -92,6 +107,49 @@ public function refactor(Node $node): ?array
         return null;
     }
 
+    /**
+     * @param Node[] $stmts
+     * @return Node[]
+     */
+    private function splitNestedAlwaysElse(array $stmts): array
+    {
+        $splitStmts = [];
+        foreach ($stmts as $stmt) {
+            $this->splitAlwaysElseInSubStmts($stmt);
+
+            if ($stmt instanceof If_) {
+                $nestedNodesToReturn = $this->splitAlwaysElse($stmt);
+                if ($nestedNodesToReturn !== null) {
+                    $splitStmts = [...$splitStmts, ...$this->splitNestedAlwaysElse($nestedNodesToReturn)];
+                    continue;
+                }
+            }
+
+            $splitStmts[] = $stmt;
+        }
+
+        return $splitStmts;
+    }
+
+    private function splitAlwaysElseInSubStmts(Node $node): void
+    {
+        foreach ($node->getSubNodeNames() as $subNodeName) {
+            $subNode = $node->{$subNodeName};
+            if (! is_array($subNode) || $subNode === []) {
+                continue;
+            }
+
+            // only statement lists are split; expression/argument lists are left untouched
+            foreach ($subNode as $subNodeItem) {
+                if (! $subNodeItem instanceof Stmt) {
+                    continue 2;
+                }
+            }
+
+            $node->{$subNodeName} = $this->splitNestedAlwaysElse($subNode);
+        }
+    }
+
     /**
      * @return Node[]
      */

rules/Php56/Rector/FuncCall/PowToExpRector.php

@@ -5,6 +5,7 @@
 namespace Rector\Php56\Rector\FuncCall;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\BinaryOp\Pow;
 use PhpParser\Node\Expr\FuncCall;
 use Rector\NodeAnalyzer\PowOperandAnalyzer;
@@ -54,10 +55,24 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $firstExpr = $node->getArgs()[0]
-            ->value;
-        $secondExpr = $node->getArgs()[1]
-            ->value;
+        return $this->createPow($node);
+    }
+
+    public function provideMinPhpVersion(): int
+    {
+        return PhpVersionFeature::EXP_OPERATOR;
+    }
+
+    private function createPow(FuncCall $funcCall): ?Pow
+    {
+        $args = $funcCall->getArgs();
+        if (count($args) < 2) {
+            return null;
+        }
+
+        // convert nested pow() calls first, so pow(pow(...), ...) resolves in a single pass
+        $firstExpr = $this->createPowFromNested($args[0]->value);
+        $secondExpr = $this->createPowFromNested($args[1]->value);
 
         // ** binds tighter than most operators, so operands with lower precedence must be
         // wrapped in parentheses to keep the original semantics, e.g. pow(~3, 4) => (~3) ** 4
@@ -72,8 +87,15 @@ public function refactor(Node $node): ?Node
         return new Pow($firstExpr, $secondExpr);
     }
 
-    public function provideMinPhpVersion(): int
+    private function createPowFromNested(Expr $expr): Expr
     {
-        return PhpVersionFeature::EXP_OPERATOR;
+        if ($expr instanceof FuncCall && ! $expr->isFirstClassCallable() && $this->isName($expr, 'pow')) {
+            $pow = $this->createPow($expr);
+            if ($pow instanceof Pow) {
+                return $pow;
+            }
+        }
+
+        return $expr;
     }
 }

rules/Php70/Rector/Ternary/TernaryToNullCoalescingRector.php

@@ -171,6 +171,14 @@ private function processTernaryWithIsset(Ternary $ternary, Isset_ $isset): ?Coal
             return null;
         }
 
+        // a nested coalescable ternary fallback is converted in the same pass; chained ?? needs no parentheses
+        if ($ternary->else instanceof Ternary) {
+            $convertedElse = $this->refactor($ternary->else);
+            if ($convertedElse instanceof Expr) {
+                return $this->createCoalesce($ternary->if, $convertedElse);
+            }
+        }
+
         if (($ternary->else instanceof Ternary || $ternary->else instanceof BinaryOp) && $this->isTernaryParenthesized(
             $this->getFile(),
             $ternary->cond,

src/Application/FileProcessor.php

@@ -22,6 +22,7 @@
 use Rector\PhpParser\Parser\RectorParser;
 use Rector\PhpParser\Printer\BetterStandardPrinter;
 use Rector\PostRector\Application\PostFileProcessor;
+use Rector\PostRector\Contract\Rector\PostRectorInterface;
 use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment;
 use Rector\ValueObject\Application\File;
 use Rector\ValueObject\Configuration;
@@ -59,7 +60,9 @@ public function processFile(File $file, Configuration $configuration): FileProce
         $fileHasChanged = false;
         $filePath = $file->getFilePath();
 
+        $previousPassRectorClasses = [];
         do {
+            $ruleChangesBefore = count($file->getRectorWithLineChanges());
             $file->changeHasChanged(false);
 
             // 1. change nodes with Rector Rules
@@ -75,6 +78,17 @@ public function processFile(File $file, Configuration $configuration): FileProce
             // important to detect if file has changed
             $this->printFile($file, $configuration, $filePath);
 
+            // in rector-src tests, fail a rule that re-applies to its own previous-pass output;
+            // such a rule is not finished in a single pass and must handle the nested/chained case itself.
+            // gated by env so sibling packages pulling rector-src as dev-main are not affected
+            if (StaticPHPUnitEnvironment::isPHPUnitRun() && getenv('RECTOR_ENFORCE_SINGLE_PASS') === '1') {
+                $previousPassRectorClasses = $this->ensureRectorsAreIdempotent(
+                    $file,
+                    $ruleChangesBefore,
+                    $previousPassRectorClasses
+                );
+            }
+
             // no change in current iteration, stop
             if (! $file->hasChanged()) {
                 break;
@@ -108,6 +122,42 @@ public function processFile(File $file, Configuration $configuration): FileProce
         return new FileProcessResult([], $file->getFileDiff(), $file->hasChanged());
     }
 
+    /**
+     * @param array<string, true> $previousPassRectorClasses rule classes that changed the file in the previous pass
+     * @return array<string, true> rule classes that changed the file in the current pass
+     */
+    private function ensureRectorsAreIdempotent(File $file, int $ruleChangesBefore, array $previousPassRectorClasses): array
+    {
+        $currentPassRectorClasses = [];
+        $rectorWithLineChanges = $file->getRectorWithLineChanges();
+
+        foreach ($rectorWithLineChanges as $key => $rectorWithLineChange) {
+            if ($key < $ruleChangesBefore) {
+                continue;
+            }
+
+            $rectorClass = $rectorWithLineChange->getRectorClass();
+
+            // post rectors (imports) legitimately converge over several passes
+            if (is_a($rectorClass, PostRectorInterface::class, true)) {
+                continue;
+            }
+
+            $currentPassRectorClasses[$rectorClass] = true;
+
+            if (isset($previousPassRectorClasses[$rectorClass])) {
+                throw new ShouldNotHappenException(sprintf(
+                    'The "%s" rule changed "%s" again after it already changed it in the previous pass. ' .
+                    'A rule must reach its final result in a single pass; handle the nested or chained case inside refactor() instead of relying on the file being re-run.',
+                    $rectorClass,
+                    $file->getFilePath()
+                ));
+            }
+        }
+
+        return $currentPassRectorClasses;
+    }
+
     private function parseFileAndDecorateNodes(File $file): ?SystemError
     {
         try {