Skip to content

Fix unmatched baseline entries in time linear in the baseline size #6977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ondrejmirtes merged 1 commit into from
Nov 6, 2026
+271 −20
Merged

Fix unmatched baseline entries in time linear in the baseline size #6977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 3 additions & 4 deletions .github/workflows/e2e-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,10 +1013,9 @@ jobs:
../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, 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"
# removing the last two messages of the PHP list conflicts, the second one goes in the next round;
# a NEON entry after the first one is removed with only its own lines, it does not conflict with its neighbours
../bashunit assert contains 'Round 2: fixed 1 error in 1 file' "$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
Expand Down
227 changes: 213 additions & 14 deletions src/Analyser/Ignore/IgnoreErrorsConfigFixer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
use Nette\Neon\Node as NeonNode;
use Nette\Neon\Node\ArrayItemNode as NeonArrayItemNode;
use Nette\Neon\Node\ArrayNode as NeonArrayNode;
use Nette\Neon\Node\BlockArrayNode as NeonBlockArrayNode;
use Nette\Neon\Node\LiteralNode as NeonLiteralNode;
use Nette\Neon\Parser as NeonParser;
use Nette\Neon\Token as NeonToken;
use Nette\Neon\TokenStream as NeonTokenStream;
use Nette\Utils\Strings;
use PhpParser\ConstExprEvaluationException;
Expand All @@ -36,12 +38,14 @@
use function array_pop;
use function array_search;
use function array_slice;
use function array_unique;
use function array_values;
use function count;
use function dirname;
use function hash;
use function implode;
use function in_array;
use function intdiv;
use function is_array;
use function is_int;
use function is_string;
Expand Down Expand Up @@ -93,6 +97,7 @@
* items: array<string, NeonArrayItemNode>,
* path: non-empty-list<NeonNode>,
* entryValue: NeonNode,
* previousItem: NeonArrayItemNode|null,
* line: int,
* }
* @phpstan-type ConfigEntry = PhpConfigEntry|NeonConfigEntry
Expand All @@ -113,13 +118,26 @@ final class IgnoreErrorsConfigFixer
'identifier' => 'identifiers',
];

/** @var array<string, list<ConfigEntry>>|null entries by normalized path, or by self::NO_PATH */
/** key of the entries with none of the MATCHED_FIELDS in getEntriesByPath() */
private const NO_MATCHED_FIELD = "\0";

/**
* Entries by normalized path, or by self::NO_PATH, and by getMatchKeys(). A baseline can have
* thousands of entries with the same path, scanning all of them for each unmatched error would be quadratic.
*
* @var array<string, array<string, list<ConfigEntry>>>|null
*/
private ?array $entriesByPath = null;

/** @var array<string, array{code: string, hash: string}> contents of the config files the entries were found in */
private array $files = [];

/** @var array<string, array{tokens: NeonTokenStream, root: NeonNode, codeWithoutCr: string, crlf: bool, mixedLineBreaks: bool}> parsed NEON config files */
/**
* Parsed NEON config files. The offsets of the tokens in the code without \r, and the offsets of its line breaks,
* let an edit of one entry be made without printing the whole ignoreErrors list again.
*
* @var array<string, array{tokens: NeonTokenStream, root: NeonNode, codeWithoutCr: string, crlf: bool, mixedLineBreaks: bool, tokenList: list<NeonToken>, offsets: list<int>, lineBreaks: list<int>}>
*/
private array $neonFiles = [];

