fix(resolution): constrain what an inheritance or import reference may resolve to (#1536, #1537)

CHANGELOG.md

@@ -21,6 +21,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- An inheritance relationship is no longer invented between a type and an unrelated symbol that merely shares a name with its supertype. When a class or struct implemented something defined outside your project — a trait from the Rust standard library, an interface from an npm package — the graph would attach that relationship to whatever local symbol happened to have the same name: an enum variant, a type alias, even a function. On a Rust project of moderate size this fabricated a dozen implementation relationships, which then showed up in diagrams and in answers about who implements what. A supertype that isn't in your project is now simply left unresolved, and when several same-named symbols compete, only ones that can actually be a supertype are considered — so the real trait wins instead of losing to a same-named variant. This covers Svelte, Vue and Astro components too, whose `<script>` imports were not previously recognized as package imports. Thanks @ctype-lab. Re-index after upgrading to clear the bad relationships from an existing project.
+
+- An import no longer connects to a class property or an interface method that happens to share its name. Importing something the project doesn't contain — `path` from Node's standard library, a helper from an npm package — left the import looking for any symbol with that name, and a common word like `path`, `url` or `join` almost always matches a member somewhere. No language lets you import a member out of a type, so those connections are simply dropped now. Re-index after upgrading to clear them from an existing project.
+
 - C, C++, Objective-C and Rust unions are now indexed as first-class `union` nodes. A `union` declaration previously produced no symbol at all, so it never appeared in search or `codegraph_explore`, and anything attached to it disappeared with it — in Rust, every `impl SomeTrait for MyUnion` lost its edge, the methods from that impl were left pointing at a type the graph did not contain, and asking which types implement a trait quietly skipped the union ones. A union-shaped dispatch table in C now resolves its function pointers like a struct-shaped one. A `typedef union { … } Name;` in C keeps the typedef's name and is no longer mistaken for a plain type alias. Thanks @ctype-lab. Re-index after upgrading to pick up unions in existing projects. (#1515)
 
 - A long-lived index no longer drifts away from what a fresh `codegraph index` would produce. When a file gained or lost a symbol, references to that name in files the sync never touched kept pointing at the definition that was correct before the change, and — because nothing distinguished two same-named definitions — the winner could come down to the order files happened to be written, which differs between a full index and a sync. On this project's own repository, replaying 80 commits through `sync` left 5.7% of connections wrong; it is now 1.3%, and the wrong-answers-still-being-asserted half drops by 99.7%. Since call edges are what flow questions follow and what `codegraph_explore` ranks files by, this quietly degraded answers as an index aged, with nothing to indicate it. Syncing is unchanged in speed, and an edit that only changes a function's body does no extra work at all. Set `CODEGRAPH_NO_REBIND=1` to opt out.

__tests__/reference-target-kind.test.ts

@@ -0,0 +1,215 @@
+/**
+ * Reference target-kind gate — `extends`/`implements` and `imports`.
+ *
+ * The name-matcher treats node kind as a scoring BONUS, never a filter, and
+ * awards no bonus at all for inheritance refs. When exactly one same-named
+ * node exists, the single-candidate shortcut adopts it unconditionally at
+ * confidence 0.9. So a supertype that lives OUTSIDE the repo — imported by a
+ * bare name — bound to whatever local symbol happened to share that name,
+ * asserting an inheritance relationship absent from the source:
+ *
+ *   use std::error::Error;        // the supertype is out-of-repo
+ *   impl Error for MapperError {} // ...but `MapperError::Error` is a variant
+ *   → implements: enum MapperError -> enum_member Error
+ *
+ * The gate drops any inheritance resolution whose target cannot be a
+ * supertype. It only ever removes edges, so the tests below pin BOTH
+ * directions: the false edge is gone, and every legitimate supertype kind
+ * (in-repo trait, interface, class, and TS object-type alias) still resolves.
+ */
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import * as os from 'node:os';
+import { CodeGraph } from '../src';
+
+describe('reference target-kind gate', () => {
+  let dir: string;
+  beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'inh-kind-')); });
+  afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); });
+
+  const write = (rel: string, body: string) => {
+    const p = path.join(dir, rel);
+    fs.mkdirSync(path.dirname(p), { recursive: true });
+    fs.writeFileSync(p, body);
+  };
+
+  type InhEdge = { src: string; srcKind: string; tgt: string; tgtKind: string; kind: string };
+
+  const load = async (): Promise<{ edges: InhEdge[]; failed: { name: string; kind: string }[] }> => {
+    const cg = await CodeGraph.init(dir, { silent: true });
+    await cg.indexAll();
+    const db = (cg as any).db.db;
+    const edges: InhEdge[] = db
+      .prepare(
+        `SELECT s.name src, s.kind srcKind, t.name tgt, t.kind tgtKind, e.kind kind
+           FROM edges e
+           JOIN nodes s ON s.id = e.source
+           JOIN nodes t ON t.id = e.target
+          WHERE e.kind IN ('extends', 'implements')`
+      )
+      .all();
+    const failed: { name: string; kind: string }[] = db
+      .prepare(
+        `SELECT reference_name name, reference_kind kind
+           FROM unresolved_refs
+          WHERE reference_kind IN ('extends', 'implements')`
+      )
+      .all();
+    cg.close?.();
+    return { edges, failed };
+  };
+
+  const has = (edges: InhEdge[], src: string, tgt: string, tgtKind: string) =>
+    edges.some((e) => e.src === src && e.tgt === tgt && e.tgtKind === tgtKind);
+
+  it('drops an out-of-repo Rust supertype that name-matched a local enum member', async () => {
+    write(
+      'src/lib.rs',
+      `use std::error::Error;\n\n` +
+        `pub enum MapperError {\n    Error,\n    Missing,\n}\n\n` +
+        `impl Error for MapperError {}\n`
+    );
+    const { edges, failed } = await load();
+    expect(has(edges, 'MapperError', 'Error', 'enum_member')).toBe(false);
+    // The reference is not silently forgotten — it stays on record as failed,
+    // which is the honest outcome for a supertype the repo does not contain.
+    expect(failed.some((r) => r.name === 'Error')).toBe(true);
+  });
+
+  it('does not relocate the false edge onto a same-named local type alias', async () => {
+    // The kind filter alone would have moved this edge from the enum member to
+    // `type Error`, which IS a legal supertype kind — still false data, and
+    // harder for a consumer to reject. Locality is what removes it.
+    write('src/alias.rs', `pub type Error = String;\n`);
+    write(
+      'src/lib.rs',
+      `mod alias;\n\nuse std::error::Error;\n\n` +
+        `pub enum MapperError {\n    Missing,\n}\n\n` +
+        `impl Error for MapperError {}\n`
+    );
+    const { edges, failed } = await load();
+    expect(edges.filter((e) => e.tgt === 'Error')).toEqual([]);
+    expect(failed.some((r) => r.name === 'Error')).toBe(true);
+  });
+
+  it('keeps a supertype imported by an in-repo `use` path', async () => {
+    write('src/ports.rs', `pub trait Sha256Port {\n    fn hash(&self) -> String;\n}\n`);
+    write(
+      'src/lib.rs',
+      `mod ports;\n\nuse crate::ports::Sha256Port;\n\n` +
+        `pub struct Hasher {\n    salt: String,\n}\n\n` +
+        `impl Sha256Port for Hasher {\n    fn hash(&self) -> String { String::new() }\n}\n`
+    );
+    const { edges } = await load();
+    expect(has(edges, 'Hasher', 'Sha256Port', 'trait')).toBe(true);
+  });
+
+  it('keeps a trait reached through a re-exported sibling-crate module', async () => {
+    // `crate::ports` here is a re-export of ANOTHER crate's module, so no
+    // `src/ports.rs` exists to walk to. Treating "module path does not resolve
+    // to a file" as proof of out-of-repo deleted 13 real trait implementations
+    // on the reference fixture — hence the rule keys on stdlib roots only.
+    write('Cargo.toml', `[workspace]\nmembers = ["core", "app"]\n`);
+    write('core/Cargo.toml', `[package]\nname = "pupil_core"\nversion = "0.1.0"\n`);
+    write('core/src/lib.rs', `pub mod ports;\n`);
+    write('core/src/ports.rs', `pub trait CacheStore {\n    fn get(&self);\n}\n`);
+    write('app/Cargo.toml', `[package]\nname = "app"\nversion = "0.1.0"\n`);
+    write('app/src/lib.rs', `pub use pupil_core::ports;\n\npub mod platform;\n`);
+    write(
+      'app/src/platform.rs',
+      `use crate::ports::CacheStore;\n\npub struct SafStorage {\n    root: String,\n}\n\n` +
+        `impl CacheStore for SafStorage {\n    fn get(&self) {}\n}\n`
+    );
+    const { edges } = await load();
+    expect(has(edges, 'SafStorage', 'CacheStore', 'trait')).toBe(true);
+  });
+
+  it('still resolves an in-repo Rust trait (the gate is not a blanket block)', async () => {
+    write(
+      'src/lib.rs',
+      `pub trait Mapper {\n    fn map(&self) -> u32;\n}\n\n` +
+        `pub enum MapperError {\n    Mapper,\n}\n\n` +
+        `pub struct Real {\n    n: u32,\n}\n\n` +
+        `impl Mapper for Real {\n    fn map(&self) -> u32 { 1 }\n}\n`
+    );
+    const { edges } = await load();
+    expect(has(edges, 'Real', 'Mapper', 'trait')).toBe(true);
+    expect(has(edges, 'Real', 'Mapper', 'enum_member')).toBe(false);
+  });
+
+  it('keeps a TypeScript class implementing an object-type alias', async () => {
+    write(
+      'src/api.ts',
+      `export type SearchApi = { query(q: string): string };\n\n` +
+        `export class LocalSearch implements SearchApi {\n` +
+        `  query(q: string): string { return q; }\n}\n`
+    );
+    const { edges } = await load();
+    expect(has(edges, 'LocalSearch', 'SearchApi', 'type_alias')).toBe(true);
+  });
+
+  it.each([
+    ['svelte', 'src/Box.svelte', '<script lang="ts">\n$IMPORT$\nexport class SfcBox implements Serializable {\n  n = 1;\n}\n</script>\n<div>hi</div>\n'],
+    ['vue', 'src/Box.vue', '<script lang="ts">\n$IMPORT$\nexport class SfcBox implements Serializable {\n  n = 1;\n}\n</script>\n<template><div/></template>\n'],
+    ['astro', 'src/Box.astro', '---\n$IMPORT$\nexport class SfcBox implements Serializable {\n  n = 1;\n}\n---\n<div/>\n'],
+  ])('drops an npm supertype in a %s single-file component', async (_lang, file, body) => {
+    // An SFC imports inside its <script> block (Astro: the `---` frontmatter)
+    // with ordinary ES module syntax, so a bare specifier there is external for
+    // exactly the same reason it is in a .ts file. Missing that, the npm
+    // supertype name-matched the local class below.
+    write('package.json', `{"name":"sfc","version":"1.0.0"}\n`);
+    write('src/models.ts', `export class Serializable {\n  a = 1;\n}\n`);
+    write(file, body.replace('$IMPORT$', `import { Serializable } from 'some-npm-pkg';\n`));
+    const { edges, failed } = await load();
+    expect(edges.filter((e) => e.tgt === 'Serializable')).toEqual([]);
+    expect(failed.some((r) => r.name === 'Serializable')).toBe(true);
+  });
+
+  it('keeps an SFC supertype imported from a relative path', async () => {
+    write('package.json', `{"name":"sfc","version":"1.0.0"}\n`);
+    write('src/models.ts', `export class Serializable {\n  a = 1;\n}\n`);
+    write(
+      'src/Box.svelte',
+      `<script lang="ts">\nimport { Serializable } from './models';\n\n` +
+        `export class SfcBox implements Serializable {\n  n = 1;\n}\n</script>\n<div>hi</div>\n`
+    );
+    const { edges } = await load();
+    expect(has(edges, 'SfcBox', 'Serializable', 'class')).toBe(true);
+  });
+
+  it('does not resolve an import to a type member that shares its name', async () => {
+    // `import * as path from 'node:path'` is unresolvable — the module is
+    // external — so the name-matcher looked for any node called `path` and
+    // found a class property. No language lets you import a type's member.
+    write('src/types.ts', `export class Request {\n  path = '';\n  url = '';\n}\n`);
+    write(
+      'src/run.ts',
+      `import * as path from 'node:path';\n\nexport function run() {\n  return path.join('a', 'b');\n}\n`
+    );
+    const cg = await CodeGraph.init(dir, { silent: true });
+    await cg.indexAll();
+    const db = (cg as any).db.db;
+    const rows: { tgt: string; tgtKind: string }[] = db
+      .prepare(
+        `SELECT t.name tgt, t.kind tgtKind
+           FROM edges e JOIN nodes t ON t.id = e.target
+          WHERE e.kind = 'imports'`
+      )
+      .all();
+    cg.close?.();
+    expect(rows.filter((r) => r.tgtKind === 'property' || r.tgtKind === 'field')).toEqual([]);
+  });
+
+  it('keeps class extends class and class implements interface', async () => {
+    write(
+      'src/base.ts',
+      `export interface Runner { run(): void }\n` +
+        `export class Base { run(): void {} }\n` +
+        `export class Child extends Base implements Runner { run(): void {} }\n`
+    );
+    const { edges } = await load();
+    expect(has(edges, 'Child', 'Base', 'class')).toBe(true);
+    expect(has(edges, 'Child', 'Runner', 'interface')).toBe(true);
+  });
+});

