Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions e2e/fix-baseline/baseline-original.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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'
1 change: 1 addition & 0 deletions e2e/fix-baseline/vendor-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
'message' => '#^Unmatched in vendor$#',
'path' => __DIR__ . '/../../../src/Foo.php',
],
'#^Unmatched in vendor without a path$#',
]]];
2 changes: 1 addition & 1 deletion phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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

-
Expand Down
99 changes: 84 additions & 15 deletions src/Analyser/Ignore/IgnoreErrorsConfigFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -83,14 +84,16 @@
* format: 'php',
* items: array<string, Node\ArrayItem>,
* removableNode: Node|null,
* line: int,
* }
* @phpstan-type NeonConfigEntry = array{
* file: string,
* values: array<string, mixed>,
* format: 'neon',
* items: array<string, NeonArrayItemNode>,
* path: non-empty-list<NeonNode>,
* entryValue: NeonArrayNode,
* entryValue: NeonNode,
* line: int,
* }
* @phpstan-type ConfigEntry = PhpConfigEntry|NeonConfigEntry
* @phpstan-type Edit = array{start: int, end: int, replacement: string, line: int}
Expand All @@ -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<string, list<ConfigEntry>>|null entries by normalized path */
/** @var array<string, list<ConfigEntry>>|null entries by normalized path, or by self::NO_PATH */
private ?array $entriesByPath = null;

/** @var array<string, array{code: string, hash: string}> contents of the config files the entries were found in */
Expand Down Expand Up @@ -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<FixedErrorDiff>
*/
public function removeUnmatched(array $ignoreError): array
public function removeUnmatched(string|array $ignoreError): array
{
$editsByFile = [];
$neonChangesByFile = [];
Expand Down Expand Up @@ -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<array{ConfigEntry, string|null, int|null}> 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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 = [];
Expand All @@ -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;
}

Expand All @@ -526,6 +593,7 @@ private function findPhpEntries(string $file): array
'format' => 'php',
'items' => $items,
'removableNode' => $removableNodes[spl_object_id($node)] ?? null,
'line' => $node->getStartLine(),
];
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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,
];
}

Expand Down
17 changes: 16 additions & 1 deletion src/Analyser/Ignore/IgnoredErrorHelperResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
}
}
}
Expand Down
84 changes: 84 additions & 0 deletions tests/PHPStan/Analyser/Ignore/IgnoreErrorsConfigFixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "<?php\nreturn ['parameters' => ['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,
"<?php\nreturn ['parameters' => ['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,
"<?php\nreturn ['parameters' => ['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"
Expand Down
Loading