/**
Expand Down Expand Up @@ -167,10 +185,12 @@ public function removeUnmatched(string|array $ignoreError): array
{
$editsByFile = [];
$neonChangesByFile = [];
$neonRemovedEntriesByFile = [];
foreach ($this->findEntries($ignoreError) as [$entry, $listField, $position]) {
if ($entry['format'] === 'neon') {
if ($listField === null || $position === null) {
$neonChangesByFile[$entry['file']][] = [$entry['path'], null];
$neonRemovedEntriesByFile[$entry['file']][] = $entry;
continue;
}

Expand Down Expand Up @@ -215,7 +235,10 @@ public function removeUnmatched(string|array $ignoreError): array
}

foreach ($neonChangesByFile as $file => $changes) {
$edit = $this->createNeonEdit($file, $changes);
// more changes of one file are printed together, they could remove all entries of the list
$edit = count($changes) === 1 && count($neonRemovedEntriesByFile[$file] ?? []) === 1
? $this->createNeonRemovalEdit($neonRemovedEntriesByFile[$file][0]) ?? $this->createNeonEdit($file, $changes)
: $this->createNeonEdit($file, $changes);
if ($edit === null) {
return [];
}
Expand Down Expand Up @@ -252,7 +275,7 @@ public function lowerCount(array $ignoreError, int $count): array
return [];
}

$edit = $this->createNeonEdit($entry['file'], [
$edit = $this->createNeonCountEdit($entry['file'], $countItem->value, $count) ?? $this->createNeonEdit($entry['file'], [
[[...$entry['path'], $entry['entryValue'], $countItem, $countItem->value], new NeonLiteralNode($count)],
]);

Expand Down Expand Up @@ -398,8 +421,16 @@ private function findEntries(string|array $ignoreError): array
return [];
}

$matchKey = self::NO_MATCHED_FIELD;
foreach (array_keys(self::MATCHED_FIELDS) as $field) {
if (isset($ignoreError[$field])) {
$matchKey = $field . "\0" . $ignoreError[$field];
break;
}
}

$found = [];
foreach ($this->getEntriesByPath()[$path !== null ? $this->normalizePath($path) : self::NO_PATH] ?? [] as $entry) {
foreach ($this->getEntriesByPath()[$path !== null ? $this->normalizePath($path) : self::NO_PATH][$matchKey] ?? [] as $entry) {
$listField = null;
$position = null;
foreach (self::MATCHED_FIELDS as $field => $matchedListField) {
Expand Down Expand Up @@ -443,7 +474,7 @@ private function findEntries(string|array $ignoreError): array
}

/**
* @return array<string, list<ConfigEntry>>
* @return array<string, array<string, list<ConfigEntry>>>
*/
private function getEntriesByPath(): array
{
Expand All @@ -462,21 +493,58 @@ private function getEntriesByPath(): array
$path = $entry['values']['path'] ?? null;
if ($path === null) {
if (!isset($entry['values']['paths'])) {
$this->entriesByPath[self::NO_PATH][] = $entry;
foreach (self::getMatchKeys($entry['values']) as $matchKey) {
$this->entriesByPath[self::NO_PATH][$matchKey][] = $entry;
}
}
continue;
}
if (!is_string($path)) {
continue;
}

$this->entriesByPath[$this->normalizePath($path)][] = $entry;
$normalizedPath = $this->normalizePath($path);
foreach (self::getMatchKeys($entry['values']) as $matchKey) {
$this->entriesByPath[$normalizedPath][$matchKey][] = $entry;
}
}
}

return $this->entriesByPath;
}

/**
* findEntries() matches an error only with an entry that has the first of the MATCHED_FIELDS the error has,
* and none of the fields before it. The entry is found by that field and each value it can match.
*
* @param array<string, mixed> $values
* @return list<string>
*/
private static function getMatchKeys(array $values): array
{
foreach (self::MATCHED_FIELDS as $field => $listField) {
if (!array_key_exists($field, $values) && !array_key_exists($listField, $values)) {
continue;
}

$matched = [];
if (is_string($values[$field] ?? null)) {
$matched[] = $values[$field];
}
foreach (is_array($values[$listField] ?? null) ? $values[$listField] : [] as $listValue) {
if (!is_string($listValue)) {
continue;
}

$matched[] = $listValue;
}

return array_values(array_unique(array_map(static fn (string $value): string => $field . "\0" . $value, $matched)));
}

return [self::NO_MATCHED_FIELD];
}