src/resolution/import-resolver.ts

@@ -287,6 +287,21 @@ const C_CPP_STDLIB_HEADERS = new Set([
   'version',
 ]);
 
+/**
+ * Languages whose imports are ES-module specifiers, extracted by
+ * `extractJSImports` and therefore classified by the same bare-specifier /
+ * alias / workspace rules. Svelte, Vue and Astro belong here: an SFC imports
+ * inside its `<script>` block (Astro: the `---` frontmatter) with exactly the
+ * same syntax, and leaving them out made `isExternalImport` answer "not
+ * external" for every npm specifier in an SFC.
+ */
+const ESM_IMPORT_LANGUAGES = new Set<Language>([
+  'typescript', 'tsx', 'javascript', 'jsx', 'arkts', 'svelte', 'vue', 'astro',
+]);
+
+/** Rust path roots that always name a standard-library crate. */
+const RUST_STDLIB_ROOTS = new Set(['std', 'core', 'alloc', 'proc_macro']);
+
 /**
  * Check if an import is external (npm package, etc.)
  *
@@ -315,7 +330,7 @@ function isExternalImport(
   }
 
   // Common external patterns
-  if (language === 'typescript' || language === 'javascript' || language === 'tsx' || language === 'jsx' || language === 'arkts') {
+  if (ESM_IMPORT_LANGUAGES.has(language)) {
     // Node built-ins
     if (['fs', 'path', 'os', 'crypto', 'http', 'https', 'url', 'util', 'events', 'stream', 'child_process', 'buffer'].includes(importPath)) {
       return true;
@@ -2274,3 +2289,123 @@ function resolveStaticMember(
   }
   return candidates[0];
 }
+
+/**
+ * Rust `use` declarations, flattened to `localName → full path`.
+ *
+ * Rust is the one supported language with NO `ImportMapping` extraction (see
+ * `extractImportMappings`), so this is the only channel that can tell whether
+ * a bare type name in a Rust file was brought in by a `use`. Handles nested
+ * groups (`use a::{b::C, d as E}`), globs (skipped — they bind no single
+ * name), and `as` aliases.
+ */
+function collectRustUseBindings(content: string): Map<string, string> {
+  const out = new Map<string, string>();
+
+  // Expand one level of `{...}` at a time so `a::{b::{C, D}, E}` flattens.
+  const expand = (spec: string): string[] => {
+    const open = spec.indexOf('{');
+    if (open === -1) return [spec.trim()];
+    const prefix = spec.slice(0, open);
+    let depth = 0;
+    let close = -1;
+    for (let i = open; i < spec.length; i++) {
+      if (spec[i] === '{') depth++;
+      else if (spec[i] === '}') {
+        depth--;
+        if (depth === 0) { close = i; break; }
+      }
+    }
+    if (close === -1) return [];
+    const suffix = spec.slice(close + 1);
+    const inner = spec.slice(open + 1, close);
+    const parts: string[] = [];
+    let depth2 = 0;
+    let start = 0;
+    for (let i = 0; i <= inner.length; i++) {
+      const ch = inner[i];
+      if (ch === '{') depth2++;
+      else if (ch === '}') depth2--;
+      if (i === inner.length || (ch === ',' && depth2 === 0)) {
+        const seg = inner.slice(start, i).trim();
+        if (seg) parts.push(seg);
+        start = i + 1;
+      }
+    }
+    return parts.flatMap((p) => expand(prefix + p + suffix));
+  };
+
+  // `use` items end at the first `;`. Attributes/visibility (`pub use`) are
+  // irrelevant to the binding itself.
+  const useRe = /(^|\n)\s*(?:pub(?:\([^)]*\))?\s+)?use\s+([^;]+);/g;
+  let m: RegExpExecArray | null;
+  while ((m = useRe.exec(content)) !== null) {
+    for (const spec of expand(m[2]!.replace(/\s+/g, ' '))) {
+      const aliasMatch = /^(.*?)\s+as\s+([A-Za-z_]\w*)$/.exec(spec);
+      const rawPath = (aliasMatch ? aliasMatch[1]! : spec).trim();
+      if (!rawPath || rawPath.endsWith('*')) continue;
+      const segments = rawPath.split('::').map((s) => s.trim()).filter(Boolean);
+      const leaf = segments[segments.length - 1];
+      if (!leaf) continue;
+      const local = aliasMatch ? aliasMatch[2]! : leaf;
+      out.set(local, segments.join('::'));
+    }
+  }
+  return out;
+}
+
+/**
+ * Is `name`, as used in `ref`'s file, bound by an import whose module lives
+ * OUTSIDE the repository?
+ *
+ * When it is, no in-repo node can be the referent: the symbol is defined in a
+ * third-party crate/package, and any same-named local symbol the name-matcher
+ * finds is a coincidence. Rust `use std::error::Error;` + `impl Error for
+ * MapperError {}` bound to a local `MapperError::Error` variant, and once
+ * non-type kinds were filtered out it simply moved to an unrelated local
+ * `type Error` alias — restricting kinds alone RELOCATES the false edge
+ * instead of removing it, so locality has to be checked too.
+ *
+ * Answers only when it can be CERTAIN, because a false "yes" deletes a real
+ * edge. Two languages qualify, each with an oracle that cannot be wrong:
+ *
+ *  - **Rust** — the `use` path is rooted at a standard-library crate
+ *    (`std`/`core`/`alloc`/`proc_macro`), which by definition ships outside
+ *    any repository. Deliberately NOT generalized to "the module path doesn't
+ *    resolve to a file": a crate can re-export another workspace crate's
+ *    modules (`pub use pupil_core::{ports, domain};`), so `crate::ports::X`
+ *    has no `src/ports/` directory to walk yet is entirely in-repo — that
+ *    generalization measured 13 real trait implementations deleted.
+ *  - **ES modules** — `isExternalImport`, which already accounts for tsconfig
+ *    path aliases and monorepo workspace packages.
+ *
+ * Everything else returns false and resolves exactly as before. JVM and Python
+ * imports notably do NOT go through `resolveImportPath` (they have dedicated
+ * FQN/module matchers), so there is no trustworthy oracle to consult here.
+ */
+export function isBoundToOutOfRepoImport(
+  ref: UnresolvedRef,
+  context: ResolutionContext
+): boolean {
+  const name = ref.referenceName;
+  if (name.includes('::') || name.includes('.')) return false; // qualified refs resolve by path
+
+  if (ref.language === 'rust') {
+    const content = context.readFile(ref.filePath);
+    if (!content) return false;
+    const usePath = collectRustUseBindings(content).get(name);
+    if (!usePath) return false;
+    const segments = usePath.split('::');
+    if (segments.length < 2 || !RUST_STDLIB_ROOTS.has(segments[0]!)) return false;
+    // 2015-edition crate-relative paths can shadow a stdlib root with a local
+    // module of the same name — if the path walks to a real file, it's local.
+    return resolveRustModuleFile(segments.slice(0, -1), ref.filePath, context) === null;
+  }
+
+  if (!ESM_IMPORT_LANGUAGES.has(ref.language)) return false;
+  for (const imp of context.getImportMappings(ref.filePath, ref.language)) {
+    if (imp.localName !== name) continue;
+    return isExternalImport(imp.source, ref.language, context);
+  }
+  return false;
+}