Skip to content

Commit 316c3c5

Browse files
ondrejmirtesondrejmirtes
committed
Keep errors ordering even when nodeCallback executed out of order
1 parent 913945c commit 316c3c5

File tree

6 files changed

+45
-35
lines changed

6 files changed

+45
-35
lines changed

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,11 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
118118
$linesToIgnore = $allLinesToIgnore[$file] ?? [];
119119
$unmatchedLineIgnores = $allUnmatchedLineIgnores[$file] ?? [];
120120
$localIgnoresProcessorResult = $this->localIgnoresProcessor->process(
121-
[$tempCollectorError],
121+
[[$tempCollectorError, 0]],
122122
$linesToIgnore,
123123
$unmatchedLineIgnores,
124124
);
125-
foreach ($localIgnoresProcessorResult->getFileErrors() as $error) {
125+
foreach ($localIgnoresProcessorResult->getFileErrors() as [$error]) {
126126
$errors[] = $error;
127127
$collectorErrors[] = $error;
128128
}

‎src/Analyser/FileAnalyser.php‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use PHPStan\Parser\ParserErrorsException;
2222
use PHPStan\Rules\Registry as RuleRegistry;
2323
use function array_keys;
24+
use function array_merge;
2425
use function array_unique;
2526
use function array_values;
2627
use function count;
@@ -42,6 +43,7 @@
4243
use const E_USER_NOTICE;
4344
use const E_USER_WARNING;
4445
use const E_WARNING;
46+
use const SORT_NUMERIC;
4547

4648
/**
4749
* @phpstan-import-type CollectorData from CollectedData
@@ -83,7 +85,7 @@ public function analyseFile(
8385
?callable $outerNodeCallback,
8486
): FileAnalyserResult
8587
{
86-
/** @var list<Error> $fileErrors */
88+
/** @var array<int, list<Error>> $fileErrors */
8789
$fileErrors = [];
8890

8991
/** @var list<Error> $locallyIgnoredErrors */
@@ -97,13 +99,14 @@ public function analyseFile(
9799
$exportedNodes = [];
98100
$linesToIgnore = [];
99101
$unmatchedLineIgnores = [];
102+
$callbackInvocationNumber = 0;
100103
if (is_file($file)) {
101104
try {
102105
$this->collectErrors($analysedFiles);
103106
$parserNodes = $this->parser->parseFile($file);
104107
$linesToIgnore = $unmatchedLineIgnores = [$file => $this->getLinesToIgnoreFromTokens($parserNodes)];
105108
$temporaryFileErrors = [];
106-
$nodeCallback = function (Node $node, $scope) use (&$fileErrors, &$fileCollectedData, &$fileDependencies, &$usedTraitFileDependencies, &$exportedNodes, $file, $ruleRegistry, $collectorRegistry, $outerNodeCallback, $analysedFiles, &$linesToIgnore, &$unmatchedLineIgnores, &$temporaryFileErrors, $parserNodes): void {
109+
$nodeCallback = function (Node $node, $scope, int $callbackInvocationNumber) use (&$fileErrors, &$fileCollectedData, &$fileDependencies, &$usedTraitFileDependencies, &$exportedNodes, $file, $ruleRegistry, $collectorRegistry, $outerNodeCallback, $analysedFiles, &$linesToIgnore, &$unmatchedLineIgnores, &$temporaryFileErrors, $parserNodes): void {
107110
/** @var Scope&NodeCallbackInvoker $scope */
108111
if ($node instanceof Node\Stmt\Trait_) {
109112
foreach (array_keys($linesToIgnore[$file] ?? []) as $lineToIgnore) {
@@ -141,23 +144,23 @@ public function analyseFile(
141144
}
142145

143146
$uniquedAnalysedCodeExceptionMessages[$e->getMessage()] = true;
144-
$fileErrors[] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, tip: $e->getTip()))
147+
$fileErrors[$callbackInvocationNumber][] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, tip: $e->getTip()))
145148
->withIdentifier('phpstan.internal')
146149
->withMetadata([
147150
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
148151
InternalError::STACK_TRACE_AS_STRING_METADATA_KEY => $e->getTraceAsString(),
149152
]);
150153
continue;
151154
} catch (IdentifierNotFound $e) {
152-
$fileErrors[] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, tip: 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))
155+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, tip: 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))
153156
->withIdentifier('phpstan.reflection')
154157
->withMetadata([
155158
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
156159
InternalError::STACK_TRACE_AS_STRING_METADATA_KEY => $e->getTraceAsString(),
157160
]);
158161
continue;
159162
} catch (UnableToCompileNode | CircularReference $e) {
160-
$fileErrors[] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))
163+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))
161164
->withIdentifier('phpstan.reflection')
162165
->withMetadata([
163166
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
@@ -177,7 +180,7 @@ public function analyseFile(
177180
}
178181
}
179182

180-
$temporaryFileErrors[] = $error;
183+
$temporaryFileErrors[] = [$error, $callbackInvocationNumber];
181184
}
182185
}
183186

@@ -190,23 +193,23 @@ public function analyseFile(
190193
}
191194

