Skip to content

Commit afca17f

Browse files
committed
fix(resolver): kind-filter same-file bare-name lookup for receiver-bearing calls
resolveCallTargets's same-file bare-name lookup (call-resolver.ts) and resolveByGlobal's exact-name fallback (resolver/strategy.ts) ran unconditionally for every call, unfiltered by symbol kind. A call with a receiver (this.x(), obj.x()) is logically "invoke a member of some instance" — a same-file class/interface/struct/variable that merely shared the call's bare name could win outright, before any more specific resolution tier (receiver typing, the Object.defineProperty accessor fallback, etc.) ever got a chance to run. Both lookups now restrict receiver-bearing calls to CALLABLE_SYMBOL_KINDS (function/method), a new shared constant in shared/kinds.ts. A genuinely bare call (no receiver at all) stays unfiltered, since at this layer it is indistinguishable from a `new ClassName()` constructor invocation, which legitimately targets a class-kind definition. Mirrored the identical fix in the native Rust engine (resolve_call_targets / resolve_exact_global_match in build_edges.rs), which reproduced the same bug. A broader fix (trying receiver-typed resolution before the bare lookup for concrete object receivers) was attempted but reverted: it regressed issue-1517's computed-property object-literal test, since that pattern emits two node representations for the same method and the bare lookup finds the more precise one. Filed #2025 to track that narrower residual. Refs #1888
1 parent 6ef1731 commit afca17f

6 files changed

Lines changed: 342 additions & 14 deletions

File tree

crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -947,14 +947,26 @@ fn find_enclosing_caller<'a>(defs: &[DefWithId<'a>], call_line: u32, file_node_i
947947
/// confidence (e.g. several files at the same directory depth from the
948948
/// caller) is genuinely ambiguous and returns nothing, letting the caller
949949
/// fall through to the narrower same-class-sibling fallback.
950+
///
951+
/// A `this`/`self`/`super` call is additionally restricted to callable kinds
952+
/// (`is_callable_kind`): such a call is logically "invoke a member of the
953+
/// current instance", which a class/interface/struct/etc. declaration can
954+
/// never satisfy, so an unrelated same-named type declaration must never
955+
/// substitute for a real callable target just because no other candidate
956+
/// exists (#1888). A genuinely bare call (no receiver at all) is left
957+
/// unfiltered — at this layer it is indistinguishable from a `new
958+
/// ClassName()` constructor invocation, which legitimately targets a
959+
/// class-kind definition.
950960
fn resolve_exact_global_match<'a>(
951961
ctx: &EdgeContext<'a>,
952962
call_name: &str,
953963
rel_path: &str,
964+
receiver: Option<&str>,
954965
) -> Vec<&'a NodeInfo> {
955966
let scored: Vec<(&'a NodeInfo, f64)> = ctx.nodes_by_name
956967
.get(call_name)
957968
.map(|v| v.iter()
969+
.filter(|&&n| receiver.is_none() || is_callable_kind(&n.kind))
958970
.map(|&n| (n, resolve::compute_confidence(rel_path, &n.file, None)))
959971
.filter(|&(_, confidence)| confidence >= 0.5)
960972
.collect())
@@ -1024,10 +1036,27 @@ fn resolve_call_targets<'a>(
10241036
if !pre_qualified.is_empty() { return pre_qualified; }
10251037
}
10261038

1027-
// 2. Same-file resolution
1028-
let targets = ctx.nodes_by_name_and_file
1039+
// 2. Same-file resolution. A receiver — concrete (`obj.x()`) or
1040+
// `this`/`self`/`super` — means the call is logically "invoke a member of
1041+
// some instance", which a class/interface/struct/etc. declaration can
1042+
// never satisfy; restrict those to definitively callable kinds
1043+
// (`is_callable_kind`) so an unrelated same-file type declaration that
1044+
// merely shares the call's name can never pre-empt a legitimate target
1045+
// that a more specific resolution tier (receiver typing, the
1046+
// Object.defineProperty accessor fallback, etc.) would otherwise find. A
1047+
// genuinely bare call (no receiver at all) is left unfiltered: at this
1048+
// layer it is indistinguishable from a `new ClassName()` constructor
1049+
// invocation, which legitimately targets a class-kind definition —
1050+
// kind-filtering it would break constructor-call resolution (#1888).
1051+
// Mirrors resolveCallTargets in call-resolver.ts.
1052+
let bare_matches = ctx.nodes_by_name_and_file
10291053
.get(&(call.name.as_str(), rel_path))
10301054
.cloned().unwrap_or_default();
1055+
let targets: Vec<&NodeInfo> = if call.receiver.is_some() {
1056+
bare_matches.into_iter().filter(|n| is_callable_kind(&n.kind)).collect()
1057+
} else {
1058+
bare_matches
1059+
};
10311060
if !targets.is_empty() { return targets; }
10321061