/**
* Every array that looks like an ignoreErrors entry, wherever it is in the file. Such an array is removable
* when it is an item of another array, or when it is added in a statement like `$ignoreErrors[] = [...];`.
Expand Down Expand Up @@ -626,20 +694,40 @@ private function findNeonEntries(string $file): array

$this->files[$file] = ['code' => $code, 'hash' => hash('sha256', $code)];
$codeWithoutCr = str_replace("\r", '', $code);
$tokenList = array_values($tokens->getTokens());
$offsets = [];
$offset = 0;
foreach ($tokenList as $token) {
$offsets[] = $offset;
$offset += strlen($token->value);
}
$offsets[] = $offset;
$lineBreaks = [];
$lineBreak = strpos($codeWithoutCr, "\n");
while ($lineBreak !== false) {
$lineBreaks[] = $lineBreak;
$lineBreak = strpos($codeWithoutCr, "\n", $lineBreak + 1);
}
$this->neonFiles[$file] = [
'tokens' => $tokens,
'root' => $root,
'codeWithoutCr' => $codeWithoutCr,
// nette/neon removes \r, it is put back when every line break had it
'crlf' => $codeWithoutCr !== $code,
'mixedLineBreaks' => $codeWithoutCr !== $code && str_replace("\n", "\r\n", $codeWithoutCr) !== $code,
'tokenList' => $tokenList,
'offsets' => $offsets,
'lineBreaks' => $lineBreaks,
];

$parameters = $this->getParameters();
// relative paths are resolved like when the config file is loaded
$adapter = new NeonAdapter(['[parameters][ignoreErrors][][path]', '[parameters][ignoreErrors][][paths][]'], $parameters);
$entries = [];
$previousItem = null;
foreach ($ignoreErrorsItem->value->items as $entryItem) {
$entryPreviousItem = $previousItem;
$previousItem = $entryItem;
$items = [];
// a plain string pattern has no items
foreach ($entryItem->value instanceof NeonArrayNode ? $entryItem->value->items : [] as $item) {
Expand Down Expand Up @@ -673,6 +761,7 @@ private function findNeonEntries(string $file): array
'items' => $items,
'path' => [$root, $parametersItem, $parametersItem->value, $ignoreErrorsItem, $ignoreErrorsItem->value, $entryItem],
'entryValue' => $entryItem->value,
'previousItem' => $ignoreErrorsItem->value instanceof NeonBlockArrayNode ? $entryPreviousItem : null,
'line' => $entryItem->startLine ?? 1,
];
}
Expand Down Expand Up @@ -704,7 +793,7 @@ private static function findNeonItem(NeonNode $node, string $key): ?NeonArrayIte
*/
private function createNeonEdit(string $file, array $changes): ?array
{
['tokens' => $tokens, 'root' => $root, 'codeWithoutCr' => $codeWithoutCr, 'crlf' => $crlf, 'mixedLineBreaks' => $mixedLineBreaks] = $this->neonFiles[$file];
['tokens' => $tokens, 'root' => $root, 'codeWithoutCr' => $codeWithoutCr, 'mixedLineBreaks' => $mixedLineBreaks] = $this->neonFiles[$file];
if ($mixedLineBreaks) {
return null;
}
Expand All @@ -726,21 +815,131 @@ private function createNeonEdit(string $file, array $changes): ?array
$replacement .= substr($codeWithoutCr, $offset, $printedChange['start'] - $offset) . $printedChange['replacement'];
$offset = $printedChange['end'];
}
$end = $offset;
if ($crlf) {
$start += substr_count($codeWithoutCr, "\n", length: $start);
$end += substr_count($codeWithoutCr, "\n", length: $end);

return $this->createEditOfCodeWithoutCr($file, $start, $offset, $replacement);
}

