fix(extract): scope this.* binding keys per enclosing class#1833
Merged
Conversation
`binding_target_names` is module-flat, so two classes in one module that
declare a same-named field collided on the `this.<field>` key via
last-write-wins: `constructor(private dep: DepA)` in one class and
`readonly dep = new DepB()` in another both wrote `this.dep`, so every
`this.dep.*` access in the module resolved against the surviving target
only, and the losing class's dep members were falsely reported as
unused-class-member cross-module. This is the collision behind the issue's
privParam row (order-dependent: reversing the class declarations flipped
which class's members were flagged).
Qualify `this.`-rooted binding keys and receiver spellings with an internal
per-class scope id during the walk (`this@<id>.<field>`), pushed and popped
alongside the class-super / class-context stacks in `visit_class`. Both the
insert side and the access / whole-object / iteration-receiver read sides
are qualified consistently, so the longest-prefix resolution and the typed
property-hop expansion keep working within one class scope. The qualifier
is an extraction-only disambiguator: `strip_this_scope_qualifiers` runs last
in `finalize_resolution_phase`, after every resolution pass, and rewrites
every `this@<id>.` spelling back to `this.` across `member_accesses` and
`whole_object_uses` before any spelling reaches `ModuleInfo`, so no
persisted spelling and no downstream consumer (core self-access `== "this"`,
heritage `!= "this"`, unused-component-output `this.<name>`, SFC template
`starts_with("this.")`) ever sees it. Bare `this` (the per-file self-access
key) and module-level `this` are never qualified.
Correcting last-write-wins removes accidental credit, so a member that
exists on both colliding classes but is called on only one can surface as a
new true-positive finding. Validated on ten real-world projects: one such
finding (`NextNodeServer.revalidate` in next.js, where a `this.server`
getter on an unrelated class had been borrowing a sibling class's
`server: NextNodeServer` field binding) and zero non-member drift.
CACHE_VERSION 233 -> 234 (the emitted member_accesses change for modules
with same-named fields across classes; warm 233 caches keep the collision).
Fixes #1821
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
1 participant
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.
binding_target_namesis module-flat with last-write-wins, so two classes in one module with a same-named field collided on thethis.<field>key: only the class declared last credited its dependency's members, and reversing declaration order flipped which class's members were falsely reported. This affected public fields, param properties, and (since #1822)#-private fields identically.this.-rooted binding keys and access/whole-object/iteration-receiver spellings are now qualified with a per-class scope id during the walk and stripped back to plainthis.as the last finalize step, so no persisted spelling changes and no downstream consumer (core self-access, heritage filter, Angular output crediting, SFC template filter) sees the qualifier. Barethisand module-levelthisare never qualified. CACHE_VERSION 233 -> 234.For the #1821 reporter: with #1822 (private-field receivers) plus this change, every row of the issue's isolation matrix is covered. Two rows were misattributed in the original analysis: the param-property row and the Gap 2 alias-hop rows were failing because of this same-file field-name collision, not because of missing shape support; local and imported alias hops credit correctly in isolation on 3.3.0.
Note the error direction: unlike #1822 this is not purely additive. Correcting last-write-wins also removes accidental credit, so a member that was only credited through a sibling class's colliding field binding can surface as a new, correct finding. Benchmark validation across ten real-world projects showed exactly one such change:
NextNodeServer.revalidatein next.js, verified in source as a genuine collision artifact (an unrelatedNextCustomServergetter namedserverwas borrowingNextServer'sserver?: NextNodeServerbinding); every other project is byte-identical on member findings and all non-member issue types are byte-identical everywhere.Fixes #1821
Tests: extract unit tests pinning both colliding classes credited in both declaration orders (public and
#-private variants) plus a no-qualifier-in-output assertion, and a cross-module integration fixture mirroring the issue's privParam collision with dead control members asserted to stay flagged.