Report outdated annotations on a plain run, not only with --remove#448
Merged
Conversation
Previously the outdated-annotation detection was gated entirely behind --remove, so a normal "annotate" run silently ignored stale docblock lines and users only discovered cruft if they happened to run the destructive flag. Detection now always runs. --remove still deletes (unchanged); a plain run instead counts the would-remove annotations and prints "N annotations outdated (run with -r to remove)" (verbose mode names each line). No file is modified and the exit code is unchanged, so the behaviour is purely additive and non-breaking; clean projects stay silent because the line is only emitted when the count is non-zero. The in-use / parent-superseded detection is no longer conditional on --remove either, since the report must apply the exact same filter chain to be truthful about what -r would actually prune.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #448 +/- ##
============================================
+ Coverage 84.55% 84.56% +0.01%
- Complexity 2217 2220 +3
============================================
Files 122 122
Lines 5893 5904 +11
============================================
+ Hits 4983 4993 +10
- Misses 910 911 +1 ☔ View full report in Codecov by Sentry. 🚀 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
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.
Summary
Outdated-annotation detection was gated entirely behind
--remove. A normalannotaterun therefore silently ignored stale docblock lines — users only discovered cruft if they happened to run the destructive flag (and then it was deleted, not reported).This makes detection always run:
--remove— unchanged: deletes the outdated lines.-rwould prune and prints-> N annotations outdated (run with -r to remove); verbose (-v) names each line.No file is modified and the exit code is unchanged on a plain run, so it is purely additive and non-breaking. Clean projects stay silent — the line is only emitted when the count is non-zero.
Why no opt-in flag
The first iteration added a
--warn-staleflag. Dropped it: a flag defeats discoverability (users who do not know to pass it never see the cruft, which is the whole point), and the output is non-destructive informational text. Making it the default is the useful behaviour — analogous to howgit statusalways surfaces untracked files. The only cost is that in-use detection now runs on plain runs too; acceptable for a dev/CI tool where "helpful and correct" beats micro-perf.Truthfulness
The in-use and parent-superseded checks are no longer conditional on
--removeeither. The report must apply the exact same filter chain (in-use heuristic, description-skip, generated-tag set, and the parent-superseded bypass) as actual removal — otherwise it would cry wolf about lines-rwould in fact keep.Tests
AbstractAnnotatorTest::testReportSurfacesRemovableCountassertsreport()emits the count.composer stanclean,composer cs-checkclean.annotate modelsover a table carrying stale overrides prints-> 7 annotations outdated (run with -r to remove)(and-vlists each), leaving the file untouched;-rstill deletes them.Possible follow-ups (not in this PR)
--cito fail when removable orphans exist) so teams can block on docblock cruft without ever running destructive removal.