feat: match baseline violations without asking the user how - #666
Open
fain182 wants to merge 1 commit into
Open
Conversation
Choosing between matching with or without line numbers was a decision about how the baseline is compared, not about what to enforce, and both answers were wrong somewhere: the default reopened known violations after any edit above them, while --ignore-baseline-linenumbers could not tell two violations of the same rule in the same class apart. Matching is now a single strategy with no knob. Violations still sitting where the baseline recorded them pair first; among what is left, class and reported problem alone decide, so an edit above a violation moves it without making it new, and identical violations in one class stay distinct. When a violation is added among moved ones, it is the added one that is reported. --ignore-baseline-linenumbers and ignoreBaselineLinenumbers() are deprecated: they have no effect and print a notice. generate-baseline always writes line numbers; baselines stored without them keep working and need no regeneration. Closes #662 Refs #636 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #666 +/- ##
============================================
- Coverage 97.79% 97.57% -0.22%
- Complexity 760 762 +2
============================================
Files 101 101
Lines 2180 2190 +10
============================================
+ Hits 2132 2137 +5
- Misses 48 53 +5
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Choosing between matching the baseline with or without line numbers is a decision about how the baseline is compared, not about what to enforce — and both answers were wrong somewhere. The default reopened a known violation after any edit above it;
--ignore-baseline-linenumberscould not tell two violations of the same rule in the same class apart, and could report the wrong one as new (#636).Matching is now a single strategy with no knob, in two passes:
So an edit above a violation moves it without making it new, identical violations in one class stay distinct, and when a violation is added among moved ones it is the added one that gets reported. The
#636scenario (three baselined violations at 10/20/30, a fourth added at 15) is covered by a test.When a group of identical violations both moved and grew in the same run, which of them is reported as new is a guess — the count is not. That ambiguity is inherent, and it is documented in the README rather than papered over.
Deprecation:
--ignore-baseline-linenumbers/ignoreBaselineLinenumbers()have no effect and print a notice, whether they come from the CLI or fromphparkitect.php. They will be removed in the next major.generate-baselinealways writes line numbers now; baselines stored without them keep working and need no regeneration, andprune-baselinestill preserves that format.Pairing stays linear: a synthetic 20k-entry baseline where every violation moved matches in 0.04s, and 2000 violations of the same rule in a single class in under 0.01s.
Built on #665, which put all matching in
Violations::matchAgainst()— this PR is mostly the removal of the boolean from it.Closes #662
Refs #636