Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -947,14 +947,26 @@ fn find_enclosing_caller<'a>(defs: &[DefWithId<'a>], call_line: u32, file_node_i
/// confidence (e.g. several files at the same directory depth from the
/// caller) is genuinely ambiguous and returns nothing, letting the caller
/// fall through to the narrower same-class-sibling fallback.
///
/// A `this`/`self`/`super` call is additionally restricted to callable kinds
/// (`is_callable_kind`): such a call is logically "invoke a member of the
/// current instance", which a class/interface/struct/etc. declaration can
/// never satisfy, so an unrelated same-named type declaration must never
/// substitute for a real callable target just because no other candidate
/// exists (#1888). A genuinely bare call (no receiver at all) is left
/// unfiltered — at this layer it is indistinguishable from a `new
/// ClassName()` constructor invocation, which legitimately targets a
/// class-kind definition.
fn resolve_exact_global_match<'a>(
ctx: &EdgeContext<'a>,
call_name: &str,
rel_path: &str,
receiver: Option<&str>,
) -> Vec<&'a NodeInfo> {
let scored: Vec<(&'a NodeInfo, f64)> = ctx.nodes_by_name
.get(call_name)
.map(|v| v.iter()
.filter(|&&n| receiver.is_none() || is_callable_kind(&n.kind))
.map(|&n| (n, resolve::compute_confidence(rel_path, &n.file, None)))
.filter(|&(_, confidence)| confidence >= 0.5)
.collect())
Expand Down Expand Up @@ -1024,10 +1036,27 @@ fn resolve_call_targets<'a>(
if !pre_qualified.is_empty() { return pre_qualified; }
}

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

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

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

