From dc3a14ec6fb6f10ba3e1c61b9e8d9d3d2151dc27 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Fri, 5 Jun 2026 20:20:34 -0600 Subject: [PATCH 1/2] fix(edge_builder): restrict same-file this-dispatch fallback to caller's own class When a file contains multiple unrelated classes all defining a method with the same name, the broader same-file suffix scan emitted false-positive call edges (e.g. this.area() in Shape.describe matched Calculator.area and Formatter.area). The fix: when the scan finds more than one method with the matching suffix, restrict the result to methods whose qualified name starts with the caller's own class prefix. A single unambiguous match is returned as-is (handles the CHA case of one subclass override). If multiple classes match and none is the caller's class, return nothing rather than emitting false edges. WASM tests are active; native tests marked todo pending next binary release. Closes #1324 docs check acknowledged --- crates/codegraph-core/src/edge_builder.rs | 22 ++++- tests/fixtures/this-dispatch-scope/shapes.ts | 24 +++++ tests/integration/this-dispatch-scope.test.ts | 98 +++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/this-dispatch-scope/shapes.ts create mode 100644 tests/integration/this-dispatch-scope.test.ts diff --git a/crates/codegraph-core/src/edge_builder.rs b/crates/codegraph-core/src/edge_builder.rs index f504cdf24..5b75a002e 100644 --- a/crates/codegraph-core/src/edge_builder.rs +++ b/crates/codegraph-core/src/edge_builder.rs @@ -461,14 +461,32 @@ fn resolve_call_targets<'a>( } // Broader fallback: same-file suffix scan to pick up CHA-expanded targets - // (subclasses that override the method). + // (e.g. a subclass that overrides the method without the parent redefining it). + // When multiple classes in the file define the same method name, restrict to + // the caller's own class prefix to avoid false-positive edges to unrelated + // classes (e.g. this.area() in Shape must not match Calculator.area). let suffix = format!(".{}", call.name); if let Some(file_nodes) = ctx.nodes_by_file.get(rel_path) { let same_file_methods: Vec<&NodeInfo> = file_nodes.iter() .filter(|n| n.kind == "method" && n.name.ends_with(&suffix)) .copied() .collect(); - if !same_file_methods.is_empty() { return same_file_methods; } + match same_file_methods.len() { + 0 => {} + 1 => return same_file_methods, + _ => { + // Multiple classes define this method — restrict to the caller's own + // class prefix; return nothing if no match to avoid false edges. + if let Some(dot_pos) = caller_name.find('.') { + let caller_prefix = format!("{}.", &caller_name[..dot_pos]); + let caller_scoped: Vec<&NodeInfo> = same_file_methods.iter() + .filter(|n| n.name.starts_with(&caller_prefix)) + .copied() + .collect(); + if !caller_scoped.is_empty() { return caller_scoped; } + } + } + } } } return exact; // empty diff --git a/tests/fixtures/this-dispatch-scope/shapes.ts b/tests/fixtures/this-dispatch-scope/shapes.ts new file mode 100644 index 000000000..bac25f6f6 --- /dev/null +++ b/tests/fixtures/this-dispatch-scope/shapes.ts @@ -0,0 +1,24 @@ +// Three unrelated classes in one file, each with an area() method. +// this.area() inside Shape.describe must resolve only to Shape.area, +// not to Calculator.area or Formatter.area. + +export class Shape { + describe(): string { + return `area=${this.area()}`; + } + area(): number { + return 0; + } +} + +export class Calculator { + area(): number { + return 100; + } +} + +export class Formatter { + area(): string { + return 'n/a'; + } +} diff --git a/tests/integration/this-dispatch-scope.test.ts b/tests/integration/this-dispatch-scope.test.ts new file mode 100644 index 000000000..be3697492 --- /dev/null +++ b/tests/integration/this-dispatch-scope.test.ts @@ -0,0 +1,98 @@ +/** + * this-dispatch scope: same-file fallback must not emit false-positive edges + * to methods in unrelated classes. + * + * Fixture: shapes.ts — three unrelated classes (Shape, Calculator, Formatter) + * all defining area(). this.area() inside Shape.describe must resolve only to + * Shape.area. Edges to Calculator.area and Formatter.area must not appear. + * + * Covers the Rust edge_builder fix in issue #1324. + */ + +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_DIR = path.join(import.meta.dirname, '..', 'fixtures', 'this-dispatch-scope'); + +interface CallEdgeRow { + caller_name: string; + callee_name: string; +} + +function readCallEdges(dbPath: string): CallEdgeRow[] { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS caller_name, n2.name AS callee_name + 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'`, + ) + .all() as CallEdgeRow[]; + } finally { + db.close(); + } +} + +const ENGINES: EngineMode[] = ['wasm', 'native']; + +describe.each(ENGINES)('this-dispatch scope (%s)', (engine) => { + let tmpDir: string; + let callEdges: CallEdgeRow[]; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `codegraph-this-scope-${engine}-`)); + fs.cpSync(FIXTURE_DIR, tmpDir, { recursive: true }); + await buildGraph(tmpDir, { incremental: false, skipRegistry: true, engine }); + callEdges = readCallEdges(path.join(tmpDir, '.codegraph', 'graph.db')); + }, 60_000); + + afterAll(() => { + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('emits Shape.describe → Shape.area (correct this-dispatch)', () => { + const edge = callEdges.find( + (e) => e.caller_name === 'Shape.describe' && e.callee_name === 'Shape.area', + ); + expect( + edge, + `Expected Shape.describe → Shape.area edge.\nActual edges:\n${JSON.stringify(callEdges, null, 2)}`, + ).toBeDefined(); + }); + + // Native binary v3.11.2 does not include the edge_builder.rs fix for issue #1324 yet. + // These assertions are active for WASM and will be re-enabled for native once a new + // binary is published that includes the Rust fix. + if (engine === 'native') { + it.todo('does NOT emit Shape.describe → Calculator.area (native binary gap #1324)'); + it.todo('does NOT emit Shape.describe → Formatter.area (native binary gap #1324)'); + } else { + it('does NOT emit Shape.describe → Calculator.area (unrelated class, same method name)', () => { + const edge = callEdges.find( + (e) => e.caller_name === 'Shape.describe' && e.callee_name === 'Calculator.area', + ); + expect( + edge, + `Expected NO Shape.describe → Calculator.area edge (false-positive from same-file scan).\nActual edges:\n${JSON.stringify(callEdges, null, 2)}`, + ).toBeUndefined(); + }); + + it('does NOT emit Shape.describe → Formatter.area (unrelated class, same method name)', () => { + const edge = callEdges.find( + (e) => e.caller_name === 'Shape.describe' && e.callee_name === 'Formatter.area', + ); + expect( + edge, + `Expected NO Shape.describe → Formatter.area edge (false-positive from same-file scan).\nActual edges:\n${JSON.stringify(callEdges, null, 2)}`, + ).toBeUndefined(); + }); + } +}); From 77086621b5e7da34a7b4eb9a44282457dd56b459 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Fri, 5 Jun 2026 22:11:22 -0600 Subject: [PATCH 2/2] fix(edge_builder): apply caller-class scope to single-match suffix scan too (#1343) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 1-match arm of the same-file suffix scan was returning any sole method regardless of whether it belonged to the caller's class. A file with Caller (no area()) and Sibling (area()) would produce a false Caller.run → Sibling.area edge — the same bug fixed for 2+ matches. Replace the three-arm match with a unified caller-prefix filter applied to all non-empty results. Add fixture single-sibling.ts and a corresponding WASM assertion to cover the single-match false-positive path. Initialize callEdges to [] for cleaner beforeAll failure mode. --- crates/codegraph-core/src/edge_builder.rs | 32 ++++++++----------- .../this-dispatch-scope/single-sibling.ts | 16 ++++++++++ tests/integration/this-dispatch-scope.test.ts | 28 +++++++++++++--- 3 files changed, 53 insertions(+), 23 deletions(-) create mode 100644 tests/fixtures/this-dispatch-scope/single-sibling.ts diff --git a/crates/codegraph-core/src/edge_builder.rs b/crates/codegraph-core/src/edge_builder.rs index 5b75a002e..3e20a60e1 100644 --- a/crates/codegraph-core/src/edge_builder.rs +++ b/crates/codegraph-core/src/edge_builder.rs @@ -460,31 +460,25 @@ fn resolve_call_targets<'a>( if !class_scoped.is_empty() { return class_scoped; } } - // Broader fallback: same-file suffix scan to pick up CHA-expanded targets - // (e.g. a subclass that overrides the method without the parent redefining it). - // When multiple classes in the file define the same method name, restrict to - // the caller's own class prefix to avoid false-positive edges to unrelated - // classes (e.g. this.area() in Shape must not match Calculator.area). + // Broader fallback: same-file suffix scan. Always restrict to the caller's + // own class prefix — regardless of how many matches are found — to avoid + // false-positive edges to unrelated classes in the same file. + // (e.g. this.area() inside Shape.describe must never yield Calculator.area, + // even when Calculator.area is the only method with that name in the file.) let suffix = format!(".{}", call.name); if let Some(file_nodes) = ctx.nodes_by_file.get(rel_path) { let same_file_methods: Vec<&NodeInfo> = file_nodes.iter() .filter(|n| n.kind == "method" && n.name.ends_with(&suffix)) .copied() .collect(); - match same_file_methods.len() { - 0 => {} - 1 => return same_file_methods, - _ => { - // Multiple classes define this method — restrict to the caller's own - // class prefix; return nothing if no match to avoid false edges. - if let Some(dot_pos) = caller_name.find('.') { - let caller_prefix = format!("{}.", &caller_name[..dot_pos]); - let caller_scoped: Vec<&NodeInfo> = same_file_methods.iter() - .filter(|n| n.name.starts_with(&caller_prefix)) - .copied() - .collect(); - if !caller_scoped.is_empty() { return caller_scoped; } - } + if !same_file_methods.is_empty() { + if let Some(dot_pos) = caller_name.find('.') { + let caller_prefix = format!("{}.", &caller_name[..dot_pos]); + let caller_scoped: Vec<&NodeInfo> = same_file_methods.iter() + .filter(|n| n.name.starts_with(&caller_prefix)) + .copied() + .collect(); + if !caller_scoped.is_empty() { return caller_scoped; } } } } diff --git a/tests/fixtures/this-dispatch-scope/single-sibling.ts b/tests/fixtures/this-dispatch-scope/single-sibling.ts new file mode 100644 index 000000000..ffe1c059e --- /dev/null +++ b/tests/fixtures/this-dispatch-scope/single-sibling.ts @@ -0,0 +1,16 @@ +// Two classes in one file; only one defines area(). +// this.area() inside Caller.run must NOT resolve to Sibling.area +// even when Sibling.area is the only method with that suffix in the file. +// The caller's own class (Caller) has no area() → the edge must be omitted. + +export class Caller { + run(): string { + return `result=${this.area()}`; + } +} + +export class Sibling { + area(): number { + return 42; + } +} diff --git a/tests/integration/this-dispatch-scope.test.ts b/tests/integration/this-dispatch-scope.test.ts index be3697492..0ef6e2906 100644 --- a/tests/integration/this-dispatch-scope.test.ts +++ b/tests/integration/this-dispatch-scope.test.ts @@ -2,9 +2,13 @@ * this-dispatch scope: same-file fallback must not emit false-positive edges * to methods in unrelated classes. * - * Fixture: shapes.ts — three unrelated classes (Shape, Calculator, Formatter) - * all defining area(). this.area() inside Shape.describe must resolve only to - * Shape.area. Edges to Calculator.area and Formatter.area must not appear. + * Fixtures: + * - shapes.ts — three unrelated classes (Shape, Calculator, Formatter) all + * defining area(). this.area() inside Shape.describe must resolve only to + * Shape.area (multi-match disambiguation path). + * - single-sibling.ts — two classes: Caller (no area()) and Sibling (area()). + * this.area() inside Caller.run must NOT resolve to Sibling.area even though + * it is the only method with that suffix in the file (single-match path). * * Covers the Rust edge_builder fix in issue #1324. */ @@ -45,7 +49,7 @@ const ENGINES: EngineMode[] = ['wasm', 'native']; describe.each(ENGINES)('this-dispatch scope (%s)', (engine) => { let tmpDir: string; - let callEdges: CallEdgeRow[]; + let callEdges: CallEdgeRow[] = []; beforeAll(async () => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `codegraph-this-scope-${engine}-`)); @@ -74,6 +78,9 @@ describe.each(ENGINES)('this-dispatch scope (%s)', (engine) => { if (engine === 'native') { it.todo('does NOT emit Shape.describe → Calculator.area (native binary gap #1324)'); it.todo('does NOT emit Shape.describe → Formatter.area (native binary gap #1324)'); + it.todo( + 'does NOT emit Caller.run → Sibling.area (single-match false-positive, native binary gap #1324)', + ); } else { it('does NOT emit Shape.describe → Calculator.area (unrelated class, same method name)', () => { const edge = callEdges.find( @@ -94,5 +101,18 @@ describe.each(ENGINES)('this-dispatch scope (%s)', (engine) => { `Expected NO Shape.describe → Formatter.area edge (false-positive from same-file scan).\nActual edges:\n${JSON.stringify(callEdges, null, 2)}`, ).toBeUndefined(); }); + + // single-sibling.ts: only one class (Sibling) has area(); Caller does not. + // The single-match arm must still check the caller's own class — Caller.run + // must not gain a false edge to Sibling.area. + it('does NOT emit Caller.run → Sibling.area (single-match false-positive, same-file scan)', () => { + const edge = callEdges.find( + (e) => e.caller_name === 'Caller.run' && e.callee_name === 'Sibling.area', + ); + expect( + edge, + `Expected NO Caller.run → Sibling.area edge (false-positive from single-match suffix scan).\nActual edges:\n${JSON.stringify(callEdges, null, 2)}`, + ).toBeUndefined(); + }); } });