fix(resolution): an out-of-repo import doesn't hide a same-file nested declaration

CHANGELOG.md

@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- In JavaScript and TypeScript, a function declared inside another function now takes calls to its name even when the file also imports that name from a package, such as `resolve` from `node:path`. Such calls used to link to nothing. (#1715)
 - In TypeScript, an awaited call with type arguments like `await fetchApi<User>("/me")` or `await api.get<T>(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586)
 - In React, a component rendered at module scope, in a top-level constant or inside a test's `it(…)` callback, like `createRoot(el).render(<App />)` or `render(<Card />)`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587)
 - In Python, `import a.b as X` written inside a function now links its calls like `X.f()`, as the same import at module level already did. Thanks @Egrn for the report. (#2600)

__tests__/out-of-repo-import-shadow.test.ts

@@ -0,0 +1,69 @@
+/**
+ * A bare JS/TS call through a name imported from outside the repository
+ * (`import { resolve } from 'node:path'`) means the package's function, never
+ * another file's same-named symbol. A declaration of that name in an inner
+ * scope of the calling file still shadows the import, and the call is that
+ * local declaration's.
+ */
+
+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-import-shadow-'));
+  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);
+  }
+}
+
+async function callTargets(caller: string): Promise<string[]> {
+  cg = await CodeGraph.init(tempDir, { index: true });
+  cg.resolveReferences();
+  const from = cg.getNodesByKind('function').find((n) => n.name === caller)!;
+  expect(from).toBeDefined();
+  return cg.getOutgoingEdges(from.id).filter((e) => e.kind === 'calls')
+    .map((e) => { const t = cg!.getNode(e.target)!; return `${t.filePath}:${t.name}:${t.startLine}`; });
+}
+
+afterEach(() => {
+  cg?.close();
+  cg = null;
+  fs.rmSync(tempDir, { recursive: true, force: true });
+});
+
+describe('a name imported from outside the repository', () => {
+  it('is shadowed by a declaration in an inner scope of the calling file', async () => {
+    project({
+      'plugin.ts': "export function resolve() { return 'plugin'; }",
+      'config.ts':
+        "import { resolve } from 'node:path';\n" +
+        "export function configure() {\n  function resolve() { return 'local'; }\n  return resolve();\n}\n",
+    });
+    expect(await callTargets('configure')).toEqual(['config.ts:resolve:3']);
+  });
+
+  it('binds no other file\'s symbol of that name', async () => {
+    project({
+      'package.json': JSON.stringify({ name: 'app', dependencies: { 'external-resolver': '^1.0.0' } }),
+      'plugin.ts': "export function resolve() { return 'plugin'; }",
+      'config.ts': "import { resolve } from 'external-resolver';\nexport function configure() { return resolve('src'); }\n",
+    });
+    expect(await callTargets('configure')).toEqual([]);
+  });
+
+  it('is not shadowed by its own top-level require binding', async () => {
+    project({
+      'package.json': JSON.stringify({ name: 'app', devDependencies: { supertest: '^6' } }),
+      'app.js': "var request = require('supertest');\nfunction run(app) { return request(app); }\nmodule.exports = run;\n",
+    });
+    expect(await callTargets('run')).toEqual([]);
+  });
+});

src/resolution/name-matcher.ts

@@ -11658,6 +11658,23 @@ function bindsAtSites(source: string, name: string, sites: LocalBindingSites): b
   return false;
 }
 
+/**
+ * The calling file's declarations of an out-of-repo imported name that sit
+ * inside a function or method — only those can shadow the import. A top-level
+ * one is the import's own binding (`request = require('supertest')`), not a
+ * shadow of it.
+ */
+function innerScopeShadows(nodes: Node[], ref: UnresolvedRef, context: ResolutionContext): Node[] {
+  const local = nodes.filter((n) => n.filePath === ref.filePath);
+  if (local.length === 0) return local;
+  const scopes = context.getNodesInFile(ref.filePath).filter((n) => n.kind === 'function' || n.kind === 'method');
+  return local.filter((n) => scopes.some((s) =>
+    s.id !== n.id &&
+    s.startLine <= n.startLine &&
+    (s.endLine ?? s.startLine) >= (n.endLine ?? n.startLine) &&
+    (s.startLine < n.startLine || (s.endLine ?? s.startLine) > (n.endLine ?? n.startLine))));
+}
+
 /**
  * Try to resolve a reference by exact name match
  */
@@ -11685,14 +11702,16 @@ export function matchByExactName(
       /^[A-Za-z_$][\w$]*$/.test(ref.referenceName) && isOutOfRepoBinding(ref.referenceName, ref, context)) {
     return null;
   }
+  // `import { useQuery } from '@tanstack/react-query'`: the call means the
+  // package's, and no other file's same-named symbol. A declaration of the
+  // name in this file's inner scope still shadows the import.
+  let outOfRepoCall = false;
   if (bareJs) {
     const storeAction = matchJsStoreBindingCall(ref, context);
     if (storeAction) return storeAction;
     const returned = matchDestructuredCallResult(ref, context);
     if (returned) return returned;
-    // `import { useQuery } from '@tanstack/react-query'`: the call means the
-    // package's, and no same-named project symbol.
-    if (isOutOfRepoBinding(ref.referenceName, ref, context)) return null;
+    outOfRepoCall = isOutOfRepoBinding(ref.referenceName, ref, context);
   }
   // Every rule below judges one candidate on its own, so they run as ONE pass,
   // the kind/language checks before the ones that read source: a common name
@@ -11704,7 +11723,9 @@ export function matchByExactName(
   // Read once, and only for a candidate in another file: it scans the file.
   let bareTypeName: boolean | undefined;
   const tsBareTypeName = (): boolean => (bareTypeName ??= isTsBareTypeName(ref, context));
-  const sameName = context.getNodesByName(ref.referenceName);
+  const sameName = outOfRepoCall
+    ? innerScopeShadows(context.getNodesByName(ref.referenceName), ref, context)
+    : context.getNodesByName(ref.referenceName);
   // `NAME(...)` where NAME is a function-like macro somewhere in the project is
   // an expansion or a call to a same-named function — never the macro itself
   // (#1839), and never a type that happens to share the name (#2070: expat's