From 15de8bc181f31b80b6e4137b1a2ef8d3d06c81a1 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Mon, 7 Sep 2026 10:16:44 +0300 Subject: [PATCH 1/2] fix(resolution): a definition its language makes file-local is not a cross-file target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Name matching accepted any same-named definition as the target of a call from another file, however the language scopes it. isVisibleAcrossFiles now declines, for a candidate in another file: C / C++ a function whose definition line carries `static` (read from source — the extractor records no storage class, and the kernel arm would need the same field) Kotlin, Java, C#, Swift, Scala, Dart, PHP visibility === 'private' Go a lowercase identifier from another directory (by the name's case: the extractor's isExported is unset for every Go method) Rust a non-`pub` item unless the reference is in the item's module subtree (a child sees its ancestors' private items via super::); a method in an `impl Trait for Type` block has the trait's visibility and is exempt The test runs in ReferenceResolver on the target the whole name-matching pipeline settled on, so a rejection ends the reference unresolved. Declining inside matchByExactName instead let the ref fall through to matchFuzzy, which committed to a same-language namesake the ranking had passed over — eight edges on one tree, all onto a local `const fail = …` arrow the graph does not hold. matchFuzzy checks its own survivor too; nothing runs after it. Five corpora, all against b9ca4b7, wasm arm, edge rows keyed with resolvedBy: betaflight fork (2,109 C files) LOST 4,451 GAINED 0 (#1730) Android/Go/JS app (114 kt, 42 go) LOST 142 GAINED 0 (#1731) emmc-reader-gui (71 rs) LOST 195 GAINED 0 skylab_hub (35 rs) LOST 92 GAINED 0 vitejs/vite (JS/TS only) LOST 0 GAINED 0 Samples read back: `Vec::new()` onto a private `fn new` in another crate, `ui.add(…)` (egui) onto a private `add`, `latch.await()` onto a test file's `private fun await`, `leaflet.js` onto an unexported Go `func add`, `usbd_get_descriptor` onto a `static get_device_descriptor` in a USB class file it never links. Fixes #1730. Fixes #1731. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 + __tests__/cross-file-visibility.test.ts | 142 ++++++++++++++++++++++++ src/resolution/index.ts | 10 +- src/resolution/name-matcher.ts | 132 +++++++++++++++++++++- 4 files changed, 283 insertions(+), 3 deletions(-) create mode 100644 __tests__/cross-file-visibility.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1815c4150..2f50f9f06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -201,6 +201,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- **A definition its language makes file-local no longer captures calls from other files.** A C `static` function, a Kotlin/Java/C#/Swift `private` member, a Go unexported name in another package and a Rust non-`pub` item outside its module subtree cannot be what a name in another file means, but name matching accepted them whenever the names agreed: on a betaflight tree 4,448 calls landed on a `static` in some other file, an Android `editor.apply()` on an unrelated class's `private fun apply`, a JavaScript `fail(...)` on a Go program's `func fail`. Such a target is now declined outright — the reference stays unresolved rather than being handed to another file's namesake. Same-file definitions and Rust trait-impl methods are unaffected. Re-index after upgrading. (#1730) + - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges. - **Production code under a `samples` or `examples` package path is no longer treated as test code.** A Kotlin or Java project whose package path runs through `com/google/samples/…` (Now in Android, for one) had nearly every file counted as a fixture, so the Map opened on `build-logic`, the entry points hid the app, and dead-code and test badges were wrong. Only the project layout above a `src/` folder decides now; the package path below it never does. diff --git a/__tests__/cross-file-visibility.test.ts b/__tests__/cross-file-visibility.test.ts new file mode 100644 index 000000000..59d25c2cb --- /dev/null +++ b/__tests__/cross-file-visibility.test.ts @@ -0,0 +1,142 @@ +/** + * A definition the language makes file-local is not a candidate for a + * cross-file name match: a C `static`, a Kotlin `private fun`, a Go unexported + * identifier in another package, a Rust non-`pub` item outside its module + * subtree. Each case pairs the invisible shape with the visible one of + * identical form, so the assertion discriminates on visibility alone. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; + +let tempDir: string; +let cg: CodeGraph | null = null; + +function project(files: Record): void { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-visibility-')); + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(tempDir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } +} + +/** `calls` targets of the function named `caller`, as `file:name` strings. */ +async function calleesOf(caller: string): Promise { + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const from = cg.getNodesByKind('function').concat(cg.getNodesByKind('method')).find((n) => n.name === caller)!; + expect(from).toBeDefined(); + return cg + .getOutgoingEdges(from.id) + .filter((e) => e.kind === 'calls') + .map((e) => cg!.getNode(e.target)) + .filter((n): n is NonNullable => !!n) + .map((n) => `${n.filePath}:${n.name}`); +} + +afterEach(() => { + cg?.close(); + cg = null; + fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +describe('C: a static function is local to its translation unit', () => { + it('does not resolve a call onto a static in another file', async () => { + project({ + 'core.c': 'void coreRun(void)\n{\n usbGetDescriptor();\n}\n', + 'usb_audio.c': 'static void usbGetDescriptor(void)\n{\n}\n', + }); + expect(await calleesOf('coreRun')).not.toContain('usb_audio.c:usbGetDescriptor'); + }); + + it('still resolves onto a non-static function in another file', async () => { + project({ + 'core.c': 'void coreRun(void)\n{\n usbGetDescriptor();\n}\n', + 'usb_audio.c': 'void usbGetDescriptor(void)\n{\n}\n', + }); + expect(await calleesOf('coreRun')).toContain('usb_audio.c:usbGetDescriptor'); + }); + + it('keeps a same-file static, whichever line the keyword is on', async () => { + project({ + 'core.c': 'static void\nhelper(void)\n{\n}\n\nvoid coreRun(void)\n{\n helper();\n}\n', + 'other.c': 'static void helper(void)\n{\n}\n', + }); + expect(await calleesOf('coreRun')).toEqual(['core.c:helper']); + }); +}); + +describe('Kotlin: a private function is class- or file-local', () => { + it('does not resolve an SDK-style call onto another file\'s private fun', async () => { + project({ + 'Budget.kt': 'class Budget {\n private fun apply(bps: Long): Long = bps\n}\n', + 'Main.kt': 'class Main {\n fun onCreate(editor: Editor) {\n editor.apply()\n }\n}\n', + }); + expect(await calleesOf('onCreate')).not.toContain('Budget.kt:apply'); + }); + + it('still resolves onto a public fun in another file', async () => { + project({ + 'Budget.kt': 'class Budget {\n fun apply(bps: Long): Long = bps\n}\n', + 'Main.kt': 'class Main {\n fun onCreate(budget: Budget) {\n budget.apply(1L)\n }\n}\n', + }); + expect(await calleesOf('onCreate')).toContain('Budget.kt:apply'); + }); +}); + +describe('Go: an unexported identifier is package-local', () => { + it('does not resolve a call onto an unexported func in another package', async () => { + project({ + 'cmd/probe/main.go': 'package main\n\nfunc fail(msg string) {}\n', + 'server/turn.go': 'package server\n\nfunc Run() {\n\tfail("x")\n}\n', + }); + expect(await calleesOf('Run')).not.toContain('cmd/probe/main.go:fail'); + }); + + it('still resolves within the package and onto an exported func elsewhere', async () => { + project({ + 'server/util.go': 'package server\n\nfunc fail(msg string) {}\n', + 'server/turn.go': 'package server\n\nfunc Run() {\n\tfail("x")\n\tReport()\n}\n', + 'report/report.go': 'package report\n\nfunc Report() {}\n', + }); + const callees = await calleesOf('Run'); + expect(callees).toContain('server/util.go:fail'); + expect(callees).toContain('report/report.go:Report'); + }); +}); + +describe('Rust: a non-pub item is visible to its module subtree only', () => { + it('does not resolve a sibling module\'s private fn, nor another crate\'s', async () => { + project({ + 'src/main.rs': 'mod util;\nmod net;\nfn main() {}\n', + 'src/util.rs': 'fn count() -> usize { 0 }\n', + 'src/net.rs': 'pub fn run() -> usize {\n count()\n}\n', + }); + expect(await calleesOf('run')).not.toContain('src/util.rs:count'); + }); + + it('keeps a trait-impl method, which has the trait\'s visibility', async () => { + project({ + 'src/main.rs': 'mod shape;\nmod draw;\nfn main() {}\n', + 'src/shape.rs': 'pub struct Circle;\npub trait Area { fn area(&self) -> f64; }\nimpl Area for Circle {\n fn area(&self) -> f64 { 1.0 }\n}\n', + 'src/draw.rs': 'use crate::shape::{Area, Circle};\npub fn render(c: &Circle) -> f64 {\n c.area()\n}\n', + }); + expect(await calleesOf('render')).toContain('src/shape.rs:area'); + }); + + it('still resolves a parent module\'s private fn from a child, and any pub fn', async () => { + project({ + 'src/main.rs': 'mod net;\nmod util;\nfn main() {}\n', + 'src/net.rs': 'pub mod tcp;\nfn shared() {}\n', + 'src/net/tcp.rs': 'use super::shared;\nuse crate::util::exported;\npub fn open() {\n shared();\n exported();\n}\n', + 'src/util.rs': 'pub fn exported() {}\n', + }); + const callees = await calleesOf('open'); + expect(callees).toContain('src/net.rs:shared'); + expect(callees).toContain('src/util.rs:exported'); + }); +}); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 7988c1bcf..2a995b985 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -16,7 +16,7 @@ import { FrameworkResolver, ImportMapping, } from './types'; -import { matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily, dumpNameMatcherProfile, clearNameMatcherMemos } from './name-matcher'; +import { isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily, dumpNameMatcherProfile, clearNameMatcherMemos } from './name-matcher'; import { resolveViaImport, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, clearImportResolverMemos } from './import-resolver'; import { ResolverPool, minRefsForPool } from './resolver-pool'; import { detectFrameworks } from './frameworks'; @@ -1012,7 +1012,13 @@ export class ReferenceResolver { // binding it happened to pick. Same-file matches only. if (nameResult) { const target = this.queries.getNodeById(nameResult.targetNodeId); - if (ref.language === 'nix') { + // A definition its language makes file-local — a C `static`, a Kotlin + // `private fun`, a Go unexported name in another package, a Rust + // non-`pub` item outside its module subtree — cannot be what a name in + // another file means, whichever strategy chose it (#1730). + if (target && !isVisibleAcrossFiles(target, ref, this.context)) { + nameResult = null; + } else if (ref.language === 'nix') { if (!target || target.filePath !== ref.filePath) { nameResult = null; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c74d8f272..b64c00e0b 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -4,6 +4,7 @@ * Handles symbol name matching for reference resolution. */ +import * as path from 'path'; import { Language, Node } from '../types'; import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types'; @@ -388,6 +389,133 @@ function isLexicallyReachable( ); } +/** + * Languages in which `visibility: 'private'` on a definition means no other + * FILE can name it: a Kotlin `private fun` is file- or class-local, and the + * same holds for Java, C#, Swift, Scala, Dart and PHP members. + */ +const PRIVATE_IS_FILE_LOCAL = new Set(['kotlin', 'java', 'csharp', 'swift', 'scala', 'dart', 'php']); + +/** Per-context memo: node id → "this C/C++ function is declared `static`". */ +const C_STATIC_MEMO = new WeakMap>(); + +/** + * Whether a C/C++ function definition carries the `static` storage class — + * read from its first source line(s), since the extractor records no storage + * class and the kernel arm would need the same field. `static` on the line + * above the name (`static void\nfoo(void)`) is the common alternative layout. + */ +function isStaticCFunction(candidate: Node, context: ResolutionContext): boolean { + let memo = C_STATIC_MEMO.get(context); + if (!memo) { + memo = new Map(); + C_STATIC_MEMO.set(context, memo); + } + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + const lines = context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split('\n') ?? []; + const head = [lines[candidate.startLine - 2] ?? '', lines[candidate.startLine - 1] ?? ''].join('\n'); + const isStatic = /(^|[\s;}])static\s/.test(head); + memo.set(candidate.id, isStatic); + return isStatic; +} + +/** Per-context memo: node id → "this Rust method implements a trait". */ +const RUST_TRAIT_IMPL_MEMO = new WeakMap>(); + +/** + * Whether a Rust method sits in an `impl Trait for Type` block. Such a method + * carries no `pub` — the trait decides its visibility — so the extractor + * records it as private; it is reachable wherever the trait is. Read from the + * nearest enclosing `impl` header above the method, memoised per node. + */ +function isRustTraitImplMethod(candidate: Node, context: ResolutionContext): boolean { + if (candidate.kind !== 'method') return false; + let memo = RUST_TRAIT_IMPL_MEMO.get(context); + if (!memo) { + memo = new Map(); + RUST_TRAIT_IMPL_MEMO.set(context, memo); + } + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + const lines = context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split('\n') ?? []; + let isTrait = false; + for (let i = candidate.startLine - 2; i >= 0; i--) { + const line = lines[i] ?? ''; + if (/^\s*(pub(\([^)]*\))?\s+)?(unsafe\s+)?impl\b/.test(line)) { + isTrait = /\sfor\s/.test(line.replace(/\/\/.*$/, '')); + break; + } + // A top-level item above the method means it was not inside an impl. + if (/^(pub(\([^)]*\))?\s+)?(fn|struct|enum|mod|trait|const|static|type)\b/.test(line)) break; + } + memo.set(candidate.id, isTrait); + return isTrait; +} + +/** + * The directory a Rust file's private items are visible from: the file's own + * module subtree. `src/net.rs` and `src/net/mod.rs` own `src/net/`; a crate + * root (`lib.rs` / `main.rs`) owns its directory. A child module reaches its + * ancestors' private items (`super::`), a sibling or another crate never does. + */ +function rustModuleDir(filePath: string): string { + const base = path.posix.basename(filePath); + const dir = path.posix.dirname(filePath); + if (base === 'mod.rs' || base === 'lib.rs' || base === 'main.rs') return dir; + return path.posix.join(dir, base.replace(/\.rs$/, '')); +} + +/** + * Whether `candidate` can be NAMED from a reference in `ref`'s file at all, + * given what its language says about the definition's visibility. A + * definition the language makes file-local is not a candidate for a + * cross-file name match, however well the names agree: + * + * - **C / C++**: a `static` function is local to its translation unit. On a + * 2,109-file betaflight tree 4,448 cross-file calls resolved onto a `static` + * in another file (#1730) — `usbd_get_descriptor` onto the `static + * get_device_descriptor` of whichever USB class file ranked first. + * - **Kotlin, Java, C#, Swift, Scala, Dart, PHP**: `private` is class- or + * file-local. An Android `editor.apply()` resolved onto an unrelated class's + * `private fun apply`. + * - **Go**: an unexported (lowercase) identifier is package-local, and a + * package is a directory. Judged by the name's case: the extractor's + * `isExported` is unset for every Go method. + * - **Rust**: a non-`pub` item is visible to its module and that module's + * descendants, never to a sibling module or another crate — `.count()` on + * an iterator resolved onto a `fn count` in a different crate. A method in + * an `impl Trait for Type` block has the trait's visibility, not `private`. + * + * Same-file candidates are always visible. Applied by ReferenceResolver to + * the target the whole name-matching pipeline settled on, so a rejection ends + * the reference unresolved: declining inside matchByExactName instead let the + * ref fall through to matchFuzzy, which then committed to a same-language + * namesake the ranking had passed over — eight such edges on one tree, all + * onto a local `const fail = …` arrow the graph does not hold. matchFuzzy + * checks its own survivor as well, since nothing runs after it. + */ +export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, context: ResolutionContext): boolean { + if (candidate.filePath === ref.filePath) return true; + const lang = candidate.language as string; + if (lang === 'c' || lang === 'cpp') { + return candidate.kind !== 'function' || !isStaticCFunction(candidate, context); + } + if (lang === 'go') { + // By the name's first letter, not the extractor's flag: the flag is unset + // for every Go method, exported or not. + return /^[A-Z]/.test(candidate.name) || path.posix.dirname(candidate.filePath) === path.posix.dirname(ref.filePath); + } + if (lang === 'rust') { + if (candidate.visibility !== 'private') return true; + if (isRustTraitImplMethod(candidate, context)) return true; + const owner = rustModuleDir(candidate.filePath); + return ref.filePath.startsWith(owner + '/'); + } + if (PRIVATE_IS_FILE_LOCAL.has(lang)) return candidate.visibility !== 'private'; + return true; +} + /** * Try to resolve a reference by exact name match */ @@ -1299,6 +1427,8 @@ function getInferScanStates(context: ResolutionContext): Map RegExp[]): RegExp[] { @@ -2418,7 +2548,7 @@ export function matchFuzzy( const sameLanguageCandidates = callableCandidates.filter(n => n.language === ref.language); const finalCandidates = sameLanguageCandidates.length > 0 ? sameLanguageCandidates : callableCandidates; - if (finalCandidates.length === 1) { + if (finalCandidates.length === 1 && isVisibleAcrossFiles(finalCandidates[0]!, ref, context)) { const isCrossLanguage = finalCandidates[0]!.language !== ref.language; return { original: ref, From 42a9267592d0ca30b938fc43e70b252a8662c8b3 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Mon, 7 Sep 2026 10:45:14 +0300 Subject: [PATCH 2/2] fix(resolution): a static in a header is part of every unit that includes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The C rule declined any `static` function defined in another file. A `static` in a SOURCE file is local to that translation unit and the rule is right there; a `static` in a header — `static inline`, the whole of MAVLink's generated `mavlink_msg_*.h` — is textually included into every unit that names it, and the call is real. On the betaflight tree 4,306 of the 4,451 rows the first cut removed were exactly that: `testsuite.h` and `mavlink_msg_*.h` calling `protocol.h`'s `_mav_put_char_array`, `mav_array_assign_char` and each other's `_pack` / `_decode` helpers. The rule now applies only to a candidate whose file is a translation unit (`.c .cc .cpp .cxx .c++ .m .mm`). Same tree: LOST 145, GAINED 0, every one onto a `static` in another `.c` — STM32 USB class sources onto GD32's `usbd_enum.c`, and the USB descriptor table shape from #1730. Header targets in the removed set: 0. Co-Authored-By: Claude Fable 5.1 --- __tests__/cross-file-visibility.test.ts | 8 ++++++++ src/resolution/name-matcher.ts | 26 ++++++++++++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/__tests__/cross-file-visibility.test.ts b/__tests__/cross-file-visibility.test.ts index 59d25c2cb..1cb0007c9 100644 --- a/__tests__/cross-file-visibility.test.ts +++ b/__tests__/cross-file-visibility.test.ts @@ -61,6 +61,14 @@ describe('C: a static function is local to its translation unit', () => { expect(await calleesOf('coreRun')).toContain('usb_audio.c:usbGetDescriptor'); }); + it('keeps a static inline defined in a header: it lives in every unit that includes it', async () => { + project({ + 'protocol.h': 'static inline void mav_put_char(char *buf, char c)\n{\n buf[0] = c;\n}\n', + 'core.c': '#include "protocol.h"\n\nvoid coreRun(char *b)\n{\n mav_put_char(b, 0);\n}\n', + }); + expect(await calleesOf('coreRun')).toContain('protocol.h:mav_put_char'); + }); + it('keeps a same-file static, whichever line the keyword is on', async () => { project({ 'core.c': 'static void\nhelper(void)\n{\n}\n\nvoid coreRun(void)\n{\n helper();\n}\n', diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index b64c00e0b..6a1863bff 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -399,6 +399,16 @@ const PRIVATE_IS_FILE_LOCAL = new Set(['kotlin', 'java', 'csharp', 'swif /** Per-context memo: node id → "this C/C++ function is declared `static`". */ const C_STATIC_MEMO = new WeakMap>(); +/** + * A C/C++ file that IS a translation unit. A `static` defined here is local + * to it. A `static` (typically `static inline`) in a header is a different + * thing: the header is textually included, so the function exists in every + * unit that includes it and is callable from each — MAVLink's generated + * `mavlink_msg_*.h` are nothing but such functions, 4,306 real calls on one + * betaflight tree. + */ +const C_SOURCE_EXT = /\.(c|cc|cpp|cxx|c\+\+|m|mm)$/i; + /** * Whether a C/C++ function definition carries the `static` storage class — * read from its first source line(s), since the extractor records no storage @@ -472,10 +482,12 @@ function rustModuleDir(filePath: string): string { * definition the language makes file-local is not a candidate for a * cross-file name match, however well the names agree: * - * - **C / C++**: a `static` function is local to its translation unit. On a - * 2,109-file betaflight tree 4,448 cross-file calls resolved onto a `static` - * in another file (#1730) — `usbd_get_descriptor` onto the `static - * get_device_descriptor` of whichever USB class file ranked first. + * - **C / C++**: a `static` function defined in a SOURCE file is local to + * that translation unit; one in a header is part of every unit that + * includes it and stays visible. On a 2,109-file betaflight tree 145 + * cross-file calls resolved onto a `static` in another `.c` (#1730) — + * `usbd_get_descriptor` onto the `static get_device_descriptor` of + * whichever USB class file ranked first. * - **Kotlin, Java, C#, Swift, Scala, Dart, PHP**: `private` is class- or * file-local. An Android `editor.apply()` resolved onto an unrelated class's * `private fun apply`. @@ -499,7 +511,11 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex if (candidate.filePath === ref.filePath) return true; const lang = candidate.language as string; if (lang === 'c' || lang === 'cpp') { - return candidate.kind !== 'function' || !isStaticCFunction(candidate, context); + return ( + candidate.kind !== 'function' || + !C_SOURCE_EXT.test(candidate.filePath) || + !isStaticCFunction(candidate, context) + ); } if (lang === 'go') { // By the name's first letter, not the extractor's flag: the flag is unset