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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
150 changes: 150 additions & 0 deletions __tests__/cross-file-visibility.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/**
* 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<string, string>): 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<string[]> {
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<typeof n> => !!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 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',
'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');
});
});
10 changes: 8 additions & 2 deletions src/resolution/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down
148 changes: 147 additions & 1 deletion src/resolution/name-matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -388,6 +389,149 @@ 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<string>(['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<ResolutionContext, Map<string, boolean>>();

/**
* 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
* 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<ResolutionContext, Map<string, boolean>>();

/**
* 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 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`.
* - **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' ||
!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
// 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
*/
Expand Down Expand Up @@ -1299,6 +1443,8 @@ function getInferScanStates(context: ResolutionContext): Map<string, InferScanSt
/** Drop the per-context scan states (see ReferenceResolver.clearCaches). */
export function clearNameMatcherMemos(context: ResolutionContext): void {
INFER_SCAN_STATES.delete(context);
C_STATIC_MEMO.delete(context);
RUST_TRAIT_IMPL_MEMO.delete(context);
}

function memoPatterns(key: string, build: () => RegExp[]): RegExp[] {
Expand Down Expand Up @@ -2418,7 +2564,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,
Expand Down