192195
$uniquedAnalysedCodeExceptionMessages[$e->getMessage()] = true;
193-
$fileErrors[] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, tip: $e->getTip()))
196+
$fileErrors[$callbackInvocationNumber][] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, tip: $e->getTip()))
194197
->withIdentifier('phpstan.internal')
195198
->withMetadata([
196199
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
197200
InternalError::STACK_TRACE_AS_STRING_METADATA_KEY => $e->getTraceAsString(),
198201
]);
199202
continue;
200203
} catch (IdentifierNotFound $e) {
201-
$fileErrors[] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, tip: 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))
204+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, $node->getStartLine(), $e, tip: 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))
202205
->withIdentifier('phpstan.reflection')
203206
->withMetadata([
204207
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
205208
InternalError::STACK_TRACE_AS_STRING_METADATA_KEY => $e->getTraceAsString(),
206209
]);
207210
continue;
208211
} catch (UnableToCompileNode | CircularReference $e) {
209-
$fileErrors[] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))
212+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, $node->getStartLine(), $e))
210213
->withIdentifier('phpstan.reflection')
211214
->withMetadata([
212215
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
@@ -248,53 +251,58 @@ public function analyseFile(
248251
}
249252
};
250253

254+
$realNodeCallback = static function (Node $node, Scope $scope) use ($nodeCallback, &$callbackInvocationNumber): void {
255+
$callbackInvocationNumber++;
256+
$nodeCallback($node, $scope, $callbackInvocationNumber);
257+
};
258+
251259
if ($this->nodeScopeResolver instanceof NodeScopeResolver) {
252-
$scope = $this->scopeFactory->create(ScopeContext::create($file), $nodeCallback);
260+
$scope = $this->scopeFactory->create(ScopeContext::create($file), $realNodeCallback);
253261
} else {
254262
$scope = $this->generatorScopeFactory->create(ScopeContext::create($file));
255263
}
256-
$nodeCallback(new FileNode($parserNodes), $scope);
264+
$realNodeCallback(new FileNode($parserNodes), $scope);
257265
$this->nodeScopeResolver->processNodes(
258266
$parserNodes,
259267
$scope,
260-
$nodeCallback,
268+
$realNodeCallback,
261269
);
262270

263271
$localIgnoresProcessorResult = $this->localIgnoresProcessor->process(
264272
$temporaryFileErrors,
265273
$linesToIgnore,
266274
$unmatchedLineIgnores,
267275
);
268-
foreach ($localIgnoresProcessorResult->getFileErrors() as $fileError) {
269-
$fileErrors[] = $fileError;
276+
foreach ($localIgnoresProcessorResult->getFileErrors() as [$fileError, $order]) {
277+
$fileErrors[$order][] = $fileError;
270278
}
271279
foreach ($localIgnoresProcessorResult->getLocallyIgnoredErrors() as $locallyIgnoredError) {
272280
$locallyIgnoredErrors[] = $locallyIgnoredError;
273281
}
274282
$linesToIgnore = $localIgnoresProcessorResult->getLinesToIgnore();
275283
$unmatchedLineIgnores = $localIgnoresProcessorResult->getUnmatchedLineIgnores();
276284
} catch (\PhpParser\Error $e) {
277-
$fileErrors[] = (new Error($e->getRawMessage(), $file, $e->getStartLine() !== -1 ? $e->getStartLine() : null, $e))->withIdentifier('phpstan.parse');
285+
$fileErrors[$callbackInvocationNumber][] = (new Error($e->getRawMessage(), $file, $e->getStartLine() !== -1 ? $e->getStartLine() : null, $e))->withIdentifier('phpstan.parse');
278286
} catch (ParserErrorsException $e) {
279287
foreach ($e->getErrors() as $error) {
280-
$fileErrors[] = (new Error($error->getMessage(), $e->getParsedFile() ?? $file, $error->getLine() !== -1 ? $error->getStartLine() : null, $e))->withIdentifier('phpstan.parse');
288+
$fileErrors[$callbackInvocationNumber][] = (new Error($error->getMessage(), $e->getParsedFile() ?? $file, $error->getLine() !== -1 ? $error->getStartLine() : null, $e))->withIdentifier('phpstan.parse');
281289
}
282290
} catch (AnalysedCodeException $e) {
283-
$fileErrors[] = (new Error($e->getMessage(), $file, canBeIgnored: $e, tip: $e->getTip()))
291+
$fileErrors[$callbackInvocationNumber][] = (new Error($e->getMessage(), $file, canBeIgnored: $e, tip: $e->getTip()))
284292
->withIdentifier('phpstan.internal')
285293
->withMetadata([
286294
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
287295
InternalError::STACK_TRACE_AS_STRING_METADATA_KEY => $e->getTraceAsString(),
288296
]);
289297
} catch (IdentifierNotFound $e) {
290-
$fileErrors[] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, canBeIgnored: $e, tip: 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))
298+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('Reflection error: %s not found.', $e->getIdentifier()->getName()), $file, canBeIgnored: $e, tip: 'Learn more at https://phpstan.org/user-guide/discovering-symbols'))
291299
->withIdentifier('phpstan.reflection')
292300
->withMetadata([
293301
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
294302
InternalError::STACK_TRACE_AS_STRING_METADATA_KEY => $e->getTraceAsString(),
295303
]);
296304
} catch (UnableToCompileNode | CircularReference $e) {
297-
$fileErrors[] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, canBeIgnored: $e))
305+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('Reflection error: %s', $e->getMessage()), $file, canBeIgnored: $e))
298306
->withIdentifier('phpstan.reflection')
299307
->withMetadata([
300308
InternalError::STACK_TRACE_METADATA_KEY => InternalError::prepareTrace($e),
@@ -304,9 +312,9 @@ public function analyseFile(
304312
$this->restoreCollectErrorsHandler();
305313
}
306314
} elseif (is_dir($file)) {
307-
$fileErrors[] = (new Error(sprintf('File %s is a directory.', $file), $file, canBeIgnored: false))->withIdentifier('phpstan.path');
315+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('File %s is a directory.', $file), $file, canBeIgnored: false))->withIdentifier('phpstan.path');
308316
} else {
309-
$fileErrors[] = (new Error(sprintf('File %s does not exist.', $file), $file, canBeIgnored: false))->withIdentifier('phpstan.path');
317+
$fileErrors[$callbackInvocationNumber][] = (new Error(sprintf('File %s does not exist.', $file), $file, canBeIgnored: false))->withIdentifier('phpstan.path');
310318
}
311319

