From 0871bc777edcf2af25ad149aefa1ff66959ec9c3 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Fri, 14 Aug 2026 00:33:19 -0600 Subject: [PATCH 1/2] fix(cha): cha dispatch omits the receiver's own instantiated type resolveChaTargets/resolve_cha_dispatch only walked implementors (subclasses) starting from the receiver's declared type, never checking whether that type itself is instantiated. When a class is instantiated directly AND an unrelated file also declares a local subclass overriding the same method (e.g. a test double), the base class's own method was dropped from the resolved edge set while the unrelated subclass's override leaked in instead (#2348). Both engines now also resolve the receiver's own type via resolveMethodViaAncestors/resolve_method_via_ancestors, gated on strict new-expression evidence only (a new newExpressionTypes/ cha_new_expression_types set) rather than the merged instantiatedTypes/ cha_instantiated_types set (which also credits a bare high-confidence type annotation as "instantiated"). The merged set is too broad for this particular check: it would resurrect a distant interface's own bodyless method whenever some unrelated concrete subclass overrides the same method name, which regressed two existing native unit tests (cha_typed_dispatch_fallback_resolves_distant_interface_implementation and ..._respects_rta_filter) until this stricter gate was added. Adds a dual-engine regression test (issue-2348-cha-base-type-own-method.test.ts) reproducing the shape with a synthetic base class + two unrelated local override subclasses, verified to fail on both engines with the fix disabled and pass with it enabled. docs check acknowledged Impact: 6 functions changed, 0 affected --- .../graph/builder/stages/build_edges.rs | 77 +++++++- src/domain/graph/builder/cha.ts | 92 +++++++++- .../builder/stages/native-orchestrator.ts | 1 + ...ssue-2348-cha-base-type-own-method.test.ts | 168 ++++++++++++++++++ tests/unit/cha.test.ts | 14 +- 5 files changed, 343 insertions(+), 9 deletions(-) create mode 100644 tests/integration/issue-2348-cha-base-type-own-method.test.ts diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs index 33564e7b9..d218cfefc 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs @@ -248,6 +248,13 @@ struct EdgeContext<'a> { /// `FileEdgeInput` has no dedicated `newExpressions` list, so only that /// fallback branch applies here). cha_instantiated_types: HashSet<&'a str>, + /// STRICT subset of `cha_instantiated_types`: class names backed ONLY by + /// a literal `new X()` expression somewhere in this build pass, never by + /// the weaker type-annotation (confidence 0.9) heuristic. See + /// `collect_cha_instantiated_types`'s doc comment (issue #2348) for why + /// `resolve_cha_dispatch`'s receiver-own-type check needs this stricter + /// bar instead of the merged `cha_instantiated_types` set. + cha_new_expression_types: HashSet<&'a str>, } impl<'a> EdgeContext<'a> { @@ -272,6 +279,8 @@ impl<'a> EdgeContext<'a> { .copied() .collect(); let cha = build_cha_context(files); + let (cha_instantiated_types, cha_new_expression_types) = + collect_cha_instantiated_types(files); Self { nodes_by_name, nodes_by_name_and_file, @@ -286,7 +295,8 @@ impl<'a> EdgeContext<'a> { cha_implementors_by_file: cha.implementors_by_file, cha_parents: cha.parents, cha_parents_by_file: cha.parents_by_file, - cha_instantiated_types: collect_cha_instantiated_types(files), + cha_instantiated_types, + cha_new_expression_types, } } } @@ -398,12 +408,28 @@ fn add_to_file_scoped<'a>( /// both qualify) — covers instantiation evidence inferred indirectly (e.g. /// cross-file return-type propagation) that never produces a literal /// `new X()` in this file. -fn collect_cha_instantiated_types(files: &[FileEdgeInput]) -> HashSet<&str> { +/// +/// Returns `(instantiated, new_expression_only)` — the second set is the +/// STRICT subset sourced from (a) alone, excluding the weaker (b) +/// type-annotation heuristic. `resolve_cha_dispatch`'s receiver-own-type +/// check (#2348) needs this stricter signal: unlike a subclass BFS hit +/// (where the weaker, merged `instantiated` set was already the trusted +/// bar before this fix), re-opening the receiver's OWN qualified method — +/// which the earlier gated qualified-lookup tier already tried and rejected +/// on proximity grounds — must not be justified by a MERE type annotation +/// (e.g. a `db: SomeInterface` parameter), or every distant +/// interface/abstract method would wrongly gain a "calls" edge whenever ANY +/// concrete subclass elsewhere also happens to override the same method +/// name (regression caught by +/// `cha_typed_dispatch_fallback_resolves_distant_interface_implementation`). +fn collect_cha_instantiated_types(files: &[FileEdgeInput]) -> (HashSet<&str>, HashSet<&str>) { let mut instantiated = HashSet::new(); + let mut new_expression_only = HashSet::new(); for file in files { if let Some(new_expressions) = &file.new_expressions { for type_name in new_expressions { instantiated.insert(type_name.as_str()); + new_expression_only.insert(type_name.as_str()); } } for tm in &file.type_map { @@ -412,7 +438,7 @@ fn collect_cha_instantiated_types(files: &[FileEdgeInput]) -> HashSet<&str> { } } } - instantiated + (instantiated, new_expression_only) } /// Resolve `${method_name}` on `cls` or, if `cls` inherits it without @@ -511,6 +537,42 @@ fn resolve_method_via_ancestors<'a>( /// scoped bucket — `cha_implementors_by_file` is populated exactly when the /// child's own file also locally declares that parent, so the child is /// *guaranteed* to live in that same file. +/// +/// The receiver's own declared type (`type_name`) is a valid dispatch target +/// too, not just its subclasses. Previously this function only walked +/// `cha_implementors`/`cha_implementors_by_file` starting FROM `type_name` to +/// find children — it never checked whether `type_name` itself is +/// instantiated. When the receiver's own type is instantiated directly and +/// ALSO has an unrelated subclass overriding the same method (even a +/// test-file-local one), the base type's own method was silently dropped +/// from the result set while the unrelated subclass's override leaked in +/// instead (#2348). Resolving `type_name` via the same +/// `resolve_method_via_ancestors` helper used for children fixes this +/// symmetrically — a duplicate resolution of an already-correctly-resolved +/// edge is a no-op thanks to the caller's `seen_call_edges` dedup, so this +/// can only add a missing edge, never introduce a wrong one. +/// +/// This root-type check deliberately uses `ctx.cha_new_expression_types` +/// (STRICT: literal `new X()` evidence only) rather than the merged +/// `ctx.cha_instantiated_types` (which also credits a bare high-confidence +/// type-annotation, e.g. a `db: SomeInterface` parameter, as "instantiated"). +/// A child's BFS hit can safely trust the weaker merged signal because it is +/// additionally gated by actually walking the class hierarchy to reach that +/// child in the first place; the root has no such gate — `type_name` here is +/// exactly what the earlier, proximity-gated qualified lookup already tried +/// (and rejected) one tier up, so re-admitting it ungated on nothing more +/// than a type annotation would wrongly resurrect a distant interface's own +/// (bodyless) method purely because some unrelated concrete subclass happens +/// to override the same method name (regression caught by +/// `cha_typed_dispatch_fallback_resolves_distant_interface_implementation`). +/// +/// `type_name` is deliberately NOT given an explicit `'a` bound here: at one +/// call site (the inline-new-expression branch of `resolve_call_targets_core`) +/// it can be a reference into a locally-computed `String` that does not live +/// as long as `'a`. `resolve_method_via_ancestors` requires `cls: &'a str`, +/// so the root-type check below re-looks-up the matching interned key +/// straight out of `ctx.cha_new_expression_types` (which is genuinely `&'a +/// str`) via `HashSet::get`, rather than passing `type_name` itself. fn resolve_cha_dispatch<'a>( ctx: &EdgeContext<'a>, type_name: &str, @@ -522,6 +584,15 @@ fn resolve_cha_dispatch<'a>( let mut visited: HashSet<&str> = HashSet::new(); visited.insert(type_name); + if let Some(&interned_type_name) = ctx.cha_new_expression_types.get(type_name) { + results.extend(resolve_method_via_ancestors( + ctx, + interned_type_name, + caller_file, + method_name, + )); + } + while let Some((current, current_file)) = queue.pop_front() { let scoped = current_file.and_then(|f| { ctx.cha_implementors_by_file diff --git a/src/domain/graph/builder/cha.ts b/src/domain/graph/builder/cha.ts index 55276a00b..653164dc9 100644 --- a/src/domain/graph/builder/cha.ts +++ b/src/domain/graph/builder/cha.ts @@ -52,6 +52,21 @@ export interface ChaContext { readonly parentsByFile: ReadonlyMap; /** RTA: class names that appear in `new X()` anywhere in the project */ readonly instantiatedTypes: ReadonlySet; + /** + * STRICT subset of `instantiatedTypes`: class names backed ONLY by a + * literal `new X()` expression somewhere in the project, never by the + * weaker type-annotation (confidence >= 0.9) heuristic that also feeds + * `instantiatedTypes`. `resolveChaTargets`'s receiver-own-type check + * (#2348) needs this stricter bar: unlike a subclass BFS hit (which is + * additionally gated by actually walking the hierarchy to reach that + * child), the root has no such gate — its own type name is exactly what + * the earlier, proximity-gated qualified lookup already tried and + * rejected one tier up, so re-admitting it ungated on nothing more than a + * type annotation would wrongly resurrect a distant interface's own + * (bodyless) method purely because some unrelated concrete subclass + * happens to override the same method name. + */ + readonly newExpressionTypes: ReadonlySet; } export const EMPTY_CHA_CONTEXT: ChaContext = { @@ -60,6 +75,7 @@ export const EMPTY_CHA_CONTEXT: ChaContext = { parents: new Map(), parentsByFile: new Map(), instantiatedTypes: new Set(), + newExpressionTypes: new Set(), }; /** @@ -143,11 +159,21 @@ function addToFileScoped( * 8.5 dedicated `newExpressions` list (all `new X()` in the file), plus the * constructor-confidence typeMap fallback (confidence >= 0.9) that covers * codebases that haven't been re-parsed since Phase 8.5 was added. + * + * `newExpressionTypes` collects ONLY the first (strict) source — see + * `ChaContext.newExpressionTypes`'s doc comment for why the receiver-own-type + * check in `resolveChaTargets` (#2348) needs that stricter signal instead of + * the merged `instantiatedTypes` set. */ -function collectInstantiatedTypes(symbols: ExtractorOutput, instantiatedTypes: Set): void { +function collectInstantiatedTypes( + symbols: ExtractorOutput, + instantiatedTypes: Set, + newExpressionTypes: Set, +): void { if (symbols.newExpressions) { for (const typeName of symbols.newExpressions) { instantiatedTypes.add(typeName); + newExpressionTypes.add(typeName); } } if (symbols.typeMap instanceof Map) { @@ -171,6 +197,7 @@ export function buildChaContext(fileSymbols: ReadonlyMap(); const parentsByFile = new Map(); const instantiatedTypes = new Set(); + const newExpressionTypes = new Set(); for (const [file, symbols] of fileSymbols) { // `symbols.classes` only lists class RELATIONS (entries with an extends/ @@ -195,10 +222,17 @@ export function buildChaContext(fileSymbols: ReadonlyMap; const instantiatedTypes = new Set(rtaRows.map((r) => r.name)); - - return { implementors, implementorsByFile, parents, parentsByFile, instantiatedTypes }; + // This path's RTA evidence is ALREADY strict (a resolved constructor call + // in the DB, not a bare type-annotation heuristic — see this function's + // doc comment), so `newExpressionTypes` can safely reuse the same set + // rather than needing a separately-collected one (contrast + // `buildChaContext`, whose in-memory `instantiatedTypes` is a weaker + // merged signal and therefore needs a distinct strict subset). + const newExpressionTypes = instantiatedTypes; + + return { + implementors, + implementorsByFile, + parents, + parentsByFile, + instantiatedTypes, + newExpressionTypes, + }; } /** @@ -641,6 +689,34 @@ function resolveMethodViaAncestors( * scoped lookup falls back to the bare one when it finds nothing, so this is * never a regression — only a precision gain when file identity happens to * be known. + * + * The receiver's own declared type (`typeName`) is a valid dispatch target + * too, not just its subclasses. Previously this function only walked + * `chaCtx.implementors`/`implementorsByFile` starting FROM `typeName` to + * find children — it never checked whether `typeName` itself is + * instantiated. When the receiver's own type is instantiated directly and + * ALSO has an unrelated subclass overriding the same method (even a + * test-file-local one), the base type's own method was silently dropped + * from the result set while the unrelated subclass's override leaked in + * instead (#2348). Resolving `typeName` via the same + * `resolveMethodViaAncestors` helper used for children fixes this + * symmetrically — a duplicate resolution of an already-correctly-resolved + * edge is a no-op thanks to `emitChaCallEdgesForCall`'s `seenCallEdges` + * dedup, so this can only add a missing edge, never introduce a wrong one. + * + * This root-type check deliberately uses `chaCtx.newExpressionTypes` + * (STRICT: literal `new X()` evidence only) rather than the merged + * `chaCtx.instantiatedTypes` (which also credits a bare high-confidence + * type annotation, e.g. a `db: SomeInterface` parameter, as "instantiated"). + * A child's BFS hit can safely trust the weaker merged signal because it is + * additionally gated by actually walking the class hierarchy to reach that + * child in the first place; the root has no such gate — `typeName` here is + * exactly what the earlier, proximity-gated qualified lookup already tried + * (and rejected) one tier up (see `CHA_TYPED_DISPATCH_CONFIDENCE`'s call + * site in `build-edges.ts`), so re-admitting it ungated on nothing more than + * a type annotation would wrongly resurrect a distant interface's own + * (bodyless) method purely because some unrelated concrete subclass happens + * to override the same method name. */ export function resolveChaTargets( typeName: string, @@ -657,6 +733,12 @@ export function resolveChaTargets( const visited = new Set(); visited.add(typeName); + if (chaCtx.newExpressionTypes.has(typeName)) { + results.push( + ...resolveMethodViaAncestors(typeName, callerFile ?? null, methodName, chaCtx, lookup), + ); + } + while (queue.length > 0) { const { name: current, file: currentFile } = queue.shift()!; const scoped = currentFile diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index cbfeebe62..6cd28e325 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -1492,6 +1492,7 @@ async function runPostNativeThisDispatch( parents, parentsByFile, instantiatedTypes: new Set(), // not needed for this/super resolution + newExpressionTypes: new Set(), // not needed for this/super resolution }; const relFiles = selectThisDispatchFiles(db, changedFiles, isFullBuild); diff --git a/tests/integration/issue-2348-cha-base-type-own-method.test.ts b/tests/integration/issue-2348-cha-base-type-own-method.test.ts new file mode 100644 index 000000000..0df210b2e --- /dev/null +++ b/tests/integration/issue-2348-cha-base-type-own-method.test.ts @@ -0,0 +1,168 @@ +/** + * Regression test for #2348: `resolveChaTargets` (`cha.ts`) / + * `resolve_cha_dispatch` (`build_edges.rs`) BFS-walk only ever considered the + * receiver's SUBCLASSES as dispatch targets — it never checked whether the + * receiver's own declared type is itself instantiated and should resolve to + * its OWN method. When a base class is instantiated directly AND some + * completely unrelated file also declares a local subclass overriding the + * same method name, the base class's own (correct) method was silently + * dropped from the resolved edge set while the unrelated subclass's + * override leaked in as the (wrong) target instead. + * + * This mirrors the real-world repro exactly (`tests/unit/in-memory- + * repository.test.ts` calling `repo.findNodesForTriage()` on an + * `InMemoryRepository`, while `tests/integration/triage.test.ts` separately + * declares two unrelated local test-double subclasses — `BrokenRepo` and + * `InvalidOptsRepo` — each overriding `findNodesForTriage` inside their own + * `it()` callback) with a minimal synthetic fixture: + * + * - `src/domain/base.ts` declares `Base.run()` and is instantiated directly + * (`new Base()`). + * - `tests/unit/caller.ts` (deliberately far from `src/domain/` — mirrors + * the real repro's cross-directory distance, which pushes the + * proximity-gated direct qualified lookup below its confidence threshold + * and forces reliance on the CHA/RTA fallback) calls `b.run()` on a + * parameter typed `b: Base`. + * - `other/rogue-a.ts` and `other/rogue-b.ts` each declare their own LOCAL, + * lexically unrelated `RogueA`/`RogueB` class extending `Base` and + * overriding `run()`, instantiated inside their own local function scope + * — unrelated to the caller and to each other, matching + * `BrokenRepo`/`InvalidOptsRepo`'s shape. + * + * Both engines must resolve `caller.ts`'s `b.run()` call to `Base.run` + * (previously missing entirely) without that resolution being crowded out + * by the unrelated `RogueA.run`/`RogueB.run` overrides. + */ +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 type { EngineMode } from '../../src/types.js'; + +const FIXTURE: Record = { + 'src/domain/base.ts': ` +export class Base { + run(): string { + return 'base'; + } +} +`, + 'tests/unit/caller.ts': ` +import { Base } from '../../src/domain/base.js'; + +function useBase(b: Base): string { + return b.run(); +} + +const liveBase = new Base(); +useBase(liveBase); +`, + 'other/rogue-a.ts': ` +import { Base } from '../src/domain/base.js'; + +function runRogueA(): string { + class RogueA extends Base { + override run(): string { + return 'rogue-a'; + } + } + return new RogueA().run(); +} +runRogueA(); +`, + 'other/rogue-b.ts': ` +import { Base } from '../src/domain/base.js'; + +function runRogueB(): string { + class RogueB extends Base { + override run(): string { + return 'rogue-b'; + } + } + return new RogueB().run(); +} +runRogueB(); +`, +}; + +function writeFixture(rootDir: string) { + for (const [rel, content] of Object.entries(FIXTURE)) { + const abs = path.join(rootDir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } +} + +interface CallEdgeRow { + caller: string; + callee: string; + calleeFile: string; +} + +function readCallEdges(dbPath: string): CallEdgeRow[] { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS caller, n2.name AS callee, n2.file AS calleeFile + 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(); + } +} + +const ENGINES: EngineMode[] = ['wasm', 'native']; + +describe.each(ENGINES)( + "CHA dispatch includes the receiver's own instantiated type (%s, #2348)", + (engine) => { + let tmpDir: string; + let edges: CallEdgeRow[]; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `codegraph-2348-${engine}-`)); + writeFixture(tmpDir); + await buildGraph(tmpDir, { incremental: false, skipRegistry: true, engine }); + edges = readCallEdges(path.join(tmpDir, '.codegraph', 'graph.db')); + }, 60_000); + + afterAll(() => { + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("useBase's b.run() resolves to Base.run (the receiver's own instantiated type)", () => { + const toBase = edges.some((e) => e.caller === 'useBase' && e.callee === 'Base.run'); + expect( + toBase, + `Expected useBase -> Base.run.\nActual edges from useBase:\n${JSON.stringify( + edges.filter((e) => e.caller === 'useBase'), + null, + 2, + )}`, + ).toBe(true); + }); + + it('the unrelated local subclass overrides are not the ONLY resolved targets', () => { + const fromUseBase = edges.filter((e) => e.caller === 'useBase'); + const onlyRogue = + fromUseBase.length > 0 && + fromUseBase.every((e) => e.callee === 'RogueA.run' || e.callee === 'RogueB.run'); + expect( + onlyRogue, + `Expected Base.run among useBase's targets, not just the unrelated Rogue overrides.\nActual edges from useBase:\n${JSON.stringify( + fromUseBase, + null, + 2, + )}`, + ).toBe(false); + }); + }, +); diff --git a/tests/unit/cha.test.ts b/tests/unit/cha.test.ts index d54d9eb50..88a6e14c9 100644 --- a/tests/unit/cha.test.ts +++ b/tests/unit/cha.test.ts @@ -68,16 +68,27 @@ function makeChaCtx( parents: new Map(Object.entries(parents)), parentsByFile: new Map(Object.entries(parentsByFile)), instantiatedTypes: new Set(), + newExpressionTypes: new Set(), }; } -/** Build a ChaContext for resolveChaTargets tests (implementors-focused). */ +/** + * Build a ChaContext for resolveChaTargets tests (implementors-focused). + * + * `newExpressionTypes` defaults to the same names as `instantiatedTypes` + * when not given explicitly — every existing caller of this helper predates + * the `newExpressionTypes`/`instantiatedTypes` split (#2348) and already + * intends its `instantiatedTypes` entries to mean genuine construction + * evidence, so this keeps them passing unchanged. Pass `newExpressionTypes` + * explicitly to exercise the strict-vs-merged distinction itself. + */ function makeChaTargetsCtx(opts: { implementors?: Record; implementorsByFile?: Record; parents?: Record; parentsByFile?: Record; instantiatedTypes?: string[]; + newExpressionTypes?: string[]; }): ChaContext { return { implementors: new Map(Object.entries(opts.implementors ?? {})), @@ -85,6 +96,7 @@ function makeChaTargetsCtx(opts: { parents: new Map(Object.entries(opts.parents ?? {})), parentsByFile: new Map(Object.entries(opts.parentsByFile ?? {})), instantiatedTypes: new Set(opts.instantiatedTypes ?? []), + newExpressionTypes: new Set(opts.newExpressionTypes ?? opts.instantiatedTypes ?? []), }; } From 4dc445e5c5f848ddd0967f13477b94fdcba12435 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Fri, 14 Aug 2026 01:03:58 -0600 Subject: [PATCH 2/2] fix(cha): scope receiver-own-type RTA evidence by declaring file Greptile review on PR #2494: newExpressionTypes/cha_new_expression_types (added for #2348's receiver-own-type check) was a bare, project-wide set with no file scoping, unlike implementorsByFile/cha_implementors_by_file which this codebase already splits carefully for the same reason (#2237). Two unrelated files can each declare their own unrelated class with the same bare name; if only one is ever instantiated, the bare set couldn't tell them apart, and resolveMethodViaAncestors/ resolve_method_via_ancestors' own bare/global fallback could then resolve to the OTHER file's method. Both engines now also track, per file: which type names it locally declares (declaredTypeNamesByFile/cha_declared_type_names_by_file, mirroring the same local-declaration anchor recordImplements/ recordExtends already use for implementorsByFile) and which type names its own new-expression evidence names (newExpressionTypesByFile/cha_new_expression_types_by_file). The root-type check now prefers the file-scoped pair whenever the caller's file locally declares the receiver type - trusting a scoped miss as authoritative - and only falls back to the bare set when the caller's file has no local declaration to anchor against (the same accepted limitation implementorsByFile already has in that case). Adds a dual-engine regression test proving the disambiguation (three TS unit tests in cha.test.ts covering the collision, the same-file positive case, and the no-anchor fallback; one Rust integration-style unit test through build_call_edges), plus re-verifies the #2348 regression test and the real-world in-memory-repository.test.ts repro are unaffected on both engines. docs check acknowledged Impact: 7 functions changed, 0 affected --- .../graph/builder/stages/build_edges.rs | 224 ++++++++++++++++-- src/domain/graph/builder/cha.ts | 111 ++++++++- .../builder/stages/native-orchestrator.ts | 2 + tests/unit/cha.test.ts | 68 ++++++ 4 files changed, 378 insertions(+), 27 deletions(-) diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs index d218cfefc..fc58c00b0 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs @@ -254,7 +254,37 @@ struct EdgeContext<'a> { /// `collect_cha_instantiated_types`'s doc comment (issue #2348) for why /// `resolve_cha_dispatch`'s receiver-own-type check needs this stricter /// bar instead of the merged `cha_instantiated_types` set. + /// + /// Still a bare, project-wide set, though — it carries the SAME + /// cross-file same-name collision risk `cha_implementors_by_file` was + /// built to fix for `cha_implementors` (Greptile review, PR #2494): two + /// unrelated files can each declare their own unrelated class named e.g. + /// `Handler`, and if only ONE of them is ever instantiated, this bare set + /// can't tell them apart. `cha_new_expression_types_by_file` below exists + /// for exactly that. cha_new_expression_types: HashSet<&'a str>, + /// `${type_name}|${file}` → present when `type_name`'s OWN `new X()` + /// evidence was recorded specifically WITHIN `file` — the file-scoped + /// counterpart to `cha_new_expression_types`, mirroring + /// `cha_implementors_by_file`'s relationship to `cha_implementors`. + /// Unlike `cha_implementors_by_file` (positive-evidence-only, falls back + /// to the bare map on a simple key miss), `resolve_cha_dispatch`'s + /// root-type check needs a scoped miss to be authoritative whenever the + /// caller's file is a declaring anchor (see `cha_declared_type_names_by_file`) + /// — otherwise falling back to the bare set would immediately re-admit + /// the exact cross-file collision this set exists to prevent. + cha_new_expression_types_by_file: HashSet, + /// `${type_name}|${file}` → present when `file` locally declares a + /// class/interface/struct/type/module named `type_name` (the same anchor + /// check `build_cha_context` already computes locally for + /// `cha_implementors_by_file`, persisted here for reuse). Distinguishes + /// "the caller's file has its OWN local `type_name` to check against" + /// (trust `cha_new_expression_types_by_file` alone, even when it says + /// no) from "the caller's file has no local anchor at all" (fall back to + /// the bare, collision-prone `cha_new_expression_types` — the same + /// accepted limitation `cha_implementors_by_file` already has when no + /// local declaration exists). + cha_declared_type_names_by_file: HashSet, } impl<'a> EdgeContext<'a> { @@ -279,8 +309,12 @@ impl<'a> EdgeContext<'a> { .copied() .collect(); let cha = build_cha_context(files); - let (cha_instantiated_types, cha_new_expression_types) = - collect_cha_instantiated_types(files); + let ( + cha_instantiated_types, + cha_new_expression_types, + cha_new_expression_types_by_file, + cha_declared_type_names_by_file, + ) = collect_cha_instantiated_types(files); Self { nodes_by_name, nodes_by_name_and_file, @@ -297,6 +331,8 @@ impl<'a> EdgeContext<'a> { cha_parents_by_file: cha.parents_by_file, cha_instantiated_types, cha_new_expression_types, + cha_new_expression_types_by_file, + cha_declared_type_names_by_file, } } } @@ -409,27 +445,63 @@ fn add_to_file_scoped<'a>( /// cross-file return-type propagation) that never produces a literal /// `new X()` in this file. /// -/// Returns `(instantiated, new_expression_only)` — the second set is the -/// STRICT subset sourced from (a) alone, excluding the weaker (b) -/// type-annotation heuristic. `resolve_cha_dispatch`'s receiver-own-type -/// check (#2348) needs this stricter signal: unlike a subclass BFS hit -/// (where the weaker, merged `instantiated` set was already the trusted -/// bar before this fix), re-opening the receiver's OWN qualified method — -/// which the earlier gated qualified-lookup tier already tried and rejected -/// on proximity grounds — must not be justified by a MERE type annotation -/// (e.g. a `db: SomeInterface` parameter), or every distant -/// interface/abstract method would wrongly gain a "calls" edge whenever ANY -/// concrete subclass elsewhere also happens to override the same method -/// name (regression caught by +/// Returns `(instantiated, new_expression_only, new_expression_only_by_file, +/// declared_type_names_by_file)`. The second set is the STRICT subset +/// sourced from (a) alone, excluding the weaker (b) type-annotation +/// heuristic. `resolve_cha_dispatch`'s receiver-own-type check (#2348) needs +/// this stricter signal: unlike a subclass BFS hit (where the weaker, merged +/// `instantiated` set was already the trusted bar before this fix), +/// re-opening the receiver's OWN qualified method — which the earlier gated +/// qualified-lookup tier already tried and rejected on proximity grounds — +/// must not be justified by a MERE type annotation (e.g. a `db: +/// SomeInterface` parameter), or every distant interface/abstract method +/// would wrongly gain a "calls" edge whenever ANY concrete subclass +/// elsewhere also happens to override the same method name (regression +/// caught by /// `cha_typed_dispatch_fallback_resolves_distant_interface_implementation`). -fn collect_cha_instantiated_types(files: &[FileEdgeInput]) -> (HashSet<&str>, HashSet<&str>) { +/// +/// The third and fourth sets (Greptile review, PR #2494) additionally break +/// (a) and the local-declaration anchor check down PER FILE — see +/// `EdgeContext::cha_new_expression_types_by_file`'s and +/// `EdgeContext::cha_declared_type_names_by_file`'s doc comments for how +/// `resolve_cha_dispatch` combines them to disambiguate two unrelated files +/// that happen to declare the same bare class name. The local-declaration +/// filter mirrors `build_cha_context`'s own `local_names` computation +/// exactly (kept as a separate pass here rather than merged into that +/// function's loop, since this function already has its own single pass +/// over `files` for an unrelated purpose). +fn collect_cha_instantiated_types( + files: &[FileEdgeInput], +) -> ( + HashSet<&str>, + HashSet<&str>, + HashSet, + HashSet, +) { let mut instantiated = HashSet::new(); let mut new_expression_only = HashSet::new(); + let mut new_expression_only_by_file = HashSet::new(); + let mut declared_type_names_by_file = HashSet::new(); for file in files { + let local_names: HashSet<&str> = file + .definitions + .iter() + .filter(|d| { + matches!( + d.kind.as_str(), + "class" | "struct" | "interface" | "type" | "module" + ) + }) + .map(|d| d.name.as_str()) + .collect(); + for name in &local_names { + declared_type_names_by_file.insert(format!("{}|{}", name, file.file)); + } if let Some(new_expressions) = &file.new_expressions { for type_name in new_expressions { instantiated.insert(type_name.as_str()); new_expression_only.insert(type_name.as_str()); + new_expression_only_by_file.insert(format!("{}|{}", type_name, file.file)); } } for tm in &file.type_map { @@ -438,7 +510,12 @@ fn collect_cha_instantiated_types(files: &[FileEdgeInput]) -> (HashSet<&str>, Ha } } } - (instantiated, new_expression_only) + ( + instantiated, + new_expression_only, + new_expression_only_by_file, + declared_type_names_by_file, + ) } /// Resolve `${method_name}` on `cls` or, if `cls` inherits it without @@ -566,6 +643,24 @@ fn resolve_method_via_ancestors<'a>( /// to override the same method name (regression caught by /// `cha_typed_dispatch_fallback_resolves_distant_interface_implementation`). /// +/// `ctx.cha_new_expression_types` is STILL a bare, project-wide set, though +/// (Greptile review, PR #2494): two unrelated files can each declare their +/// own unrelated class with the same bare name (e.g. both name a class +/// `Handler`), and if only ONE of them is ever instantiated, a bare +/// `cha_new_expression_types.contains(type_name)` can't tell them apart — it +/// would treat that as proof THIS caller's `Handler` was instantiated too, +/// and `resolve_method_via_ancestors`'s own bare/global fallback could then +/// resolve to the OTHER file's `Handler.method`. So the check below prefers +/// the file-scoped `cha_new_expression_types_by_file` whenever `caller_file` +/// itself locally declares `type_name` (`cha_declared_type_names_by_file` — +/// the same anchor `cha_implementors_by_file` uses) — in that case a scoped +/// miss is trusted as an authoritative "not instantiated (in THIS file's +/// sense of `type_name`)", never falling through to the bare set. Only when +/// `caller_file` has no such local anchor at all (imports `type_name` from +/// elsewhere, or `caller_file` is unknown) does this fall back to the bare, +/// collision-prone `cha_new_expression_types` — the same accepted limitation +/// `cha_implementors_by_file` already has for that exact situation. +/// /// `type_name` is deliberately NOT given an explicit `'a` bound here: at one /// call site (the inline-new-expression branch of `resolve_call_targets_core`) /// it can be a reference into a locally-computed `String` that does not live @@ -584,13 +679,31 @@ fn resolve_cha_dispatch<'a>( let mut visited: HashSet<&str> = HashSet::new(); visited.insert(type_name); - if let Some(&interned_type_name) = ctx.cha_new_expression_types.get(type_name) { - results.extend(resolve_method_via_ancestors( - ctx, - interned_type_name, - caller_file, - method_name, - )); + let has_local_declaration = caller_file + .map(|f| { + ctx.cha_declared_type_names_by_file + .contains(&format!("{}|{}", type_name, f)) + }) + .unwrap_or(false); + let is_root_instantiated = if has_local_declaration { + caller_file + .map(|f| { + ctx.cha_new_expression_types_by_file + .contains(&format!("{}|{}", type_name, f)) + }) + .unwrap_or(false) + } else { + ctx.cha_new_expression_types.contains(type_name) + }; + if is_root_instantiated { + if let Some(&interned_type_name) = ctx.cha_new_expression_types.get(type_name) { + results.extend(resolve_method_via_ancestors( + ctx, + interned_type_name, + caller_file, + method_name, + )); + } } while let Some((current, current_file)) = queue.pop_front() { @@ -5697,6 +5810,71 @@ mod call_edge_tests { ); } + /// #2348 root-type check, cross-file same-name collision (Greptile review + /// on PR #2494): `src/domain/mod_a.ts` declares its OWN `Handler` class + /// AND instantiates it (`new_expressions` contains `Handler`). + /// `tests/unit/mod_b.ts` independently declares an UNRELATED `Handler` + /// with no `method` of its own, and never instantiates it anywhere. + /// Only mod_a's `Handler.method` exists under the bare qualified name + /// "Handler.method" project-wide. `useHandler` (in mod_b.ts) calls + /// `h.method()` on a parameter typed `Handler` — since mod_b.ts is far + /// enough from mod_a.ts that the proximity-gated qualified lookup (tier + /// 3, `typed`) rejects the cross-file match, resolution falls through to + /// the CHA fallback this test is guarding. Before the file-scoped fix, + /// the bare (project-wide) `cha_new_expression_types.contains("Handler")` + /// would have been true purely because of mod_a's UNRELATED instance, + /// wrongly admitting an edge to mod_a's `Handler.method` for a caller + /// whose own (never-instantiated) `Handler` has nothing to do with it. + #[test] + fn resolve_cha_dispatch_root_check_does_not_leak_across_same_named_unrelated_classes() { + let all_nodes = vec![ + node(1, "useHandler", "function", "tests/unit/mod_b.ts", 5), + node(2, "Handler", "class", "src/domain/mod_a.ts", 1), + node(3, "Handler.method", "method", "src/domain/mod_a.ts", 2), + node(4, "Handler", "class", "tests/unit/mod_b.ts", 1), + ]; + + let mut mod_a = make_file( + "src/domain/mod_a.ts", + 10, + vec![def("Handler", "class", 1, 3)], + vec![], + vec![], + vec![], + ); + mod_a.new_expressions = Some(vec!["Handler".to_string()]); + + let mod_b = make_file( + "tests/unit/mod_b.ts", + 20, + vec![ + def("Handler", "class", 1, 2), + def("useHandler", "function", 5, 8), + ], + vec![call("method", 6, Some("h"))], + vec![type_map_entry("h", "Handler", 0.9)], + vec![], + ); + + let edges = build_call_edges( + vec![mod_a, mod_b], + all_nodes, + vec![], + MAX_SOLVER_ITERATIONS, + None, + ); + + let calls_edges: Vec<_> = edges.iter().filter(|e| e.kind == "calls").collect(); + assert!( + calls_edges.iter().all(|e| e.target_id != 3), + "expected no calls edge to mod_a's unrelated Handler.method; got: {:?}", + calls_edges + .iter() + .map(|e| (e.source_id, e.target_id)) + .collect::>() + ); + } + /// #2139: CHA dispatch is additive, not a last-resort fallback — when the /// interface's own qualified method already passes the proximity gate /// (tier 3, `typed`), the caller still ALSO gets a CHA-expanded edge to diff --git a/src/domain/graph/builder/cha.ts b/src/domain/graph/builder/cha.ts index 653164dc9..8ea39c58b 100644 --- a/src/domain/graph/builder/cha.ts +++ b/src/domain/graph/builder/cha.ts @@ -65,8 +65,43 @@ export interface ChaContext { * type annotation would wrongly resurrect a distant interface's own * (bodyless) method purely because some unrelated concrete subclass * happens to override the same method name. + * + * `newExpressionTypes` is STILL a bare, project-wide set, though — it + * carries the SAME cross-file same-name collision risk `implementorsByFile` + * was built to fix for the implementors map (Greptile review, PR #2494): + * two unrelated files can each declare their own unrelated class named + * e.g. `Handler`, and if only ONE of them is ever instantiated, this bare + * set can't tell them apart — `newExpressionTypesByFile` below exists for + * exactly that. */ readonly newExpressionTypes: ReadonlySet; + /** + * `${typeName}|${file}` → present when `typeName`'s OWN `new X()` evidence + * was recorded specifically WITHIN `file` (i.e. `file`'s own + * `newExpressions` list contains `typeName`) — the file-scoped counterpart + * to `newExpressionTypes`, mirroring `implementorsByFile`'s relationship to + * `implementors`. Unlike `implementorsByFile` (a positive-evidence-only + * map that falls back to the bare map when a key is simply absent), + * `resolveChaTargets`'s root-type check needs to treat a scoped MISS as an + * authoritative "no" whenever the caller's file is a declaring anchor (see + * `declaredTypeNamesByFile`) — otherwise the bare `newExpressionTypes` + * fallback would immediately re-admit the exact cross-file collision this + * set exists to prevent. + */ + readonly newExpressionTypesByFile: ReadonlySet; + /** + * `${typeName}|${file}` → present when `file` locally declares a + * class/interface/struct/type/module named `typeName` (mirrors the + * `localClassNames` anchor check already used by `recordImplements`/ + * `recordExtends` for `implementorsByFile`, persisted here for reuse by + * `resolveChaTargets`'s root-type check). This is the signal that + * distinguishes "the caller's file has its OWN local `typeName` to check + * against" (trust `newExpressionTypesByFile` alone, even when it says no) + * from "the caller's file has no local anchor at all" (fall back to the + * bare, collision-prone `newExpressionTypes`, same accepted limitation + * `implementorsByFile` already has when no local declaration exists). + */ + readonly declaredTypeNamesByFile: ReadonlySet; } export const EMPTY_CHA_CONTEXT: ChaContext = { @@ -76,6 +111,8 @@ export const EMPTY_CHA_CONTEXT: ChaContext = { parentsByFile: new Map(), instantiatedTypes: new Set(), newExpressionTypes: new Set(), + newExpressionTypesByFile: new Set(), + declaredTypeNamesByFile: new Set(), }; /** @@ -164,16 +201,32 @@ function addToFileScoped( * `ChaContext.newExpressionTypes`'s doc comment for why the receiver-own-type * check in `resolveChaTargets` (#2348) needs that stricter signal instead of * the merged `instantiatedTypes` set. + * + * `newExpressionTypesByFile` and `declaredTypeNamesByFile` (Greptile review, + * PR #2494) additionally record, for THIS file specifically: which type + * names it locally declares (`localClassNames`, the same anchor set + * `recordImplements`/`recordExtends` already use), and which type names its + * own `newExpressions` evidence names — see their doc comments on + * `ChaContext` for how `resolveChaTargets` combines the two to disambiguate + * two unrelated files that happen to declare the same bare class name. */ function collectInstantiatedTypes( symbols: ExtractorOutput, instantiatedTypes: Set, newExpressionTypes: Set, + file: string, + localClassNames: ReadonlySet, + newExpressionTypesByFile: Set, + declaredTypeNamesByFile: Set, ): void { + for (const name of localClassNames) { + declaredTypeNamesByFile.add(`${name}|${file}`); + } if (symbols.newExpressions) { for (const typeName of symbols.newExpressions) { instantiatedTypes.add(typeName); newExpressionTypes.add(typeName); + newExpressionTypesByFile.add(`${typeName}|${file}`); } } if (symbols.typeMap instanceof Map) { @@ -198,6 +251,8 @@ export function buildChaContext(fileSymbols: ReadonlyMap(); const instantiatedTypes = new Set(); const newExpressionTypes = new Set(); + const newExpressionTypesByFile = new Set(); + const declaredTypeNamesByFile = new Set(); for (const [file, symbols] of fileSymbols) { // `symbols.classes` only lists class RELATIONS (entries with an extends/ @@ -222,7 +277,15 @@ export function buildChaContext(fileSymbols: ReadonlyMap; + .all() as Array<{ file: string; name: string }>; const instantiatedTypes = new Set(rtaRows.map((r) => r.name)); // This path's RTA evidence is ALREADY strict (a resolved constructor call // in the DB, not a bare type-annotation heuristic — see this function's @@ -332,6 +402,13 @@ export function buildChaContextFromDb(db: BetterSqlite3Database): ChaContext { // `buildChaContext`, whose in-memory `instantiatedTypes` is a weaker // merged signal and therefore needs a distinct strict subset). const newExpressionTypes = instantiatedTypes; + const newExpressionTypesByFile = new Set(rtaRows.map((r) => `${r.name}|${r.file}`)); + const declaredTypeNamesByFile = new Set(); + for (const [file, names] of localNamesByFile) { + for (const name of names) { + declaredTypeNamesByFile.add(`${name}|${file}`); + } + } return { implementors, @@ -340,6 +417,8 @@ export function buildChaContextFromDb(db: BetterSqlite3Database): ChaContext { parentsByFile, instantiatedTypes, newExpressionTypes, + newExpressionTypesByFile, + declaredTypeNamesByFile, }; } @@ -717,6 +796,24 @@ function resolveMethodViaAncestors( * a type annotation would wrongly resurrect a distant interface's own * (bodyless) method purely because some unrelated concrete subclass happens * to override the same method name. + * + * `newExpressionTypes` is STILL a bare, project-wide set, though (Greptile + * review, PR #2494): two unrelated files can each declare their own + * unrelated class with the same bare name (e.g. both name a class + * `Handler`), and if only ONE of them is ever instantiated, a bare + * `newExpressionTypes.has(typeName)` can't tell them apart — it would treat + * that as proof THIS caller's `Handler` was instantiated too, and + * `resolveMethodViaAncestors`'s own bare/global fallback could then resolve + * to the OTHER file's `Handler.method`. So the check below prefers the + * file-scoped `newExpressionTypesByFile` whenever `callerFile` itself + * locally declares `typeName` (`declaredTypeNamesByFile` — the same anchor + * `implementorsByFile` uses) — in that case a scoped MISS is trusted as an + * authoritative "not instantiated (in THIS file's sense of `typeName`)", + * never falling through to the bare set. Only when `callerFile` has no such + * local anchor at all (imports `typeName` from elsewhere, or `callerFile` is + * unknown) does this fall back to the bare, collision-prone + * `newExpressionTypes` — the same accepted limitation `implementorsByFile` + * already has for that exact situation. */ export function resolveChaTargets( typeName: string, @@ -733,7 +830,13 @@ export function resolveChaTargets( const visited = new Set(); visited.add(typeName); - if (chaCtx.newExpressionTypes.has(typeName)) { + const hasLocalDeclaration = callerFile + ? chaCtx.declaredTypeNamesByFile.has(`${typeName}|${callerFile}`) + : false; + const isRootInstantiated = hasLocalDeclaration + ? chaCtx.newExpressionTypesByFile.has(`${typeName}|${callerFile}`) + : chaCtx.newExpressionTypes.has(typeName); + if (isRootInstantiated) { results.push( ...resolveMethodViaAncestors(typeName, callerFile ?? null, methodName, chaCtx, lookup), ); diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index 6cd28e325..a2c1e4d4e 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -1493,6 +1493,8 @@ async function runPostNativeThisDispatch( parentsByFile, instantiatedTypes: new Set(), // not needed for this/super resolution newExpressionTypes: new Set(), // not needed for this/super resolution + newExpressionTypesByFile: new Set(), // not needed for this/super resolution + declaredTypeNamesByFile: new Set(), // not needed for this/super resolution }; const relFiles = selectThisDispatchFiles(db, changedFiles, isFullBuild); diff --git a/tests/unit/cha.test.ts b/tests/unit/cha.test.ts index 88a6e14c9..caa22ecac 100644 --- a/tests/unit/cha.test.ts +++ b/tests/unit/cha.test.ts @@ -69,6 +69,8 @@ function makeChaCtx( parentsByFile: new Map(Object.entries(parentsByFile)), instantiatedTypes: new Set(), newExpressionTypes: new Set(), + newExpressionTypesByFile: new Set(), + declaredTypeNamesByFile: new Set(), }; } @@ -81,6 +83,13 @@ function makeChaCtx( * intends its `instantiatedTypes` entries to mean genuine construction * evidence, so this keeps them passing unchanged. Pass `newExpressionTypes` * explicitly to exercise the strict-vs-merged distinction itself. + * + * `newExpressionTypesByFile`/`declaredTypeNamesByFile` (Greptile review, PR + * #2494) default to empty — every existing caller of this helper predates + * the file-scoped disambiguation split too, and an empty scoped set means + * `resolveChaTargets`'s root-type check always falls back to the bare + * `newExpressionTypes` set, exactly matching pre-#2494 behavior. Pass + * entries as `${typeName}|${file}` strings to exercise the scoping itself. */ function makeChaTargetsCtx(opts: { implementors?: Record; @@ -89,6 +98,8 @@ function makeChaTargetsCtx(opts: { parentsByFile?: Record; instantiatedTypes?: string[]; newExpressionTypes?: string[]; + newExpressionTypesByFile?: string[]; + declaredTypeNamesByFile?: string[]; }): ChaContext { return { implementors: new Map(Object.entries(opts.implementors ?? {})), @@ -97,6 +108,8 @@ function makeChaTargetsCtx(opts: { parentsByFile: new Map(Object.entries(opts.parentsByFile ?? {})), instantiatedTypes: new Set(opts.instantiatedTypes ?? []), newExpressionTypes: new Set(opts.newExpressionTypes ?? opts.instantiatedTypes ?? []), + newExpressionTypesByFile: new Set(opts.newExpressionTypesByFile ?? []), + declaredTypeNamesByFile: new Set(opts.declaredTypeNamesByFile ?? []), }; } @@ -393,6 +406,61 @@ describe('resolveChaTargets — cross-file same-name collision (issue #2237, par }); }); +describe('resolveChaTargets — receiver-own-type cross-file same-name collision (issue #2348, Greptile review on PR #2494)', () => { + // mod_a.ts declares its OWN Handler AND instantiates it (`new Handler()` + // recorded within mod_a.ts itself). mod_b.ts independently declares an + // UNRELATED Handler with no `method` of its own, and NEVER instantiates + // it anywhere. Only mod_a's Handler.method exists under the bare qualified + // name "Handler.method" in the lookup. A caller inside mod_b.ts must not + // have its own (never-instantiated) Handler treated as instantiated just + // because some unrelated same-named Handler elsewhere happens to be. + const lookup = makeLookup({ + 'Handler.method': [{ id: 1, file: 'mod_a.ts', kind: 'method', line: 1 }], + }); + + it("does not treat the caller's own type as instantiated merely because an unrelated same-named type elsewhere is", () => { + const chaCtx = makeChaTargetsCtx({ + instantiatedTypes: ['Handler'], + newExpressionTypes: ['Handler'], + newExpressionTypesByFile: ['Handler|mod_a.ts'], + declaredTypeNamesByFile: ['Handler|mod_a.ts', 'Handler|mod_b.ts'], + }); + + const result = resolveChaTargets('Handler', 'method', chaCtx, lookup, 'mod_b.ts'); + expect( + result, + `Expected [] (mod_b's own Handler was never instantiated); got: ${JSON.stringify(result)}`, + ).toEqual([]); + }); + + it('still resolves when the caller IS the file whose own type was instantiated', () => { + const chaCtx = makeChaTargetsCtx({ + instantiatedTypes: ['Handler'], + newExpressionTypes: ['Handler'], + newExpressionTypesByFile: ['Handler|mod_a.ts'], + declaredTypeNamesByFile: ['Handler|mod_a.ts', 'Handler|mod_b.ts'], + }); + + const result = resolveChaTargets('Handler', 'method', chaCtx, lookup, 'mod_a.ts'); + expect(result).toEqual([{ id: 1, file: 'mod_a.ts', kind: 'method', line: 1 }]); + }); + + it('falls back to the bare set when the caller file has no local declaration to disambiguate against', () => { + // mod_c.ts never declares its own Handler at all (only imports one) — no + // scoped anchor exists to trust, so this keeps the pre-#2494 fallback + // behavior, same accepted limitation `implementorsByFile` already has. + const chaCtx = makeChaTargetsCtx({ + instantiatedTypes: ['Handler'], + newExpressionTypes: ['Handler'], + newExpressionTypesByFile: ['Handler|mod_a.ts'], + declaredTypeNamesByFile: ['Handler|mod_a.ts'], + }); + + const result = resolveChaTargets('Handler', 'method', chaCtx, lookup, 'mod_c.ts'); + expect(result).toEqual([{ id: 1, file: 'mod_a.ts', kind: 'method', line: 1 }]); + }); +}); + describe('resolveChaTargets — inherited (non-overriding) method walk (issue #2237, part 2)', () => { it('walks up to the declaring ancestor when the instantiated class inherits without overriding', () => { // ConcreteHandler is instantiated and implements Handler transitively via