// Class-scoped exact lookup: prefer `ClassName.method` when the caller is a qualified
Expand Down
29 changes: 19 additions & 10 deletions src/domain/graph/builder/call-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* `resolveByMethodOrGlobal` delegates its two branches to strategy helpers
* in `../resolver/strategy.ts` to keep per-strategy complexity manageable.
*/
import { CALLABLE_SYMBOL_KINDS } from '../../../shared/kinds.js';
import { computeConfidence, isSameLanguageFamily } from '../resolve.js';
import {
isModuleScopedLanguage,
Expand Down Expand Up @@ -113,14 +114,6 @@ export function resolveDefinePropertyAccessorTarget(

// ── Shared resolution functions ──────────────────────────────────────────

/**
* Callable definition kinds — variable/constant bindings are NOT callable
* in the function-as-enclosing-scope sense (they are local declarations, not
* function bodies). Top-level variable bindings (e.g. Haskell `main = do …`)
* are handled separately as a fallback tier.
*/
const CALLABLE_KINDS = new Set(['function', 'method']);

/**
* Variable-like binding kinds that may act as top-level callers when no
* enclosing function/method exists (e.g. Haskell top-level `main` is a
Expand All @@ -145,7 +138,7 @@ function findEnclosingCallable(
let best: CallerMatch = null;
let bestSpan = Infinity;
for (const def of definitions) {
if (!CALLABLE_KINDS.has(def.kind)) continue;
if (!CALLABLE_SYMBOL_KINDS.has(def.kind)) continue;
if (def.line > callLine) continue;
const end = def.endLine ?? Infinity;
if (callLine > end) continue;
Expand Down Expand Up @@ -300,7 +293,23 @@ export function resolveCallTargets(
}

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

if (targets.length === 0) {
targets = resolveByMethodOrGlobal(
lookup,
Expand Down
14 changes: 13 additions & 1 deletion src/domain/graph/resolver/strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* a circular dependency. The StrategyLookup interface mirrors CallNodeLookup structurally
* (TypeScript structural typing ensures compatibility without an explicit import).
*/
import { CALLABLE_SYMBOL_KINDS } from '../../../shared/kinds.js';
import { computeConfidence } from '../resolve.js';

// ── Lookup adapter (structural mirror of CallNodeLookup) ──────────────────────
Expand Down Expand Up @@ -357,14 +358,25 @@ function resolveViaSameClassSibling(
* ambiguous: there is no principled way to prefer one over another, so
* the match is dropped rather than fanning out, letting the caller fall
* through to the narrower resolveViaSameClassSibling fallback.
*
* A `this`/`self`/`super` call is additionally restricted to
* `CALLABLE_SYMBOL_KINDS`: such a call is logically "invoke a member of the
* current instance", which a class/interface/struct/etc. declaration can
* never satisfy, so an unrelated same-named type declaration must never
* substitute for a real callable target just because no other candidate
* exists (#1888). A genuinely bare call (no receiver at all) is left
* unfiltered — at this layer it is indistinguishable from a `new
* ClassName()` constructor invocation, which legitimately targets a
* class-kind definition.
*/
function resolveExactGlobalMatch(
lookup: StrategyLookup,
call: { name: string },
call: { name: string; receiver?: string | null },
relPath: string,
): ReadonlyArray<{ id: number; file: string }> {
const scored = lookup
.byName(call.name)
.filter((target) => !call.receiver || CALLABLE_SYMBOL_KINDS.has(target.kind ?? ''))
.map((target) => ({ target, confidence: computeConfidence(relPath, target.file, null) }))
.filter((candidate) => candidate.confidence >= 0.5);
if (scored.length === 0) return [];
Expand Down
15 changes: 15 additions & 0 deletions src/shared/kinds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ export const EVERY_SYMBOL_KIND: readonly SymbolKind[] = [
...EXTENDED_SYMBOL_KINDS,
];

/**
* Symbol kinds that represent an actual invocable definition. A call site can
* legitimately target one of these — a class, interface, struct, or plain
* variable/constant binding cannot, in the general case, and must never win a
* same-name lookup that has no other type/receiver information to narrow it.
*
* Guards the "no other signal" tiers of call resolution — the same-file
* bare-name lookup in `resolveCallTargets` and `resolveByGlobal`'s exact
* global-name match (`call-resolver.ts`, `resolver/strategy.ts`) — so an
* unrelated same-named class/interface/variable never masquerades as a real
* callable target purely because those lookups otherwise carry no kind filter
* (#1888).
*/
export const CALLABLE_SYMBOL_KINDS: ReadonlySet<string> = new Set(['function', 'method']);

// Backward compat: ALL_SYMBOL_KINDS stays as the core 10
export const ALL_SYMBOL_KINDS: readonly CoreSymbolKind[] = CORE_SYMBOL_KINDS;

Expand Down
131 changes: 131 additions & 0 deletions tests/integration/issue-1888-same-file-bare-name-kind-filter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/**
* Regression test for #1888: the same-file bare-name lookup in
* `resolveCallTargets` (`lookup.byNameAndFile(call.name, relPath)`) ran
* unconditionally for every call and was unfiltered by symbol kind. A call
* with a receiver — `this.x()`, `obj.x()` — is logically "invoke a member of
* some instance", so an unrelated same-file class/interface/etc. that merely
* shared the call's bare name won outright, before any more specific
* resolution tier (receiver typing, the Object.defineProperty accessor
* fallback, etc.) ever got a chance to run. The exact same defect existed in
* the mirrored Rust `resolve_call_targets` / `resolve_exact_global_match` in
* `build_edges.rs` — confirmed identical on both engines before the fix.
*
* Repro (from the issue): `this.bar()` inside a plain function `getter`,
* registered as a get-accessor for `obj` (an instance of `Registry`, which
* defines `bar()`) via `Object.defineProperty`, plus an unrelated
* `class bar {}` declared later in the same file.
*
* class Registry { bar() { return 1; } }
* const obj = new Registry();
* function getter() { this.bar(); }
* Object.defineProperty(obj, 'x', { get: getter });
* class bar {}
*
* Before the fix: `getter -> bar` (kind: class) — the coincidentally-named
* class pre-empted the correctly-typed `Registry.bar` method, which was only
* reachable via a later, more specific fallback tier that never got a
* chance to run.
*
* After the fix: `getter -> Registry.bar` (kind: method), and the unrelated
* `bar` class no longer receives a `calls` edge at all.
*
* The `new Registry()` constructor call (`obj -> Registry`) is also asserted
* to still resolve — that call has no receiver at all, so it is
* indistinguishable, at this resolution layer, from a plain bare call; kind
* must NOT be filtered for it, or constructor-call resolution regresses.
*/
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import Database from 'better-sqlite3';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { buildGraph } from '../../src/domain/graph/builder.js';
import { isNativeAvailable } from '../../src/infrastructure/native.js';
import type { EngineMode } from '../../src/types.js';

const FIXTURE = `
class Registry {
bar() {
return 1;
}
}

const obj = new Registry();

function getter() {
this.bar();
}

Object.defineProperty(obj, 'x', { get: getter });

class bar {}
`;

interface CallEdgeRow {
src: string;
srcKind: string;
tgt: string;
tgtKind: string;
}

function readCallEdges(dbPath: string): CallEdgeRow[] {
const db = new Database(dbPath, { readonly: true });
try {
return db
.prepare(
`SELECT n1.name AS src, n1.kind AS srcKind, n2.name AS tgt, n2.kind AS tgtKind
FROM edges e
JOIN nodes n1 ON e.source_id = n1.id
JOIN nodes n2 ON e.target_id = n2.id
WHERE e.kind = 'calls'
ORDER BY n1.name, n2.name`,
)
.all() as CallEdgeRow[];
} finally {
db.close();
}
}

function runScenario(engine: EngineMode): void {
describe(`same-file bare-name lookup kind filter (#1888) — ${engine}`, () => {
let dir: string;

beforeAll(async () => {
dir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1888-${engine}-`));
fs.writeFileSync(path.join(dir, 'repro.js'), FIXTURE);
await buildGraph(dir, { engine, incremental: false, skipRegistry: true });
}, 30_000);

afterAll(() => {
fs.rmSync(dir, { recursive: true, force: true });
});

it('resolves this.bar() to Registry.bar, not the unrelated class bar', () => {
const edges = readCallEdges(path.join(dir, '.codegraph', 'graph.db'));

expect(edges).toContainEqual({
src: 'getter',
srcKind: 'function',
tgt: 'Registry.bar',
tgtKind: 'method',
});
expect(edges.some((e) => e.tgt === 'bar' && e.tgtKind === 'class')).toBe(false);
});

it('still resolves the new Registry() constructor call (bare call, no receiver)', () => {
const edges = readCallEdges(path.join(dir, '.codegraph', 'graph.db'));

expect(edges).toContainEqual({
src: 'obj',
srcKind: 'constant',
tgt: 'Registry',
tgtKind: 'class',
});
});
});
}

runScenario('wasm');
describe.skipIf(!isNativeAvailable())('native engine coverage', () => {
runScenario('native');
});
Loading
Loading