From d539b0ccf72086dc38daae5a8d06b1d547e109c3 Mon Sep 17 00:00:00 2001 From: Matthias Vogel Date: Wed, 11 Nov 2026 14:56:15 +0000 Subject: [PATCH 1/4] Remove ignores made stale by --fix-ignored for a list of files too For a list of files, unmatched ignoreErrors entries with a path are not reported, because errors from other files can still match them. So the later rounds of --fix-ignored never saw the baseline entry that matched a fixed ignored error, left it in place, and the next full analysis failed with ignore.unmatched. The rounds already get the files the earlier rounds changed. They are now passed to IgnoredErrorHelperResult::process(), which reports the unmatched entries for those files for a list of files too, so the next round removes them. Entries for other files are still not reported. --- .../fix-ignored-entries_test.sh | 21 +++++++++ .../Ignore/IgnoredErrorHelperResult.php | 9 +++- src/Command/AnalyseApplication.php | 4 +- src/Command/AnalyseCommand.php | 4 ++ tests/PHPStan/Analyser/AnalyserTest.php | 45 ++++++++++++++++++- 5 files changed, 80 insertions(+), 3 deletions(-) diff --git a/e2e/fix-ignored-entries/fix-ignored-entries_test.sh b/e2e/fix-ignored-entries/fix-ignored-entries_test.sh index 399409a3b5c..114b46faf49 100644 --- a/e2e/fix-ignored-entries/fix-ignored-entries_test.sh +++ b/e2e/fix-ignored-entries/fix-ignored-entries_test.sh @@ -30,3 +30,24 @@ function test_fix_ignored_entries() { assert_contains 'Untouched.php:?:Ignored error pattern #^Stale$#' "$OUTPUT" assert_not_contains 'src/In' "$OUTPUT" } + +function test_fix_ignored_entries_in_list_of_files() { + composer install + rm -rf src tmp && cp -r original src && cp baseline-original.php baseline.php && cp baseline-original.neon baseline.neon + # a list of files is a partial analysis, like a git hook that passes the changed files, + # the ignores in the files the first round changed are still fixed in the second round + OUTPUT=$(../../bin/phpstan analyse --fix-ignored src/InComment.php src/InNeonBaseline.php src/InPhpBaseline.php src/Untouched.php src/functions.php 2>&1) && CODE=0 || CODE=$? + assert_same "0" "$CODE" + echo "$OUTPUT" + assert_contains 'Round 1: fixed 5 errors in 3 files' "$OUTPUT" + assert_contains 'Round 2: fixed 3 errors in 3 files' "$OUTPUT" + assert_contains '[OK] 8 errors fixed in 5 files' "$OUTPUT" + for file in InComment.php InNeonBaseline.php InPhpBaseline.php; do diff -u expected/$file src/$file; done + diff -u expected/baseline.php baseline.php + diff -u expected/baseline.neon baseline.neon + OUTPUT=$(../../bin/phpstan analyse --error-format raw 2>&1) && CODE=0 || CODE=$? + assert_same "1" "$CODE" + echo "$OUTPUT" + assert_contains 'Untouched.php:?:Ignored error pattern #^Stale$#' "$OUTPUT" + assert_not_contains 'src/In' "$OUTPUT" +} diff --git a/src/Analyser/Ignore/IgnoredErrorHelperResult.php b/src/Analyser/Ignore/IgnoredErrorHelperResult.php index f1bed8885d9..8bc52a4e05c 100644 --- a/src/Analyser/Ignore/IgnoredErrorHelperResult.php +++ b/src/Analyser/Ignore/IgnoredErrorHelperResult.php @@ -9,6 +9,7 @@ use function array_fill_keys; use function array_key_exists; use function array_keys; +use function array_map; use function count; use function is_array; use function is_string; @@ -91,12 +92,14 @@ private static function filterIgnoreErrorsByIdentifier(array $ignoreErrors, ?str /** * @param list $errors * @param string[] $analysedFiles + * @param string[] $filesChangedByFix Files changed by the previous rounds of --fix, their unmatched ignores are reported even for a list of files. */ public function process( array $errors, bool $onlyFiles, array $analysedFiles, bool $hasInternalErrors, + array $filesChangedByFix = [], ): IgnoredErrorHelperProcessedResult { $unmatchedIgnoredErrors = $this->ignoreErrors; @@ -355,6 +358,7 @@ public function process( } $analysedFilesKeys = array_fill_keys($analysedFiles, true); + $filesChangedByFixKeys = array_fill_keys(array_map(fn (string $file): string => $this->fileHelper->normalizePath($file), $filesChangedByFix), true); if (!$hasInternalErrors) { foreach ($unmatchedIgnoredErrors as $i => $unmatchedIgnoredError) { @@ -396,7 +400,10 @@ public function process( continue; } - if ($onlyFiles) { + // an earlier round of --fix can leave an ignore in a file it changed unmatched, like the one + // that matched an error fixed by --fix-ignored, so it is reported for a list of files too + // and the next round removes it + if ($onlyFiles && !array_key_exists($unmatchedIgnoredError['realPath'], $filesChangedByFixKeys)) { continue; } diff --git a/src/Command/AnalyseApplication.php b/src/Command/AnalyseApplication.php index d9c223b153d..78546f855d5 100644 --- a/src/Command/AnalyseApplication.php +++ b/src/Command/AnalyseApplication.php @@ -53,6 +53,7 @@ public function __construct( /** * @param string[] $files * @param mixed[]|null $projectConfigArray + * @param string[] $filesChangedByFix * @throws InceptionNotSuccessfulException */ public function analyse( @@ -67,6 +68,7 @@ public function analyse( ?string $tmpFile, ?string $insteadOfFile, InputInterface $input, + array $filesChangedByFix = [], ): AnalysisResult { $isResultCacheUsed = false; @@ -167,7 +169,7 @@ public function analyse( $workerCount = $analyserResult->getWorkerCount(); $isResultCacheUsed = !$resultCache->isFullAnalysis(); - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors, $filesChangedByFix); $fileSpecificErrors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors(); $notFileSpecificErrors = $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages(); foreach ($ignoredErrorHelperProcessedResult->getIgnoredErrors() as [$ignoredError]) { diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index 114757add92..7b2a5e4d758 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -397,6 +397,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int $inceptionResult->getErrorOutput()->writeLineFormatted('Analysing files...'); } + /** @var string[] $filesChangedByFix */ + $filesChangedByFix = $input->getOption(self::OPTION_FIXED_IGNORED_IN); + try { $analysisResult = $application->analyse( $files, @@ -410,6 +413,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $inceptionResult->getEditorModeTmpFile(), $inceptionResult->getEditorModeInsteadOfFile(), $input, + $filesChangedByFix, ); } catch (Throwable $t) { if ($debug) { diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 3a800ea97a0..77b385ab1b3 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -657,6 +657,41 @@ public function testDoNotReportUnmatchedIgnoredErrorsFromPathWithCountIfPathWasN $this->assertNoErrors($result); } + /** + * @return iterable, bool}> + */ + public static function dataReportUnmatchedIgnoredErrorFromPathInFileChangedByFix(): iterable + { + yield 'list of files' => [true, [], false]; + yield 'list of files, file changed by an earlier fix round' => [true, [__DIR__ . '/data/empty/empty.php'], true]; + yield 'list of files, another file changed by an earlier fix round' => [true, [__DIR__ . '/data/two-fails.php'], false]; + yield 'full analysis' => [false, [], true]; + } + + /** + * @param list $filesChangedByFix + */ + #[DataProvider('dataReportUnmatchedIgnoredErrorFromPathInFileChangedByFix')] + public function testReportUnmatchedIgnoredErrorFromPathInFileChangedByFix(bool $onlyFiles, array $filesChangedByFix, bool $expectReported): void + { + $ignoreErrors = [ + [ + 'message' => '#Fail\.#', + 'path' => __DIR__ . '/data/empty/empty.php', + ], + ]; + $result = $this->runAnalyser($ignoreErrors, true, __DIR__ . '/data/empty/empty.php', $onlyFiles, filesChangedByFix: $filesChangedByFix); + if (!$expectReported) { + $this->assertNoErrors($result); + return; + } + + $this->assertCount(1, $result); + $this->assertInstanceOf(Error::class, $result[0]); + $this->assertSame('Ignored error pattern #Fail\.# in path ' . __DIR__ . '/data/empty/empty.php was not matched in reported errors.', $result[0]->getMessage()); + $this->assertSame('ignore.unmatched', $result[0]->getIdentifier()); + } + public function testIgnoreNextLine(): void { $result = $this->runAnalyser([], false, [ @@ -851,6 +886,7 @@ public function testIgnoreErrorExplicitReportUnmatchedEnableMulti(): void * @param string|string[] $filePaths * @param list> $collectors * @param list> $collectedDataRules + * @param list $filesChangedByFix * @return string[]|Error[] */ private function runAnalyser( @@ -861,6 +897,7 @@ private function runAnalyser( bool $reportIgnoresWithoutComments = false, array $collectors = [], array $collectedDataRules = [], + array $filesChangedByFix = [], ): array { $analyser = $this->createAnalyser($reportIgnoresWithoutComments, $collectors); @@ -901,7 +938,13 @@ private function runAnalyser( ); $analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult(); - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($analyserResult->getErrors(), $onlyFiles, $normalizedFilePaths, $analyserResult->hasReachedInternalErrorsCountLimit()); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process( + $analyserResult->getErrors(), + $onlyFiles, + $normalizedFilePaths, + $analyserResult->hasReachedInternalErrorsCountLimit(), + $filesChangedByFix, + ); $errors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors(); $errors = array_merge($errors, $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages()); if ($analyserResult->hasReachedInternalErrorsCountLimit()) { From 0b3fa4b7a020f19a62701f5b3d1f6a416211a035 Mon Sep 17 00:00:00 2001 From: Matthias Vogel Date: Wed, 11 Nov 2026 15:14:47 +0000 Subject: [PATCH 2/4] Pass the argument to runAnalyser() by position, PHP 7.4 has no named arguments --- tests/PHPStan/Analyser/AnalyserTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 77b385ab1b3..3fbc81c6a2e 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -680,7 +680,7 @@ public function testReportUnmatchedIgnoredErrorFromPathInFileChangedByFix(bool $ 'path' => __DIR__ . '/data/empty/empty.php', ], ]; - $result = $this->runAnalyser($ignoreErrors, true, __DIR__ . '/data/empty/empty.php', $onlyFiles, filesChangedByFix: $filesChangedByFix); + $result = $this->runAnalyser($ignoreErrors, true, __DIR__ . '/data/empty/empty.php', $onlyFiles, false, [], [], $filesChangedByFix); if (!$expectReported) { $this->assertNoErrors($result); return; From 94f35f02f6df0750e34198bdbe26a1436f6a28fd Mon Sep 17 00:00:00 2001 From: Matthias Vogel Date: Wed, 11 Nov 2026 15:16:34 +0000 Subject: [PATCH 3/4] Normalize the expected path in the test, it has backslashes on Windows --- tests/PHPStan/Analyser/AnalyserTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 3fbc81c6a2e..27725e4f04c 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -686,9 +686,11 @@ public function testReportUnmatchedIgnoredErrorFromPathInFileChangedByFix(bool $ return; } + $expectedPath = $this->getFileHelper()->normalizePath(__DIR__ . '/data/empty/empty.php'); $this->assertCount(1, $result); $this->assertInstanceOf(Error::class, $result[0]); - $this->assertSame('Ignored error pattern #Fail\.# in path ' . __DIR__ . '/data/empty/empty.php was not matched in reported errors.', $result[0]->getMessage()); + $this->assertSame('Ignored error pattern #Fail\.# in path ' . $expectedPath . ' was not matched in reported errors.', $result[0]->getMessage()); + $this->assertSame($expectedPath, $result[0]->getFilePath()); $this->assertSame('ignore.unmatched', $result[0]->getIdentifier()); } From 9e0c05dfebabcaf2cd34b7878463db0c66391bac Mon Sep 17 00:00:00 2001 From: Matthias Vogel Date: Wed, 11 Nov 2026 15:36:18 +0000 Subject: [PATCH 4/4] Look up the files changed by --fix with array_flip() and test a path that is not normalized --- src/Analyser/Ignore/IgnoredErrorHelperResult.php | 3 ++- tests/PHPStan/Analyser/AnalyserTest.php | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Analyser/Ignore/IgnoredErrorHelperResult.php b/src/Analyser/Ignore/IgnoredErrorHelperResult.php index 8bc52a4e05c..0d0390b67a8 100644 --- a/src/Analyser/Ignore/IgnoredErrorHelperResult.php +++ b/src/Analyser/Ignore/IgnoredErrorHelperResult.php @@ -7,6 +7,7 @@ use PHPStan\File\FileHelper; use PHPStan\ShouldNotHappenException; use function array_fill_keys; +use function array_flip; use function array_key_exists; use function array_keys; use function array_map; @@ -358,7 +359,7 @@ public function process( } $analysedFilesKeys = array_fill_keys($analysedFiles, true); - $filesChangedByFixKeys = array_fill_keys(array_map(fn (string $file): string => $this->fileHelper->normalizePath($file), $filesChangedByFix), true); + $filesChangedByFixKeys = array_flip(array_map(fn (string $file): string => $this->fileHelper->normalizePath($file), $filesChangedByFix)); if (!$hasInternalErrors) { foreach ($unmatchedIgnoredErrors as $i => $unmatchedIgnoredError) { diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 27725e4f04c..5fb7be8581a 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -664,6 +664,7 @@ public static function dataReportUnmatchedIgnoredErrorFromPathInFileChangedByFix { yield 'list of files' => [true, [], false]; yield 'list of files, file changed by an earlier fix round' => [true, [__DIR__ . '/data/empty/empty.php'], true]; + yield 'list of files, file changed by an earlier fix round, path not normalized' => [true, [__DIR__ . '/data/empty/../empty/empty.php'], true]; yield 'list of files, another file changed by an earlier fix round' => [true, [__DIR__ . '/data/two-fails.php'], false]; yield 'full analysis' => [false, [], true]; }