Skip to content

[skip] fix match class + path matching in reporting of unused skips#8073

Merged
TomasVotruba merged 1 commit into
mainfrom
tv-skip-10
Jun 21, 2026
Merged

[skip] fix match class + path matching in reporting of unused skips#8073
TomasVotruba merged 1 commit into
mainfrom
tv-skip-10

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Jun 21, 2026

Copy link
Copy Markdown
Member

Bug

UnusedSkipResolver::resolveUnusedRuleScopedSkips() matched used skips by path only, ignoring the rule class.

Used skips live in one flat list (UsedSkipCollector), mixing rule-scoped paths and global paths. The same path skipped under two rules collapsed to a single key, so a match under one rule silently marked the same path under every other rule as "used".

Before (false negative)

->withSkip([
    SomeRector::class    => ['src/Foo.php'], // matched a file -> used
    AnotherRector::class => ['src/Foo.php'], // never matched -> still UNUSED
])

src/Foo.php matched under SomeRector, landed in the used list as the bare path src/Foo.php. The resolver then checked in_array('src/Foo.php', $usedSkips) for AnotherRector too — found it — and reported the genuinely unused skip as used. It was never flagged for removal.

After (fixed)

Used skips are now tracked scoped to their rule as class|path:

  • SkipSkipper::doesMatchSkip() marks $skippedClass . '|' . $matchedPath
  • UnusedSkipResolver checks in_array($rectorClass . '|' . $path, $usedSkips, true)

SomeRector|src/Foo.php and AnotherRector|src/Foo.php are distinct keys, so the unmatched skip under AnotherRector is correctly reported as unused.

Global skips and skip-everywhere (null path) rules are unchanged — they carry no rule scope.

Tests

  • New regression testSamePathUnderAnotherRuleDoesNotMarkSkipUsed — same path under two rules, only one matched; asserts the other still reports unused.
  • Updated UnusedSkipResolverTest + UsedSkipCollectorTest to the class|path key shape.

@TomasVotruba TomasVotruba enabled auto-merge (squash) June 21, 2026 11:34
@TomasVotruba TomasVotruba disabled auto-merge June 21, 2026 11:35
@TomasVotruba TomasVotruba merged commit 19dcdb7 into main Jun 21, 2026
67 checks passed
@TomasVotruba TomasVotruba deleted the tv-skip-10 branch June 21, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant