From 0e6c91efcba239a38cc1e2ded838bb15c69765a4 Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Tue, 3 Nov 2026 10:09:44 +0000 Subject: [PATCH] Make unmatched ignoreErrors entries without a path fixable An unmatched entry without a path, a plain string pattern, or an entry with an fnmatch path pattern used to be reported as a message that is not specific to any file, and so it could not be fixed. It is now reported as ignore.unmatched in the config file, on the line of the entry, and --fix removes it like the path-specific ones. A too high count of such an entry can be lowered too. The entry stays a plain message when it is not found in the config files, e.g. in a config file of an installed Composer package, or when it has `paths` with some of them matched. Amp-Thread-ID: https://ampcode.com/threads/T-01a1a0da-23d4-77fd-aee3-a6a5a4016baf --- .github/workflows/e2e-tests.yml | 14 ++- e2e/fix-baseline/baseline-original.neon | 5 + e2e/fix-baseline/vendor-baseline.php | 1 + phpstan-baseline.neon | 2 +- .../Ignore/IgnoreErrorsConfigFixer.php | 99 ++++++++++++++++--- .../Ignore/IgnoredErrorHelperResult.php | 17 +++- .../Ignore/IgnoreErrorsConfigFixerTest.php | 84 ++++++++++++++++ 7 files changed, 200 insertions(+), 22 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index c957a105144..f0d1a19ad6e 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -1004,22 +1004,26 @@ jobs: echo "$OUTPUT" # two unmatched messages, an unmatched entry, and a count higher than the occurrences in the PHP baseline, # an unmatched message, an unmatched entry, and a count higher than the occurrences in the NEON baseline; - # not the entry in vendor/, and not a count lower than the occurrences (--generate-baseline does that) - ../bashunit assert contains '7 of the errors can be fixed automatically. Run PHPStan again with --fix' "$OUTPUT" + # and two entries without a path in the NEON baseline, reported in it; + # not the entries in vendor/, and not a count lower than the occurrences (--generate-baseline does that) + ../bashunit assert contains '9 of the errors can be fixed automatically. Run PHPStan again with --fix' "$OUTPUT" + ../bashunit assert contains 'baseline.neon:21:Ignored error pattern #^Unmatched without a path$# was not matched' "$OUTPUT" OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse --fix --generate-baseline") echo "$OUTPUT" ../bashunit assert contains 'Errors cannot be fixed when generating the baseline.' "$OUTPUT" OUTPUT=$(../bashunit assert exit_code "0" "../../bin/phpstan analyse --fix") echo "$OUTPUT" # removing the last two messages of the PHP list conflicts, and so does removing the NEON entry - # and the first message of the entry after it, the second one goes in the next round - ../bashunit assert contains 'Round 2: fixed 2 errors in 2 files' "$OUTPUT" - ../bashunit assert contains '[OK] 7 errors fixed in 2 files' "$OUTPUT" + # and the first message of the entry after it, and so does removing the two neighbouring entries without a path, + # the second one goes in the next round + ../bashunit assert contains 'Round 2: fixed 3 errors in 2 files' "$OUTPUT" + ../bashunit assert contains '[OK] 9 errors fixed in 2 files' "$OUTPUT" diff -u baseline-expected.php baseline.php diff -u baseline-expected.neon baseline.neon OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse --error-format raw") echo "$OUTPUT" ../bashunit assert contains 'Ignored error pattern #^Unmatched in vendor$#' "$OUTPUT" + ../bashunit assert contains 'Ignored error pattern #^Unmatched in vendor without a path$# was not matched' "$OUTPUT" ../bashunit assert contains 'is expected to occur 1 time, but occurred 2 times.' "$OUTPUT" ../bashunit assert not_contains 'Unmatched one' "$OUTPUT" ../bashunit assert not_contains 'NEON' "$OUTPUT" diff --git a/e2e/fix-baseline/baseline-original.neon b/e2e/fix-baseline/baseline-original.neon index b31a8187b86..cdab5299174 100644 --- a/e2e/fix-baseline/baseline-original.neon +++ b/e2e/fix-baseline/baseline-original.neon @@ -16,3 +16,8 @@ parameters: - '#^Unmatched in a NEON list$#' - '#^Undefined variable\: \$h$#' path: src/Foo.php + + # entries without a path are reported in this file + - '#^Unmatched without a path$#' + - + rawMessage: 'Unmatched without a path either' diff --git a/e2e/fix-baseline/vendor-baseline.php b/e2e/fix-baseline/vendor-baseline.php index f6862a300cd..efaf918d114 100644 --- a/e2e/fix-baseline/vendor-baseline.php +++ b/e2e/fix-baseline/vendor-baseline.php @@ -5,4 +5,5 @@ 'message' => '#^Unmatched in vendor$#', 'path' => __DIR__ . '/../../../src/Foo.php', ], + '#^Unmatched in vendor without a path$#', ]]]; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 51a6d0e58d5..fc79f566e9e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -57,7 +57,7 @@ parameters: - rawMessage: 'Call to static method expand() of internal class Nette\DI\Helpers from outside its root namespace Nette.' identifier: staticMethod.internalClass - count: 2 + count: 3 path: src/Analyser/Ignore/IgnoreErrorsConfigFixer.php - diff --git a/src/Analyser/Ignore/IgnoreErrorsConfigFixer.php b/src/Analyser/Ignore/IgnoreErrorsConfigFixer.php index e16d619c194..6a4ce9296aa 100644 --- a/src/Analyser/Ignore/IgnoreErrorsConfigFixer.php +++ b/src/Analyser/Ignore/IgnoreErrorsConfigFixer.php @@ -69,6 +69,7 @@ * * The entry is looked for in the whole config cascade, except for files of installed Composer packages. * The ignoreErrors parameter is merged from all of them, so the entry can come from more than one file. + * Entries without a path, including the ones written as a plain string pattern, are fixed too. * * Baselines can have tens of thousands of entries and each fixed error needs its own diff, which covers * only the changed lines. In a PHP config file, a fix is an edit of the source range of the parsed node, @@ -83,6 +84,7 @@ * format: 'php', * items: array, * removableNode: Node|null, + * line: int, * } * @phpstan-type NeonConfigEntry = array{ * file: string, @@ -90,7 +92,8 @@ * format: 'neon', * items: array, * path: non-empty-list, - * entryValue: NeonArrayNode, + * entryValue: NeonNode, + * line: int, * } * @phpstan-type ConfigEntry = PhpConfigEntry|NeonConfigEntry * @phpstan-type Edit = array{start: int, end: int, replacement: string, line: int} @@ -101,13 +104,16 @@ final class IgnoreErrorsConfigFixer private const ENTRY_KEYS = ['message', 'messages', 'rawMessage', 'rawMessages', 'identifier', 'identifiers', 'path', 'paths', 'count', 'reportUnmatched']; + /** key of the entries without a path in getEntriesByPath(), a normalized path is never empty */ + private const NO_PATH = ''; + private const MATCHED_FIELDS = [ 'message' => 'messages', 'rawMessage' => 'rawMessages', 'identifier' => 'identifiers', ]; - /** @var array>|null entries by normalized path */ + /** @var array>|null entries by normalized path, or by self::NO_PATH */ private ?array $entriesByPath = null; /** @var array contents of the config files the entries were found in */ @@ -136,10 +142,28 @@ public function __construct( } /** - * @param ExpandedIgnoredErrorData $ignoreError + * Config file and line of the entry, for reporting an entry without a path. + * + * @param string|ExpandedIgnoredErrorData $ignoreError + * @return array{string, int}|null + */ + public function findEntryLocation(string|array $ignoreError): ?array + { + $entries = $this->findEntries($ignoreError); + if ($entries === []) { + return null; + } + + $entry = $entries[0][0]; + + return [$entry['file'], $entry['line']]; + } + + /** + * @param string|ExpandedIgnoredErrorData $ignoreError * @return list */ - public function removeUnmatched(array $ignoreError): array + public function removeUnmatched(string|array $ignoreError): array { $editsByFile = []; $neonChangesByFile = []; @@ -360,18 +384,22 @@ private function createDiff(string $code, array $edits): ?string * Entries the expanded entry was made of. The list item is set when the entry matched through a list * like `messages` that has other items too, only that item can then be removed. * - * @param ExpandedIgnoredErrorData $ignoreError + * @param string|ExpandedIgnoredErrorData $ignoreError * @return list the entry, and the list field and position of the item */ - private function findEntries(array $ignoreError): array + private function findEntries(string|array $ignoreError): array { + if (is_string($ignoreError)) { + $ignoreError = ['message' => $ignoreError]; + } + $path = $ignoreError['realPath'] ?? $ignoreError['path'] ?? null; - if ($path === null) { + if ($path === null && isset($ignoreError['paths'])) { return []; } $found = []; - foreach ($this->getEntriesByPath()[$this->normalizePath($path)] ?? [] as $entry) { + foreach ($this->getEntriesByPath()[$path !== null ? $this->normalizePath($path) : self::NO_PATH] ?? [] as $entry) { $listField = null; $position = null; foreach (self::MATCHED_FIELDS as $field => $matchedListField) { @@ -432,6 +460,12 @@ private function getEntriesByPath(): array $entries = str_ends_with($configFile, '.php') ? $this->findPhpEntries($configFile) : $this->findNeonEntries($configFile); foreach ($entries as $entry) { $path = $entry['values']['path'] ?? null; + if ($path === null) { + if (!isset($entry['values']['paths'])) { + $this->entriesByPath[self::NO_PATH][] = $entry; + } + continue; + } if (!is_string($path)) { continue; } @@ -496,6 +530,39 @@ private function findPhpEntries(string $file): array foreach ($node->items as $item) { $removableNodes[spl_object_id($item->value)] = $item; + + // plain string patterns, unlike entry arrays, are told apart from other strings only by being in an ignoreErrors list + if ( + !$item->key instanceof Node\Scalar\String_ + || $item->key->value !== 'ignoreErrors' + || !$item->value instanceof Expr\Array_ + ) { + continue; + } + + foreach ($item->value->items as $patternItem) { + if ($patternItem->key !== null || $patternItem->value instanceof Expr\Array_) { + continue; + } + + try { + $pattern = Helpers::expand($evaluator->evaluateDirectly($patternItem->value), $parameters); + } catch (ConstExprEvaluationException | InvalidArgumentException) { + continue; + } + if (!is_string($pattern)) { + continue; + } + + $entries[] = [ + 'file' => $file, + 'values' => ['message' => $pattern], + 'format' => 'php', + 'items' => [], + 'removableNode' => $patternItem, + 'line' => $patternItem->getStartLine(), + ]; + } } $items = []; @@ -516,7 +583,7 @@ private function findPhpEntries(string $file): array $values[$item->key->value] = $value; } - if (!isset($values['path']) || (isset($values['count']) && !is_int($values['count']))) { + if ($values === [] || (isset($values['count']) && !is_int($values['count']))) { continue; } @@ -526,6 +593,7 @@ private function findPhpEntries(string $file): array 'format' => 'php', 'items' => $items, 'removableNode' => $removableNodes[spl_object_id($node)] ?? null, + 'line' => $node->getStartLine(), ]; } @@ -572,12 +640,9 @@ private function findNeonEntries(string $file): array $adapter = new NeonAdapter(['[parameters][ignoreErrors][][path]', '[parameters][ignoreErrors][][paths][]'], $parameters); $entries = []; foreach ($ignoreErrorsItem->value->items as $entryItem) { - if (!$entryItem->value instanceof NeonArrayNode) { - continue; - } - $items = []; - foreach ($entryItem->value->items as $item) { + // a plain string pattern has no items + foreach ($entryItem->value instanceof NeonArrayNode ? $entryItem->value->items : [] as $item) { $key = $item->key !== null ? $item->key->toValue() : null; if (!is_string($key) || !in_array($key, self::ENTRY_KEYS, true)) { continue 2; @@ -594,7 +659,10 @@ private function findNeonEntries(string $file): array continue; } - if (!is_array($values) || !isset($values['path']) || (isset($values['count']) && !is_int($values['count']))) { + if (is_string($values)) { + $values = ['message' => $values]; + } + if (!is_array($values) || $values === [] || (isset($values['count']) && !is_int($values['count']))) { continue; } @@ -605,6 +673,7 @@ private function findNeonEntries(string $file): array 'items' => $items, 'path' => [$root, $parametersItem, $parametersItem->value, $ignoreErrorsItem, $ignoreErrorsItem->value, $entryItem], 'entryValue' => $entryItem->value, + 'line' => $entryItem->startLine ?? 1, ]; } diff --git a/src/Analyser/Ignore/IgnoredErrorHelperResult.php b/src/Analyser/Ignore/IgnoredErrorHelperResult.php index e5ebbbf0fbf..f1bed8885d9 100644 --- a/src/Analyser/Ignore/IgnoredErrorHelperResult.php +++ b/src/Analyser/Ignore/IgnoredErrorHelperResult.php @@ -412,11 +412,26 @@ public function process( fixedErrorDiffs: $this->configFixer->removeUnmatched($unmatchedIgnoredError), ); } elseif (!$onlyFiles) { - $stringErrors[] = sprintf( + $message = sprintf( '%s %s was not matched in reported errors.', IgnoredError::getIgnoredErrorLabel($unmatchedIgnoredError), IgnoredError::stringifyPattern($unmatchedIgnoredError), ); + // without a file of its own, the error is reported in the config file the entry is in + $location = $this->configFixer->findEntryLocation($unmatchedIgnoredError); + if ($location === null) { + $stringErrors[] = $message; + continue; + } + + $errors[] = new Error( + $message, + $location[0], + $location[1], + canBeIgnored: false, + identifier: 'ignore.unmatched', + fixedErrorDiffs: $this->configFixer->removeUnmatched($unmatchedIgnoredError), + ); } } } diff --git a/tests/PHPStan/Analyser/Ignore/IgnoreErrorsConfigFixerTest.php b/tests/PHPStan/Analyser/Ignore/IgnoreErrorsConfigFixerTest.php index 0f8c91ddf65..7923ca94577 100644 --- a/tests/PHPStan/Analyser/Ignore/IgnoreErrorsConfigFixerTest.php +++ b/tests/PHPStan/Analyser/Ignore/IgnoreErrorsConfigFixerTest.php @@ -216,6 +216,90 @@ public function testNotFixable(string $code, array $ignoreError, bool $lowerCoun $this->assertSame([], $lowerCount ? $fixer->lowerCount($ignoreError, 1) : $fixer->removeUnmatched($ignoreError)); } + public static function dataEntryWithoutPath(): Generator + { + $php = " ['ignoreErrors' => [\n" + . "\t'#^A$#',\n" + . "\t['message' => '#^A$#', 'path' => __DIR__ . '/Foo.php'],\n" + . "\t['message' => '#^A$#', 'identifier' => 'foo.bar'],\n" + . "\t['rawMessage' => 'B'],\n" + . "]]];\n"; + + yield 'PHP string pattern, not the entries with a path or an identifier' => [ + $php, + 'php', + '#^A$#', + 3, + " ['ignoreErrors' => [\n" + . "\t['message' => '#^A$#', 'path' => __DIR__ . '/Foo.php'],\n" + . "\t['message' => '#^A$#', 'identifier' => 'foo.bar'],\n" + . "\t['rawMessage' => 'B'],\n" + . "]]];\n", + ]; + + yield 'PHP entry' => [ + $php, + 'php', + ['rawMessage' => 'B'], + 6, + " ['ignoreErrors' => [\n" + . "\t'#^A$#',\n" + . "\t['message' => '#^A$#', 'path' => __DIR__ . '/Foo.php'],\n" + . "\t['message' => '#^A$#', 'identifier' => 'foo.bar'],\n" + . "]]];\n", + ]; + + $neon = "parameters:\n\tignoreErrors:\n" + . "\t\t- '#^100%% sure$#'\n" + . "\t\t-\n\t\t\tmessage: '#^100%% sure$#'\n\t\t\tpath: Foo.php\n" + . "\t\t-\n\t\t\trawMessages:\n\t\t\t\t- B\n\t\t\t\t- C\n" + . "\tlevel: 8\n"; + + yield 'NEON string pattern' => [ + $neon, + 'neon', + '#^100% sure$#', + 3, + "parameters:\n\tignoreErrors:\n" + . "\t\t-\n\t\t\tmessage: '#^100%% sure$#'\n\t\t\tpath: Foo.php\n" + . "\t\t-\n\t\t\trawMessages:\n\t\t\t\t- B\n\t\t\t\t- C\n" + . "\tlevel: 8\n", + ]; + + yield 'NEON item of a list' => [ + $neon, + 'neon', + ['rawMessage' => 'C'], + 7, + "parameters:\n\tignoreErrors:\n" + . "\t\t- '#^100%% sure$#'\n" + . "\t\t-\n\t\t\tmessage: '#^100%% sure$#'\n\t\t\tpath: Foo.php\n" + . "\t\t-\n\t\t\trawMessages:\n\t\t\t\t- B\n" + . "\tlevel: 8\n", + ]; + } + + /** + * @param string|array{message?: string, rawMessage?: string} $ignoreError + */ + #[DataProvider('dataEntryWithoutPath')] + public function testEntryWithoutPath(string $code, string $extension, string|array $ignoreError, int $line, string $expected): void + { + [$fixer, $file] = $this->createFixer($code, $extension); + + $this->assertSame([$file, $line], $fixer->findEntryLocation($ignoreError)); + $this->assertSame($expected, $this->apply($file, $code, $fixer->removeUnmatched($ignoreError))); + } + + public function testEntryWithPathsIsNotFound(): void + { + [$fixer] = $this->createFixer("parameters:\n\tignoreErrors:\n\t\t-\n\t\t\tmessage: '#^A$#'\n\t\t\tpaths:\n\t\t\t\t- Foo.php\n\t\t\t\t- Bar.php\n", 'neon'); + $ignoreError = ['message' => '#^A$#', 'paths' => [1 => __DIR__ . '/Bar.php']]; + + $this->assertNull($fixer->findEntryLocation($ignoreError)); + $this->assertSame([], $fixer->removeUnmatched($ignoreError)); + } + public static function dataNeon(): Generator { $entries = "parameters:\n\tignoreErrors:\n"