10331062
// 3. Type-aware resolution via receiver → type map.
@@ -1212,7 +1241,7 @@ fn resolve_call_targets<'a>(
12121241
}
12131242

12141243
// First try exact name match (e.g. an unqualified function named "area").
1215-
let exact = resolve_exact_global_match(ctx, call.name.as_str(), rel_path);
1244+
let exact = resolve_exact_global_match(ctx, call.name.as_str(), rel_path, call.receiver.as_deref());
12161245
if !exact.is_empty() { return exact; }
12171246

12181247
// Class-scoped exact lookup: prefer `ClassName.method` when the caller is a qualified

src/domain/graph/builder/call-resolver.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* `resolveByMethodOrGlobal` delegates its two branches to strategy helpers
1010
* in `../resolver/strategy.ts` to keep per-strategy complexity manageable.
1111
*/
12+
import { CALLABLE_SYMBOL_KINDS } from '../../../shared/kinds.js';
1213
import { computeConfidence, isSameLanguageFamily } from '../resolve.js';
1314
import {
1415
isModuleScopedLanguage,
@@ -113,14 +114,6 @@ export function resolveDefinePropertyAccessorTarget(
113114

114115
// ── Shared resolution functions ──────────────────────────────────────────
115116

116-
/**
117-
* Callable definition kinds — variable/constant bindings are NOT callable
118-
* in the function-as-enclosing-scope sense (they are local declarations, not
119-
* function bodies). Top-level variable bindings (e.g. Haskell `main = do …`)
120-
* are handled separately as a fallback tier.
121-
*/
122-
const CALLABLE_KINDS = new Set(['function', 'method']);
123-
124117
/**
125118
* Variable-like binding kinds that may act as top-level callers when no
126119
* enclosing function/method exists (e.g. Haskell top-level `main` is a
@@ -145,7 +138,7 @@ function findEnclosingCallable(
145138
let best: CallerMatch = null;
146139
let bestSpan = Infinity;
147140
for (const def of definitions) {
148-
if (!CALLABLE_KINDS.has(def.kind)) continue;
141+
if (!CALLABLE_SYMBOL_KINDS.has(def.kind)) continue;
149142
if (def.line > callLine) continue;
150143
const end = def.endLine ?? Infinity;
151144
if (callLine > end) continue;
@@ -300,7 +293,23 @@ export function resolveCallTargets(
300293
}
301294

302295
if (!targets || targets.length === 0) {
303-
targets = lookup.byNameAndFile(call.name, relPath);
296+
// Same-file bare-name lookup. A receiver — concrete (`obj.x()`) or
297+
// `this`/`self`/`super` — means the call is logically "invoke a member of
298+
// some instance", which a class/interface/struct/etc. declaration can
299+
// never satisfy; restrict those to definitively callable kinds so an
300+
// unrelated same-file type declaration that merely shares the call's name
301+
// can never pre-empt a legitimate target that a more specific resolution
302+
// tier (receiver typing, the Object.defineProperty accessor fallback,
303+
// etc.) would otherwise find. A genuinely bare call (no receiver at all)
304+
// is left unfiltered: at this layer it is indistinguishable from a `new
305+
// ClassName()` constructor invocation, which legitimately targets a
306+
// class-kind definition — kind-filtering it would break constructor-call
307+
// resolution (#1888).
308+
const bareMatches = lookup.byNameAndFile(call.name, relPath);
309+
targets = call.receiver
310+
? bareMatches.filter((n) => CALLABLE_SYMBOL_KINDS.has(n.kind ?? ''))
311+
: bareMatches;
312+
304313
if (targets.length === 0) {
305314
targets = resolveByMethodOrGlobal(
306315
lookup,

src/domain/graph/resolver/strategy.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* a circular dependency. The StrategyLookup interface mirrors CallNodeLookup structurally
1313
* (TypeScript structural typing ensures compatibility without an explicit import).
1414
*/
15+
import { CALLABLE_SYMBOL_KINDS } from '../../../shared/kinds.js';
1516
import { computeConfidence } from '../resolve.js';
1617

1718
// ── Lookup adapter (structural mirror of CallNodeLookup) ──────────────────────
@@ -357,14 +358,25 @@ function resolveViaSameClassSibling(
357358
* ambiguous: there is no principled way to prefer one over another, so
358359
* the match is dropped rather than fanning out, letting the caller fall
359360
* through to the narrower resolveViaSameClassSibling fallback.
361+
*
362+
* A `this`/`self`/`super` call is additionally restricted to
363+
* `CALLABLE_SYMBOL_KINDS`: such a call is logically "invoke a member of the
364+
* current instance", which a class/interface/struct/etc. declaration can
365+
* never satisfy, so an unrelated same-named type declaration must never
366+
* substitute for a real callable target just because no other candidate
367+
* exists (#1888). A genuinely bare call (no receiver at all) is left
368+
* unfiltered — at this layer it is indistinguishable from a `new
369+
* ClassName()` constructor invocation, which legitimately targets a
370+
* class-kind definition.
360371
*/
361372
function resolveExactGlobalMatch(
362373
lookup: StrategyLookup,
363-
call: { name: string },
374+
call: { name: string; receiver?: string | null },
364375
relPath: string,
365376
): ReadonlyArray<{ id: number; file: string }> {
366377
const scored = lookup
367378
.byName(call.name)
379+
.filter((target) => !call.receiver || CALLABLE_SYMBOL_KINDS.has(target.kind ?? ''))
368380
.map((target) => ({ target, confidence: computeConfidence(relPath, target.file, null) }))
369381
.filter((candidate) => candidate.confidence >= 0.5);
370382
if (scored.length === 0) return [];

src/shared/kinds.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,21 @@ export const EVERY_SYMBOL_KIND: readonly SymbolKind[] = [
4242
...EXTENDED_SYMBOL_KINDS,
4343
];
4444

45+
/**
46+
* Symbol kinds that represent an actual invocable definition. A call site can
47+
* legitimately target one of these — a class, interface, struct, or plain
48+
* variable/constant binding cannot, in the general case, and must never win a
49+
* same-name lookup that has no other type/receiver information to narrow it.
50+
*
51+
* Guards the "no other signal" tiers of call resolution — the same-file
52+
* bare-name lookup in `resolveCallTargets` and `resolveByGlobal`'s exact
53+
* global-name match (`call-resolver.ts`, `resolver/strategy.ts`) — so an
54+
* unrelated same-named class/interface/variable never masquerades as a real
55+
* callable target purely because those lookups otherwise carry no kind filter
56+
* (#1888).
57+
*/
58+
export const CALLABLE_SYMBOL_KINDS: ReadonlySet<string> = new Set(['function', 'method']);
59+
4560
// Backward compat: ALL_SYMBOL_KINDS stays as the core 10
4661
export const ALL_SYMBOL_KINDS: readonly CoreSymbolKind[] = CORE_SYMBOL_KINDS;
4762

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* Regression test for #1888: the same-file bare-name lookup in
3+
* `resolveCallTargets` (`lookup.byNameAndFile(call.name, relPath)`) ran
4+
* unconditionally for every call and was unfiltered by symbol kind. A call
5+
* with a receiver — `this.x()`, `obj.x()` — is logically "invoke a member of
6+
* some instance", so an unrelated same-file class/interface/etc. that merely
7+
* shared the call's bare name won outright, before any more specific
8+
* resolution tier (receiver typing, the Object.defineProperty accessor
9+
* fallback, etc.) ever got a chance to run. The exact same defect existed in
10+
* the mirrored Rust `resolve_call_targets` / `resolve_exact_global_match` in
11+
* `build_edges.rs` — confirmed identical on both engines before the fix.
12+
*
13+
* Repro (from the issue): `this.bar()` inside a plain function `getter`,
14+
* registered as a get-accessor for `obj` (an instance of `Registry`, which
15+
* defines `bar()`) via `Object.defineProperty`, plus an unrelated
16+
* `class bar {}` declared later in the same file.
17+
*
18+
* class Registry { bar() { return 1; } }
19+
* const obj = new Registry();
20+
* function getter() { this.bar(); }
21+
* Object.defineProperty(obj, 'x', { get: getter });
22+
* class bar {}
23+
*
24+
* Before the fix: `getter -> bar` (kind: class) — the coincidentally-named
25+
* class pre-empted the correctly-typed `Registry.bar` method, which was only
26+
* reachable via a later, more specific fallback tier that never got a
27+
* chance to run.
28+
*
29+
* After the fix: `getter -> Registry.bar` (kind: method), and the unrelated
30+
* `bar` class no longer receives a `calls` edge at all.
31+
*
32+
* The `new Registry()` constructor call (`obj -> Registry`) is also asserted
33+
* to still resolve — that call has no receiver at all, so it is
34+
* indistinguishable, at this resolution layer, from a plain bare call; kind
35+
* must NOT be filtered for it, or constructor-call resolution regresses.
36+
*/
37+
import fs from 'node:fs';
38+
import os from 'node:os';
39+
import path from 'node:path';
40+
import Database from 'better-sqlite3';
41+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
42+
import { buildGraph } from '../../src/domain/graph/builder.js';
43+
import { isNativeAvailable } from '../../src/infrastructure/native.js';
44+
import type { EngineMode } from '../../src/types.js';
45+
46+
const FIXTURE = `
47+
class Registry {
48+
bar() {
49+
return 1;
50+
}
51+
}
52+
53+
const obj = new Registry();
54+
55+
function getter() {
56+
this.bar();
57+
}
58+
59+
Object.defineProperty(obj, 'x', { get: getter });
60+
61+
class bar {}
62+
`;
63+
64+
interface CallEdgeRow {
65+
src: string;
66+
srcKind: string;
67+
tgt: string;
68+
tgtKind: string;
69+
}
70+
71+
function readCallEdges(dbPath: string): CallEdgeRow[] {
72+
const db = new Database(dbPath, { readonly: true });
73+
try {
74+
return db
75+
.prepare(
76+
`SELECT n1.name AS src, n1.kind AS srcKind, n2.name AS tgt, n2.kind AS tgtKind
77+
FROM edges e
78+
JOIN nodes n1 ON e.source_id = n1.id
79+
JOIN nodes n2 ON e.target_id = n2.id
80+
WHERE e.kind = 'calls'
81+
ORDER BY n1.name, n2.name`,
82+
)
83+
.all() as CallEdgeRow[];
84+
} finally {
85+
db.close();
86+
}
87+
}
88+
89+
function runScenario(engine: EngineMode): void {
90+
describe(`same-file bare-name lookup kind filter (#1888) — ${engine}`, () => {
91+
let dir: string;
92+
93+
beforeAll(async () => {
94+
dir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1888-${engine}-`));
95+
fs.writeFileSync(path.join(dir, 'repro.js'), FIXTURE);
96+
await buildGraph(dir, { engine, incremental: false, skipRegistry: true });
97+
}, 30_000);
98+
99+
afterAll(() => {
100+
fs.rmSync(dir, { recursive: true, force: true });
101+
});
102+
103+
it('resolves this.bar() to Registry.bar, not the unrelated class bar', () => {
104+
const edges = readCallEdges(path.join(dir, '.codegraph', 'graph.db'));
105+
106+
expect(edges).toContainEqual({
107+
src: 'getter',
108+
srcKind: 'function',
109+
tgt: 'Registry.bar',
110+
tgtKind: 'method',
111+
});
112+
expect(edges.some((e) => e.tgt === 'bar' && e.tgtKind === 'class')).toBe(false);
113+
});
114+
115+
it('still resolves the new Registry() constructor call (bare call, no receiver)', () => {
116+
const edges = readCallEdges(path.join(dir, '.codegraph', 'graph.db'));
117+
118+
expect(edges).toContainEqual({
119+
src: 'obj',
120+
srcKind: 'constant',
121+
tgt: 'Registry',
122+
tgtKind: 'class',
123+
});
124+
});
125+
});
126+
}
127+
128+
runScenario('wasm');
129+
describe.skipIf(!isNativeAvailable())('native engine coverage', () => {
130+
runScenario('native');
131+
});

0 commit comments

Comments
 (0)