312320
foreach ($linesToIgnore as $fileKey => $lines) {
@@ -325,8 +333,10 @@ public function analyseFile(
325333
unset($unmatchedLineIgnores[$fileKey]);
326334
}
327335

336+
ksort($fileErrors, SORT_NUMERIC);
337+
328338
return new FileAnalyserResult(
329-
$fileErrors,
339+
array_merge(...$fileErrors),
330340
array_values($this->filteredPhpErrors),
331341
array_values($this->allPhpErrors),
332342
$locallyIgnoredErrors,

‎src/Analyser/LocalIgnoresProcessor.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ final class LocalIgnoresProcessor
1616
{
1717

1818
/**
19-
* @param list<Error> $temporaryFileErrors
19+
* @param list<array{Error, int}> $temporaryFileErrors
2020
* @param LinesToIgnore $linesToIgnore
2121
* @param LinesToIgnore $unmatchedLineIgnores
2222
*/
@@ -28,7 +28,7 @@ public function process(
2828
{
2929
$fileErrors = [];
3030
$locallyIgnoredErrors = [];
31-
foreach ($temporaryFileErrors as $tmpFileError) {
31+
foreach ($temporaryFileErrors as [$tmpFileError, $order]) {
3232
$line = $tmpFileError->getLine();
3333
if (
3434
$line !== null
@@ -44,7 +44,7 @@ public function process(
4444
}
4545

4646
if ($tmpFileError->getIdentifier() === null) {
47-
$fileErrors[] = $tmpFileError;
47+
$fileErrors[] = [$tmpFileError, $order];
4848
continue;
4949
}
5050

@@ -89,7 +89,7 @@ public function process(
8989
}
9090
}
9191

92-
$fileErrors[] = $tmpFileError;
92+
$fileErrors[] = [$tmpFileError, $order];
9393
}
9494

9595
return new LocalIgnoresProcessorResult(

‎src/Analyser/LocalIgnoresProcessorResult.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ final class LocalIgnoresProcessorResult
99
{
1010

1111
/**
12-
* @param list<Error> $fileErrors
12+
* @param list<array{Error, int}> $fileErrors
1313
* @param list<Error> $locallyIgnoredErrors
1414
* @param LinesToIgnore $linesToIgnore
1515
* @param LinesToIgnore $unmatchedLineIgnores
@@ -24,7 +24,7 @@ public function __construct(
2424
}
2525

2626
/**
27-
* @return list<Error>
27+
* @return list<array{Error, int}>
2828
*/
2929
public function getFileErrors(): array
3030
{

‎tests/PHPStan/Analyser/Generator/GeneratorNodeScopeResolverRuleTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,11 @@ static function (Node $node, Scope $scope) {
158158
];
159159
},
160160
[
161-
['exit through', 666],
162161
['Called on GeneratorNodeScopeResolverRule\\Foo, arg: 1', 21],
163162
['exit through', 666],
164163
['Virtual node invoked: \'foo\', 3', 23],
165164
['Called on GeneratorNodeScopeResolverRule\\Foo, arg: \'foo\'', 23],
165+
['exit through', 666],
166166
],
167167
];
168168
}

‎tests/PHPStan/Rules/NodeCallbackInvokerRuleTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ protected function getRule(): Rule
1818
public function testRule(): void
1919
{
2020
$this->analyse([__DIR__ . '/data/node-callback-invoker.php'], [
21-
[
22-
'found virtual echo',
23-
6,
24-
],
2521
[
2622
'found echo',
2723
5,
2824
],
25+
[
26+
'found virtual echo',
27+
6,
28+
],
2929
]);
3030
}
3131

0 commit comments

Comments
 (0)