mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(scope-resolution): a named receiver's member never resolves lexically (#2699)
`lookupCore` Step 1 walked the lexical scope chain for every lookup, including explicit-receiver property reads. So `options.baseUrl` could bind to an unrelated function-local `const baseUrl` in the same file, and `config.extractVisibility(node)` to the enclosing class's own method. This is the residual half of the defect JS/TS block scopes narrowed in #2695. Blocks moved nested-block locals off the chain of a reference outside the block, which removed 114 false edges; a local declared directly in the function body stayed on it, and no amount of extra scopes reaches that case. Fixed at the cause instead: `recv.name` names a member of whatever `recv` denotes, so a binding of the bare tail name in an enclosing scope is never the right answer. Steps 2 and 3 (receiver type / owner members) are the legitimate routes. `this` and `self` are EXEMPT, and that exemption was measured, not assumed. Skipping Step 1 for every explicit receiver removed 711 edges on a 762-file corpus — but 2 of those were genuine: `self.srcIx` and `self.streamedAt(...)` after `const self = this`, reaching their own class's members through the class-body scope. For a self-receiver the members and the lexical chain legitimately overlap; for a named receiver they never do. Exempting the self names keeps both true edges and still removes 709 false ones, adding none. The removals were classified by reading source at the site, not by pattern- matching ids — an "is the target a member of the source's owner?" heuristic labelled 43 of them plausible and every one I then read was false: language = config.language; -> the class's own `language` dirMap.get(...) / exactMap.get(...) -> a sibling object-literal `get` return config.extractVisibility(n); -> the class's own method (self-edge) writer.close(); -> GraphEmitSink.close Residual, deliberately kept: a `this.x` read can still bind lexically to a same-named local. That is the price of the two true self-alias edges above. `INCREMENTAL_SCHEMA_VERSION` 19 -> 20: a v19 index holds these false CALLS/ACCESSES on every unchanged file and would keep serving them through the reuse gate. Test confirmed discriminating: it fails with the guard reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184RmD24KFJidYqpM7v3XjR
This commit is contained in:
parent
4906daf27b
commit
59b892cae7
4 changed files with 91 additions and 5 deletions
|
|
@ -108,7 +108,31 @@ export function lookupCore(
|
||||||
const perCandidate = new Map<DefId, CandidateState>();
|
const perCandidate = new Map<DefId, CandidateState>();
|
||||||
|
|
||||||
// ── Step 1: lexical scope-chain walk ──────────────────────────────────
|
// ── Step 1: lexical scope-chain walk ──────────────────────────────────
|
||||||
const lexicalShadowed = walkLexicalChain(name, startScope, acceptedKinds, ctx, perCandidate);
|
//
|
||||||
|
// SKIPPED for a NAMED explicit receiver. `recv.name` names a MEMBER of
|
||||||
|
// whatever `recv` denotes; it is not a lexical reference to `name`, so a
|
||||||
|
// binding of the bare tail name in an enclosing scope is never the right
|
||||||
|
// answer. Steps 2 and 3 (receiver type / owner members) are the routes.
|
||||||
|
//
|
||||||
|
// Without this, `options.baseUrl` bound to an unrelated function-local
|
||||||
|
// `const baseUrl` in the same file. This is the residual half of the defect
|
||||||
|
// JS/TS block scopes narrowed in #2699 — blocks moved nested-block locals
|
||||||
|
// off the chain, but a local declared directly in the function body stayed
|
||||||
|
// on it, and no amount of extra scopes reaches that case.
|
||||||
|
//
|
||||||
|
// `this` / `self` are deliberately EXEMPT. For a self-receiver the members
|
||||||
|
// and the lexical chain legitimately overlap — a class body is itself a
|
||||||
|
// scope that binds its members — so Step 1 is a real resolution route
|
||||||
|
// there, not a coincidence. Measured on a 762-file corpus: skipping Step 1
|
||||||
|
// for every explicit receiver dropped 711 edges, of which 43 were
|
||||||
|
// `this.member` reads reaching their own owner. Exempting the self names
|
||||||
|
// keeps those and still removes the 668 named-receiver false positives.
|
||||||
|
const skipLexical =
|
||||||
|
params.explicitReceiver !== undefined &&
|
||||||
|
!IMPLICIT_RECEIVERS.includes(params.explicitReceiver.name);
|
||||||
|
const lexicalShadowed = skipLexical
|
||||||
|
? false
|
||||||
|
: walkLexicalChain(name, startScope, acceptedKinds, ctx, perCandidate);
|
||||||
|
|
||||||
// ── Step 2: type-binding / MRO walk (methods/fields) ──────────────────
|
// ── Step 2: type-binding / MRO walk (methods/fields) ──────────────────
|
||||||
if (params.useReceiverTypeBinding && ctx.methodDispatch !== undefined) {
|
if (params.useReceiverTypeBinding && ctx.methodDispatch !== undefined) {
|
||||||
|
|
|
||||||
|
|
@ -518,8 +518,15 @@ export interface RepoMeta {
|
||||||
* destroying the javac-compatible JLS identity of #2550/#2555/#2562. An index stamped
|
* destroying the javac-compatible JLS identity of #2550/#2555/#2562. An index stamped
|
||||||
* v18 therefore holds WRONG Java ids, and without this bump it passes the reuse gate
|
* v18 therefore holds WRONG Java ids, and without this bump it passes the reuse gate
|
||||||
* and keeps them on every unchanged file; force a full re-analyze instead.
|
* and keeps them on every unchanged file; force a full re-analyze instead.
|
||||||
|
* v20: a NAMED explicit receiver no longer resolves its member through the lexical
|
||||||
|
* scope chain (#2699 follow-up). `options.baseUrl` used to bind to an unrelated
|
||||||
|
* function-local `const baseUrl`; measured on a 762-file corpus this removes 709
|
||||||
|
* such edges and adds none. `this`/`self` are exempt, so the 2 genuine self-alias
|
||||||
|
* reads it also covered are kept. A v19 index holds those false CALLS/ACCESSES on
|
||||||
|
* every unchanged file and would keep serving them through the reuse gate; force a
|
||||||
|
* full re-analyze instead.
|
||||||
*/
|
*/
|
||||||
export const INCREMENTAL_SCHEMA_VERSION = 19;
|
export const INCREMENTAL_SCHEMA_VERSION = 20;
|
||||||
|
|
||||||
export interface IndexedRepo {
|
export interface IndexedRepo {
|
||||||
repoPath: string;
|
repoPath: string;
|
||||||
|
|
|
||||||
|
|
@ -114,3 +114,53 @@ describeIfWorkerBuilt('block scopes keep a property read off a same-named block
|
||||||
expect(edges[0]).toContain('-> Property:box.ts:Box.baseUrl');
|
expect(edges[0]).toContain('-> Property:box.ts:Box.baseUrl');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describeIfWorkerBuilt('a property read never resolves to a lexical binding of its own name', () => {
|
||||||
|
// The residual half. Block scopes moved a NESTED-block local off the chain of
|
||||||
|
// a reference outside that block, which removed 114 false edges on a 762-file
|
||||||
|
// corpus. A local declared directly in the FUNCTION BODY stayed on the chain,
|
||||||
|
// so `options.baseUrl` still bound to it — same defect, one scope level up,
|
||||||
|
// and not fixable by adding more scopes.
|
||||||
|
//
|
||||||
|
// Fixed in `lookupCore` instead: Step 1's lexical walk is skipped when the
|
||||||
|
// site has an explicit receiver. `recv.name` names a member of whatever
|
||||||
|
// `recv` denotes; a binding of the bare tail name in an enclosing scope is
|
||||||
|
// never the right answer.
|
||||||
|
|
||||||
|
it('TypeScript: `options.baseUrl` does not ACCESS a function-body-level `const baseUrl`', async () => {
|
||||||
|
const edges = await accessEdgesFor(
|
||||||
|
'body.ts',
|
||||||
|
[
|
||||||
|
'export function pick(options: { baseUrl?: string }, fallback: string): string {',
|
||||||
|
' const baseUrl = fallback.trim();',
|
||||||
|
' if (baseUrl.length > 0) return baseUrl;',
|
||||||
|
' return options.baseUrl ?? fallback;',
|
||||||
|
'}',
|
||||||
|
'',
|
||||||
|
].join('\n'),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(edges.filter((e) => e.endsWith('baseUrl'))).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('TypeScript: a real member read still resolves through the receiver type', async () => {
|
||||||
|
// The guard against over-suppression: skipping Step 1 must not take Steps
|
||||||
|
// 2 and 3 with it. `this.baseUrl` has an explicit receiver too, and it
|
||||||
|
// must still reach the class property.
|
||||||
|
const edges = await accessEdgesFor(
|
||||||
|
'recv.ts',
|
||||||
|
[
|
||||||
|
'export class Box {',
|
||||||
|
" baseUrl = 'https://example.com';",
|
||||||
|
' read(): string {',
|
||||||
|
' return this.baseUrl;',
|
||||||
|
' }',
|
||||||
|
'}',
|
||||||
|
'',
|
||||||
|
].join('\n'),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(edges).toHaveLength(1);
|
||||||
|
expect(edges[0]).toContain('-> Property:recv.ts:Box.baseUrl');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -73,8 +73,8 @@ describe('CALL_SUMMARY relation-type exclusion (U-C1)', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => {
|
describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => {
|
||||||
it('INCREMENTAL_SCHEMA_VERSION is bumped to 19 (class-body boundary fix, #2699)', () => {
|
it('INCREMENTAL_SCHEMA_VERSION is bumped to 20 (named-receiver lexical fallback, #2699)', () => {
|
||||||
expect(INCREMENTAL_SCHEMA_VERSION).toBe(19);
|
expect(INCREMENTAL_SCHEMA_VERSION).toBe(20);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('a pre-current stamp fails the `=== INCREMENTAL_SCHEMA_VERSION` reuse gate → forces full re-analyze', () => {
|
it('a pre-current stamp fails the `=== INCREMENTAL_SCHEMA_VERSION` reuse gate → forces full re-analyze', () => {
|
||||||
|
|
@ -150,7 +150,12 @@ describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => {
|
||||||
// enclosing-callable walk on class DECLARATIONS only, so `Worker$1.run` was re-keyed
|
// enclosing-callable walk on class DECLARATIONS only, so `Worker$1.run` was re-keyed
|
||||||
// as `Worker.makeHandler.run@7:12`. Reusing it would keep those on unchanged files.
|
// as `Worker.makeHandler.run@7:12`. Reusing it would keep those on unchanged files.
|
||||||
expect(passesReuseGate(18)).toBe(false);
|
expect(passesReuseGate(18)).toBe(false);
|
||||||
|
// A pre-v20 (v19) index holds the false CALLS/ACCESSES a NAMED explicit receiver
|
||||||
|
// used to mint through the lexical chain (`options.baseUrl` → a function-local
|
||||||
|
// `const baseUrl`) — 709 of them on a 762-file corpus. Reusing it would keep
|
||||||
|
// every one on unchanged files.
|
||||||
|
expect(passesReuseGate(19)).toBe(false);
|
||||||
// A current-version stamp passes the gate (incremental top-up eligible).
|
// A current-version stamp passes the gate (incremental top-up eligible).
|
||||||
expect(passesReuseGate(19)).toBe(true);
|
expect(passesReuseGate(20)).toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue