1
0
Эх сурвалжийг харах

fix(resolution): leave built-in Map calls unresolved (#1566) (#1790)

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 1 цаг өмнө
parent
commit
de5adba7ea

+ 2 - 0
CHANGELOG.md

@@ -219,6 +219,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 #### Symbols, tests and the viewer
 
+- TypeScript and JavaScript collection calls through local variables and their nested properties no longer link to unrelated project methods; re-index after upgrading. (#1566)
+
 - Objective-C headers now index in a project that has no `.m` file. A `.h` file is read as C from its name alone, and only later — once its contents are read — recognized as Objective-C; the grammar for that was never loaded up front, so the file failed with a parser error and nothing in it reached the index. Adding any `.m` file used to make the same header work, which is what made this look arbitrary. Thanks @Juddd. (#1628)
 
 - TypeScript interface methods and properties are now indexed, so `node`, `callers` and impact can find platform `.d.ts` APIs while declaration-only files keep their lower ranking on flow queries; re-index TypeScript projects after upgrading. (#1638)

+ 25 - 1
__tests__/kernel-tsjs-parity.test.ts

@@ -76,7 +76,7 @@ describe.skipIf(!kernelBuilt)('kernel TS/JS extraction parity', () => {
     resetKernelForTests();
   });
 
-  function assertParity(filePath: string, source: string, language: Language): void {
+  function assertParity(filePath: string, source: string, language: Language): ExtractionResult {
     process.env.CODEGRAPH_KERNEL_LANGS = 'all';
     delete process.env.CODEGRAPH_KERNEL;
     const viaKernel = tryKernelExtract(filePath, source, language);
@@ -93,8 +93,32 @@ describe.skipIf(!kernelBuilt)('kernel TS/JS extraction parity', () => {
     expect(k.refs, `${filePath}: refs`).toEqual(w.refs);
     // Meaningful comparison, not empty-vs-empty.
     expect(viaWasm.nodes.length).toBeGreaterThan(3);
+    return viaWasm;
   }
 
+  it.each([
+    ['ts', 'typescript'], ['tsx', 'tsx'], ['js', 'javascript'], ['jsx', 'jsx'],
+  ] as const)('leaves nested identifier receivers unresolved and keeps argument calls: %s (#1566)', (ext, language) => {
+    const result = assertParity(`fixture.${ext}`, `
+function readKey() { return 'answer'; }
+function local() {
+  const values = new Map();
+  return values.get(readKey());
+}
+function nested(holder) {
+  holder.values.get(readKey());
+  holder.values?.get(readKey());
+  holder['values'].get(readKey());
+  holder.deep.values.get(readKey());
+}
+`, language);
+    const nested = result.nodes.find((n) => n.name === 'nested' && n.kind === 'function');
+    expect(nested).toBeDefined();
+    expect(result.unresolvedReferences.filter((r) => r.referenceKind === 'calls' && r.fromNodeId === nested!.id)
+      .map((r) => r.referenceName)).toEqual(['readKey', 'readKey', 'readKey', 'readKey']);
+    expect(result.unresolvedReferences.some((r) => r.referenceName === 'values.get')).toBe(true);
+  });
+
   it('torture fixture (tsx): components, stores, RTK, fn-refs, value-refs, decorators', () => {
     const file = path.join(FIXTURE_DIR, 'torture.tsx');
     assertParity('fixtures/torture.tsx', fs.readFileSync(file, 'utf8'), 'tsx');

+ 81 - 0
__tests__/resolution.test.ts

@@ -2316,6 +2316,87 @@ func main() {
   });
 
   describe('Local-variable receiver-type inference (#1108)', () => {
+    it.each(['ts', 'tsx', 'js', 'jsx'])('keeps built-in Map calls off project methods — %s (#1566)', async (ext) => {
+      const typed = ext === 'ts' || ext === 'tsx';
+      fs.writeFileSync(path.join(tempDir, `cache.${ext}`), `
+export class LRUCache {
+  get(key) { return key; }
+  set(key, value) { return value; }
+  has(key) { return true; }
+}
+export function useLocalMap() {
+  const values = new Map${typed ? '<string, string>' : ''}();
+  values.set('answer', '42');
+  values.get('answer');
+  return values.has('answer');
+}
+export function useNestedMap(holder${typed ? ': { values: Map<string, string> }' : ''}) {
+  return holder.values.get('answer');
+}
+export function useProjectCache() {
+  const cache = new LRUCache();
+  cache.set('answer', '42');
+  cache.get('answer');
+  return cache.has('answer');
+}
+`);
+      cg = await CodeGraph.init(tempDir, { index: true });
+      cg.resolveReferences();
+
+      for (const name of ['useLocalMap', 'useNestedMap', 'useProjectCache']) {
+        const caller = cg.getNodesByName(name).find((n) => n.kind === 'function');
+        expect(caller, name).toBeDefined();
+        const calls = cg.getOutgoingEdges(caller!.id).filter((e) => e.kind === 'calls');
+        if (name === 'useProjectCache') {
+          const methods = cg.getNodesByKind('method').filter((n) => n.qualifiedName.startsWith('LRUCache::'));
+          expect(methods).toHaveLength(3);
+          expect(calls.map((e) => e.target).sort()).toEqual(methods.map((n) => n.id).sort());
+          expect(calls.every((e) => e.metadata?.confidence === 0.9)).toBe(true);
+        } else {
+          expect.soft(calls, `${ext}: ${name} must not call a project method`).toEqual([]);
+        }
+      }
+    });
+
+    it('keeps a validated project class that shadows Map (#1566)', async () => {
+      fs.writeFileSync(path.join(tempDir, 'shadow.ts'), `
+export class Map { get() { return 1; } }
+export class Other { get() { return 2; } }
+export function useShadow() {
+  const values = new Map();
+  return values.get();
+}
+`);
+      cg = await CodeGraph.init(tempDir, { index: true });
+      const caller = cg.getNodesByName('useShadow').find((n) => n.kind === 'function');
+      expect(caller).toBeDefined();
+      expect(cg.getCallees(caller!.id).filter(({ edge }) => edge.kind === 'calls').map(({ node }) => node.qualifiedName))
+        .toEqual(['Map::get']);
+    });
+
+    it.each([
+      ['Set', 'has'], ['WeakMap', 'get'], ['WeakSet', 'has'], ['Array', 'map'], ['Promise', 'then'],
+    ])('declines same-name guesses for an inferred %s receiver (#1566)', async (type, method) => {
+      fs.writeFileSync(path.join(tempDir, 'builtin.ts'), `
+export class Collision { ${method}() { return 1; } }
+export function constructed() {
+  const values = new ${type}();
+  return values.${method}();
+}
+export function annotated(values: ${type}<string>) {
+  return values.${method}();
+}
+`);
+      cg = await CodeGraph.init(tempDir, { index: true });
+      cg.resolveReferences();
+      expect(cg.getNodesByKind('method').some((n) => n.name === method)).toBe(true);
+      for (const name of ['constructed', 'annotated']) {
+        const caller = cg.getNodesByName(name).find((n) => n.kind === 'function');
+        expect(caller, name).toBeDefined();
+        expect.soft(cg.getOutgoingEdges(caller!.id).filter((e) => e.kind === 'calls'), name).toEqual([]);
+      }
+    });
+
     // `lg.log()` where `lg` is a local whose type is inferred from its
     // declaration/initializer. Before this, only C++ resolved these; every
     // other language produced no method edge. Each case is one file with a

+ 2 - 2
__tests__/ts-chained-receiver.test.ts

@@ -3,8 +3,8 @@
  * .get(k)`, `document.body.querySelector(s)` — ends in a platform API. Emitting
  * the bare method name for it let every such call exact-match whatever project
  * symbol shared the name, so a storage wrapper's `get` called itself (#1707).
- * Those are dropped. A chain rooted at a project value keeps the bare name:
- * `window.MyNs.run()` and `this.<field>.m()` reach real targets.
+ * Those are dropped, as are untyped identifier chains (#1566). The existing
+ * `window.MyNs.run()` and `this.<field>.m()` paths remain outside that guard.
  */
 
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';

+ 11 - 18
codegraph-kernel/src/tsjs/extractors.rs

@@ -1122,15 +1122,10 @@ impl<'t> Walker<'t> {
 
     // --- extractCall (TS/JS generic tail) -------------------------------------------------
 
-    /// Whether a member-call receiver is a chain rooted at a host object a
-    /// TS/JS project never declares. `window` is absent on purpose:
-    /// `window.MyNs.doThing()` reaches a project symbol (#1707).
-    fn is_host_global_chain(&self, receiver: Node<'t>) -> bool {
-        const HOST_GLOBAL_ROOTS: [&str; 19] = [
-            "chrome", "browser", "document", "navigator", "performance", "console",
-            "localStorage", "sessionStorage", "indexedDB", "crypto", "globalThis",
-            "process", "Math", "JSON", "Object", "Array", "Reflect", "Promise", "Intl",
-        ];
+    /// Identifier-rooted member chains have no inferred property type (#1566),
+    /// including host API chains (#1707). Keep the existing window namespace
+    /// escape; call-result and `this` receivers are outside this guard.
+    fn is_unresolved_member_chain(&self, receiver: Node<'t>) -> bool {
         let mut cur = receiver;
         if !matches!(cur.kind(), "member_expression" | "subscript_expression") {
             return false;
@@ -1141,7 +1136,7 @@ impl<'t> Walker<'t> {
                 None => return false,
             }
         }
-        cur.kind() == "identifier" && HOST_GLOBAL_ROOTS.contains(&self.text(cur))
+        cur.kind() == "identifier" && self.text(cur) != "window"
     }
 
     pub(super) fn extract_call(&mut self, node: Node<'t>) {
@@ -1171,14 +1166,12 @@ impl<'t> Walker<'t> {
                         if is_literal_receiver(r.kind()) {
                             return;
                         }
-                        // A chain rooted at a host namespace — `chrome.storage
-                        // .local.get(k)`, `document.body.querySelector(s)` —
-                        // ends in a platform API, so the bare method name emitted
-                        // here could only exact-match an unrelated project symbol
-                        // sharing it (#1707). Emit nothing. A chain rooted at a
-                        // project value keeps the bare name. Mirrors the TS
-                        // extractor's extractCall (extraction/tree-sitter.ts).
-                        if self.is_host_global_chain(r) {
+                        // `holder.values.get()` has no inferred property type
+                        // (#1566). Dropping the receiver or merely preserving it
+                        // would allow unrelated same-name method guesses. Emit
+                        // nothing, as for host chains (#1707); argument calls are
+                        // visited independently. Mirrors extractCall in TS.
+                        if self.is_unresolved_member_chain(r) {
                             return;
                         }
                     }

+ 13 - 30
src/extraction/tree-sitter.ts

@@ -407,34 +407,21 @@ const LITERAL_RECEIVER_TYPES = new Set([
  */
 const TS_JS_CHAIN_LANGUAGES = new Set(['typescript', 'tsx', 'javascript', 'jsx']);
 
-/**
- * Host objects a TS/JS project never declares: the browser, extension, and
- * runtime namespaces, plus the builtin constructors whose statics are library
- * calls. A member chain ROOTED at one of these ends in a platform API, so the
- * bare method name the extractor used to emit for `chrome.storage.local.get(k)`
- * or `document.body.querySelector(s)` could only ever exact-match an unrelated
- * project symbol that happened to share the name (#1707). `window` is absent on
- * purpose: `window.MyNamespace.doThing()` reaches a project symbol.
- */
-const TS_JS_HOST_GLOBAL_ROOTS = new Set([
-  'chrome', 'browser', 'document', 'navigator', 'performance', 'console',
-  'localStorage', 'sessionStorage', 'indexedDB', 'crypto', 'globalThis',
-  'process', 'Math', 'JSON', 'Object', 'Array', 'Reflect', 'Promise', 'Intl',
-]);
-
 /** Receiver node types (TS/JS grammars) that continue a member chain downward. */
 const TS_JS_CHAIN_RECEIVER_TYPES = new Set(['member_expression', 'subscript_expression']);
 
 /**
- * Root identifier of a TS/JS member chain — `chrome` for `chrome.storage.local`
- * — or null when the chain bottoms out in a call, a literal, or `this`.
+ * Identifier-rooted member chains have no inferred property type (#1566),
+ * including host API chains (#1707). Keep the existing `window.MyNamespace`
+ * escape for project globals; call-result and `this` receivers have their own
+ * paths and are outside this guard.
  */
-function tsJsChainRoot(node: SyntaxNode, source: string): string | null {
+function isUnresolvedTsJsChain(node: SyntaxNode, source: string): boolean {
   let cur: SyntaxNode | null = node;
   while (cur && TS_JS_CHAIN_RECEIVER_TYPES.has(cur.type)) {
     cur = getChildByField(cur, 'object');
   }
-  return cur && cur.type === 'identifier' ? getNodeText(cur, source) : null;
+  return !!cur && cur.type === 'identifier' && getNodeText(cur, source) !== 'window';
 }
 
 /**
@@ -4786,18 +4773,14 @@ export class TreeSitterExtractor {
               TS_JS_CHAIN_LANGUAGES.has(this.language) &&
               receiver &&
               TS_JS_CHAIN_RECEIVER_TYPES.has(receiver.type) &&
-              TS_JS_HOST_GLOBAL_ROOTS.has(tsJsChainRoot(receiver, this.source) ?? '')
+              isUnresolvedTsJsChain(receiver, this.source)
             ) {
-              // TS/JS member call reached through a host namespace —
-              // `chrome.storage.local.get(key)`, `document.body.querySelector(s)`.
-              // The bare method name this used to emit exact-matched whatever
-              // project symbol shared it: every `chrome.storage.local.get/set`
-              // in a storage wrapper bound to the wrapper's own `get`/`set`,
-              // a self-edge not in the source (#1707). Emit nothing: a silent
-              // miss, never a wrong edge. A chain rooted at a project value
-              // (`window.MyNs.run()`, `store.getState().act()`, `ref.value.m()`)
-              // keeps the bare name — those targets are real, and dropping them
-              // would cost far more recall than the mis-bind costs precision.
+              // `holder.values.get()` has no inferred property type (#1566).
+              // Emitting bare `get` exact-matches an unrelated project method;
+              // preserving the chain alone would still allow receiver guessing.
+              // Emit nothing until the property type can be established. This
+              // also covers host chains such as `chrome.storage.local.get()`
+              // (#1707). Calls inside arguments are visited independently.
               // Mirrored in the kernel's extract_call (tsjs/extractors.rs).
               return;
             } else {

+ 1 - 8
src/resolution/index.ts

@@ -29,6 +29,7 @@ import { logDebug } from '../errors';
 import { lexicalPathWithinRoot } from '../utils';
 import type { ReExport } from './types';
 import { LRUCache } from './lru-cache';
+import { JS_BUILT_INS } from './js-builtins';
 
 /** Node kinds that can declare supertypes (extends/implements). */
 const SUPERTYPE_BEARING_KINDS = new Set<Node['kind']>([
@@ -70,14 +71,6 @@ function resolveCacheLimit(): number {
 export * from './types';
 
 // Pre-built Sets for O(1) built-in lookups (allocated once, shared across all instances)
-const JS_BUILT_INS = new Set([
-  'console', 'window', 'document', 'global', 'process',
-  'Promise', 'Array', 'Object', 'String', 'Number', 'Boolean',
-  'Date', 'Math', 'JSON', 'RegExp', 'Error', 'Map', 'Set',
-  'setTimeout', 'setInterval', 'clearTimeout', 'clearInterval',
-  'fetch', 'require', 'module', 'exports', '__dirname', '__filename',
-]);
-
 const REACT_HOOKS = new Set([
   'useState', 'useEffect', 'useContext', 'useReducer', 'useCallback',
   'useMemo', 'useRef', 'useLayoutEffect', 'useImperativeHandle', 'useDebugValue',

+ 8 - 0
src/resolution/js-builtins.ts

@@ -0,0 +1,8 @@
+/** Shared JS/TS built-ins for direct references and inferred receiver types. */
+export const JS_BUILT_INS = new Set([
+  'console', 'window', 'document', 'global', 'process',
+  'Promise', 'Array', 'Object', 'String', 'Number', 'Boolean',
+  'Date', 'Math', 'JSON', 'RegExp', 'Error', 'Map', 'Set', 'WeakMap', 'WeakSet',
+  'setTimeout', 'setInterval', 'clearTimeout', 'clearInterval',
+  'fetch', 'require', 'module', 'exports', '__dirname', '__filename',
+]);

+ 8 - 0
src/resolution/name-matcher.ts

@@ -8,6 +8,7 @@ import * as path from 'path';
 import { Language, Node } from '../types';
 import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types';
 import { blankStringContents, stripCommentsForRegex } from './strip-comments';
+import { JS_BUILT_INS } from './js-builtins';
 
 /**
  * Ceiling on how many same-named definitions a FUZZY name-match strategy will
@@ -2194,6 +2195,13 @@ export function matchMethodCall(
       if (typedMatch) {
         return typedMatch;
       }
+      // A known JS/TS builtin receiver is external when it has no project
+      // method (#1566). Inference already strips generics (`Map<K, V>` →
+      // `Map`); do not let Strategy 3 guess an unrelated `get`/`set`/`has`.
+      // Keep the validated match above for a project type shadowing a builtin.
+      if (ESM_FAMILY.has(ref.language) && JS_BUILT_INS.has(inferredType)) {
+        return null;
+      }
     }
   }