diff --git a/gitnexus/src/core/ingestion/languages/javascript/captures.ts b/gitnexus/src/core/ingestion/languages/javascript/captures.ts index a04844059..dc8175bdf 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/captures.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/captures.ts @@ -38,6 +38,7 @@ import { splitImportStatement } from '../typescript/import-decomposer.js'; import { getJsParser, getJsScopeQuery, jsCachedTreeMatchesGrammar } from './query.js'; import { computeTsArityMetadata } from '../typescript/arity-metadata.js'; import { synthesizeTsReceiverBinding } from '../typescript/receiver-binding.js'; +import { isArrayMethodCallbackArrow } from '../typescript/array-callback.js'; import { getTreeSitterBufferSize } from '../../constants.js'; import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; @@ -640,6 +641,21 @@ export function emitJsScopeCaptures( } } + // #1876: drop @declaration.function for array higher-order-method + // callbacks (`const x = arr.map(a => …)`). The HOC-wrapped-arrow + // pattern matches them, but the binding holds a value, not a callable. + // The binding keeps its separate @declaration.const / .variable match, + // and the arrow's own @scope.function match (a different pattern) is + // untouched, so inner-call attribution falls through to the enclosing + // scope instead of a phantom Function. + const fnDeclAnchor = grouped['@declaration.function']; + if (fnDeclAnchor !== undefined) { + const arrowNode = findFunctionNode(tree.rootNode, fnDeclAnchor.range); + if (arrowNode !== null && isArrayMethodCallbackArrow(arrowNode)) { + continue; + } + } + // Synthesize arity metadata on function-like declarations. const declAnchor = pickFirstDefined(grouped, FUNCTION_DECL_TAGS); if (declAnchor !== undefined) { diff --git a/gitnexus/src/core/ingestion/languages/javascript/query.ts b/gitnexus/src/core/ingestion/languages/javascript/query.ts index 20cfc4f2e..42b043fb0 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/query.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/query.ts @@ -148,6 +148,12 @@ const JAVASCRIPT_SCOPE_QUERY = ` ;; HOC-wrapped variable declarations: const X = HOC((args) => { ... }). ;; Covers React.forwardRef, memo, useCallback, useMemo, observer, ;; debounce, and any user-defined HOC factory. +;; +;; #1876: this shape also matches array higher-order-method callbacks +;; (const x = arr.map(a => ...)), where x is a value, not a function. +;; Those are filtered out emit-side in captures.ts via +;; isArrayMethodCallbackArrow (member-expression callee whose property +;; is a known Array method), so only the @declaration.const survives. (lexical_declaration (variable_declarator name: (identifier) @declaration.name diff --git a/gitnexus/src/core/ingestion/languages/typescript/array-callback.ts b/gitnexus/src/core/ingestion/languages/typescript/array-callback.ts new file mode 100644 index 000000000..14daa9a4c --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/typescript/array-callback.ts @@ -0,0 +1,98 @@ +/** + * Array higher-order-method callback detection (issue #1876). + * + * The HOC-wrapped-arrow declaration pattern in the JS/TS scope queries + * (`const X = call((args) => …)`) was added for React idioms + * (`forwardRef` / `memo` / `useCallback`). It has the same AST shape as + * an array higher-order-method call (`const x = arr.map(a => …)`), so + * those callbacks also match and produce a spurious `@declaration.function` + * named after the binding — duplicating the `@declaration.const` / + * `@declaration.variable` def that the same binding already gets. + * + * For an array-method callback the binding holds a *value* (the method's + * result), not a callable, so the `Function` def is semantically wrong. + * `isArrayMethodCallbackArrow` lets the emitter (`captures.ts`) drop that + * `@declaration.function` match, leaving only the value def. + * + * Shared by both the JavaScript and TypeScript capture emitters — the + * relevant grammar nodes (`arrow_function`, `function_expression`, + * `arguments`, `call_expression`, `member_expression`, + * `property_identifier`) are identical across `tree-sitter-javascript` + * and `tree-sitter-typescript`. + * + * Pure given the input node. No I/O, no globals. + */ + +import type { SyntaxNode } from '../../utils/ast-helpers.js'; + +/** + * Array prototype higher-order methods whose result is a value, not a + * function. A callback passed to one of these is an anonymous callback, + * never a top-level function definition. Identifier-callee HOCs + * (`forwardRef(...)`, `useCallback(...)`, custom factories) are + * deliberately NOT listed — they keep their `Function` classification. + * + * Trade-off (unchanged from before #1876): a custom *fluent-API* member + * call with a callback whose method name is not in this set + * (`qb.where(x => …)`) still classifies as `Function`. There is no clean + * syntactic line beyond the well-known Array surface, so the set is + * intentionally closed and easy to extend. + * + * Receiver-blind, by design: the match keys on the method NAME only, never + * the receiver type (tree-sitter has no type information here). So an in-set + * name on a NON-array receiver — `Map`/`Set` `.forEach`, an RxJS + * `observable.map(…)`, a query builder `.sort(…)`, a lodash chain + * `.filter(…)` — is ALSO treated as a callback and has its + * `@declaration.function` dropped. This is an accepted limitation, not a + * regression: those bindings hold the call's *result value*, not a callable, + * so a value def is the correct classification anyway. The only genuine loss + * is a bespoke DSL whose in-set-named method returns something callable — + * rare enough to accept rather than guard with type inference. Pinned by the + * "in-set method on a non-array receiver" case in `*-captures.test.ts`. + */ +export const ARRAY_CALLBACK_METHODS: ReadonlySet = new Set([ + 'map', + 'filter', + 'find', + 'findIndex', + 'findLast', + 'findLastIndex', + 'forEach', + 'reduce', + 'reduceRight', + 'some', + 'every', + 'flatMap', + 'sort', +]); + +/** + * True when `node` (an `arrow_function` / `function_expression`) is the + * callback argument of an array higher-order-method call, i.e. the + * enclosing call's callee is a `member_expression` whose property is one + * of {@link ARRAY_CALLBACK_METHODS}. + * + * Returns false for direct assignments (`const fn = () => {}` — parent is + * `variable_declarator`, not `arguments`) and for identifier-callee HOCs + * (`forwardRef(() => …)` — callee is an `identifier`, not a + * `member_expression`), so neither is ever suppressed. + * + * Intentional non-suppressing gaps (preserve current behavior, no + * regression): parenthesized callee `(arr.map)(cb)` (`parenthesized_expression`) + * and computed callee `arr['map'](cb)` (`subscript_expression`). + */ +export function isArrayMethodCallbackArrow(node: SyntaxNode): boolean { + const args = node.parent; + if (args === null || args.type !== 'arguments') return false; + + const call = args.parent; + if (call === null || call.type !== 'call_expression') return false; + + const callee = call.childForFieldName('function'); + if (callee === null || callee.type !== 'member_expression') return false; + + const property = callee.childForFieldName('property'); + if (property === null || property.type !== 'property_identifier') return false; + + return ARRAY_CALLBACK_METHODS.has(property.text); +} diff --git a/gitnexus/src/core/ingestion/languages/typescript/captures.ts b/gitnexus/src/core/ingestion/languages/typescript/captures.ts index 22f48e80d..bc9d1a4f7 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/captures.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/captures.ts @@ -37,6 +37,7 @@ import { getTsParser, getTsScopeQuery, tsCachedTreeMatchesGrammar } from './quer import { recordCacheHit, recordCacheMiss } from './cache-stats.js'; import { synthesizeTsReceiverBinding } from './receiver-binding.js'; import { computeTsArityMetadata } from './arity-metadata.js'; +import { isArrayMethodCallbackArrow } from './array-callback.js'; import { getTreeSitterBufferSize } from '../../constants.js'; import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; @@ -252,6 +253,25 @@ export function emitTsScopeCaptures( } } + // #1876: drop @declaration.function for array higher-order-method + // callbacks (`const x = arr.map(a => …)`). The HOC-wrapped-arrow + // pattern matches them, but the binding holds a value, not a callable. + // The binding keeps its separate @declaration.const / .variable match, + // and the arrow's own @scope.function match (a different pattern) is + // untouched, so inner-call attribution falls through to the enclosing + // scope instead of a phantom Function. + const fnDeclAnchor = grouped['@declaration.function']; + if (fnDeclAnchor !== undefined) { + const arrowNode = findFunctionNode( + tree.rootNode, + fnDeclAnchor.range, + groupedNodes['@declaration.function'], + ); + if (arrowNode !== null && isArrayMethodCallbackArrow(arrowNode)) { + continue; + } + } + // Synthesize arity metadata on function-like declaration anchors // before pushing the match. The registry uses these to narrow // overloads — TypeScript supports overload signatures via diff --git a/gitnexus/src/core/ingestion/languages/typescript/query.ts b/gitnexus/src/core/ingestion/languages/typescript/query.ts index 9e0d9b809..9d2ebe5db 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/query.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/query.ts @@ -250,20 +250,22 @@ const TYPESCRIPT_SCOPE_QUERY = ` ;; that promotes the binding to the parent scope (where \`const X\` ;; lives). ;; -;; Trade-off — chained array-method form: \`const x = arr.find((y) => p(y))\` -;; has the same syntactic shape and would also match, naming the -;; \`.find\` callback as \`x\`. The resulting \`Function:x\` is mostly -;; harmless: \`x\` is consumed as a value (\`if (x) { ... }\`), never -;; invoked as a function, so it gets zero incoming \`CALLS\` edges. The -;; one outgoing edge \`Function:x → p\` is a minor mis-attribution that -;; could in principle be fixed by adding a \`function: [(identifier) -;; (member_expression)]\` predicate that excludes property-identifiers -;; matching a known array-method blocklist (\`map\` / \`filter\` / \`find\` -;; / \`reduce\` / \`forEach\` / \`some\` / \`every\`). We don't do that here -;; because (a) the false-positive cost is negligible, (b) the blocklist -;; would need maintenance, and (c) any user-defined fluent-API method -;; with a callback argument would still false-positive — there's no -;; clean syntactic line. +;; #1876 — chained array-method form: \`const x = arr.find((y) => p(y))\` +;; has the same syntactic shape and matches here too, naming the +;; \`.find\` callback as \`x\`. Because \`x\` holds a value (the method +;; result), not a callable, the spurious \`Function:x\` def is dropped +;; emit-side in captures.ts: \`isArrayMethodCallbackArrow\` skips any +;; \`@declaration.function\` whose enclosing call has a member-expression +;; callee with a known Array-method property (\`ARRAY_CALLBACK_METHODS\`: +;; \`map\` / \`filter\` / \`find\` / \`reduce\` / \`forEach\` / \`some\` / +;; \`every\` / …). Only the \`@declaration.variable\` survives, so the +;; binding is a single value def and calls inside the callback attribute +;; to the enclosing scope rather than \`Function:x\`. +;; +;; Residual (intentional): a user-defined fluent-API method with a +;; callback (\`qb.where(x => …)\`) is NOT in the blocklist and still +;; classifies as \`Function\` — there's no clean syntactic line beyond +;; the well-known Array surface, so the set is closed and easy to extend. ;; ;; Trade-off — multi-arrow arguments: \`const x = call(arrow1, arrow2)\` ;; would emit TWO matches with the same name \`x\`. tree-sitter-query diff --git a/gitnexus/src/core/ingestion/scope-extractor.ts b/gitnexus/src/core/ingestion/scope-extractor.ts index 973d4ac76..31a59d2f5 100644 --- a/gitnexus/src/core/ingestion/scope-extractor.ts +++ b/gitnexus/src/core/ingestion/scope-extractor.ts @@ -750,6 +750,61 @@ function normalizeNodeLabel(kindStr: string): SymbolDefinition['type'] | undefin } } +/** Function-like labels: callable defs that must keep incoming CALLS edges. */ +const NODE_BEARING_FUNCTION_LABELS: ReadonlySet = new Set([ + 'Function', + 'Method', + 'Constructor', +]); + +/** Value labels: non-callable bindings (a `const`/`let`/`var` holds a value). */ +const NODE_BEARING_VALUE_LABELS: ReadonlySet = new Set([ + 'Const', + 'Variable', +]); + +/** + * Collapse rule for the deferred node-creation migration (#1876). + * + * When graph-node creation moves from the legacy DAG onto the + * registry-primary path, a single source binding can carry more than one + * `SymbolDefinition` for the same name in the same scope — e.g. a direct + * arrow `const fn = () => {}` is classified BOTH as a `Function` (the + * arrow) and a `Variable` (the binding). Emitting one graph node per def + * would reproduce exactly the duplicate-node bug this issue tracks. + * + * `selectNodeBearingDef` picks the ONE def that should bear the graph node + * for such a binding group: + * + * 1. a function-like def (`Function` / `Method` / `Constructor`) if any — + * the binding is callable and must keep incoming `CALLS` edges; + * 2. otherwise a value def (`Const` / `Variable`) — the binding holds a + * value (e.g. an array-method result after the U1/U2 narrowing); + * 3. otherwise the first def — deterministic fallback for label sets this + * rule does not rank. + * + * INPUT CONTRACT: `group` must be the defs bound to ONE name within ONE + * scope (a binding group). It deliberately does NOT dedup by range — + * `SymbolDefinition` carries no range and `makeDefId` encodes only the + * start position, so containment is uncomputable here; the caller forms the + * group (e.g. from a scope's `ownedDefs` keyed by name) before calling. + * + * Pure. No production call site yet — this dead export is intentional and + * tracked by #1876 (the deferred node-creation migration); it is the + * executable contract that follow-up will consume, pinned today by the + * scope-extractor unit test. + */ +export function selectNodeBearingDef( + group: readonly SymbolDefinition[], +): SymbolDefinition | undefined { + if (group.length === 0) return undefined; + const functionLike = group.find((def) => NODE_BEARING_FUNCTION_LABELS.has(def.type)); + if (functionLike !== undefined) return functionLike; + const value = group.find((def) => NODE_BEARING_VALUE_LABELS.has(def.type)); + if (value !== undefined) return value; + return group[0]; +} + function makeDefId( filePath: string, range: Range, diff --git a/gitnexus/test/fixtures/lang-resolution/javascript-array-method-callback/src/index.js b/gitnexus/test/fixtures/lang-resolution/javascript-array-method-callback/src/index.js new file mode 100644 index 000000000..7b42c6b08 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/javascript-array-method-callback/src/index.js @@ -0,0 +1,27 @@ +function transform(account) { + return account.id; +} + +function predicate(account) { + return account.active; +} + +// Control: a normal named function whose body calls `transform` directly. +// Proves the registry-primary resolver wires same-file free calls for this +// fixture, so the callback assertions below are not vacuous. +function run(account) { + return transform(account); +} + +const accountsList = []; + +// #1876: array higher-order-method callbacks at module scope. Pre-fix the JS +// scope model emitted a phantom `Function:exportData` / `Function:firstActive` +// for these callbacks (they match the HOC-wrapped-arrow declaration pattern), +// and calls INSIDE the callbacks (`transform`, `predicate`) attributed to that +// phantom Function. Post-fix the callback is no longer a `Function` def, so the +// inner calls fall through to the enclosing File scope. +const exportData = accountsList.map((account) => transform(account)); +const firstActive = accountsList.find((account) => predicate(account)); + +module.exports = { run, exportData, firstActive }; diff --git a/gitnexus/test/integration/js-array-method-callback-attribution.test.ts b/gitnexus/test/integration/js-array-method-callback-attribution.test.ts new file mode 100644 index 000000000..cbfc22807 --- /dev/null +++ b/gitnexus/test/integration/js-array-method-callback-attribution.test.ts @@ -0,0 +1,81 @@ +/** + * JavaScript: CALLS-edge attribution for calls inside array higher-order- + * method callbacks (issue #1876). + * + * `const exportData = accountsList.map(account => transform(account))` matches + * the HOC-wrapped-arrow declaration pattern, so before this fix the JS scope + * model emitted a phantom `Function:exportData` for the `.map` callback (on + * top of the value binding). Calls nested in the callback (`transform`) then + * attributed to that phantom `Function` instead of the enclosing scope. + * + * U1 drops the `@declaration.function` for array-method callbacks, so the + * binding is value-only and the inner call falls through to the File scope — + * exactly the Zustand module-level-call behavior already pinned for TS. + * + * SCOPE: this asserts the registry-primary CALLS-edge ATTRIBUTION change only. + * The duplicate *graph node* (`Function:exportData`) is created by the legacy + * parse-worker node path, which this change does not touch; collapsing it is + * the deferred node-creation migration. Accordingly this file makes NO node- + * count assertion. + * + * Registry-primary-only correctness win: under the forced-legacy parity flag + * (`REGISTRY_PRIMARY_JAVASCRIPT=0`) the legacy DAG still emits the phantom + * attribution, so the suite is skipped there (mirrors the per-language + * expected-failure handling in `resolvers/helpers.ts`). + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import path from 'path'; +import { + FIXTURES, + getRelationships, + isLegacyResolverParityRun, + runPipelineFromRepo, + type PipelineResult, +} from './resolvers/helpers.js'; + +describe.skipIf(isLegacyResolverParityRun('javascript'))( + 'JavaScript array-method-callback CALLS attribution (#1876)', + () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'javascript-array-method-callback'), + () => {}, + ); + }, 60000); + + it('control: run() body calls transform directly (resolver is wired)', () => { + const calls = getRelationships(result, 'CALLS').filter((c) => c.target === 'transform'); + expect(calls.map((c) => `${c.source} → ${c.target}`)).toContain('run → transform'); + }); + + it('call inside .map callback attributes to File, not a phantom Function:exportData', () => { + const calls = getRelationships(result, 'CALLS').filter((c) => c.target === 'transform'); + const fromExportData = calls.filter((c) => c.source === 'exportData'); + expect( + fromExportData, + 'transform must NOT be attributed to exportData (phantom Function)', + ).toEqual([]); + const fromFile = calls.filter((c) => c.sourceLabel === 'File'); + expect( + fromFile, + 'the .map callback call to transform must source from the File node (exactly once)', + ).toHaveLength(1); + }); + + it('call inside .find callback attributes to File, not a phantom Function:firstActive', () => { + const calls = getRelationships(result, 'CALLS').filter((c) => c.target === 'predicate'); + const fromFirstActive = calls.filter((c) => c.source === 'firstActive'); + expect( + fromFirstActive, + 'predicate must NOT be attributed to firstActive (phantom Function)', + ).toEqual([]); + const fromFile = calls.filter((c) => c.sourceLabel === 'File'); + expect( + fromFile, + 'the .find callback call to predicate must source from the File node (exactly once)', + ).toHaveLength(1); + }); + }, +); diff --git a/gitnexus/test/unit/scope-resolution/javascript/javascript-captures.test.ts b/gitnexus/test/unit/scope-resolution/javascript/javascript-captures.test.ts new file mode 100644 index 000000000..40b892761 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/javascript/javascript-captures.test.ts @@ -0,0 +1,124 @@ +/** + * Coverage for the JavaScript scope-captures orchestrator, focused on the + * #1876 array-method-callback narrowing. + * + * `const x = arr.map(a => …)` must NOT produce a `@declaration.function` + * named `x` (the binding holds a value, not a callable) — only the + * `@declaration.const`. Identifier-callee HOCs (`forwardRef`, `useMemo`) + * and direct arrow assignments keep their `@declaration.function`. + * + * Runs against tree-sitter-javascript so it catches grammar drift before + * the integration parity gate. + */ + +import { describe, it, expect } from 'vitest'; +import { emitJsScopeCaptures } from '../../../../src/core/ingestion/languages/javascript/captures.js'; + +function matchesFor(src: string) { + return emitJsScopeCaptures(src, 'test.js'); +} + +/** True when some match carries `tag` and its @declaration.name is `name`. */ +function hasDecl(src: string, tag: string, name: string): boolean { + return matchesFor(src).some((m) => m[tag] !== undefined && m['@declaration.name']?.text === name); +} + +/** Count matches carrying `tag` (any name). */ +function countTag(src: string, tag: string): number { + return matchesFor(src).filter((m) => m[tag] !== undefined).length; +} + +describe('emitJsScopeCaptures — #1876 array-method-callback narrowing', () => { + it('does not emit @declaration.function for `const x = arr.map(a => …)`', () => { + const src = 'const exportData = accountsList.map(account => ({ id: account.id }));'; + expect(hasDecl(src, '@declaration.const', 'exportData')).toBe(true); + expect(hasDecl(src, '@declaration.function', 'exportData')).toBe(false); + // Exactly one binding-bearing declaration for the name. + expect(countTag(src, '@declaration.function')).toBe(0); + }); + + // Every method in ARRAY_CALLBACK_METHODS except `map` (covered above). + it.each([ + 'filter', + 'find', + 'findIndex', + 'findLast', + 'findLastIndex', + 'reduce', + 'reduceRight', + 'forEach', + 'some', + 'every', + 'flatMap', + 'sort', + ])('suppresses the Function def for array method .%s()', (method) => { + const src = `const x = arr.${method}((a) => a);`; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(false); + expect(hasDecl(src, '@declaration.const', 'x')).toBe(true); + }); + + it('keeps @declaration.function for an identifier-callee HOC (forwardRef)', () => { + const src = 'const Button = forwardRef((props, ref) => null);'; + expect(hasDecl(src, '@declaration.function', 'Button')).toBe(true); + }); + + it('keeps @declaration.function for useMemo (identifier callee, unchanged this round)', () => { + const src = 'const value = useMemo(() => compute(), []);'; + expect(hasDecl(src, '@declaration.function', 'value')).toBe(true); + }); + + it('keeps dual classification for a direct arrow `const fn = () => {}`', () => { + const src = 'const fn = () => { doThing(); };'; + expect(hasDecl(src, '@declaration.function', 'fn')).toBe(true); + expect(hasDecl(src, '@declaration.const', 'fn')).toBe(true); + }); + + it('keeps @declaration.function for a non-array fluent-API member call (accepted limitation)', () => { + const src = 'const q = qb.where((row) => row.ok);'; + expect(hasDecl(src, '@declaration.function', 'q')).toBe(true); + }); + + it('suppresses an in-set method name on a NON-array receiver (accepted receiver-blind limitation)', () => { + // The predicate keys on the method NAME only, never the receiver type — + // tree-sitter has no type info. So `.map` on an RxJS observable (or + // Map/Set `.forEach`, a query builder `.sort`, a lodash chain `.filter`) + // is also treated as a callback and loses its Function def. Accepted: the + // binding holds the call's result value, so a value def is correct anyway. + const src = 'const stream = source$.map((event) => handle(event));'; + expect(hasDecl(src, '@declaration.function', 'stream')).toBe(false); + expect(hasDecl(src, '@declaration.const', 'stream')).toBe(true); + }); + + it('suppresses the outer .map() callback in a chained array call', () => { + const src = 'const x = arr.filter((a) => a).map((b) => b);'; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(false); + expect(hasDecl(src, '@declaration.const', 'x')).toBe(true); + }); + + it('suppresses through an export_statement wrapper', () => { + const src = 'export const x = arr.map((a) => a);'; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(false); + expect(hasDecl(src, '@declaration.const', 'x')).toBe(true); + }); + + it('suppresses a function_expression callback', () => { + const src = 'const x = arr.map(function (a) { return a; });'; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(false); + expect(hasDecl(src, '@declaration.const', 'x')).toBe(true); + }); + + it('suppresses an optional-chained array call `arr?.map(...)`', () => { + const src = 'const x = arr?.map((a) => a);'; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(false); + }); + + it('does NOT suppress a parenthesized callee `(arr.map)(cb)` (intentional gap)', () => { + const src = 'const x = (arr.map)((a) => a);'; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(true); + }); + + it('does NOT suppress a computed callee `arr["map"](cb)` (intentional gap)', () => { + const src = 'const x = arr["map"]((a) => a);'; + expect(hasDecl(src, '@declaration.function', 'x')).toBe(true); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts b/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts index 5143bce41..a3e3bde8c 100644 --- a/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts +++ b/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts @@ -16,8 +16,13 @@ import type { ReferenceKind, Scope, ScopeKind, + SymbolDefinition, } from 'gitnexus-shared'; -import { extract, type ScopeExtractorHooks } from '../../../src/core/ingestion/scope-extractor.js'; +import { + extract, + selectNodeBearingDef, + type ScopeExtractorHooks, +} from '../../../src/core/ingestion/scope-extractor.js'; // ─── Synthetic-capture helpers ────────────────────────────────────────────── @@ -550,3 +555,54 @@ describe('end-to-end fixture (all 5 passes together)', () => { expect(result.moduleScope).toBe(mod.id); }); }); + +describe('selectNodeBearingDef — #1876 one-node-per-binding collapse rule', () => { + const def = (type: SymbolDefinition['type'], name = 'x'): SymbolDefinition => ({ + nodeId: `def:test.ts#1:0:${type}:${name}`, + filePath: 'test.ts', + type, + qualifiedName: name, + }); + + it('returns undefined for an empty group', () => { + expect(selectNodeBearingDef([])).toBeUndefined(); + }); + + it('returns the only def for a single-element group', () => { + const only = def('Variable'); + expect(selectNodeBearingDef([only])).toBe(only); + }); + + it('prefers a Function over a co-bound Variable (direct arrow / HOC)', () => { + const fn = def('Function'); + const variable = def('Variable'); + // Order-independent: function-like wins regardless of position. + expect(selectNodeBearingDef([variable, fn])).toBe(fn); + expect(selectNodeBearingDef([fn, variable])).toBe(fn); + }); + + it('prefers a Method over a co-bound value def', () => { + const method = def('Method'); + const variable = def('Variable'); + expect(selectNodeBearingDef([variable, method])).toBe(method); + }); + + it('returns the value def when no function-like def is present (array-method result)', () => { + const constDef = def('Const'); + expect(selectNodeBearingDef([constDef])).toBe(constDef); + const variable = def('Variable'); + expect(selectNodeBearingDef([variable])).toBe(variable); + }); + + it('prefers a value def even when an unranked label appears first', () => { + const cls = def('Class'); + const variable = def('Variable'); + expect(selectNodeBearingDef([cls, variable])).toBe(variable); + }); + + it('falls back to the first def for label sets the rule does not rank', () => { + const cls = def('Class'); + const iface = def('Interface'); + expect(selectNodeBearingDef([cls, iface])).toBe(cls); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/typescript/typescript-captures.test.ts b/gitnexus/test/unit/scope-resolution/typescript/typescript-captures.test.ts index 8d465ab82..0e481835a 100644 --- a/gitnexus/test/unit/scope-resolution/typescript/typescript-captures.test.ts +++ b/gitnexus/test/unit/scope-resolution/typescript/typescript-captures.test.ts @@ -565,3 +565,100 @@ describe('emitTsScopeCaptures — edge cases', () => { expect(() => emitTsScopeCaptures('', 'test.ts')).not.toThrow(); }); }); + +describe('emitTsScopeCaptures — #1876 array-method-callback narrowing', () => { + // True when some match carries `tag` and its @declaration.name is `name`. + const declWithName = (src: string, tag: string, name: string): boolean => + emitTsScopeCaptures(src, 'test.ts').some( + (m) => m[tag] !== undefined && m['@declaration.name']?.text === name, + ); + + it('does not emit @declaration.function for `const x = arr.map(a => …)`', () => { + const src = 'const exportData = accountsList.map((account) => ({ id: account.id }));'; + expect(declWithName(src, '@declaration.variable', 'exportData')).toBe(true); + expect(declWithName(src, '@declaration.function', 'exportData')).toBe(false); + }); + + // Every method in ARRAY_CALLBACK_METHODS except `map` (covered above). + it.each([ + 'filter', + 'find', + 'findIndex', + 'findLast', + 'findLastIndex', + 'reduce', + 'reduceRight', + 'forEach', + 'some', + 'every', + 'flatMap', + 'sort', + ])('suppresses the Function def for array method .%s()', (method) => { + const src = `const x = arr.${method}((a) => a);`; + expect(declWithName(src, '@declaration.function', 'x')).toBe(false); + expect(declWithName(src, '@declaration.variable', 'x')).toBe(true); + }); + + it('keeps @declaration.function for an identifier-callee HOC (forwardRef)', () => { + const src = 'const Button = forwardRef((props, ref) => null);'; + expect(declWithName(src, '@declaration.function', 'Button')).toBe(true); + }); + + it('keeps @declaration.function for useCallback (identifier callee, unchanged this round)', () => { + const src = 'const cb = useCallback(() => doThing(), []);'; + expect(declWithName(src, '@declaration.function', 'cb')).toBe(true); + }); + + it('keeps dual classification for a direct arrow `const fn = () => {}`', () => { + const src = 'const fn = () => { doThing(); };'; + expect(declWithName(src, '@declaration.function', 'fn')).toBe(true); + expect(declWithName(src, '@declaration.variable', 'fn')).toBe(true); + }); + + it('keeps @declaration.function for a non-array fluent-API member call (accepted limitation)', () => { + const src = 'const q = qb.where((row) => row.ok);'; + expect(declWithName(src, '@declaration.function', 'q')).toBe(true); + }); + + it('suppresses an in-set method name on a NON-array receiver (accepted receiver-blind limitation)', () => { + // Receiver-blind by design — see array-callback.ts. An in-set method name + // on a non-array receiver (RxJS observable, Map/Set, query builder) also + // loses its Function def. Accepted: the binding holds a value, not a callable. + const src = 'const stream = source$.map((event) => handle(event));'; + expect(declWithName(src, '@declaration.function', 'stream')).toBe(false); + expect(declWithName(src, '@declaration.variable', 'stream')).toBe(true); + }); + + it('suppresses the outer .map() callback in a chained array call', () => { + const src = 'const x = arr.filter((a) => a).map((b) => b);'; + expect(declWithName(src, '@declaration.function', 'x')).toBe(false); + expect(declWithName(src, '@declaration.variable', 'x')).toBe(true); + }); + + it('suppresses through an export_statement wrapper', () => { + const src = 'export const x = arr.map((a) => a);'; + expect(declWithName(src, '@declaration.function', 'x')).toBe(false); + expect(declWithName(src, '@declaration.variable', 'x')).toBe(true); + }); + + it('suppresses a function_expression callback', () => { + const src = 'const x = arr.map(function (a) { return a; });'; + expect(declWithName(src, '@declaration.function', 'x')).toBe(false); + expect(declWithName(src, '@declaration.variable', 'x')).toBe(true); + }); + + it('suppresses an optional-chained array call `arr?.map(...)`', () => { + const src = 'const x = arr?.map((a) => a);'; + expect(declWithName(src, '@declaration.function', 'x')).toBe(false); + }); + + it('does NOT suppress a parenthesized callee `(arr.map)(cb)` (intentional gap)', () => { + const src = 'const x = (arr.map)((a) => a);'; + expect(declWithName(src, '@declaration.function', 'x')).toBe(true); + }); + + it('does NOT suppress a computed callee `arr["map"](cb)` (intentional gap)', () => { + const src = 'const x = arr["map"]((a) => a);'; + expect(declWithName(src, '@declaration.function', 'x')).toBe(true); + }); +});