Remove a dead member's overrides along with it - #63
Merged
Merged
Conversation
An override annotated @OverRide is not a candidate, so removing only the member the finder reports left every override of it overriding nothing: override_on_non_overriding_member on code that analyzed cleanly before. Ask textDocument/implementation for the overrides of each dead member and couple them to its removal, wherever they live — the request answers transitively and across files. A member is instead left report-only when an override cannot be deleted: one in a file the run did not scan, one declared as a field (it can be a declaring parameter, or constructor-initialized), or one that has references of its own. Fixes #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Trim the doc comments, fixture comments, verbose lines and the report-only hint this change added to what they need to say. The `field` note also had the reason wrong: a constructor-initialized field is referenced by its own constructor, so it never reads as dead in the first place; what makes a field override unsafe to delete is a declaring parameter of a primary constructor, which the outline reports as a field too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
PiotrRogulski
commented
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
PiotrRogulski
marked this pull request as ready for review
September 18, 2026 18:16
Komoszek
reviewed
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
A field override was left report-only for being a field. Only two field shapes actually have to stay: a declaring parameter of a primary constructor, which the outline also reports as a field and whose removal changes the constructor signature at every call site, and a declarator sharing a statement with others, whose span covers only its own text. Both are read from the selection range already fetched for the override; a plain body field is now coupled like any other override. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Komoszek
reviewed
Sep 18, 2026
A declarator sharing its statement with others was left report-only, but the remover already handles the shape: it deletes the statement whole when every declarator goes, and takes one out of the list, comma and all, when only some do. What it could not do was read a coupled removal, which arrived as a whole-node span. Give CoupledRemoval the kind and full range of what it deletes, so a field override goes through the same declarator path as a reported one. One declarator can now arrive twice, reported in its own right and coupled to the member it overrides; the span arithmetic read it twice and threw a RangeError, so it is deduplicated. Only a declaring parameter of a primary constructor still blocks removal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Komoszek
reviewed
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Komoszek
reviewed
Sep 18, 2026
Komoszek
reviewed
Sep 18, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
`b` in `@override final int a, b;` carries no doc comment, annotations or modifiers of its own — they all sit on the statement's first declarator — so it was checked where an @OverRide member is skipped, and reported when its uses were invisible to a reference search. A declarator that starts at its own name continues the statement before it, and reads that statement's metadata. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
Komoszek
approved these changes
Sep 18, 2026
Komoszek
left a comment
Contributor
There was a problem hiding this comment.
We could probably cut down most of the comments and we should update PR's description
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw
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.
Fixes #62.
An
@overridemember is not a candidate, so removing only the member the finder reports left every override of it overriding nothing —override_on_non_overriding_memberon code that analyzed cleanly before.textDocument/implementationanswers with every member overriding the one asked about, transitively and across files, so each dead member is asked once and its overrides are coupled to its removal. They stay out of the report, as coupled removals always have.A member is left report-only, marked
unsafe to auto-remove, when an override cannot be deleted: it is a declaring parameter of a primary constructor (deleting one changes the constructor signature at every call site), it is in a file the run did not scan, or it has references of its own.Two more fixes came out of review:
final int a = 1, b = 2;could not be touched safely.CoupledRemovalnow carries the kind and full range of what it deletes, and the remover's existing declarator handling takes over: the statement goes whole when every declarator does, one declarator comes out with its comma when only some do. One declarator can now reach the remover twice — reported in its own right and coupled to the member it overrides — which threw aRangeError, so it is deduplicated.bin@override final int a, b;reported none of its own and was checked where an@overridemember is skipped. It now reads the metadata of the declarator that opened the statement, which covers@pragma('vm:entry-point')the same way.Verification
On the issue's repro,
--removedeletes the member and both of its overrides, anddart analyzestays clean where it previously reported a dangling@override. The same holds for a three-level chain and for overrides in another file.New fixture (
example/lib/scenarios/overrides.dart+overrides_impl.dart) and nine tests cover: a transitive override, an override from another file, a plain body field, a declarator sharing its statement with another dead one and with a live one, the two report-only cases, a member called through the interface staying unreported, and the existingAnimal.sound/Dog.soundpair in the default fixture. Two remover tests cover the duplicate declarator and the partial statement.--removeover the whole override scenario leavesdart analyzeclean; published 0.5.0 leaves five errors on the same fixture.On lichess-org/mobile (641 files, 13,043 declarations,
-e 'lib/l10n/**'), where this was first hit: same 159 findings and same 13 report-only as before, 89 dead members asked about overrides, 2 coupled —EngineOpponent.displayNameandMaiaWeightsService.downloading, each with its subclass implementation.flutter analyzeafter--removegoes from 18 issues to 16, the two removed being exactly theoverride_on_non_overriding_memberwarnings.Cost
One
textDocument/implementationper reported dead member, onetextDocument/referencesper override found, and onetextDocument/selectionRangeper field override — findings are a small fraction of the declarations a run already queries. The lichess run above spent 0.5s on all 89 members.🤖 Generated with Claude Code
https://claude.ai/code/session_012YuAi9rcyBRnHD6gyGBRqw