Skip to content

Commit 9211a83

Browse files
ondrejmirtesondrejmirtes
committed
Fix ignores of all line errors in AnalyserResultFinalizer
The identifiers now include the errors of CollectedDataNode rules reported on the ignored line, which are known only after FileAnalyser. Amp-Thread-ID: https://ampcode.com/threads/T-01a19bdb-a0f8-76c3-8540-9b8919996af4
1 parent 93c37ba commit 9211a83

7 files changed

Lines changed: 161 additions & 53 deletions

File tree

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 109 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Closure;
66
use PHPStan\AnalysedCodeException;
7+
use PHPStan\Analyser\Ignore\LineIgnoreFixer;
78
use PHPStan\Analyser\Ignore\UnmatchedIgnoreFixer;
89
use PHPStan\Analyser\ResultCache\LazyCollectedData;
910
use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
@@ -19,6 +20,7 @@
1920
use PHPStan\Rules\Registry as RuleRegistry;
2021
use Throwable;
2122
use function array_key_exists;
23+
use function array_map;
2224
use function array_merge;
2325
use function count;
2426
use function get_class;
@@ -47,6 +49,7 @@ public function __construct(
4749
private ScopeFactory $scopeFactory,
4850
private LocalIgnoresProcessor $localIgnoresProcessor,
4951
private UnmatchedIgnoreFixer $unmatchedIgnoreFixer,
52+
private LineIgnoreFixer $lineIgnoreFixer,
5053
#[AutowiredParameter]
5154
private bool $reportUnmatchedIgnoredErrors,
5255
private ValueDependencyCollector $valueDependencyCollector,
@@ -74,12 +77,12 @@ public function finalize(
7477
): FinalizerResult
7578
{
7679
if ($analyserResult->getLazyCollectedData()->isEmpty()) {
77-
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
80+
return $this->processInlineIgnores($this->mergeFilteredPhpErrors($analyserResult), [], []);
7881
}
7982

8083
$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
8184
if ($hasInternalErrors) {
82-
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
85+
return $this->processInlineIgnores($this->mergeFilteredPhpErrors($analyserResult), [], []);
8386
}
8487

8588
try {
@@ -89,7 +92,7 @@ public function finalize(
8992
throw $t;
9093
}
9194

92-
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($this->withUnreadableCollectedData($analyserResult, $t)), [], []);
95+
return $this->processInlineIgnores($this->mergeFilteredPhpErrors($this->withUnreadableCollectedData($analyserResult, $t)), [], []);
9396
}
9497

9598
$nodeType = CollectedDataNode::class;
@@ -230,7 +233,7 @@ public function finalize(
230233
$allUnmatchedLineIgnores[$file] = $localIgnoresProcessorResult->getUnmatchedLineIgnores();
231234
}
232235

233-
return $this->addUnmatchedIgnoredErrors(new AnalyserResult(
236+
return $this->processInlineIgnores(new AnalyserResult(
234237
unorderedErrors: array_merge($errors, $analyserResult->getFilteredPhpErrors()),
235238
filteredPhpErrors: [],
236239
allPhpErrors: $analyserResult->getAllPhpErrors(),
@@ -402,6 +405,83 @@ private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): Analyse
402405
);
403406
}
404407

408+
/**
409+
* Replaces each ignore of all errors on a line with an ignore of the identifiers of the errors reported there,
410+
* including the errors of CollectedDataNode rules.
411+
*
412+
* @param list<Error> $errors
413+
* @return list<Error>
414+
*/
415+
private function fixAllLineErrorsIgnores(array $errors): array
416+
{
417+
$allLineErrors = [];
418+
foreach ($errors as $error) {
419+
if ($error->getIdentifier() !== 'ignore.allLineErrors' || $error->getLine() === null) {
420+
continue;
421+
}
422+
423+
$allLineErrors[$error->getFile()][$error->getLine()] = true;
424+
}
425+
if ($allLineErrors === []) {
426+
return $errors;
427+
}
428+
429+
$identifiers = [];
430+
foreach ($errors as $error) {
431+
$line = $error->getLine();
432+
if ($line === null || !isset($allLineErrors[$error->getFile()][$line]) || !$error->canBeIgnored()) {
433+
continue;
434+
}
435+
436+
$identifier = $error->getIdentifier();
437+
if ($identifier === null) {
438+
// such an error cannot be ignored by an identifier
439+
$identifiers[$error->getFile()][$line] = false;
440+
} elseif (($identifiers[$error->getFile()][$line] ?? null) !== false) {
441+
$identifiers[$error->getFile()][$line][] = $identifier;
442+
}
443+
}
444+
445+
$fixes = [];
446+
foreach ($identifiers as $file => $lines) {
447+
if (!is_file($file)) {
448+
continue;
449+
}
450+
451+
$code = FileReader::read($file);
452+
foreach ($lines as $line => $lineIdentifiers) {
453+
if ($lineIdentifiers === false) {
454+
continue;
455+
}
456+
457+
$newCode = $this->lineIgnoreFixer->fix($code, $line, $lineIdentifiers);
458+
if ($newCode === null) {
459+
continue;
460+
}
461+
462+
$fixes[$file][$line] = $this->ruleErrorTransformer->createFixedErrorDiff($file, $code, $newCode);
463+
}
464+
}
465+
466+
return array_map(static function (Error $error) use ($fixes): Error {
467+
$line = $error->getLine();
468+
if ($line === null || $error->getIdentifier() !== 'ignore.allLineErrors' || !isset($fixes[$error->getFile()][$line])) {
469+
return $error;
470+
}
471+
472+
return new Error(
473+
$error->getMessage(),
474+
$error->getFile(),
475+
$line,
476+
false,
477+
$error->getFilePath(),
478+
tip: $error->getTip(),
479+
identifier: 'ignore.allLineErrors',
480+
fixedErrorDiffs: [$fixes[$error->getFile()][$line]],
481+
);
482+
}, $errors);
483+
}
484+
405485
/**
406486
* @return list<FixedErrorDiff>
407487
*/
@@ -420,23 +500,46 @@ private function fixUnmatchedIgnore(string $file, ?string $code, int $line, ?str
420500
}
421501

422502
/**
503+
* Fixes ignores of all errors on a line, and reports unmatched inline ignores. Runs after the errors of CollectedDataNode rules
504+
* are known, because they can be reported on the ignored lines too.
505+
*
423506
* @param list<Error> $collectorErrors
424507
* @param list<Error> $locallyIgnoredCollectorErrors
425508
* @param array<string, CollectedDataRuleResult> $collectedDataRuleResults
426509
*/
427-
private function addUnmatchedIgnoredErrors(
510+
private function processInlineIgnores(
428511
AnalyserResult $analyserResult,
429512
array $collectorErrors,
430513
array $locallyIgnoredCollectorErrors,
431514
array $collectedDataRuleResults = [],
432515
int $reusedRulesCount = 0,
433516
): FinalizerResult
434517
{
518+
$errors = $this->fixAllLineErrorsIgnores($analyserResult->getUnorderedErrors());
435519
if (!$this->reportUnmatchedIgnoredErrors) {
520+
$analyserResult = new AnalyserResult(
521+
unorderedErrors: $errors,
522+
filteredPhpErrors: $analyserResult->getFilteredPhpErrors(),
523+
allPhpErrors: $analyserResult->getAllPhpErrors(),
524+
locallyIgnoredErrors: $analyserResult->getLocallyIgnoredErrors(),
525+
linesToIgnore: $analyserResult->getLinesToIgnore(),
526+
unmatchedLineIgnores: $analyserResult->getUnmatchedLineIgnores(),
527+
internalErrors: $analyserResult->getInternalErrors(),
528+
collectedData: $analyserResult->getLazyCollectedData(),
529+
dependencies: $analyserResult->getDependencies(),
530+
usedTraitDependencies: $analyserResult->getUsedTraitDependencies(),
531+
valueDependencies: $analyserResult->getValueDependencies(),
532+
packageDependencies: $analyserResult->getPackageDependencies(),
533+
exportedNodes: $analyserResult->getExportedNodes(),
534+
reachedInternalErrorsCountLimit: $analyserResult->hasReachedInternalErrorsCountLimit(),
535+
peakMemoryUsageBytes: $analyserResult->getPeakMemoryUsageBytes(),
536+
processedFiles: $analyserResult->getProcessedFiles(),
537+
workerCount: $analyserResult->getWorkerCount(),
538+
);
539+
436540
return new FinalizerResult($analyserResult, $collectorErrors, $locallyIgnoredCollectorErrors, $collectedDataRuleResults, $reusedRulesCount);
437541
}
438542

439-
$errors = $analyserResult->getUnorderedErrors();
440543
foreach ($analyserResult->getUnmatchedLineIgnores() as $file => $data) {
441544
foreach ($data as $ignoredFile => $lines) {
442545
if ($ignoredFile !== $file) {

‎src/Analyser/FileAnalyser.php‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
use PhpParser\Node;
66
use PHPStan\AnalysedCodeException;
7-
use PHPStan\Analyser\Ignore\LineIgnoreFixer;
87
use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
98
use PHPStan\BetterReflection\Reflection\Exception\CircularReference;
109
use PHPStan\BetterReflection\Reflector\Exception\IdentifierNotFound;
@@ -15,7 +14,6 @@
1514
use PHPStan\DependencyInjection\AutowiredParameter;
1615
use PHPStan\DependencyInjection\AutowiredService;
1716
use PHPStan\DependencyInjection\ExtensionsCollection;
18-
use PHPStan\File\FileReader;
1917
use PHPStan\Node\FileNode;
2018
use PHPStan\Parser\Parser;
2119
use PHPStan\Parser\ParserErrorsException;
@@ -67,7 +65,6 @@ public function __construct(
6765
private ExtensionsCollection $ignoreErrorExtensions,
6866
private RuleErrorTransformer $ruleErrorTransformer,
6967
private LocalIgnoresProcessor $localIgnoresProcessor,
70-
private LineIgnoreFixer $lineIgnoreFixer,
7168
#[AutowiredParameter]
7269
private bool $reportIgnoresWithoutComments,
7370
private ValueDependencyCollector $valueDependencyCollector,
@@ -153,17 +150,15 @@ public function analyseFile(
153150
foreach ($linesToIgnore as $ignoredFile => $lines) {
154151
foreach ($lines as $line => $identifiers) {
155152
if ($identifiers === null) {
156-
$fileErrors[] = new Error(
153+
$fileErrors[] = (new Error(
157154
'Ignoring all errors on a line is not allowed.',
158155
$ignoredFile,
159156
$line,
160157
false,
161158
$ignoredFile,
162159
null,
163160
'Use @phpstan-ignore with an identifier and a comment (in parentheses).',
164-
identifier: 'ignore.allLineErrors',
165-
fixedErrorDiffs: $this->fixAllLineErrorsIgnore($ignoredFile, $line, $temporaryFileErrors),
166-
);
161+
))->withIdentifier('ignore.allLineErrors');
167162
unset($linesToIgnore[$ignoredFile][$line]);
168163
unset($unmatchedLineIgnores[$ignoredFile][$line]);
169164
continue;
@@ -272,40 +267,6 @@ public function analyseFile(
272267
);
273268
}
274269

275-
/**
276-
* Replaces the ignore of all errors on the line with an ignore of the identifiers of the errors reported there.
277-
*
278-
* @param list<Error> $temporaryFileErrors
279-
* @return list<FixedErrorDiff>
280-
*/
281-
private function fixAllLineErrorsIgnore(string $file, int $line, array $temporaryFileErrors): array
282-
{
283-
$identifiers = [];
284-
foreach ($temporaryFileErrors as $error) {
285-
if ($error->getFile() !== $file || $error->getLine() !== $line || !$error->canBeIgnored()) {
286-
continue;
287-
}
288-
289-
$identifier = $error->getIdentifier();
290-
if ($identifier === null) {
291-
return [];
292-
}
293-
294-
$identifiers[] = $identifier;
295-
}
296-
if ($identifiers === [] || !is_file($file)) {
297-
return [];
298-
}
299-
300-
$code = FileReader::read($file);
301-
$newCode = $this->lineIgnoreFixer->fix($code, $line, $identifiers);
302-
if ($newCode === null) {
303-
return [];
304-
}
305-
306-
return [$this->ruleErrorTransformer->createFixedErrorDiff($file, $code, $newCode)];
307-
}
308-
309270
/**
310271
* @param array<string, true> $analysedFiles
311272
*/

‎src/Testing/RuleTestCase.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ private function getAnalyser(DirectRuleRegistry $ruleRegistry): Analyser
129129
self::getContainer()->getExtensionsCollection(IgnoreErrorExtension::class),
130130
self::getContainer()->getByType(RuleErrorTransformer::class),
131131
new LocalIgnoresProcessor(),
132-
self::getContainer()->getByType(LineIgnoreFixer::class),
133132
false,
134133
self::getContainer()->getByType(ValueDependencyCollector::class),
135134
self::getContainer()->getByType(DependencyResolver::class),
@@ -387,6 +386,7 @@ private function gatherAnalyserErrorsWithDelayedErrors(array $files): array
387386
self::createScopeFactory($reflectionProvider, self::getContainer()->getService('typeSpecifier')),
388387
new LocalIgnoresProcessor(),
389388
self::getContainer()->getByType(UnmatchedIgnoreFixer::class),
389+
self::getContainer()->getByType(LineIgnoreFixer::class),
390390
true,
391391
self::getContainer()->getByType(ValueDependencyCollector::class),
392392
false,

‎tests/PHPStan/Analyser/AnalyserResultFinalizerTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PHPStan\Analyser;
44

55
use PhpParser\Node;
6+
use PHPStan\Analyser\Ignore\LineIgnoreFixer;
67
use PHPStan\Analyser\Ignore\UnmatchedIgnoreFixer;
78
use PHPStan\Analyser\ResultCache\LazyCollectedData;
89
use PHPStan\DependencyInjection\DirectExtensionsCollection;
@@ -91,6 +92,7 @@ private function createFinalizer(array $rules): AnalyserResultFinalizer
9192
),
9293
new LocalIgnoresProcessor(),
9394
self::getContainer()->getByType(UnmatchedIgnoreFixer::class),
95+
self::getContainer()->getByType(LineIgnoreFixer::class),
9496
false,
9597
self::getContainer()->getByType(ValueDependencyCollector::class),
9698
false,

0 commit comments

Comments
 (0)