Просмотр исходного кода

fix(resolution): a definition its language makes file-local is not a cross-file target (#1731) (#1745)

* fix(resolution): a definition its language makes file-local is not a cross-file target

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 <noreply@anthropic.com>

* fix(resolution): a static in a header is part of every unit that includes it

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 <noreply@anthropic.com>

* docs(changelog): cite #1731 and narrow the C file-local note

Rebased #1732 onto latest main. Clarify that only a static in another
source file is declined (header static inline stays), name the Kotlin/Go/Rust
shapes from #1731, and note the post-pipeline placement that avoids fuzzy
fallback.

---------

Co-authored-by: danusha2345 <ewidusoc498@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 8 часов назад
Родитель
Сommit
2c251e2c61
4 измененных файлов с 307 добавлено и 3 удалено
  1. 2 0
      CHANGELOG.md
  2. 150 0
      __tests__/cross-file-visibility.test.ts
  3. 8 2
      src/resolution/index.ts
  4. 147 1
      src/resolution/name-matcher.ts

+ 2 - 0
CHANGELOG.md

@@ -207,6 +207,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` in another source file (`.c`/`.cc`/… — not a header's `static inline`, which is textually included), a Kotlin/Java/C#/Swift/Scala/Dart/PHP `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: an Android `editor.apply()` onto an unrelated class's `private fun apply`, a JavaScript `fail(...)` onto a Go `func fail`, a Rust `.count()` onto a private `fn count` in another crate, and C USB helpers onto a `static` in a `.c` they never link. Such a target is now declined after the whole name-matching pipeline settles — the reference stays unresolved rather than falling through to a fuzzy namesake. Same-file definitions, a child Rust module reaching its ancestors' private items, and Rust `impl Trait for Type` methods stay resolvable. Re-index after upgrading. (#1730, #1731)
+
 - **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.

+ 150 - 0
__tests__/cross-file-visibility.test.ts

@@ -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');
+  });
+});

+ 8 - 2
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;
         }

+ 147 - 1
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,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
  */
@@ -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[] {
@@ -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,