diff --git a/__tests__/fixtures/kernel-parity/torture.tsx b/__tests__/fixtures/kernel-parity/torture.tsx index b7f4be749..313542284 100644 --- a/__tests__/fixtures/kernel-parity/torture.tsx +++ b/__tests__/fixtures/kernel-parity/torture.tsx @@ -221,6 +221,14 @@ export class FieldDelegator { direct(): void { this.send('x'); super.toString(); } } +// --- call through an ES private field (#1987) -------------------------------- +export class PrivateDelegator { + #mailer = new FieldDelegator({ send: (m: string) => m }, []); + #items = new Set(); + send(msg: string): string { return this.#mailer.send(msg); } + add(x: string): void { this.#items.add(x); } +} + // --- const-bound functions inside a body (#1669) ----------------------------- export function NestedHandlers({ items, onPick }: { items: string[]; onPick: (a: unknown, b: unknown) => void }) { const handleClear = () => { onPick(null, null); }; diff --git a/__tests__/js-builtin-method-calls.test.ts b/__tests__/js-builtin-method-calls.test.ts new file mode 100644 index 000000000..d2d1f8f4c --- /dev/null +++ b/__tests__/js-builtin-method-calls.test.ts @@ -0,0 +1,117 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let dir: string; +let cg: CodeGraph | undefined; +afterEach(() => { + cg?.destroy(); + cg = undefined; + if (dir) fs.rmSync(dir, { recursive: true, force: true }); +}); + +const families = { + array: ['map', 'filter', 'reduce', 'forEach', 'find', 'findIndex', 'some', 'every', 'push', 'pop', 'slice', 'splice', 'sort', 'flatMap', 'includes', 'join'], + collection: ['get', 'set', 'has', 'add', 'delete', 'clear', 'keys', 'values', 'entries'], + string: ['trim', 'split', 'replace', 'replaceAll', 'match', 'matchAll', 'search', 'startsWith', 'endsWith', 'substring', 'toLowerCase', 'toUpperCase'], + promise: ['then', 'catch', 'finally'], + function: ['call', 'apply', 'bind'], + event: ['on', 'once', 'off', 'emit', 'addListener', 'removeListener', 'removeAllListeners', 'addEventListener', 'removeEventListener', 'dispatchEvent'], + iterator: ['next', 'return', 'throw'], +}; + +describe.each(['ts', 'tsx', 'js', 'jsx'])('built-in method calls in %s (#1987)', (ext) => { + it('does not guess project methods for unknown receivers, including class-name overlap', async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1987-')); + const declarations: string[] = []; + const calls: string[] = []; + for (const [family, methods] of Object.entries(families)) { + // Receiver and class share a word; this is still no evidence of its type. + declarations.push(`export class ${family}Wrapper {\n${methods.map(m => ` ${m}() {}\n`).join('')}}\n`); + methods.forEach(m => calls.push(`export function ${family}_${m}(${family}) { ${family}.${m}(); }\n`)); + } + fs.writeFileSync(path.join(dir, `decoys.${ext}`), declarations.join('')); + fs.writeFileSync(path.join(dir, `calls.${ext}`), calls.join('')); + cg = await CodeGraph.init(dir, { index: true }); + for (const caller of cg.getNodesByKind('function').filter(n => n.filePath === `calls.${ext}`)) { + expect(cg.getCallees(caller.id).map(c => c.node.qualifiedName), caller.name).toEqual([]); + } + expect(cg.getNodesByKind('function').filter(n => n.filePath === `calls.${ext}`)).toHaveLength(calls.length); + }); + + it('retains every method family on a constructed or typed project receiver', async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1987-')); + const methods = [...new Set(Object.values(families).flat())]; + const typed = ext === 'ts' || ext === 'tsx'; + fs.writeFileSync(path.join(dir, `project.${ext}`), ` +export class Project { +${methods.map(m => ` ${m}() {}\n`).join('')} +} +export function useConstructed() { + const receiver = new Project(); +${methods.map(m => ` receiver.${m}();\n`).join('')} +} +${typed ? `export function useTyped(receiver: Project) { +${methods.map(m => ` receiver.${m}();\n`).join('')} +}` : ''} +`); + cg = await CodeGraph.init(dir, { index: true }); + for (const name of typed ? ['useConstructed', 'useTyped'] : ['useConstructed']) { + const caller = cg.getNodesByKind('function').find(n => n.name === name)!; + expect(cg.getCallees(caller.id).filter(c => c.node.kind === 'method').map(c => c.node.name).sort()) + .toEqual([...methods].sort()); + } + }); + + it('rejects built-in values but retains constructed, imported, literal and class receivers', async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1987-')); + const typed = ext === 'ts' || ext === 'tsx'; + fs.writeFileSync(path.join(dir, `cart.${ext}`), ` +export class Cart { + map() {} + add() {} + get() {} + static bind() {} + self() { this.add(); } +} +export const api = { map() {} }; +export class LRUCache { get() {} } +`); + fs.writeFileSync(path.join(dir, `calls.${ext}`), ` +import { Cart, api } from './cart'; +export function tidy(list${typed ? ': string[]' : ''}) { list.map(); } +export function cached() { + const cache = new Map(); + cache.get('x'); +} +export function unknownCache(cache) { cache.get('x'); } +export function literalArray() { const cart = []; cart.map(); } +export function literalString() { const cart = 'x'; cart.trim(); } +export function constructed() { + const cart = new Cart(); + cart.add(); +} +export function imported() { Cart.bind(); } +export function objectLiteral() { api.map(); } +export const local = { map() {} }; +export function localLiteral() { local.map(); } +${typed ? 'export function typed(cart: Cart) { cart.map(); }' : ''} +`); + cg = await CodeGraph.init(dir, { index: true }); + const callees = (name: string) => { + const fn = cg!.getNodesByKind('function').find(n => n.name === name)!; + expect(fn, name).toBeDefined(); + return cg!.getCallees(fn.id).filter(({ edge }) => edge.kind === 'calls').map(c => c.node.qualifiedName); + }; + for (const name of ['tidy', 'cached', 'unknownCache', 'literalArray', 'literalString']) expect(callees(name), name).toEqual([]); + expect(callees('constructed')).toContain('Cart::add'); + expect(callees('imported')).toContain('Cart::bind'); + expect(callees('objectLiteral')).toEqual(['map']); + expect(callees('localLiteral')).toEqual(['map']); + if (typed) expect(callees('typed')).toEqual(['Cart::map']); + const self = cg.getNodesByKind('method').find(n => n.qualifiedName === 'Cart::self')!; + expect(cg.getCallees(self.id).map(c => c.node.qualifiedName)).toEqual(['Cart::add']); + }); +}); diff --git a/__tests__/kernel-tsjs-parity.test.ts b/__tests__/kernel-tsjs-parity.test.ts index 3fe2834ab..7bfd3caa9 100644 --- a/__tests__/kernel-tsjs-parity.test.ts +++ b/__tests__/kernel-tsjs-parity.test.ts @@ -124,6 +124,26 @@ function nested(holder) { expect(result.unresolvedReferences.some((r) => r.referenceName === 'values.get')).toBe(true); }); + describe.each([ + ['ts', 'typescript'], ['tsx', 'tsx'], ['js', 'javascript'], ['jsx', 'jsx'], + ] as const)('private field receivers: %s (#1987)', (ext, language) => { + it.each(['LF', 'CRLF'])('preserves private fields and optional calls (%s)', (ending) => { + const source = ` +class Mailer { send() {} } +class Vault { + #mailer = new Mailer(); + #items = new Set(); + notify() { this.#mailer?.send(); } + optional() { this.#mailer.send?.(); } + put() { this.#items?.add('x'); } +} +`; + const result = assertParity(`vault.${ext}`, ending === 'CRLF' ? source.replace(/\n/g, '\r\n') : source, language); + expect(result.unresolvedReferences.filter(r => r.referenceKind === 'calls').map(r => r.referenceName)) + .toEqual(['this.#mailer.send', 'this.#mailer.send', 'this.#items.add']); + }); + }); + 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__/ts-this-field-call.test.ts b/__tests__/ts-this-field-call.test.ts index 69bcf2086..4a194fb8c 100644 --- a/__tests__/ts-this-field-call.test.ts +++ b/__tests__/ts-this-field-call.test.ts @@ -9,7 +9,7 @@ * shape a delegating wrapper takes. The identical call resolved correctly * whenever the wrapper had any other name. */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -65,6 +65,23 @@ beforeAll(async () => { ' async settings(): Promise { return this.storage.getSettings(); }\n' + '}\n' ); + // ES private fields (#1987). `Outbox::send` and `Cart::add` sit in the same + // file, so a bare-name guess would pick them over the field's real type. + w( + 'vault.ts', + "import { Mailer } from './mailer';\n" + + 'export class Outbox {\n send(msg: string): string { return msg; }\n}\n' + + 'export class Cart {\n add(item: string): void {}\n}\n' + + 'export class Vault {\n' + + ' #mailer: Mailer;\n' + + ' #backup = new Mailer();\n' + + ' #items = new Set();\n' + + ' constructor(m: Mailer) { this.#mailer = m; }\n' + + ' notify(msg: string): string { return this.#mailer.send(msg); }\n' + + ' fallback(msg: string): string { return this.#backup.send(msg); }\n' + + ' put(x: string): void { this.#items.add(x); }\n' + + '}\n' + ); cg = CodeGraph.initSync(dir); await cg.indexAll(); }); @@ -103,4 +120,52 @@ describe('this..() (#1496)', () => { const self = cg.getCallers(method('Keeper::get').id).some(({ node }) => node.id === method('Keeper::get').id); expect(self).toBe(false); }); + + it('resolves an ES private field on its declared or constructed type (#1987)', () => { + expect(calleesOf('Vault::notify')).toEqual(['Mailer::send']); + expect(calleesOf('Vault::fallback')).toEqual(['Mailer::send']); + }); + + it('leaves a builtin-typed ES private field unresolved (#1987)', () => { + // `this.#items.add()` on a Set must not bind to the project's `Cart::add`. + expect(calleesOf('Vault::put')).toEqual([]); + }); +}); + +describe.each(['ts', 'tsx', 'js', 'jsx'])('private field receivers in %s (#1987)', (ext) => { + let temp: string; + let graph: CodeGraph | undefined; + afterEach(() => { + graph?.destroy(); + graph = undefined; + if (temp) fs.rmSync(temp, { recursive: true, force: true }); + }); + + it.each(['LF', 'CRLF'])('keeps optional receivers distinct from public fields (%s)', async (ending) => { + temp = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-private-1987-')); + const source = ` +export class Mailer { send() {} } +export class Cart { add() {} } +export class Vault { + #mailer = new Mailer(); + #items = new Set(); + items = new Cart(); + notify() { this.#mailer?.send(); } + optional() { this.#mailer.send?.(); } + put() { this.#items?.add('x'); } + publicPut() { this.items.add('x'); } +} +`; + fs.writeFileSync(path.join(temp, `vault.${ext}`), ending === 'CRLF' ? source.replace(/\n/g, '\r\n') : source); + graph = await CodeGraph.init(temp, { index: true }); + const callees = (name: string) => { + const caller = graph!.getNodesByKind('method').find(n => n.qualifiedName === `Vault::${name}`)!; + expect(caller).toBeDefined(); + return graph!.getCallees(caller.id).map(c => c.node.qualifiedName); + }; + expect(callees('notify')).toEqual(['Mailer::send']); + expect(callees('optional')).toEqual(['Mailer::send']); + expect(callees('put')).toEqual([]); + expect(callees('publicPut')).toEqual(['Cart::add']); + }); }); diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index 8af774945..525cbc00d 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -1237,14 +1237,17 @@ impl<'t> Walker<'t> { // --- extractInstantiation ----------------------------------------------------------- - /// `this.` as a member_expression receiver → Some(field) (#1496). + /// `this.` as a member_expression receiver → Some(field) (#1496, #1987). fn this_field_of(&self, receiver: Node<'t>) -> Option { if receiver.kind() != "member_expression" { return None; } let object = receiver.child_by_field_name("object")?; let property = receiver.child_by_field_name("property")?; - if object.kind() != "this" || property.kind() != "property_identifier" { + // An ES private field (`this.#items`) keeps its `#` (#1987). + if object.kind() != "this" + || !matches!(property.kind(), "property_identifier" | "private_property_identifier") + { return None; } Some(self.text(property).to_string()) diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index fb2dfd097..1b3f7edf0 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -409,6 +409,8 @@ const TS_JS_CHAIN_LANGUAGES = new Set(['typescript', 'tsx', 'javascript', 'jsx'] /** Receiver node types (TS/JS grammars) that continue a member chain downward. */ const TS_JS_CHAIN_RECEIVER_TYPES = new Set(['member_expression', 'subscript_expression']); +/** The field of a `this..()` receiver: public or ES private (#1496, #1987). */ +const THIS_FIELD_PROPERTY_TYPES = new Set(['property_identifier', 'private_property_identifier']); /** * Identifier-rooted member chains have no inferred property type (#1566), @@ -4842,7 +4844,7 @@ export class TreeSitterExtractor { receiver && receiver.type === 'member_expression' && getChildByField(receiver, 'object')?.type === 'this' && - getChildByField(receiver, 'property')?.type === 'property_identifier' + THIS_FIELD_PROPERTY_TYPES.has(getChildByField(receiver, 'property')?.type ?? '') ) { // TS/JS call through a field of the enclosing class — // `this.mailer.send()` (#1496). Keep the `this.` prefix: @@ -4855,6 +4857,8 @@ export class TreeSitterExtractor { // itself when the two share a name, a self-edge not in the // source. Same discipline as Rust's `self.` (#1585). // Mirrored in the kernel's extract_call (tsjs/extractors.rs). + // An ES private field (`this.#items.add()`) is the same shape; + // its name keeps the `#` (#1987). const fieldName = getNodeText(getChildByField(receiver, 'property')!, this.source); calleeName = `this.${fieldName}.${methodName}`; } else if ( diff --git a/src/resolution/js-builtins.ts b/src/resolution/js-builtins.ts index f02a9742d..ef0d88cd9 100644 --- a/src/resolution/js-builtins.ts +++ b/src/resolution/js-builtins.ts @@ -7,6 +7,30 @@ export const JS_BUILT_INS = new Set([ 'fetch', 'require', 'module', 'exports', '__dirname', '__filename', ]); +/** Method names that require receiver evidence before linking to project code. */ +export const JS_BUILTIN_METHODS = new Set([ + // Array / typed arrays and collections. + 'at', 'concat', 'copyWithin', 'entries', 'every', 'fill', 'filter', 'find', + 'findIndex', 'findLast', 'findLastIndex', 'flat', 'flatMap', 'forEach', + 'includes', 'indexOf', 'join', 'keys', 'lastIndexOf', 'map', 'pop', 'push', + 'reduce', 'reduceRight', 'reverse', 'shift', 'slice', 'some', 'sort', 'splice', + 'toReversed', 'toSorted', 'toSpliced', 'unshift', 'values', 'with', 'subarray', + 'get', 'set', 'has', 'add', 'delete', 'clear', + // String. + 'charAt', 'charCodeAt', 'codePointAt', 'endsWith', 'localeCompare', 'match', + 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', + 'search', 'split', 'startsWith', 'substring', 'substr', 'toLowerCase', + 'toUpperCase', 'toLocaleLowerCase', 'toLocaleUpperCase', 'trim', 'trimStart', + 'trimEnd', 'trimLeft', 'trimRight', 'toString', 'toLocaleString', 'valueOf', + // Promise, Function, EventTarget / EventEmitter and iterators. + 'then', 'catch', 'finally', 'call', 'apply', 'bind', + 'addEventListener', 'removeEventListener', 'dispatchEvent', 'on', 'once', + 'off', 'emit', 'addListener', 'removeListener', 'removeAllListeners', + 'prependListener', 'prependOnceListener', 'listeners', 'rawListeners', + 'listenerCount', 'eventNames', 'setMaxListeners', 'getMaxListeners', + 'next', 'return', 'throw', 'drop', 'take', 'toArray', +]); + /** * TypeScript primitive type names. Distinct from JS_BUILT_INS on purpose: those * are runtime globals a receiver can be constructed from, these only ever come diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 72f407fe2..a1404be2b 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -8,7 +8,7 @@ import * as path from 'path'; import { Language, Node } from '../types'; import { UnresolvedRef, ResolvedRef, ResolutionContext, SUPERTYPE_TARGET_KINDS, isInheritanceRef, isImportableKind } from './types'; import { blankStringContents, stripCommentsForRegex } from './strip-comments'; -import { JS_BUILT_INS, TS_PRIMITIVE_TYPES } from './js-builtins'; +import { JS_BUILT_INS, JS_BUILTIN_METHODS, TS_PRIMITIVE_TYPES } from './js-builtins'; /** * Ceiling on how many same-named definitions a FUZZY name-match strategy will @@ -2363,6 +2363,15 @@ export function matchMethodCall( ); } + // A TS/JS call through an ES private field of the enclosing class — + // `this.#items.add()`, emitted as `this.#items.add` (#1987) — resolves + // exactly like `this.` below (#1496). `#` is outside dotMatch's + // receiver class, so the shape is matched here. + if (ref.language === 'typescript' || ref.language === 'javascript' || ref.language === 'tsx' || ref.language === 'jsx') { + const privateField = ref.referenceName.match(/^this\.(#[\w$]+)\.(\w+)$/); + if (privateField) return matchTsThisFieldCall(privateField[1]!, privateField[2]!, ref, context); + } + const match = dotMatch || colonMatch || luaColonMatch || rDollarMatch; if (!match) { return null; @@ -2572,6 +2581,14 @@ export function matchMethodCall( }); if (strat1) return strat1; + // Built-in method names need a validated receiver (#1987). Typed, imported, + // object-literal and direct class receivers have had their chance above; + // capitalization, word overlap or a unique method name are not evidence + // that `list.map()` / `cache.get()` calls a project class. + if (ref.referenceKind === 'calls' && JS_FAMILY.has(ref.language) && + objectOrClass !== 'this' && objectOrClass !== 'super' && + JS_BUILTIN_METHODS.has(methodName!)) return null; + // Strategy 2: Instance variable receiver - try capitalized form to find class // e.g., "permissionEngine" → look for classes containing "PermissionEngine" const capitalizedReceiver = objectOrClass!.charAt(0).toUpperCase() + objectOrClass!.slice(1); @@ -2949,6 +2966,9 @@ function matchTsThisFieldCall( (n) => (n.kind === 'class' || n.kind === 'component') && sameLanguageFamily(n.language, ref.language) ); const fieldEsc = field.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // A word boundary cannot open a private name; it also lets a public + // `items` match `#items`. Keep the two field namespaces distinct (#1987). + const fieldStart = '(? = [ // `storage: typeof DraftHubStorage` — the type OF a value: an object // literal used as a namespace. Its members are bare-named functions inside @@ -2956,18 +2976,18 @@ function matchTsThisFieldCall( // `Type::method`. Tried first: the declared-type pattern below would // otherwise capture the word `typeof`. { - re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?typeof\\s+([A-Za-z_$][\\w.$]*)`), + re: new RegExp(`${fieldStart}${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?typeof\\s+([A-Za-z_$][\\w.$]*)`), valueType: true, }, // `private readonly mailer?: Mailer` — a class field or a constructor // parameter property; the capture stops at `<`, `[` or `|`, so a generic // or union type yields its head and resolveMethodOnType decides. { - re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?([A-Za-z_$][\\w.$]*)`), + re: new RegExp(`${fieldStart}${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?([A-Za-z_$][\\w.$]*)`), valueType: false, }, // `mailer = new Mailer()` / `this.mailer = new Mailer()` - { re: new RegExp(`\\b${fieldEsc}\\b\\s*=\\s*new\\s+([A-Za-z_$][\\w.$]*)`), valueType: false }, + { re: new RegExp(`${fieldStart}${fieldEsc}\\b\\s*=\\s*new\\s+([A-Za-z_$][\\w.$]*)`), valueType: false }, ]; for (const cls of owners) { const source = context.readFile(cls.filePath);