Skip to content

Commit 34e3372

Browse files
committed
Replace fixed-point iteration cap with content-cycle guard
Remove the hard MAX_ITERATIONS cap on the FileProcessor fixed-point loop. Legit rule chains (rule A enables rule B enables rule C) and post-rector import passes need several passes to converge, so a fixed numeric cap either trips on valid input or is an arbitrary guess. Instead detect a real non-convergence: hash each file content the loop produces and stop if one repeats, which only happens when rules flip the file between states and would otherwise loop forever.
1 parent 86311b5 commit 34e3372

1 file changed

Lines changed: 17 additions & 17 deletions

File tree

‎src/Application/FileProcessor.php‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@
3232

3333
final readonly class FileProcessor
3434
{
35-
/**
36-
* Safety cap for the fixed-point loop; a well-behaved rule set settles within a few passes,
37-
* so this only trips on a non-idempotent rule that keeps changing the same node.
38-
*/
39-
private const int MAX_ITERATIONS = 5;
40-
4135
public function __construct(
4236
private BetterStandardPrinter $betterStandardPrinter,
4337
private RectorNodeTraverser $rectorNodeTraverser,
@@ -65,16 +59,14 @@ public function processFile(File $file, Configuration $configuration): FileProce
6559
$fileHasChanged = false;
6660
$filePath = $file->getFilePath();
6761

68-
$iteration = 0;
69-
do {
70-
// safety cap: a non-idempotent rule can keep changing a node every pass, so the
71-
// fixed-point loop would never settle; stop and report instead of looping forever
72-
if ($iteration >= self::MAX_ITERATIONS) {
73-
return $this->createNotConvergingResult($file, $fileHasChanged);
74-
}
75-
76-
++$iteration;
62+
// infinite-loop guard: remember every file content the loop produces; a rule set that
63+
// keeps flipping the file between states would never settle, so once a content repeats
64+
// we stop and report instead of looping forever
65+
$seenContentHashes = [
66+
hash('xxh128', $file->getOriginalFileContent()) => true,
67+
];
7768

69+
do {
7870
$file->changeHasChanged(false);
7971

8072
// 1. change nodes with Rector Rules
@@ -96,6 +88,15 @@ public function processFile(File $file, Configuration $configuration): FileProce
9688
}
9789

9890
$fileHasChanged = true;
91+
92+
// this exact content was already produced in an earlier pass: the rules cycle
93+
// and will never converge, so stop here
94+
$contentHash = hash('xxh128', $file->getFileContent());
95+
if (isset($seenContentHashes[$contentHash])) {
96+
return $this->createNotConvergingResult($file, $fileHasChanged);
97+
}
98+
99+
$seenContentHashes[$contentHash] = true;
99100
} while (true);
100101

101102
// 5. add as cacheable if not changed at all
@@ -131,8 +132,7 @@ private function createNotConvergingResult(File $file, bool $fileHasChanged): Fi
131132
}
132133

133134
$message = sprintf(
134-
'File did not stabilize after %d passes, a rule keeps changing it. Rules applied: %s',
135-
self::MAX_ITERATIONS,
135+
'File did not stabilize, the rules keep flipping it between states. Rules applied: %s',
136136
implode(', ', array_keys($rectorClasses))
137137
);
138138

0 commit comments

Comments
 (0)