From e116594b4b11c942247c24f8798fc1f3e63b51f7 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Fri, 23 Oct 2026 20:50:50 -0500 Subject: [PATCH] fix(resolution): a call reaches the caller's own copy of a duplicated module A module kept in several copies, like two Android build flavors that each hold one package, gives every name in it one candidate per copy. Directory proximity is capped at six shared folders, so in a deep tree the copies scored the same and the first indexed won: a call in one flavor could land on the other flavor's copy. And since the name is not in doubt among copies, only which copy, scoring the right pick by proximity (0.7, or 0.4 in a shallow tree) understated it. findBestMatch now breaks a tie among copies of one symbol (same qualified name, kind and file name) toward the copy whose file shares the longest directory prefix with the reference, when that copy is uniquely closest. matchByExactName scores that pick 0.8: above a proximity guess, below a unique name or the file's own definition (0.9, which keeps precedence). On top of the PR: the copies are judged among the candidates findBestMatch weighed, so a namesake in another language no longer blocks the 0.8; a match of the caller itself keeps the proximity score, as it does for a same-file match; and Kotlin and Java flavor tests, since the PR's Python fixture calls a function it never imports. Validation, main (cde4cd74) vs this commit, 21 repos in 11 languages: no edge added or removed; 4 targets moved, all right: spring-modulith's example-full tests now reach example-full's own OrderManagement and OrderCompleted, where Spring's naming rule (0.7) took the epr-jdbc example's copy, and playframework's shutdown-happy-path test its own src/main HomeController, not the copy its script swaps in later. 109 edges keep their target and go to 0.8, each onto the copy in the caller's own module or app; 4 of them are vben's `defineConfig` calls from `import { defineConfig } from 'tsdown'`, which main already links to a namesake because that dependency is declared only in the root package.json. In 18 sites the 0.8 now beats a framework rule's 0.7. Index CPU, median of 3 interleaved: okhttp 34.8 vs 34.8 s, etcd 36.5 vs 36.5 s. Tests: 3 of 6 fail on main, 2 on the PR as written. Original PR #1936 by @danusha2345. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/same-module-copy-resolution.test.ts | 150 ++++++++++++++++++ src/resolution/name-matcher.ts | 67 +++++++- 3 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 __tests__/same-module-copy-resolution.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 17022e6343..e4b287422a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -212,6 +212,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Framework support now switches on when an app's `package.json` sits three or more folders deep, as with an ASP.NET solution's Angular or React app in `src/Web/ClientApp/`, or a React app in a workspace folder like `web/ui/mantine-ui/`. Before, only the project root and the first two folder levels were read, so frameworks such as Angular, React Router, Vue Router, SvelteKit and TanStack Router could go undetected there: an Angular app's routes, its `routerLink` and `router.navigate(…)` navigation and its template's event bindings were missing, so a handler used only from a `(click)` binding had no callers, and a React app's `navigate('/login')` calls didn't reach their route. Re-index projects laid out this way after upgrading. - In a repository that mixes languages, a web framework's naming conventions now link only code written in that framework's own languages. Before, Express's also linked Go and Python code: in etcd a Go method's `*zap.Logger` result type linked to the method itself, and in Harbor a Python `self.jaeger.validate()` call linked to another class's `validate`. Links that cross languages on purpose, like React Native and Expo native modules, Swift and Objective-C, Drupal's `routing.yml`, SvelteKit's `$lib/…` imports in `.ts` files and a Razor page's `@model`, are unchanged. - A name that only one definition in its own file can mean, like a call to a function the file defines, is no longer marked as an uncertain match when the file sits near the top of the project, and it now wins over a framework's weaker same-named guess in another file: in an ASP.NET app, `ToastService.OnShow += …` now reads the component's own `ToastService` property instead of linking to the `ToastService` class. Thanks @danusha2345. +- When a project keeps copies of the same code in several folders, like Android build flavors that each hold one package, a call now links to the copy in the caller's own folder, even in deeply nested trees where it could land on another copy. Thanks @danusha2345. - In C and C++, a call written from the global scope, like `::leveldb::RepairDB(…)` or `::hpb::CreateMessage(…)`, now links to the function of exactly that name, and one through a namespace alias, like protobuf's `::_pbi::…` calls, to the function in the aliased namespace. Before, these calls were dropped unless the repository also held Swift and Objective-C files, and even then a name spelled out in full, like `::google::protobuf::internal::TSanWrite(…)`, linked to nothing, while `::operator delete(…)` linked to one class's own `operator delete`. A global call never links to a function or method of the same name inside a namespace or class, such as a wrapper `open()` that calls the C library's `::open(…)`, and `::std::…` and `::testing::…` calls stay unlinked unless that library's source is part of your project. Re-index C and C++ projects after upgrading. - In Go, an interface that embeds another, like gin's `IRouter` embedding `IRoutes`, now links to it, so `codegraph_explore` and impact show the embedded interface as its base. Before, every embedded interface was dropped. An embedded field written through another package of your project, like `*refresh.Discovery` or `storage.Appender`, or with type arguments, like `List[int]`, now links too. One from the standard library or another module, like `io.Closer` or `sync.Mutex`, links to nothing rather than to a project type that shares its name, and a type-set constraint like `~int | ~float64` is not an embedding. An embedded type written without a package now always links to its own package's type: before, etcd's `Client`, which embeds its own `Lease` interface, was linked to a `Lease` struct in another package. Re-index Go projects after upgrading. - In Go, working out which structs implement an interface now counts the methods of the interfaces it embeds, and the methods a struct gets from the types it embeds. Before, only each type's own methods counted: etcd's `AuthReadTx`, which embeds `UnsafeAuthReader` and adds `RLock` and `RUnlock`, was listed as implemented by every read-write lock in the project, an interface that only embeds others had no implementations at all, and a struct that gets its methods from an embedded base, like prometheus's service discoveries embedding `refresh.Discovery` or gin's `Engine` embedding `RouterGroup`, was missing. `codegraph_explore`, impact and the type hierarchy now list the right implementations, and a call through the interface reaches a method the embedding struct overrides, like gin's `Engine.Use`. Re-index Go projects after upgrading. diff --git a/__tests__/same-module-copy-resolution.test.ts b/__tests__/same-module-copy-resolution.test.ts new file mode 100644 index 0000000000..7d88744fef --- /dev/null +++ b/__tests__/same-module-copy-resolution.test.ts @@ -0,0 +1,150 @@ +/** + * A module duplicated under another source root (`rc2/…` vs `rcpro2/…`) gives + * every bare name two same-named candidates, one per copy. Directory proximity + * is capped, so on a deep tree both copies score the same and the first-indexed + * one used to win — a call in module B could land on module A's copy. The + * caller's own copy, when it is uniquely closest, must win; when no copy is + * closer (the caller sits above both), behavior is unchanged. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { CodeGraph } from '../src'; + +// Six shared segments already reach the proximity cap. +const ROOT = 'a/b/c/d/e/f'; + +function write(dir: string, rel: string, lines: string[]): void { + const full = path.join(dir, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, lines.join('\n') + '\n'); +} + +describe('exact-name resolution prefers the caller\'s own copy of a duplicated module', () => { + let tempDir: string; + let cg: CodeGraph | null = null; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-same-module-')); + for (const mod of ['rc2', 'rcpro2']) { + write(tempDir, `${ROOT}/${mod}/util.py`, ['def compute():', ' return 1']); + write(tempDir, `${ROOT}/${mod}/main.py`, ['def run():', ' return compute()']); + } + write(tempDir, `${ROOT}/rc2/only.py`, ['def solo():', ' return 2']); + write(tempDir, `${ROOT}/rc2/solo_caller.py`, ['def use_solo():', ' return solo()']); + // Equidistant from both copies. + write(tempDir, `${ROOT}/top.py`, ['def top():', ' return compute()']); + }); + + afterEach(() => { + cg?.destroy(); + cg = null; + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch { + // Windows can still hold the SQLite handle for a moment; the OS temp dir is swept anyway. + } + }); + + async function callsFrom(fnName: string, filePath: string) { + const caller = cg!.getNodesByKind('function').find((n) => n.name === fnName && n.filePath === filePath); + expect(caller).toBeDefined(); + return cg!.getOutgoingEdges(caller!.id).filter((e) => e.kind === 'calls'); + } + + function fn(name: string, filePath: string) { + const node = cg!.getNodesByKind('function').find((n) => n.name === name && n.filePath === filePath); + expect(node).toBeDefined(); + return node!; + } + + it('resolves each module\'s call to its own copy', async () => { + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + + const calls = await Promise.all( + ['rc2', 'rcpro2'].map((mod) => callsFrom('run', `${ROOT}/${mod}/main.py`)), + ); + expect(calls.map((c) => c.map((e) => e.target))).toEqual( + ['rc2', 'rcpro2'].map((mod) => [fn('compute', `${ROOT}/${mod}/util.py`).id]), + ); + expect(calls.map((c) => c[0]!.metadata?.confidence)).toEqual([0.8, 0.8]); + }); + + it('keeps the prior choice and confidence when the caller is equidistant', async () => { + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + + const calls = await callsFrom('top', `${ROOT}/top.py`); + expect(calls).toHaveLength(1); + const copies = ['rc2', 'rcpro2'].map((m) => fn('compute', `${ROOT}/${m}/util.py`).id); + expect(copies).toContain(calls[0]!.target); + expect(calls[0]!.metadata?.confidence).toBe(0.7); + }); + + // Android build flavors keep one package in two source sets that are never + // compiled together, so a name the package declares needs no import and + // means the caller's own flavor's copy. + const FLAVORS = 'platform/apps/mobile/android/app/src'; + + it("resolves a Kotlin flavor's call to its own copy of the package", async () => { + for (const flavor of ['rc2', 'rcpro2']) { + const dir = `${FLAVORS}/${flavor}/kotlin/com/app/util`; + write(tempDir, `${dir}/Util.kt`, ['package com.app.util', '', 'fun compute(): Int = 1']); + write(tempDir, `${dir}/Main.kt`, ['package com.app.util', '', 'fun run(): Int = compute()']); + } + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + + const flavors = ['rc2', 'rcpro2']; + const calls = await Promise.all(flavors.map((f) => callsFrom('run', `${FLAVORS}/${f}/kotlin/com/app/util/Main.kt`))); + expect(calls.map((c) => c.map((e) => e.target))).toEqual( + flavors.map((f) => [fn('compute', `${FLAVORS}/${f}/kotlin/com/app/util/Util.kt`).id]), + ); + expect(calls.map((c) => c[0]!.metadata?.confidence)).toEqual([0.8, 0.8]); + }); + + it("resolves a Java flavor's `new Util()` to its own copy of the class", async () => { + for (const flavor of ['rc2', 'rcpro2']) { + const dir = `${FLAVORS}/${flavor}/java/com/app/util`; + write(tempDir, `${dir}/Util.java`, ['package com.app.util;', '', 'public class Util {', ' public int compute() { return 1; }', '}']); + write(tempDir, `${dir}/Main.java`, ['package com.app.util;', '', 'public class Main {', ' int run() { return new Util().compute(); }', '}']); + } + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + + for (const flavor of ['rc2', 'rcpro2']) { + const dir = `${FLAVORS}/${flavor}/java/com/app/util`; + const run = cg.getNodesByKind('method').find((n) => n.name === 'run' && n.filePath === `${dir}/Main.java`); + const util = cg.getNodesByKind('class').find((n) => n.name === 'Util' && n.filePath === `${dir}/Util.java`); + const made = cg.getOutgoingEdges(run!.id).filter((e) => e.kind === 'instantiates').map((e) => e.target); + expect(made).toEqual([util!.id]); + } + }); + + it('leaves a match of the caller itself at the proximity score', async () => { + // Recursion, or a receiver the call lost (`json.dumps` inside `dumps`): + // the caller's own copy being the caller says nothing of which copy. + for (const mod of ['rc2', 'rcpro2']) { + write(tempDir, `${ROOT}/${mod}/walk.py`, ['def walk(n):', ' return walk(n - 1)']); + } + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + + const calls = await callsFrom('walk', `${ROOT}/rcpro2/walk.py`); + expect(calls.map((e) => e.target)).toEqual([fn('walk', `${ROOT}/rcpro2/walk.py`).id]); + expect(calls[0]!.metadata?.confidence).toBe(0.7); + }); + + it('leaves a single-candidate name unaffected', async () => { + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + + const calls = await callsFrom('use_solo', `${ROOT}/rc2/solo_caller.py`); + expect(calls).toHaveLength(1); + expect(calls[0]!.target).toBe(fn('solo', `${ROOT}/rc2/only.py`).id); + expect(calls[0]!.metadata?.confidence).toBe(0.9); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 6b582f6d9c..7a43a5e329 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -12049,8 +12049,19 @@ export function matchByExactName( if (bestMatch && isCrossFileReachable(bestMatch, ref, context)) { // Lower confidence when the match is from a distant/unrelated module const proximity = computePathProximity(ref.filePath, bestMatch.filePath); + // Every candidate is a copy of one symbol and this is the caller's own + // copy: the name itself is not ambiguous, only which copy — so rank it + // above a proximity guess, but below a unique name or the file's own + // definition (0.9). The candidates are those findBestMatch weighed: a + // namesake in another language never wins while one in the reference's + // own language exists. The caller's own copy being the caller itself is + // no evidence, as for a same-file match (isDecisiveSameFileMatch). + const weighed = candidates.some((c) => c.language === ref.language) + ? candidates.filter((c) => c.language === ref.language) + : candidates; const confidence = isDecisiveSameFileMatch(bestMatch, ref, candidates, context) ? 0.9 + : bestMatch.id !== ref.fromNodeId && closestCopy(ref, weighed) === bestMatch ? 0.8 : proximity >= 30 ? 0.7 : 0.4; return { original: ref, @@ -26798,6 +26809,7 @@ function findBestMatch( let bestScore = -1; let bestNode: Node | null = null; + let tied: Node[] = []; // Split the ref's path once (it's the same across every candidate) instead of // re-splitting it inside computePathProximity per candidate (#915 hot spot). @@ -26888,10 +26900,63 @@ function findBestMatch( if (score > bestScore) { bestScore = score; bestNode = candidate; + tied = [candidate]; + } else if (score === bestScore) { + tied.push(candidate); } } - return bestNode; + // Proximity is capped, so on deep trees two copies of the same symbol in + // sibling modules can tie; the caller's own copy, if uniquely closest, wins. + return (tied.length > 1 && closestCopy(ref, tied)) || bestNode; +} + +/** + * Number of leading directory segments two file paths share (uncapped). + */ +function sharedDirDepth(filePath1: string, filePath2: string): number { + const a = filePath1.split('/'); + const b = filePath2.split('/'); + a.pop(); + b.pop(); + let shared = 0; + while (shared < a.length && shared < b.length && a[shared] === b[shared]) shared++; + return shared; +} + +/** + * When every candidate is a copy of the same symbol — same qualified name, kind + * and file basename, as when a module is duplicated under another source root + * (`rc2/flymod/src/…` vs `rcpro2/flymod/src/…`) — return the copy whose file + * shares the longest directory prefix with the reference, i.e. the one in the + * caller's own module. Returns null when the candidates are not all copies or + * no copy is uniquely closest, so callers keep their existing choice. + */ +function closestCopy(ref: UnresolvedRef, candidates: Node[]): Node | null { + if (candidates.length < 2) return null; + const first = candidates[0]!; + const base = path.posix.basename(first.filePath); + let best: Node | null = null; + let bestDepth = -1; + let unique = false; + for (const c of candidates) { + if ( + c.qualifiedName !== first.qualifiedName || + c.kind !== first.kind || + path.posix.basename(c.filePath) !== base + ) { + return null; + } + const depth = sharedDirDepth(ref.filePath, c.filePath); + if (depth > bestDepth) { + bestDepth = depth; + best = c; + unique = true; + } else if (depth === bestDepth) { + unique = false; + } + } + return unique ? best : null; } /**