/**
* Removes a whole entry of a block list with its lines, and with the lines between it and the previous entry,
* like comments, the same as printing the list without it. Unlike printing the list, the time does not grow
* with its length, and the edit covers only the lines of the entry, so that the next entry can be removed
* in the same round. The first entry is left to the printer, it removes the empty lines after it as well.
*
* @param NeonConfigEntry $entry
* @return Edit|null
*/
private function createNeonRemovalEdit(array $entry): ?array
{
$previousItem = $entry['previousItem'];
$entryItem = $entry['path'][count($entry['path']) - 1];
if (
$previousItem === null
|| $previousItem->endTokenPos === null
|| $entryItem->startTokenPos === null
|| $entryItem->endTokenPos === null
|| $this->neonFiles[$entry['file']]['mixedLineBreaks']
) {
return null;
}

['tokenList' => $tokens, 'offsets' => $offsets, 'codeWithoutCr' => $codeWithoutCr] = $this->neonFiles[$entry['file']];
$previousLineBreak = self::findNeonLineBreak($tokens, $previousItem->endTokenPos + 1);
for ($i = $previousLineBreak; $i < $entryItem->startTokenPos; $i++) {
// only empty lines, comments and indentation are between the entries of a block list
if (!in_array($tokens[$i]->type, [NeonToken::Newline, NeonToken::Whitespace, NeonToken::Comment], true)) {
return null;
}
}
if (!isset($tokens[$previousLineBreak]) || $previousLineBreak >= $entryItem->startTokenPos) {
return null;
}

$lineBreak = self::findNeonLineBreak($tokens, $entryItem->endTokenPos + 1);

return $this->createEditOfCodeWithoutCr(
$entry['file'],
$offsets[$previousLineBreak] + 1,
isset($tokens[$lineBreak]) ? $offsets[$lineBreak] + 1 : strlen($codeWithoutCr),
'',
);
}

/**
* Replaces the count with a new literal, the same as the printer prints it.
*
* @return Edit|null
*/
private function createNeonCountEdit(string $file, NeonNode $countNode, int $count): ?array
{
['tokenList' => $tokens, 'offsets' => $offsets, 'mixedLineBreaks' => $mixedLineBreaks] = $this->neonFiles[$file];
$pos = $countNode->startTokenPos;
if (
$mixedLineBreaks
|| !$countNode instanceof NeonLiteralNode
|| $pos === null
|| $countNode->endTokenPos !== $pos
|| ($tokens[$pos]->type ?? null) !== NeonToken::Literal
) {
return null;
}

return $this->createEditOfCodeWithoutCr($file, $offsets[$pos], $offsets[$pos + 1], (string) $count);
}

/**
* Position of the first line break token from $pos, or the count of the tokens.
*
* @param list<NeonToken> $tokens
*/
private static function findNeonLineBreak(array $tokens, int $pos): int
{
while (isset($tokens[$pos]) && $tokens[$pos]->type !== NeonToken::Newline) {
$pos++;
}

return $pos;
}

/**
* @return Edit
*/
private function createEditOfCodeWithoutCr(string $file, int $start, int $end, string $replacement): array
{
$lineBreaksBeforeStart = $this->countNeonLineBreaksBefore($file, $start);
if ($this->neonFiles[$file]['crlf']) {
$end += $this->countNeonLineBreaksBefore($file, $end);
$start += $lineBreaksBeforeStart;
$replacement = str_replace("\n", "\r\n", $replacement);
}

return [
'start' => $start,
'end' => $end,
'replacement' => $replacement,
'line' => substr_count($this->files[$file]['code'], "\n", length: $start) + 1,
'line' => $lineBreaksBeforeStart + 1,
];
}

/**
* The number of line breaks before the offset in the code without \r.
*/
private function countNeonLineBreaksBefore(string $file, int $offset): int
{
$lineBreaks = $this->neonFiles[$file]['lineBreaks'];
$low = 0;
$high = count($lineBreaks);
while ($low < $high) {
$mid = intdiv($low + $high, 2);
if ($lineBreaks[$mid] < $offset) {
$low = $mid + 1;
} else {
$high = $mid;
}
}

return $low;
}

/**
* Replaces the changed nodes and clones their ancestors, the original tree stays as it is.
* The clones keep the token positions of the original nodes, so the printer knows what they stand for.
Expand Down
Loading
Loading