diff --git a/CHANGELOG.md b/CHANGELOG.md index 1758fc1..1fd25a1 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/__tests__/kernel-tsjs-parity.test.ts b/__tests__/kernel-tsjs-parity.test.ts index c16d41f..3d4316a 100644 --- a/__tests__/kernel-tsjs-parity.test.ts +++ b/__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'); diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index 7c697a3..b8d25f6 100644 --- a/__tests__/resolution.test.ts +++ b/__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 ? '' : ''}(); + values.set('answer', '42'); + values.get('answer'); + return values.has('answer'); +} +export function useNestedMap(holder${typed ? ': { values: Map }' : ''}) { + 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}) { + 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 diff --git a/__tests__/ts-chained-receiver.test.ts b/__tests__/ts-chained-receiver.test.ts index 71049d4..cdfa95b 100644 --- a/__tests__/ts-chained-receiver.test.ts +++ b/__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..m()` reach real targets. + * Those are dropped, as are untyped identifier chains (#1566). The existing + * `window.MyNs.run()` and `this..m()` paths remain outside that guard. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index 844916b..c46495e 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/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; } } diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index d1b5b7e..030607b 100644 --- a/src/extraction/tree-sitter.ts +++ b/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 { diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 2502ccb..4d6b664 100644 --- a/src/resolution/index.ts +++ b/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([ @@ -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', diff --git a/src/resolution/js-builtins.ts b/src/resolution/js-builtins.ts new file mode 100644 index 0000000..ad3e338 --- /dev/null +++ b/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', +]); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index f18acf2..9f9b3b3 100644 --- a/src/resolution/name-matcher.ts +++ b/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` → + // `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; + } } }