Przeglądaj źródła

fix(resolution): an import naming the emitted .js extension resolves to its .ts source (#1767)

Fixes #1705. Lands #1706 (thanks @bompus), rebased onto main.
Colby Mchenry 7 godzin temu
rodzic
commit
28033f62f8

+ 1 - 0
CHANGELOG.md

@@ -220,6 +220,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - **A binding in a module that exports nothing is no longer a cross-file target.** On vite, every `import { defineConfig } from 'vite'` across the playground resolved onto a `const vite = await createServer(…)` sitting at module scope in `playground/ssr-html/test-stacktrace.js` — a file with an import and no export, so that binding is reachable from nowhere but itself. Name matching commits as soon as one candidate survives, and nothing asked whether an import could reach the survivor; that one binding took 157 edges. A JS/TS file holding an `import` and no export of any kind now offers its locals to no other file. Classic scripts, CommonJS (including `exports["x"] = …`), a later `export { … }`, and names contributed through `declare global` are all unaffected. Across vite this removed 320 wrong edges and added 18, each addition a reference that was previously ambiguous rather than newly invented. Re-index after upgrading. (#1719)
 - **A bare call inside a JavaScript or TypeScript method no longer resolves to the method itself.** When a method and a module-scope function share a name, `serialize(this.raw)` written inside `Record.serialize` means the function, but the nearest same-named definition won the tie and the graph recorded the method calling itself. A call written without a receiver can never reach a method in JS/TS, so methods are no longer candidates for it; `this.serialize()` and `other.serialize()` resolve as before. (#1714)
 - **Fuzzy matching no longer lands on a closure it cannot reach.** A function nested inside another function is only callable from inside its container, and exact-name matching already declined such candidates; the fuzzy fallback did not, so a builtin method call (`res.text()`, `items.push()`) whose only same-named project symbol was some file's closure resolved onto that closure. The fallback now checks that the one candidate it would commit to is reachable, and declines otherwise — it does not filter the candidate list first, which would turn a crowd of same-named definitions into a single "unique" survivor and hand it every call of that name. On vite that removes the 12 edges onto nested functions and adds none. Re-index after upgrading. Thanks @bompus. (#1708, #1709)
+- **An import that names the emitted extension resolves to its source.** Under `moduleResolution: node16 | nodenext | bundler` TypeScript requires `import { x } from './util.js'` for `util.ts`, and no file of that name exists, so the import resolver returned nothing and every name imported that way fell through to bare-name matching: a method wrapping the same-named helper it imports (`renderDockStyles() { return renderDockStyles(); }`) resolved to itself, and cross-module edges in such projects were name guesses. `.js` / `.jsx` / `.mjs` / `.cjs` specifiers now retry with the source extensions TypeScript compiles from when the emitted file is absent; a real `.js` beside the `.ts` still wins. On a 582-file repo whose `.ts` files import this way, import-backed `calls`/`imports` edges went from 4,002 to 7,312 and the eight wrapper-method self-edges disappeared. Re-index after upgrading. Thanks @bompus. (#1705, #1706)
 
 - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges.
 

+ 126 - 0
__tests__/import-emitted-specifier.test.ts

@@ -0,0 +1,126 @@
+/**
+ * TypeScript's node16/nodenext/bundler resolution writes the EMITTED extension
+ * in a relative specifier (`./util.js` for `util.ts`). The import resolver must
+ * map that back to the source file that is actually in the repo; otherwise the
+ * imported names fall through to bare-name matching and a method that wraps a
+ * same-named import resolves to itself.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import * as fs from 'fs';
+import * as path from 'path';
+import * as os from 'os';
+import { CodeGraph } from '../src';
+import { resolveImportPath } from '../src/resolution/import-resolver';
+import type { ResolutionContext } from '../src/resolution';
+
+function contextWithFiles(files: string[]): ResolutionContext {
+  const set = new Set(files);
+  return {
+    getNodesInFile: () => [],
+    getNodesByName: () => [],
+    getNodesByQualifiedName: () => [],
+    getNodesByKind: () => [],
+    fileExists: (p: string) => set.has(p),
+    readFile: () => null,
+    getProjectRoot: () => '/test',
+    getAllFiles: () => files,
+    getNodesByLowerName: () => [],
+    getImportMappings: () => [],
+  } as unknown as ResolutionContext;
+}
+
+describe('emitted-extension import specifiers (`./x.js` naming `x.ts`)', () => {
+  it('maps a relative .js specifier onto the .ts source', () => {
+    const ctx = contextWithFiles(['shared/engine.ts', 'shared/util.ts']);
+    expect(resolveImportPath('./util.js', 'shared/engine.ts', 'typescript', ctx)).toBe('shared/util.ts');
+  });
+
+  it('prefers a real .js file over the remap when both exist', () => {
+    const ctx = contextWithFiles(['shared/engine.ts', 'shared/util.js', 'shared/util.ts']);
+    expect(resolveImportPath('./util.js', 'shared/engine.ts', 'typescript', ctx)).toBe('shared/util.js');
+  });
+
+  it('maps .jsx, .mjs and .cjs onto their TypeScript sources', () => {
+    const ctx = contextWithFiles(['app/a.tsx', 'app/View.tsx', 'app/esm.mts', 'app/cjs.cts']);
+    expect(resolveImportPath('./View.jsx', 'app/a.tsx', 'tsx', ctx)).toBe('app/View.tsx');
+    expect(resolveImportPath('./esm.mjs', 'app/a.tsx', 'tsx', ctx)).toBe('app/esm.mts');
+    expect(resolveImportPath('./cjs.cjs', 'app/a.tsx', 'tsx', ctx)).toBe('app/cjs.cts');
+  });
+
+  it('maps an aliased .js specifier through tsconfig paths', () => {
+    const files = ['src/main.ts', 'src/lib/util.ts'];
+    const ctx = {
+      ...contextWithFiles(files),
+      getProjectAliases: () => ({
+        baseUrl: '/test',
+        patterns: [{ prefix: '@/', suffix: '', hasWildcard: true, replacements: ['src/*'] }],
+      }),
+    } as unknown as ResolutionContext;
+    expect(resolveImportPath('@/lib/util.js', 'src/main.ts', 'typescript', ctx)).toBe('src/lib/util.ts');
+  });
+
+  it('leaves a specifier that names no source unresolved', () => {
+    const ctx = contextWithFiles(['shared/engine.ts']);
+    expect(resolveImportPath('./missing.js', 'shared/engine.ts', 'typescript', ctx)).toBeNull();
+  });
+
+  it('does not remap for a language without TypeScript emit (python)', () => {
+    const ctx = contextWithFiles(['pkg/a.py', 'pkg/b.ts']);
+    expect(resolveImportPath('./b.js', 'pkg/a.py', 'python', ctx)).toBeNull();
+  });
+});
+
+describe('end to end: a wrapper method calling the same-named import it wraps', () => {
+  let tempDir: string;
+  let cg: CodeGraph | null = null;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-emitted-spec-'));
+  });
+
+  afterEach(() => {
+    cg?.destroy();
+    cg = null;
+    try {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    } catch {
+      // Windows can still hold the SQLite handle for a moment; the OS temp dir is swept anyway.
+    }
+  });
+
+  it('links the call to the imported function, not to the method itself', async () => {
+    fs.writeFileSync(
+      path.join(tempDir, 'template.ts'),
+      'export function renderDockStyles(): string {\n  return ".dock {}";\n}\n'
+    );
+    fs.writeFileSync(
+      path.join(tempDir, 'sidebar.ts'),
+      [
+        'import { renderDockStyles } from "./template.js";',
+        '',
+        'export class Sidebar {',
+        '  renderDockStyles(): string {',
+        '    return renderDockStyles();',
+        '  }',
+        '}',
+        '',
+      ].join('\n')
+    );
+    cg = await CodeGraph.init(tempDir, { index: true });
+    cg.resolveReferences();
+
+    const method = cg.getNodesByKind('method').find((n) => n.name === 'renderDockStyles');
+    const fn = cg
+      .getNodesByKind('function')
+      .find((n) => n.name === 'renderDockStyles' && n.filePath === 'template.ts');
+    expect(method).toBeDefined();
+    expect(fn).toBeDefined();
+    const targets = cg
+      .getOutgoingEdges(method!.id)
+      .filter((e) => e.kind === 'calls')
+      .map((e) => e.target);
+    expect(targets).toContain(fn!.id);
+    expect(targets).not.toContain(method!.id);
+  });
+});

+ 41 - 1
src/resolution/import-resolver.ts

@@ -452,9 +452,49 @@ function resolveRelativeImport(
     return relativePath;
   }
 
+  return findSourceForEmittedSpecifier(relativePath, language, context);
+}
+
+/**
+ * TypeScript under `moduleResolution: node16 | nodenext | bundler` writes the
+ * EMITTED extension in the specifier (`import x from './util.js'` for
+ * `util.ts`, `.mjs` for `.mts`, `.cjs` for `.cts`), and the source file with that
+ * exact name never exists in the repo. Without this remap the import resolver
+ * returned null for every such import, so each imported name fell through to
+ * bare-name matching: a method wrapping the same-named helper it imports
+ * (`renderDockStyles() { return renderDockStyles() }`) resolved to ITSELF, and
+ * any repo-wide same-named symbol could win the cross-module edge.
+ */
+const EMITTED_TO_SOURCE_EXTENSIONS: ReadonlyArray<readonly [RegExp, readonly string[]]> = [
+  [/\.js$/, ['.ts', '.tsx', '.d.ts']],
+  [/\.jsx$/, ['.tsx']],
+  [/\.mjs$/, ['.mts', '.d.mts']],
+  [/\.cjs$/, ['.cts', '.d.cts']],
+];
+
+function findSourceForEmittedSpecifier(
+  relativePath: string,
+  language: Language,
+  context: ResolutionContext
+): string | null {
+  if (!EMITTED_SPECIFIER_LANGUAGES.has(language)) return null;
+  for (const [emitted, sources] of EMITTED_TO_SOURCE_EXTENSIONS) {
+    if (!emitted.test(relativePath)) continue;
+    const stem = relativePath.replace(emitted, '');
+    for (const ext of sources) {
+      const candidate = stem + ext;
+      if (context.fileExists(candidate)) return candidate;
+    }
+    return null;
+  }
   return null;
 }
 
+/** Languages whose import specifiers can name the emitted `.js` of a `.ts` source. */
+const EMITTED_SPECIFIER_LANGUAGES: ReadonlySet<string> = new Set([
+  'typescript', 'tsx', 'javascript', 'jsx', 'vue', 'svelte', 'astro', 'arkts',
+]);
+
 /**
  * Resolve an aliased/absolute import.
  *
@@ -479,7 +519,7 @@ function resolveAliasedImport(
       if (context.fileExists(candidate)) return candidate;
     }
     if (context.fileExists(basePath)) return basePath;
-    return null;
+    return findSourceForEmittedSpecifier(basePath, language, context);
   };
 
   // 1. Project tsconfig/jsconfig paths.