Forráskód Böngészése

fix(resolution,cli): cross-file static method calls + affected path normalization (#825) (#865)

Cross-file `ClassName.staticMethod()` calls resolved to the class, not the
method: the import resolver matched the receiver `Foo` to the named class
import but dropped the `.bar` member, and createEdges then mis-promoted the
`calls` edge to `instantiates`. So callers/impact for the static method came
back empty. Descend from the resolved class into its `Container::member` so the
call links to the method; fall back to the class when no such member exists
(non-`::` languages and genuine class references are unaffected).

Also normalize `codegraph affected` inputs to the project-relative,
forward-slash form the index stores, so `./src/x.ts`, an absolute path, and a
Windows back-slash path all match (previously silently returned 0).

Validated on luxon (24 files): node/edge totals identical (no explosion), 69
mis-promoted `instantiates` edges become `calls`, and real static factories
(DateTime.fromISO, etc.) resolve their callers. Full suite: 1534 passed.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Colby Mchenry 1 hete
szülő
commit
f7441f2124

+ 2 - 0
CHANGELOG.md

@@ -17,6 +17,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- Cross-file static method calls like `ClassName.staticMethod()` now resolve correctly. CodeGraph was linking the call to the *class* instead of the method (and recording it as a construction), so `callers` and `impact` for a static method came back empty — a real blind spot in TypeScript and JavaScript codebases that lean on static utility classes (Python and other languages with the same call shape benefit too). The call now links to the method itself. Thanks @contextFlow-lab. (#825)
+- `codegraph affected` now accepts `./`-prefixed and absolute file paths, not just bare project-relative ones. Passing `./src/x.ts` or an absolute path — common when the file list comes from another tool — used to silently match nothing and report no affected tests. Thanks @contextFlow-lab. (#825)
 - The CodeGraph MCP server no longer risks getting stuck at 100% CPU after an unexpected internal error. Previously such an error was logged but the process was left running in a broken state, where it could spin a CPU core indefinitely and had to be killed by hand. The server now logs the error and exits cleanly, so a fresh one starts on the next request. Thanks @songhlc. (#850)
 - CodeGraph no longer indexes your entire home directory by accident. Running the installer — or `codegraph init` / `codegraph index` — from your home folder or a filesystem root would index everything underneath it (caches, `Library`, every other project), producing a multi-gigabyte index and constant file-watching churn. CodeGraph now refuses these roots and points you at a specific project instead; pass `--force` if you genuinely mean to. (Combined with the macOS file-descriptor fix already in 1.0.0, this closes the report of a runaway watcher exhausting the system file limit.) Thanks @ligson. (#845)
 

+ 63 - 0
__tests__/cli-affected-paths.test.ts

@@ -0,0 +1,63 @@
+/**
+ * `codegraph affected` input-path normalization (#825).
+ *
+ * The index stores project-relative, forward-slash paths. A user (or a wrapping
+ * script) may pass a `./`-prefixed path or an absolute path; before #825 those
+ * silently matched nothing and reported 0 affected tests. All three spellings
+ * must now resolve the same affected test file.
+ *
+ * Exercised end-to-end against the built binary.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { execFileSync } from 'child_process';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { CodeGraph } from '../src';
+
+const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js');
+
+function affected(cwd: string, arg: string): string[] {
+  const out = execFileSync(process.execPath, [BIN, 'affected', arg, '--quiet', '-p', cwd], {
+    encoding: 'utf-8',
+    env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1' },
+    stdio: ['ignore', 'pipe', 'pipe'],
+  });
+  return out.split('\n').map((s) => s.trim()).filter(Boolean);
+}
+
+describe('codegraph affected — input path normalization (#825)', () => {
+  let tempDir: string;
+
+  beforeEach(async () => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-affected-paths-'));
+    fs.mkdirSync(path.join(tempDir, 'src'));
+    // util.ts <- helper.ts <- helper.test.ts (transitive test dependency)
+    fs.writeFileSync(path.join(tempDir, 'src/util.ts'), 'export function util(x: number){ return x + 1; }\n');
+    fs.writeFileSync(
+      path.join(tempDir, 'src/helper.ts'),
+      "import { util } from './util';\nexport function helper(){ return util(1); }\n",
+    );
+    fs.writeFileSync(
+      path.join(tempDir, 'src/helper.test.ts'),
+      "import { helper } from './helper';\ntest('t', () => helper());\n",
+    );
+    const cg = CodeGraph.initSync(tempDir);
+    await cg.indexAll();
+    cg.close();
+  });
+
+  afterEach(() => {
+    fs.rmSync(tempDir, { recursive: true, force: true });
+  });
+
+  it('bare-relative, ./-prefixed, and absolute paths all resolve the same affected test', () => {
+    const expected = ['src/helper.test.ts'];
+    // Baseline that always worked.
+    expect(affected(tempDir, 'src/util.ts')).toEqual(expected);
+    // Both of these returned [] before the normalization fix.
+    expect(affected(tempDir, './src/util.ts')).toEqual(expected);
+    expect(affected(tempDir, path.join(tempDir, 'src/util.ts'))).toEqual(expected);
+  });
+});

+ 40 - 0
__tests__/resolution.test.ts

@@ -768,6 +768,46 @@ def bootstrap():
       expect(callsToUserService).toHaveLength(0);
     });
 
+    it('resolves a cross-file static method call to the method, not the class (#825)', async () => {
+      // `Foo.bar()` where `Foo` is an imported class must link to the static
+      // method `Foo::bar`, NOT to the class `Foo`. Previously the import
+      // resolver dropped the `.bar` member and resolved to `Foo`, which the
+      // calls→instantiates promotion then turned into `run instantiates Foo`,
+      // leaving the static method with zero callers and a hollow impact radius.
+      fs.writeFileSync(
+        path.join(tempDir, 'helpers.ts'),
+        `export class Foo {\n  static bar(x: number) { return x + 1; }\n}\n`
+      );
+      fs.writeFileSync(
+        path.join(tempDir, 'caller.ts'),
+        `import { Foo } from './helpers';\nexport function run() { return Foo.bar(41); }\n`
+      );
+
+      cg = await CodeGraph.init(tempDir, { index: true });
+      cg.resolveReferences();
+
+      const bar = cg.getNodesByKind('method').find((n) => n.name === 'bar');
+      const foo = cg.getNodesByKind('class').find((n) => n.name === 'Foo');
+      const run = cg.getNodesByKind('function').find((n) => n.name === 'run');
+      expect(bar).toBeDefined();
+      expect(foo).toBeDefined();
+      expect(run).toBeDefined();
+
+      // `run` is reported as a caller of the static method `Foo.bar`.
+      const barCallers = cg.getCallers(bar!.id).map((c) => c.node.name);
+      expect(barCallers).toContain('run');
+
+      // And the call is NOT mis-promoted to `run instantiates Foo`.
+      const outgoing = cg.getOutgoingEdges(run!.id);
+      expect(
+        outgoing.filter((e) => e.kind === 'instantiates' && e.target === foo!.id)
+      ).toHaveLength(0);
+      // The real edge is a `calls` edge to the method.
+      expect(
+        outgoing.some((e) => e.kind === 'calls' && e.target === bar!.id)
+      ).toBe(true);
+    });
+
     it('resolves Go cross-package qualified calls via go.mod module path (#388)', async () => {
       // Pre-#388, every `pkga.FuncX(...)` call in a Go monorepo was flagged
       // external (isExternalImport returned true for any non-`/internal/`

+ 25 - 0
src/bin/codegraph.ts

@@ -1198,6 +1198,23 @@ program
     }
   });
 
+/**
+ * Normalize a user-supplied file path to the project-relative, forward-slash
+ * form CodeGraph stores in the index. Accepts an absolute path, a `./`-prefixed
+ * path, or Windows back-slashes; an empty string when the input is blank. Used
+ * by `codegraph affected` so `./src/x.ts`, `/abs/repo/src/x.ts`, and
+ * `src/x.ts` all match the same indexed file. (#825)
+ */
+function normalizeIndexPath(filePath: string, projectPath: string): string {
+  let f = filePath.trim();
+  if (!f) return '';
+  if (path.isAbsolute(f)) f = path.relative(projectPath, f);
+  // Collapse `.`/`..` segments, then force forward slashes and drop a leading
+  // `./` (path.normalize already strips it on POSIX; explicit for Windows).
+  f = path.normalize(f).replace(/\\/g, '/').replace(/^\.\//, '');
+  return f;
+}
+
 /**
  * Convert glob pattern to regex
  */
@@ -1710,6 +1727,14 @@ program
         changedFiles.push(...stdinFiles);
       }
 
+      // Normalize inputs to the project-relative, forward-slash form the index
+      // stores. Without this, `affected ./src/x.ts`, an absolute path (what a
+      // wrapping script often passes), or a Windows back-slash path silently
+      // matches nothing and reports 0 affected tests. (#825)
+      changedFiles = changedFiles
+        .map((f) => normalizeIndexPath(f, projectPath))
+        .filter(Boolean);
+
       if (changedFiles.length === 0) {
         if (!options.quiet) info('No files provided. Use file arguments or --stdin.');
         process.exit(0);

+ 62 - 0
src/resolution/import-resolver.ts

@@ -1296,6 +1296,25 @@ export function resolveViaImport(
         );
 
         if (targetNode) {
+          // `Foo.bar()` / `Foo.CONST` — a NAMED (non-namespace) class import
+          // accessed through a member. `findExportedSymbol` resolved `Foo` to
+          // the class itself; descend into it so the reference links to the
+          // member `bar`, not the class. Without this the edge points at the
+          // class and `createEdges` then mis-promotes the call to an
+          // `instantiates` edge, so the static method shows zero callers and a
+          // hollow impact radius. (#825)
+          if (!imp.isNamespace && ref.referenceName.startsWith(imp.localName + '.')) {
+            const memberNode = resolveStaticMember(targetNode, ref, imp.localName, context);
+            if (memberNode) {
+              return {
+                original: ref,
+                targetNodeId: memberNode.id,
+                confidence: 0.9,
+                resolvedBy: 'import',
+              };
+            }
+          }
+
           return {
             original: ref,
             targetNodeId: targetNode.id,
@@ -1896,3 +1915,46 @@ function findExportedSymbol(
 
   return undefined;
 }
+
+/** Node kinds that own static members reachable as `Container.member`. */
+const STATIC_MEMBER_CONTAINERS = new Set<Node['kind']>([
+  'class', 'struct', 'interface', 'enum', 'trait', 'protocol',
+]);
+
+/**
+ * Resolve `Container.member` — a static method/property access on a NAMED class
+ * import (`import { Foo } …; Foo.bar()`) — to the member node, given the
+ * already-resolved container class.
+ *
+ * Members carry a `Container::member` qualifiedName, so we look up
+ * `${container.qualifiedName}::${member}` within the container's own file (the
+ * file filter disambiguates same-named classes in other modules). Returns
+ * undefined when the container isn't a member-owning kind or the member isn't
+ * found, so the caller falls back to the container itself (prior behavior) —
+ * languages whose members aren't `::`-qualified, and genuine class references,
+ * are unaffected. See #825.
+ */
+function resolveStaticMember(
+  container: Node,
+  ref: UnresolvedRef,
+  localName: string,
+  context: ResolutionContext
+): Node | undefined {
+  if (!STATIC_MEMBER_CONTAINERS.has(container.kind)) return undefined;
+  // First segment after the receiver: `Foo.bar.baz` → `bar`.
+  const member = ref.referenceName.slice(localName.length + 1).split('.')[0];
+  if (!member) return undefined;
+
+  const candidates = context
+    .getNodesByQualifiedName(`${container.qualifiedName}::${member}`)
+    .filter((n) => n.filePath === container.filePath);
+  if (candidates.length === 0) return undefined;
+
+  // When the reference is a call, prefer a callable member if several nodes
+  // share the qualifiedName (e.g. a static property and a method).
+  if (ref.referenceKind === 'calls') {
+    const callable = candidates.find((n) => n.kind === 'method' || n.kind === 'function');
+    if (callable) return callable;
+  }
+  return candidates[0];
+}