Skip to content

Commit fa18276

Browse files
committed
Cap FileProcessor fixed-point loop at 5 iterations
Guard the rule do-while loop so a non-idempotent rule cannot loop forever; stop after 5 passes and surface it (throw in tests, SystemError otherwise).
1 parent 1779451 commit fa18276

1 file changed

Lines changed: 37 additions & 0 deletions

File tree

‎src/Application/FileProcessor.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@
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+
3541
public function __construct(
3642
private BetterStandardPrinter $betterStandardPrinter,
3743
private RectorNodeTraverser $rectorNodeTraverser,
@@ -59,7 +65,16 @@ public function processFile(File $file, Configuration $configuration): FileProce
5965
$fileHasChanged = false;
6066
$filePath = $file->getFilePath();
6167

68+
$iteration = 0;
6269
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;
77+
6378
$file->changeHasChanged(false);
6479

6580
// 1. change nodes with Rector Rules
@@ -108,6 +123,28 @@ public function processFile(File $file, Configuration $configuration): FileProce
108123
return new FileProcessResult([], $file->getFileDiff(), $file->hasChanged());
109124
}
110125

126+
private function createNotConvergingResult(File $file, bool $fileHasChanged): FileProcessResult
127+
{
128+
$rectorClasses = [];
129+
foreach ($file->getRectorWithLineChanges() as $rectorWithLineChange) {
130+
$rectorClasses[$rectorWithLineChange->getRectorClass()] = true;
131+
}
132+
133+
$message = sprintf(
134+
'File did not stabilize after %d passes, a rule keeps changing it. Rules applied: %s',
135+
self::MAX_ITERATIONS,
136+
implode(', ', array_keys($rectorClasses))
137+
);
138+
139+
// in tests a non-converging file is a rule bug that must surface, not a silent partial result
140+
if (StaticPHPUnitEnvironment::isPHPUnitRun()) {
141+
throw new ShouldNotHappenException($message . ' in ' . $file->getFilePath());
142+
}
143+
144+
$systemError = new SystemError($message, $this->filePathHelper->relativePath($file->getFilePath()));
145+
return new FileProcessResult([$systemError], $file->getFileDiff(), $fileHasChanged);
146+
}
147+
111148
private function parseFileAndDecorateNodes(File $file): ?SystemError
112149
{
113150
try {

0 commit comments

Comments
 (0)