diff --git a/gitnexus/src/core/ingestion/languages/javascript/captures.ts b/gitnexus/src/core/ingestion/languages/javascript/captures.ts index c8fe0446f..2ce0561dc 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/captures.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/captures.ts @@ -39,6 +39,12 @@ import { getJsParser, getJsScopeQuery, jsCachedTreeMatchesGrammar } from './quer import { computeTsArityMetadata } from '../typescript/arity-metadata.js'; import { synthesizeTsReceiverBinding } from '../typescript/receiver-binding.js'; import { isArrayMethodCallbackArrow } from '../typescript/array-callback.js'; +import { synthesizeCjsModuleExports } from '../typescript/cjs-module-exports.js'; +import { + isShadowedCjsExportAssignment, + isUnexportedMemberAssignmentValue, + isUndeclarableThisMemberValue, +} from '../typescript/cjs-export-assignment.js'; import { getTreeSitterBufferSize } from '../../constants.js'; import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; import { synthesizeCallableFlowCaptures } from '../../utils/callable-flow-captures.js'; @@ -881,6 +887,25 @@ export function emitJsScopeCaptures( if (arrowNode !== null && isBlockedDefaultExportHoc(arrowNode)) { continue; } + // #2723 — see the matching filter in `typescript/captures.ts`. + if (arrowNode !== null && isShadowedCjsExportAssignment(arrowNode, tree.rootNode)) { + continue; + } + // #2723 follow-up: the member-assignment rule matches ANY identifier + // receiver so an `exports` alias can be recognised. A receiver that is + // not the exports object declares nothing at module scope — drop it, or + // every `obj.handler = fn` would bind `handler` as a module symbol. + if (arrowNode !== null && isUnexportedMemberAssignmentValue(arrowNode, tree.rootNode)) { + continue; + } + + // A `this.X = fn` declares a module symbol ONLY at the top level of a + // CommonJS file, where `this` is `module.exports`. Inside a function it + // is an instance member (a Method with an owner, no module binding), and + // in ESM top-level `this` is undefined and exports nothing. + if (arrowNode !== null && isUndeclarableThisMemberValue(arrowNode, tree.rootNode)) { + continue; + } } if (fnDeclAnchor !== undefined) { @@ -991,6 +1016,7 @@ export function emitJsScopeCaptures( // Post-query synthesis passes. synthesizeCjsImports(tree.rootNode, out); + synthesizeCjsModuleExports(tree.rootNode, filePath, out); synthesizeJsDocBindings(tree.rootNode, out); synthesizeConstructorFieldBindings(tree.rootNode, out); synthesizeDestructuringBindings(tree.rootNode, out); diff --git a/gitnexus/src/core/ingestion/languages/javascript/index.ts b/gitnexus/src/core/ingestion/languages/javascript/index.ts index e887bab51..15411f504 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/index.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/index.ts @@ -34,11 +34,60 @@ * resolved. * 3. **Dynamic require** — `require(computedPath)` is skipped (non-literal * argument — cannot statically resolve the target). - * 4. **`module.exports` / `exports.X`** — CJS export forms are not yet - * modeled as re-exports. The finalize algorithm treats the exporting - * module as a namespace; importers that do `const X = require('./m')` - * bind the module namespace, and member-call resolution walks the - * class graph from there. + * 4. **CommonJS `require()` in TypeScript** — `require()` decomposition is a + * JavaScript-emitter concern, so a `.ts` file's `const m = require('./m')` + * is not decomposed into an import. The EXPORT side of a `.ts` CommonJS + * module is now declared (shared with the JS emitter), but an importer + * written in TypeScript still cannot resolve through it. + * 5. **`.cjs` module-level `this`** — the CommonJS/ESM gate consults the file + * extension where it can, but `provider.labelOverride` receives no file + * path, so a `.cjs` file whose only export is a module-level `this.X = fn` + * is not labelled. It now emits nothing rather than an ownerless `Method`. + * 6. **Calling a renamed default-export binding** — `module.exports = fn` IS + * indexed (#2723, named after the file), but resolving a CALL through a + * renamed local binding (`const renamed = require('./mod'); renamed()`) + * needs the finalize layer to treat a called namespace binding as the + * target module's default export. `const mod = require('./mod'); mod()` + * happens to resolve because the names coincide. + * 7. **Anonymous ESM default** — `export default function () {}` (no name) + * is not indexed either. Same class as the CJS default above, different + * construct; not addressed by #2723. + * + * ## CommonJS export forms (#2723) + * + * Each of these declares its name in the module scope, so importers resolve to + * it by name — `const { foo } = require('./m')` matches directly and a + * namespace `m.foo()` walks the module's defs: + * + * - `exports.foo = function () {}` / `module.exports.foo = (a) => a`, in + * every value form (function, async, arrow, async arrow, generator). + * - `const e = exports; e.foo = fn` — an alias bound at module scope. The + * receiver cannot be pinned in a query, so the member-assignment rules + * match any identifier receiver and the emitters prune anything that is + * not the exports object. + * - `this.foo = fn` at MODULE level of a CommonJS file, where `this` is + * `module.exports`. Gated on the file being CommonJS — in ESM top-level + * `this` is undefined and the same line exports nothing. + * - `exports.foo = lib.imported` / `exports.foo = importedName` — forwarded + * as a re-export (`reexport-alias`), so callers reach the ORIGINAL + * definition. A plain import binding would not do: it is private to its + * module, exactly as in ESM. + * - `module.exports = fn` — the whole module is the callable, so it is named + * after the file (`index.js` takes its parent directory). A named function + * expression keeps its own name. `exports = fn` is NOT indexed: rebinding + * `exports` exports nothing in CommonJS, it only breaks the alias. + * + * Two deliberate non-cases. `exports.foo = localFn`, where the value is a + * locally declared function, needs nothing — the module scope already binds + * `localFn` and importers already resolve through it. And a CJS export whose + * name the module ALSO declares lexically is suppressed rather than declared + * twice: two declarations of one name are ambiguous, and the resolver would + * drop the intra-module edge entirely. + * + * Callable members assigned through a receiver are Methods with an owner edge + * rather than free functions: `Foo.prototype.bar = fn` and `this.bar = fn` + * inside a constructor. Ownership resolves to what the file declares, so an + * owner it cannot see (`External.prototype.x = fn`) claims no edge. */ export { emitJsScopeCaptures } from './captures.js'; diff --git a/gitnexus/src/core/ingestion/languages/javascript/query.ts b/gitnexus/src/core/ingestion/languages/javascript/query.ts index d79db10cb..7445929ba 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/query.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/query.ts @@ -152,6 +152,67 @@ export const JAVASCRIPT_SCOPE_QUERY = ` name: (identifier) @declaration.name value: (function_expression) @declaration.function)) +;; CJS property-assignment exports (#2723): \`exports.foo = function () {}\`, +;; \`module.exports.foo = (a) => a\`. The graph node for these comes from +;; TYPESCRIPT/JAVASCRIPT_QUERIES; this block is the other half — without a +;; scope-resolution declaration the node exists but nothing resolves TO it, +;; so \`impact\` answered "found, zero callers" on a whole CommonJS API. +;; +;; The declaration binds the BARE property name into the enclosing (module) +;; scope, which is what importers see: \`const { foo } = require('./m')\` +;; matches by name, and a namespace \`m.foo()\` walks the module's defs. +;; +;; Same anchor discipline as the blocks above — \`@declaration.function\` sits +;; on the INNER arrow / function_expression so its range matches the +;; \`@scope.function\` range. +;; The three right-hand-side forms share one pattern via an inner LEAF +;; alternation. tree-sitter 0.21.1 has a known hazard where a top-level +;; \`[...]\` alternation makes sibling branches share one predicate bucket and +;; silently drops matches; an inner leaf alternation whose predicates all sit +;; on captures OUTSIDE it (here \`@_cjs.exports\` / \`@_cjs.module\`, both on the +;; left-hand side and bound in every branch) is the safe form. Verified by +;; probing all six receiver × RHS combinations, not by reading. +;; +;; \`(generator_function) @scope.function\` is declared near the top of this +;; query, so the anchor aligns for that branch too. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.receiver + property: (property_identifier) @declaration.name) + right: [ + (arrow_function) + (function_expression) + (generator_function) + ] @declaration.function) + +;; \`this.X = fn\` at MODULE level of a CommonJS file — there \`this\` IS +;; \`module.exports\`, so this declares an export. Pruned emit-side for ESM +;; files (where top-level \`this\` is undefined) and for a \`this\` inside a +;; function, which is an instance member rather than an export. +(assignment_expression + left: (member_expression + object: (this) + property: (property_identifier) @declaration.name) + right: [ + (arrow_function) + (function_expression) + (generator_function) + ] @declaration.function) + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @declaration.name) + right: [ + (arrow_function) + (function_expression) + (generator_function) + ] @declaration.function + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) + ;; Object-property arrows / function expressions named by their pair key. ;; Same anchor discipline as the lexical_declaration block above: the ;; @declaration.function capture must sit on the INNER arrow/fn-expression. diff --git a/gitnexus/src/core/ingestion/languages/typescript.ts b/gitnexus/src/core/ingestion/languages/typescript.ts index 4c42b7375..2ccfbb577 100644 --- a/gitnexus/src/core/ingestion/languages/typescript.ts +++ b/gitnexus/src/core/ingestion/languages/typescript.ts @@ -16,7 +16,61 @@ import { javascriptClassConfig, } from '../class-extractors/configs/typescript-javascript.js'; import type { SyntaxNode } from '../utils/ast-helpers.js'; -import { createLeadingDocDescriptionExtractor } from '../utils/ast-helpers.js'; +import { + createLeadingDocDescriptionExtractor, + isCjsDefaultExportAssignment, + isPrototypeMemberAssignmentNode, +} from '../utils/ast-helpers.js'; +import { + cjsExportedNameFor, + isModuleLevelThisAssignment, + isModuleLevelThisExport, + isShadowedCjsExportAssignmentNode, +} from './typescript/cjs-export-assignment.js'; + +const rootOf = (node: SyntaxNode): SyntaxNode | undefined => + (node as { tree?: { rootNode?: SyntaxNode } }).tree?.rootNode; + +/** `exports.X = fn` where the module already declares `X` — see #2723. */ +const isShadowedCjsExportNode = (node: SyntaxNode): boolean => { + const root = rootOf(node); + return root !== undefined && isShadowedCjsExportAssignmentNode(node, root); +}; + +/** + * Label for a `. = ` capture (#2723 follow-up). + * + * The member-assignment queries match ANY identifier receiver, because an + * `exports` alias (`const e = exports; e.foo = fn`) cannot be pinned in the + * query. Everything that is not a recognised shape is dropped HERE — without + * that, every `obj.handler = function () {}` in every JS/TS file would emit a + * spurious top-level `Function` named `handler`. + * + * Recognised: a CJS export (direct, `module.exports`, or via a module-scope + * alias) stays a Function; a prototype or `this` member becomes a Method; a + * CJS export shadowing a name the module already declares emits nothing. + */ +const memberAssignmentLabel = (node: SyntaxNode): NodeLabel | null => { + const root = rootOf(node); + + // Module-level `this.X = fn` in CommonJS IS `module.exports.X = fn`, so it + // is an exported Function, not an instance member. Checked before the + // Method branch, which would otherwise claim every `this` receiver. + if (root !== undefined && isModuleLevelThisExport(node, root)) return 'Function'; + // `module.exports = fn` — the module itself is the callable. + if (isCjsDefaultExportAssignment(node)) return 'Function'; + // A module-level `this` member in ESM or a no-signal file is neither an + // export (top-level `this` is undefined) nor an instance member. Emitting a + // Method there minted an ownerless node (#2729 review F13). + if (isModuleLevelThisAssignment(node)) return null; + if (isPrototypeMemberAssignmentNode(node)) return 'Method'; + if (isShadowedCjsExportNode(node)) return null; + + const value = node.childForFieldName('right'); + if (root === undefined || value === null) return null; + + return cjsExportedNameFor(value, root) !== null ? 'Function' : null; +}; import { createTypeScriptCfgVisitor } from '../cfg/visitors/typescript.js'; import { typeConfig as typescriptConfig } from '../type-extractors/typescript.js'; import { tsExportChecker } from '../export-detection.js'; @@ -365,6 +419,21 @@ export const typescriptProvider = defineLanguage({ }), builtInNames: BUILT_INS, + // Member-assignment shapes are not free functions (#2723 follow-up). The + // capture arrives anchored on the assignment, so the shape is decided from + // the left-hand side: + // - `Foo.prototype.bar = fn` / `this.bar = fn` -> a Method + // - a CJS export shadowing a name the module already declares -> no node + // at all, since the scope declaration that would reach it is suppressed + // too (an orphan `Function` twin beside `class Dup {}` otherwise). + // Every other `definition.function` capture keeps its default label. + labelOverride: (functionNode, defaultLabel) => + defaultLabel !== 'Function' + ? defaultLabel + : functionNode.type === 'assignment_expression' + ? memberAssignmentLabel(functionNode) + : defaultLabel, + // ── RFC #909 Ring 3: scope-based resolution hooks (RFC §5) ────────── // TypeScript is the third migration after Python and C#. See // ./typescript/index.ts for the full per-hook rationale and the @@ -433,6 +502,14 @@ export const javascriptProvider = defineLanguage({ }), builtInNames: BUILT_INS, + // Member-assignment shapes — see the TypeScript provider above. + labelOverride: (functionNode, defaultLabel) => + defaultLabel !== 'Function' + ? defaultLabel + : functionNode.type === 'assignment_expression' + ? memberAssignmentLabel(functionNode) + : defaultLabel, + // ── RFC #909 Ring 3: scope-based resolution hooks (RFC §5) ────────── // JavaScript is the fourth migration after Python, C#, and TypeScript. // Hooks are thin wrappers over the TypeScript implementations where diff --git a/gitnexus/src/core/ingestion/languages/typescript/captures.ts b/gitnexus/src/core/ingestion/languages/typescript/captures.ts index 3f0bea26f..31e01e539 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/captures.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/captures.ts @@ -38,9 +38,15 @@ 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 { + isShadowedCjsExportAssignment, + isUnexportedMemberAssignmentValue, + isUndeclarableThisMemberValue, +} from './cjs-export-assignment.js'; import { getTreeSitterBufferSize } from '../../constants.js'; import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; import { synthesizeCallableFlowCaptures } from '../../utils/callable-flow-captures.js'; +import { synthesizeCjsModuleExports } from './cjs-module-exports.js'; import { deriveDefaultExportHocName, isBlockedDefaultExportHoc, @@ -358,6 +364,28 @@ export function emitTsScopeCaptures( if (arrowNode !== null && isBlockedDefaultExportHoc(arrowNode)) { continue; } + // #2723: a CJS export assignment must not register a SECOND module-scope + // declaration for a name the file already declares lexically — the name + // would become ambiguous and the resolver would drop the intra-module + // edge that resolved before #2723. + if (arrowNode !== null && isShadowedCjsExportAssignment(arrowNode, tree.rootNode)) { + continue; + } + // #2723 follow-up: the member-assignment rule matches ANY identifier + // receiver so an `exports` alias can be recognised. A receiver that is + // not the exports object declares nothing at module scope — drop it, or + // every `obj.handler = fn` would bind `handler` as a module symbol. + if (arrowNode !== null && isUnexportedMemberAssignmentValue(arrowNode, tree.rootNode)) { + continue; + } + + // A `this.X = fn` declares a module symbol ONLY at the top level of a + // CommonJS file, where `this` is `module.exports`. Inside a function it + // is an instance member (a Method with an owner, no module binding), and + // in ESM top-level `this` is undefined and exports nothing. + if (arrowNode !== null && isUndeclarableThisMemberValue(arrowNode, tree.rootNode)) { + continue; + } } if (fnDeclAnchor !== undefined) { @@ -502,6 +530,12 @@ export function emitTsScopeCaptures( synthesizeTsInheritanceReferences(tree.rootNode, out); out.push(...synthesizeCallableFlowCaptures(tree.rootNode, TS_CALLABLE_CAPTURE_OPTIONS)); + // CommonJS module-export declarations (#2723). Shared with the JavaScript + // emitter: a `.ts` file in a CommonJS package uses the same forms, and + // without this the default-export NODE was emitted with nothing declaring it + // — the "found, zero callers" state this work exists to remove (#2729 F7). + synthesizeCjsModuleExports(tree.rootNode, filePath, out); + return out; } diff --git a/gitnexus/src/core/ingestion/languages/typescript/cjs-export-assignment.ts b/gitnexus/src/core/ingestion/languages/typescript/cjs-export-assignment.ts new file mode 100644 index 000000000..a82557ac0 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/typescript/cjs-export-assignment.ts @@ -0,0 +1,543 @@ +/** + * Shadowed CommonJS export-assignment detection (issue #2723). + * + * The CJS declaration rules in the JS/TS scope queries bind the bare property + * name of `exports.X = function () {}` / `module.exports.X = (a) => a` into the + * enclosing module scope, so importers resolve to it by name. That is the whole + * point of #2723 — without it the graph node exists and nothing resolves to it. + * + * But a file may ALSO declare that same name lexically: + * + * function dup(v) { return v; } + * exports.dup = function (v) { return !v; }; + * function callIt(v) { return dup(v); } + * + * Then the module scope holds TWO declarations named `dup`, the name is + * ambiguous, and the resolver drops `callIt -> dup` altogether — an edge that + * resolved fine before #2723. A silently missing caller is worse than the gap + * #2723 set out to close, so the emitter drops the CJS declaration in exactly + * this case: the lexical declaration already supplies the module-scope name, + * so importers still resolve, and intra-module resolution is unchanged from + * before #2723. + * + * Only the `@declaration.function` match is suppressed. The graph node comes + * from a separate query (`tree-sitter-queries.ts`) and collapses onto the + * lexical declaration's node by name anyway, so no node is lost. + * + * Shared by both the JavaScript and TypeScript capture emitters — every + * grammar node named here (`assignment_expression`, `member_expression`, + * `property_identifier`, `function_declaration`, `lexical_declaration`, + * `variable_declaration`, `export_statement`) exists in both + * `tree-sitter-javascript` and `tree-sitter-typescript`. + * + * Pure given the input nodes. No I/O, no globals; the cache is keyed on the + * root node it derives from, so it cannot outlive its tree. + */ + +import type { SyntaxNode } from '../../utils/ast-helpers.js'; + +/** + * Module-scope declared names per program root. + * + * Memoized because the emitter asks once per CJS export match: a file with + * 1000 `exports.X = function () {}` lines would otherwise re-walk the top + * level 1000 times, which is the O(n²) shape that has bitten this repo's + * scope capture before. Keyed on the root `SyntaxNode`, so it is dropped with + * the tree. + */ +const moduleScopeNamesByRoot = new WeakMap>(); + +/** Declaration nodes whose name binds directly in the enclosing scope. */ +const NAMED_DECLARATION_TYPES = new Set([ + 'function_declaration', + 'generator_function_declaration', + 'class_declaration', +]); + +/** Declaration nodes that carry one or more `variable_declarator` children. */ +const VARIABLE_DECLARATION_TYPES = new Set(['lexical_declaration', 'variable_declaration']); + +/** + * Identifiers bound to the exports object at module scope, per program root. + * + * `const e = exports;` / `const e = module.exports;` makes `e.foo = fn` an + * export just as surely as `exports.foo = fn`. The queries cannot express + * "an identifier that happens to alias exports", so they match any identifier + * receiver and the emitters prune here. + * + * Memoized for the same reason as {@link moduleScopeDeclaredNames}: asked once + * per candidate assignment, and a large CommonJS module has many. + */ +const exportAliasesByRoot = new WeakMap>(); + +/** Nodes that introduce their own binding scope for the purposes below. */ +const BINDING_SCOPE_NODE_TYPES = new Set([ + 'function_declaration', + 'generator_function_declaration', + 'function_expression', + 'generator_function', + 'arrow_function', + 'method_definition', +]); + +/** + * True when the identifier `name` is SHADOWED by a local binding somewhere + * between `node` and the module root — a parameter, or a `var`/`let`/`const` + * declared inside an enclosing function. + * + * This is the difference between reading source and understanding it. The + * canonical UMD wrapper takes the exports object as a PARAMETER: + * + * (function (exports) { exports.publicApi = function () {}; })(this); + * + * A text comparison sees `exports` and claims a module export. It is a local + * binding, so the assignment exports nothing — and treating it as an export + * both invents a symbol and, because the name then collides with the real + * module scope, deletes call edges that resolved before (#2729 review F2). + */ +function isShadowedByLocalBinding(node: SyntaxNode, name: string): boolean { + for (let scope: SyntaxNode | null = node.parent; scope !== null; scope = scope.parent) { + if (!BINDING_SCOPE_NODE_TYPES.has(scope.type)) continue; + + // Parameters of this function. + const params = scope.childForFieldName('parameters'); + if (params !== null && declaresName(params, name)) return true; + + // `var`/`let`/`const` declared directly in this function's body. + const body = scope.childForFieldName('body'); + if (body !== null) { + for (const stmt of body.namedChildren) { + if (!VARIABLE_DECLARATION_TYPES.has(stmt.type)) continue; + if (declaresName(stmt, name)) return true; + } + } + } + return false; +} + +/** True when `subtree` binds `name` as an identifier (parameter or declarator). */ +function declaresName(subtree: SyntaxNode, name: string): boolean { + const stack: SyntaxNode[] = [subtree]; + while (stack.length > 0) { + const n = stack.pop(); + if (n === undefined) continue; + if (n.type === 'identifier' && n.text === name) return true; + // Do not descend into nested function bodies — their bindings are not ours. + if (n !== subtree && BINDING_SCOPE_NODE_TYPES.has(n.type)) continue; + for (const child of n.namedChildren) stack.push(child); + } + return false; +} + +/** + * True when `node` is the `exports` / `module.exports` object itself AND that + * name is not shadowed by a local binding at this site. + */ +function isExportsObjectExpression(node: SyntaxNode): boolean { + if (node.type === 'identifier') { + return node.text === 'exports' && !isShadowedByLocalBinding(node, 'exports'); + } + if (node.type !== 'member_expression') return false; + return ( + node.childForFieldName('object')?.text === 'module' && + node.childForFieldName('property')?.text === 'exports' && + !isShadowedByLocalBinding(node, 'module') + ); +} + +/** The set of module-scope identifiers aliasing the exports object. */ +function exportAliases(root: SyntaxNode): Set { + const cached = exportAliasesByRoot.get(root); + if (cached !== undefined) return cached; + + const aliases = new Set(); + for (const child of root.namedChildren) { + if (!VARIABLE_DECLARATION_TYPES.has(child.type)) continue; + for (const declarator of child.namedChildren) { + if (declarator.type !== 'variable_declarator') continue; + const name = declarator.childForFieldName('name'); + const value = declarator.childForFieldName('value'); + if (name === null || name.type !== 'identifier' || value === null) continue; + if (isExportsObjectExpression(value)) aliases.add(name.text); + } + } + + exportAliasesByRoot.set(root, aliases); + return aliases; +} + +/** Whether each program root is a CommonJS module, memoized. */ +const isCommonJsByRoot = new WeakMap(); + +/** Function forms that BIND their own `this` (every form except an arrow). */ +const THIS_BINDING_NODE_TYPES = new Set([ + 'function_declaration', + 'generator_function_declaration', + 'function_expression', + 'generator_function', + 'method_definition', + 'class_declaration', + 'class', +]); + +/** + * True when the file is a CommonJS module, so module-level `this` is + * `module.exports` rather than `undefined`. + * + * This gate is load-bearing, not decoration: `this.foo = fn` at the top level + * of an ESM file assigns to `undefined` and exports nothing, so indexing it as + * an export would be wrong for every `.mjs`, every `"type": "module"` package, + * and every `.ts` that compiles to ESM. An `import`/`export` statement makes + * the file unambiguously ESM; otherwise a `require()` call or an + * `exports`/`module` reference marks it CommonJS. A file with neither signal + * is left alone — silence is not evidence of CommonJS. + */ +function isCommonJsModule(root: SyntaxNode, filePath?: string): boolean { + // The extension settles it outright where Node itself does: `.cjs`/`.cts` are + // CommonJS whatever the body contains, `.mjs`/`.mts` are ESM. Without this a + // `.cjs` file whose only content is `this.x = fn` carries no AST signal and + // was classified non-CommonJS, silently dropping the export (#2729 review F14). + if (filePath !== undefined) { + if (/\.(cjs|cts)$/.test(filePath)) return true; + if (/\.(mjs|mts)$/.test(filePath)) return false; + } + + const cached = isCommonJsByRoot.get(root); + if (cached !== undefined) return cached; + + let sawCjsSignal = false; + const visit = (node: SyntaxNode): boolean => { + // An ESM statement settles it immediately: top-level `this` is undefined. + if (node.type === 'import_statement' || node.type === 'export_statement') return false; + if (node.type === 'identifier' && (node.text === 'exports' || node.text === 'module')) + sawCjsSignal = true; + if (node.type === 'call_expression' && node.childForFieldName('function')?.text === 'require') + sawCjsSignal = true; + + for (const child of node.namedChildren) if (!visit(child)) return false; + return true; + }; + + const result = visit(root) && sawCjsSignal; + isCommonJsByRoot.set(root, result); + return result; +} + +/** + * True when `assignment` is a `this.X = ` at MODULE level — i.e. no + * `this`-binding function encloses it — inside a CommonJS module, which makes + * it an export of `X`. + * + * Arrow functions are transparent here: ECMA-262 gives an arrow + * `[[ThisMode]] = lexical`, so a top-level arrow's `this` is still the + * module's. Every other function form binds its own receiver and stops the + * walk — that is an instance member, handled as a Method instead. + */ +/** + * True when `assignment` is a `this.X = ` at MODULE level, regardless + * of module system. + * + * Distinct from {@link isModuleLevelThisExport}, which additionally requires the + * file to be CommonJS. A module-level `this` member in ESM (or in a file with no + * module-system signal) is neither an export nor an instance member — `this` is + * `undefined` there — so it should produce nothing rather than an ownerless + * `Method` node (#2729 review F13). + */ +export function isModuleLevelThisAssignment(assignment: SyntaxNode): boolean { + const left = + assignment.type === 'assignment_expression' ? assignment.childForFieldName('left') : null; + if (left === null || left.type !== 'member_expression') return false; + if (left.childForFieldName('object')?.type !== 'this') return false; + const right = assignment.childForFieldName('right'); + if (right === null || !CALLABLE_VALUE_TYPES.has(right.type)) return false; + + for (let anc: SyntaxNode | null = assignment.parent; anc !== null; anc = anc.parent) { + if (THIS_BINDING_NODE_TYPES.has(anc.type)) return false; + } + return true; +} + +export function isModuleLevelThisExport( + assignment: SyntaxNode, + root: SyntaxNode, + filePath?: string, +): boolean { + const left = + assignment.type === 'assignment_expression' ? assignment.childForFieldName('left') : null; + if (left === null || left.type !== 'member_expression') return false; + if (left.childForFieldName('object')?.type !== 'this') return false; + + for (let anc: SyntaxNode | null = assignment.parent; anc !== null; anc = anc.parent) { + if (THIS_BINDING_NODE_TYPES.has(anc.type)) return false; + } + return isCommonJsModule(root, filePath); +} + +/** The property name of a module-level `this.X = fn` export, or null. */ +export function moduleLevelThisExportName( + assignment: SyntaxNode, + root: SyntaxNode, + filePath?: string, +): string | null { + if (!isModuleLevelThisExport(assignment, root, filePath)) return null; + const property = assignment.childForFieldName('left')?.childForFieldName('property'); + return property?.type === 'property_identifier' ? property.text : null; +} + +/** Right-hand-side node types that make a binding CALLABLE. */ +const CALLABLE_VALUE_TYPES = new Set([ + 'arrow_function', + 'function_expression', + 'generator_function', +]); + +/** + * Collect the CALLABLE names `node` binds into its enclosing scope, into `out`. + * + * Callable-only is load-bearing. The guard exists to stop TWO declarations of + * one callable name making the name ambiguous, which drops the intra-module + * call edge. A non-callable binding cannot be a call target and so cannot + * create that ambiguity — including it instead deletes a genuine export: + * + * let cache = null; + * exports.cache = function (v) { cache = v; return cache; }; + * + * With every module-scope name collected, `cache` matched the `let` and the + * export vanished entirely — no node, no edge (#2729 review F8). + */ +function collectDeclaredNames(node: SyntaxNode, out: Set): void { + if (NAMED_DECLARATION_TYPES.has(node.type)) { + const name = node.childForFieldName('name'); + if (name !== null && name.type === 'identifier') out.add(name.text); + return; + } + + if (VARIABLE_DECLARATION_TYPES.has(node.type)) { + for (const declarator of node.namedChildren) { + if (declarator.type !== 'variable_declarator') continue; + const name = declarator.childForFieldName('name'); + // Destructuring patterns bind several names; they never collide with a + // CJS export assignment's single property name in a way this guard + // needs to model, so only plain identifiers are collected. + if (name === null || name.type !== 'identifier') continue; + const value = declarator.childForFieldName('value'); + if (value !== null && CALLABLE_VALUE_TYPES.has(value.type)) out.add(name.text); + } + return; + } + + // `export function f() {}` / `export const f = …` — the binding is the + // wrapped declaration's, so recurse one level through the wrapper. + if (node.type === 'export_statement') { + const declaration = node.childForFieldName('declaration'); + if (declaration !== null) collectDeclaredNames(declaration, out); + } +} + +/** The set of names declared at module scope (top level) of `root`. */ +function moduleScopeDeclaredNames(root: SyntaxNode): Set { + const cached = moduleScopeNamesByRoot.get(root); + if (cached !== undefined) return cached; + + const names = new Set(); + for (const child of root.namedChildren) collectDeclaredNames(child, names); + + moduleScopeNamesByRoot.set(root, names); + return names; +} + +/** + * The property name of the `exports.X = …` / `module.exports.X = …` assignment + * whose right-hand side is `node`, or null when `node` is not such a value. + * + * Mirrors the receiver pinning in the scope queries: a bare `exports` receiver, + * or a `module.exports` member expression. Any other receiver (`Foo.prototype`, + * `this`, an aliased `const e = exports`) returns null, so this guard never + * fires for a construct the CJS declaration rules did not create. + */ +function cjsExportedPropertyName(node: SyntaxNode, root?: SyntaxNode): string | null { + const assignment = node.parent; + if (assignment === null || assignment.type !== 'assignment_expression') return null; + if (assignment.childForFieldName('right')?.id !== node.id) return null; + + const left = assignment.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') return null; + + const property = left.childForFieldName('property'); + if (property === null || property.type !== 'property_identifier') return null; + + const receiver = left.childForFieldName('object'); + if (receiver === null) return null; + + if (isExportsObjectExpression(receiver)) return property.text; + + // An identifier aliasing the exports object (`const e = exports; e.foo = fn`). + // The alias is a MODULE-scope binding, so it only means "exports" where it is + // not shadowed — `const e = exports; function f(e) { e.x = fn }` assigns to + // the parameter, not to the module (#2729 review F2). + if ( + receiver.type === 'identifier' && + root !== undefined && + exportAliases(root).has(receiver.text) && + !isShadowedByLocalBinding(receiver, receiver.text) + ) + return property.text; + + return null; +} + +/** + * The name this assignment exports, or null when it exports nothing. + * + * Takes the VALUE node (the function literal) and the program root, because + * alias resolution is a whole-file question. Used by the capture emitters to + * keep the widened `.X = fn` match only when the receiver really + * is the exports object. + */ +export function cjsExportedNameFor(node: SyntaxNode, root: SyntaxNode): string | null { + return cjsExportedPropertyName(node, root); +} + +/** + * True when `node` is the value of a `this.X = fn` assignment that declares + * NOTHING at module scope — either because a function encloses it (an instance + * member, which gets a Method + owner edge instead) or because the file is not + * CommonJS (top-level `this` is undefined in ESM). + */ +export function isUndeclarableThisMemberValue(node: SyntaxNode, root: SyntaxNode): boolean { + const assignment = node.parent; + if (assignment === null || assignment.type !== 'assignment_expression') return false; + if (assignment.childForFieldName('right')?.id !== node.id) return false; + + const left = assignment.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') return false; + if (left.childForFieldName('object')?.type !== 'this') return false; + + return !isModuleLevelThisExport(assignment, root); +} + +/** + * True when `node` is the value of a `. = fn` assignment + * whose receiver is NOT the exports object — the over-match the widened + * member-assignment rule accepts so an `exports` alias can be recognised. + * + * Such a receiver declares nothing at module scope: `obj.handler = fn` binds a + * property of `obj`, not a module symbol named `handler`. Prototype and `this` + * receivers are not identifiers, so they never reach this guard and keep their + * own (Method) treatment. + */ +export function isUnexportedMemberAssignmentValue(node: SyntaxNode, root: SyntaxNode): boolean { + const assignment = node.parent; + if (assignment === null || assignment.type !== 'assignment_expression') return false; + if (assignment.childForFieldName('right')?.id !== node.id) return false; + + const left = assignment.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') return false; + if (left.childForFieldName('object')?.type !== 'identifier') return false; + + return cjsExportedPropertyName(node, root) === null; +} + +/** + * True when `node` (an `arrow_function` / `function_expression` / + * `generator_function`) is the value of a CJS export assignment whose property + * name is ALREADY declared at module scope of `root`. + * + * False for a CJS export whose name is declared only by the assignment (the + * ordinary #2723 case — the declaration must be emitted), and false for + * anything that is not a CJS export assignment at all. + */ +export function isShadowedCjsExportAssignment(node: SyntaxNode, root: SyntaxNode): boolean { + const exportedName = cjsExportedName(node, root); + if (exportedName === null) return false; + + return moduleScopeDeclaredNames(root).has(exportedName); +} + +/** + * The module-scope name this assignment value exports, across EVERY export + * form — direct `exports.X`, `module.exports.X`, an alias receiver, a + * module-level `this.X`, and the anonymous/named default export. + * + * One entry point so the shadow guard cannot silently miss a form. Previously + * it consulted only the direct path: the alias path was skipped because `root` + * was not forwarded, `this` never reached it at all, and the default export had + * no check — three holes that each dropped a real call edge or, for the + * default, fabricated a self-recursive one (#2729 review F3/F4). + */ +export function cjsExportedName(value: SyntaxNode, root: SyntaxNode): string | null { + const direct = cjsExportedPropertyName(value, root); + if (direct !== null) return direct; + + const assignment = value.parent; + if (assignment === null || assignment.type !== 'assignment_expression') return null; + if (assignment.childForFieldName('right')?.id !== value.id) return null; + + // Module-level `this.X = fn` in CommonJS — `this` IS `module.exports`. + const viaThis = moduleLevelThisExportName(assignment, root); + if (viaThis !== null) return viaThis; + + // `module.exports = fn` — the whole module is the callable. A named function + // expression names itself; an anonymous one takes the file-derived name, + // which the caller supplies since a tree has no file path. + if (isCjsDefaultExportAssignmentNode(assignment)) { + return assignment.childForFieldName('right')?.childForFieldName('name')?.text ?? null; + } + + return null; +} + +/** `module.exports = `, named or anonymous. */ +function isCjsDefaultExportAssignmentNode(assignment: SyntaxNode): boolean { + const right = assignment.childForFieldName('right'); + if (right === null || !CALLABLE_VALUE_TYPES.has(right.type)) return false; + const left = assignment.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') return false; + return ( + left.childForFieldName('object')?.text === 'module' && + left.childForFieldName('property')?.text === 'exports' + ); +} + +/** + * True when the DERIVED default-export name collides with a callable the module + * already declares. + * + * `format.js` containing `function format() {}` plus an anonymous + * `module.exports = function () { return format(v); }` merged both onto one + * node, and the inner call to `format` then resolved to the merged node — + * fabricating a self-recursion edge present in no source (#2729 review F4). + * A fabricated edge is worse than a missing one: it misleads `impact` with a + * caller that does not exist. + */ +export function defaultExportNameCollides( + assignment: SyntaxNode, + root: SyntaxNode, + derivedName: string, +): boolean { + if (!isCjsDefaultExportAssignmentNode(assignment)) return false; + return moduleScopeDeclaredNames(root).has(derivedName); +} + +/** + * Same question as {@link isShadowedCjsExportAssignment}, asked of the + * ASSIGNMENT node rather than its value — the shape `provider.labelOverride` + * receives, since the graph-node capture is anchored on the assignment. + * + * Used to drop the graph node too, not just the scope declaration. When the + * shadowed name is a `function`, the node collapsed onto the lexical + * declaration's node anyway (same label, same id), but a `class Dup {}` plus + * `exports.Dup = function () {}` produced `Class:f:Dup` AND an orphan + * `Function:f:Dup` — a node nothing can reach, because the declaration that + * would have made it reachable is suppressed by the rule above. + */ +export function isShadowedCjsExportAssignmentNode(node: SyntaxNode, root: SyntaxNode): boolean { + if (node.type !== 'assignment_expression') return false; + const value = node.childForFieldName('right'); + if (value === null) return false; + return isShadowedCjsExportAssignment(value, root); +} + +// `Foo.prototype.bar = function () {}` is handled as a MEMBER assignment — see +// `prototypeAssignmentOwnerName` / `findMemberAssignmentOwnerInfo` in +// `utils/ast-helpers.ts`, next to the object-literal owner helper it mirrors. diff --git a/gitnexus/src/core/ingestion/languages/typescript/cjs-module-exports.ts b/gitnexus/src/core/ingestion/languages/typescript/cjs-module-exports.ts new file mode 100644 index 000000000..012815a09 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/typescript/cjs-module-exports.ts @@ -0,0 +1,196 @@ +/** + * CommonJS module-export capture synthesis, shared by the JavaScript and + * TypeScript emitters (#2723; TS parity per the #2729 review, F7). + * + * These declarations cannot come from the tree-sitter queries: the anonymous + * default export takes its name from the FILE, which a query pattern cannot + * see. They therefore had to be synthesized in the emitter — and living only in + * `javascript/captures.ts` meant a `.ts` file emitted the default-export NODE + * (the query and `labelOverride` are wired on both providers) while nothing + * ever declared it. That is precisely the "found, zero callers" half-fix state + * this issue set out to remove, reintroduced for TypeScript. + * + * Every grammar node named here exists in both `tree-sitter-javascript` and + * `tree-sitter-typescript`, so one implementation serves both. + * + * Pure given the input nodes. No I/O, no globals. + */ + +import type { CaptureMatch } from 'gitnexus-shared'; +import { nodeToCapture, syntheticCapture, type SyntaxNode } from '../../utils/ast-helpers.js'; +import { deriveDefaultExportHocName } from '../../ts-js-hoc-utils.js'; +import { defaultExportNameCollides } from './cjs-export-assignment.js'; + +/** + * Synthesize re-export markers for CJS forwarding assignments (#2723). + * + * const lib = require('./lib'); + * exports.forwarded = lib.imported; // re-export of `imported` from ./lib + * const { imported } = require('./lib'); + * exports.alsoForwarded = imported; // same, through a named binding + * + * The right-hand side is an existing symbol rather than a function literal, so + * no definition rule reaches it and importers of `forwarded` resolved to + * nothing. Emitted with the `named-alias` vocabulary the destructured + * `require()` form already uses, so `interpretJsImport` needs no new case. + * + * `exports.foo = localFn` (a locally DECLARED function) is deliberately not + * handled here: the module scope already binds `localFn`, importers already + * resolve through it, and synthesizing a second binding would re-create the + * ambiguity the shadow guard exists to prevent. + */ +export function synthesizeCjsReExports(root: SyntaxNode, out: CaptureMatch[]): void { + // Namespace and named bindings introduced by require(), by local name. + const namespaceSources = new Map(); + const namedSources = new Map(); + + for (const child of root.namedChildren) { + if (child.type !== 'lexical_declaration' && child.type !== 'variable_declaration') continue; + for (const declarator of child.namedChildren) { + if (declarator.type !== 'variable_declarator') continue; + const value = declarator.childForFieldName('value'); + if (value === null || value.type !== 'call_expression') continue; + if (value.childForFieldName('function')?.text !== 'require') continue; + const arg = value.childForFieldName('arguments')?.namedChild(0); + if (arg === undefined || arg === null || arg.type !== 'string') continue; + const source = arg.namedChild(0)?.text ?? arg.text.slice(1, -1); + + const nameNode = declarator.childForFieldName('name'); + if (nameNode === null) continue; + if (nameNode.type === 'identifier') { + namespaceSources.set(nameNode.text, { source, node: arg }); + } else if (nameNode.type === 'object_pattern') { + for (const field of nameNode.namedChildren) { + if (field.type === 'shorthand_property_identifier_pattern') { + namedSources.set(field.text, { source, name: field.text, node: arg }); + } else if (field.type === 'pair_pattern') { + const key = field.childForFieldName('key'); + const local = field.childForFieldName('value'); + if (key !== null && local !== null && local.type === 'identifier') + namedSources.set(local.text, { source, name: key.text, node: arg }); + } + } + } + } + } + + if (namespaceSources.size === 0 && namedSources.size === 0) return; + + for (const child of root.namedChildren) { + if (child.type !== 'expression_statement') continue; + const assignment = child.namedChild(0); + if (assignment === null || assignment.type !== 'assignment_expression') continue; + + const left = assignment.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') continue; + const receiver = left.childForFieldName('object'); + const exposed = left.childForFieldName('property'); + if (receiver === null || exposed === null || exposed.type !== 'property_identifier') continue; + + const isExportsReceiver = + (receiver.type === 'identifier' && receiver.text === 'exports') || + (receiver.type === 'member_expression' && + receiver.childForFieldName('object')?.text === 'module' && + receiver.childForFieldName('property')?.text === 'exports'); + if (!isExportsReceiver) continue; + + const right = assignment.childForFieldName('right'); + if (right === null) continue; + + // `exports.fwd = ns.member` + if (right.type === 'member_expression') { + const ns = right.childForFieldName('object'); + const member = right.childForFieldName('property'); + if (ns === null || member === null || ns.type !== 'identifier') continue; + const origin = namespaceSources.get(ns.text); + if (origin === undefined) continue; + out.push({ + '@import.statement': syntheticCapture('@import.statement', assignment, origin.source), + '@import.kind': syntheticCapture('@import.kind', assignment, 'reexport-alias'), + '@import.name': syntheticCapture('@import.name', member, member.text), + '@import.alias': syntheticCapture('@import.alias', exposed, exposed.text), + '@import.source': syntheticCapture('@import.source', origin.node, origin.source), + }); + continue; + } + + // `exports.fwd = importedName` + if (right.type === 'identifier') { + const origin = namedSources.get(right.text); + if (origin === undefined) continue; + out.push({ + '@import.statement': syntheticCapture('@import.statement', assignment, origin.source), + '@import.kind': syntheticCapture('@import.kind', assignment, 'reexport-alias'), + '@import.name': syntheticCapture('@import.name', right, origin.name), + '@import.alias': syntheticCapture('@import.alias', exposed, exposed.text), + '@import.source': syntheticCapture('@import.source', origin.node, origin.source), + }); + } + } +} + +/** + * Synthesize the scope declaration for `module.exports = ` (#2723). + * + * The graph node for this comes from the definition query, but the scope query + * cannot produce its name: an anonymous default export takes its name from the + * FILE, which a tree-sitter pattern has no access to. Without a declaration the + * node exists and nothing resolves to it — the half-fixed state this issue's + * first commit already had to correct once. + * + * A named function expression (`module.exports = function named() {}`) keeps + * its own name; the anonymous forms use `deriveDefaultExportHocName`, the + * convention already applied to anonymous default exports elsewhere. + */ +export function synthesizeCjsDefaultExport( + root: SyntaxNode, + filePath: string, + out: CaptureMatch[], +): void { + for (const child of root.namedChildren) { + if (child.type !== 'expression_statement') continue; + const assignment = child.namedChild(0); + if (assignment === null || assignment.type !== 'assignment_expression') continue; + + const left = assignment.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') continue; + if (left.childForFieldName('object')?.text !== 'module') continue; + if (left.childForFieldName('property')?.text !== 'exports') continue; + + const value = assignment.childForFieldName('right'); + if (value === null) continue; + if ( + value.type !== 'function_expression' && + value.type !== 'arrow_function' && + value.type !== 'generator_function' + ) + continue; + + const ownName = value.childForFieldName('name'); + const name = ownName?.text ?? deriveDefaultExportHocName(filePath); + + // Mirror the worker's suppression: when the DERIVED name collides with a + // callable the module already declares, emit nothing. Declaring it anyway + // would bind a second callable under one name, merging two functions onto + // one node and fabricating a self-recursive call edge (#2729 review F4). + if (ownName === null && defaultExportNameCollides(assignment, root, name)) continue; + + // Anchored on the INNER function so the declaration range matches the + // `@scope.function` range — the same anchor discipline as every other + // closure-binding rule. + out.push({ + '@declaration.function': nodeToCapture('@declaration.function', value), + '@declaration.name': syntheticCapture('@declaration.name', ownName ?? value, name), + }); + } +} + +/** Run every CommonJS export-capture synthesis pass for one file. */ +export function synthesizeCjsModuleExports( + root: SyntaxNode, + filePath: string, + out: CaptureMatch[], +): void { + synthesizeCjsReExports(root, out); + synthesizeCjsDefaultExport(root, filePath, out); +} diff --git a/gitnexus/src/core/ingestion/languages/typescript/query.ts b/gitnexus/src/core/ingestion/languages/typescript/query.ts index fa7ff4089..57ad0b5f7 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/query.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/query.ts @@ -215,6 +215,51 @@ export const TYPESCRIPT_SCOPE_QUERY = ` name: (identifier) @declaration.name value: (function_expression) @declaration.function)) +;; CJS property-assignment exports (#2723) — see the matching block in +;; \`languages/javascript/query.ts\` for the rationale. Mirrored here because +;; \`.ts\` files in a CommonJS package use the same form, and because the JS +;; provider delegates several hooks to these TypeScript counterparts. +;; One pattern per receiver form, RHS forms folded into an inner LEAF +;; alternation — see the matching note in \`languages/javascript/query.ts\` for +;; why that shape is safe under the tree-sitter 0.21.1 alternation hazard. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.receiver + property: (property_identifier) @declaration.name) + right: [ + (arrow_function) + (function_expression) + (generator_function) + ] @declaration.function) + +;; \`this.X = fn\` at MODULE level of a CommonJS file — there \`this\` IS +;; \`module.exports\`, so this declares an export. Pruned emit-side for ESM +;; files (where top-level \`this\` is undefined) and for a \`this\` inside a +;; function, which is an instance member rather than an export. +(assignment_expression + left: (member_expression + object: (this) + property: (property_identifier) @declaration.name) + right: [ + (arrow_function) + (function_expression) + (generator_function) + ] @declaration.function) + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @declaration.name) + right: [ + (arrow_function) + (function_expression) + (generator_function) + ] @declaration.function + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) + ;; Object-property arrows / function expressions named by their pair key: ;; \`{ addItem: (item) => ..., removeItem: (item) => ... }\`. The legacy ;; TYPESCRIPT_QUERIES emits the same shape; mirroring it here keeps diff --git a/gitnexus/src/core/ingestion/tree-sitter-queries.ts b/gitnexus/src/core/ingestion/tree-sitter-queries.ts index 60d947d31..d4ce4860f 100644 --- a/gitnexus/src/core/ingestion/tree-sitter-queries.ts +++ b/gitnexus/src/core/ingestion/tree-sitter-queries.ts @@ -390,6 +390,50 @@ export const TYPESCRIPT_QUERIES = ` name: (property_identifier) @name value: (function_expression)) @definition.method +; CJS property-assignment exports (#2723) — see JAVASCRIPT_QUERIES for the +; rationale and for why the receiver is pinned to \`exports\`/\`module.exports\`. +; Mirrored here because \`.ts\` files in a CommonJS package use the same form. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.receiver + property: (property_identifier) @name) + right: [(function_expression) (arrow_function) (generator_function)]) @definition.function + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @name) + right: [(function_expression) (arrow_function) (generator_function)] + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) @definition.function + +; Instance members assigned through \`this\` (#2723 follow-up) — see +; JAVASCRIPT_QUERIES for rationale. +(assignment_expression + left: (member_expression + object: (this) + property: (property_identifier) @name) + right: [ + (function_expression) + (arrow_function) + (generator_function) + ]) @definition.function + +; Prototype methods (#2723 follow-up) — see JAVASCRIPT_QUERIES for rationale. +(assignment_expression + left: (member_expression + object: (member_expression + property: (property_identifier) @_proto.kw) + property: (property_identifier) @name) + right: [ + (function_expression) + (arrow_function) + (generator_function) + ] + (#eq? @_proto.kw "prototype")) @definition.function + ; Constructor parameter properties: constructor(public address: Address) (required_parameter (accessibility_modifier) @@ -540,6 +584,66 @@ export const JAVASCRIPT_QUERIES = ` name: (identifier) @name value: (generator_function)))) @definition.function +; CJS property-assignment exports (#2723): \`exports.foo = function () {}\`, +; \`module.exports.foo = (a) => a\`. This is the dominant export style in +; pre-ESM Node (Express, Firebase Functions), and without these rules a +; CommonJS codebase indexed its internals while every symbol on its public +; API was missing — \`impact\`/\`context\`/\`rename\` all answered "not found". +; +; Scoped to the \`exports\` / \`module.exports\` receivers on purpose. The +; general \`X.foo = function () {}\` shape also covers \`Foo.prototype.bar\` and +; \`this.handler\`, which are member constructs with their own ownership +; questions (an owning Class, a function-local binding) — a broader rule +; would emit ownerless top-level Functions for them. Same rationale as the +; other closure-binding rules above: the label means "is a call target". +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.receiver + property: (property_identifier) @name) + right: [(function_expression) (arrow_function) (generator_function)]) @definition.function + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @name) + right: [(function_expression) (arrow_function) (generator_function)] + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) @definition.function + +; Prototype methods (#2723 follow-up): \`Foo.prototype.bar = function () {}\`. +; The dominant pre-ES6 method form, and previously invisible — no node at all, +; so \`impact\` could not reach a single prototype method. Emitted as a MEMBER: +; \`labelOverride\` reclassifies it to Method and the owner resolves to whatever +; \`Foo\` names, so \`HAS_METHOD\` makes it reachable the way a class method is. +(assignment_expression + left: (member_expression + object: (member_expression + property: (property_identifier) @_proto.kw) + property: (property_identifier) @name) + right: [ + (function_expression) + (arrow_function) + (generator_function) + ] + (#eq? @_proto.kw "prototype")) @definition.function + +; Instance members assigned through \`this\` (#2723 follow-up): +; \`function Widget() { this.handler = function () {}; }\`. The pre-ES6 sibling +; of a closure-valued class field, which #2693 already models as a Method. +; Ownership resolves to the enclosing constructor/class; a \`this\` at module +; top level owns nothing and stays a plain top-level definition. +(assignment_expression + left: (member_expression + object: (this) + property: (property_identifier) @name) + right: [ + (function_expression) + (arrow_function) + (generator_function) + ]) @definition.function + ; Object-property arrows / function expressions: \`{ addItem: () => ... }\`. ; See TYPESCRIPT_QUERIES for rationale (issue #1166). (pair diff --git a/gitnexus/src/core/ingestion/utils/ast-helpers.ts b/gitnexus/src/core/ingestion/utils/ast-helpers.ts index f8ee547b8..cb38f19a2 100644 --- a/gitnexus/src/core/ingestion/utils/ast-helpers.ts +++ b/gitnexus/src/core/ingestion/utils/ast-helpers.ts @@ -1036,6 +1036,15 @@ export const findEnclosingClassInfo = ( /** Object literal binding info for TS/JS shorthand methods. */ export interface ObjectLiteralBindingInfo { ownerId: string; + /** + * Owner name, when the owner is also the member's qualifier. + * + * Set by {@link findMemberAssignmentOwnerInfo} so a prototype method keys as + * `Foo.bar` — without it two constructors in one file that each define + * `bar` collapse onto a single `Method::bar` id. Left undefined by + * {@link findObjectLiteralBindingInfo}, whose ids stay exactly as they were. + */ + ownerName?: string; } /** @@ -1150,6 +1159,201 @@ export const findObjectLiteralBindingInfo = ( }; }; +/** + * Find the owner of a member assigned by `.prototype. = fn` + * (#2723 follow-up). + * + * Sibling of {@link findObjectLiteralBindingInfo}: same seam, same return + * shape, different syntax. There the owner is the variable the literal is + * bound to; here it is the identifier to the left of `.prototype`. + * + * The owner label is read from the file's own module-scope declaration, so the + * edge points at the node that actually exists — `function Foo() {}` is a + * `Function` node, `class Foo {}` is a `Class` node. When the file declares no + * such name (the constructor lives in another module) no owner is claimed: + * a HAS_METHOD edge to a fabricated node is worse than a top-level Method. + */ +export const findMemberAssignmentOwnerInfo = ( + node: SyntaxNode, + filePath: string, +): ObjectLiteralBindingInfo | null => { + const ownerName = prototypeAssignmentOwnerName(node) ?? thisAssignmentOwnerName(node); + if (ownerName === null) return null; + + const root = (node as { tree?: { rootNode?: SyntaxNode } }).tree?.rootNode; + if (!root) return null; + + const ownerLabel = prototypeOwnerLabel(root, ownerName); + if (ownerLabel === null) return null; + + return { ownerId: generateId(ownerLabel, `${filePath}:${ownerName}`), ownerName }; +}; + +/** Right-hand-side node types that make an assignment a callable binding. */ +const CALLABLE_ASSIGNMENT_VALUE_TYPES: ReadonlySet = new Set([ + 'arrow_function', + 'function_expression', + 'generator_function', +]); + +/** + * The receiver name of a `.prototype. = ` assignment, + * or null when `assignment` is not that shape. + * + * Only a bare identifier owner is accepted. `a.b.prototype.c = …` and + * `getClass().prototype.c = …` name an owner this layer cannot resolve to a + * definition, so they are left alone rather than attributed to a guess. + */ +export const prototypeAssignmentOwnerName = (assignment: SyntaxNode): string | null => { + const left = callableAssignmentTarget(assignment); + if (left === null) return null; + + const protoRef = left.childForFieldName('object'); + if (protoRef === null || protoRef.type !== 'member_expression') return null; + + if (protoRef.childForFieldName('property')?.text !== 'prototype') return null; + + const owner = protoRef.childForFieldName('object'); + if (owner === null || owner.type !== 'identifier') return null; + + return owner.text; +}; + +/** The `member_expression` being assigned a function value, or null. */ +const callableAssignmentTarget = (assignment: SyntaxNode): SyntaxNode | null => { + if (assignment.type !== 'assignment_expression') return null; + + const right = assignment.childForFieldName('right'); + if (right === null || !CALLABLE_ASSIGNMENT_VALUE_TYPES.has(right.type)) return null; + + const left = assignment.childForFieldName('left'); + return left !== null && left.type === 'member_expression' ? left : null; +}; + +/** + * The constructor function that owns a `this.member = ` assignment, + * or null when there is none (module top level, or an owner this layer cannot + * name). + * + * Only a `function_declaration` counts. An `arrow_function` does NOT bind its + * own `this` (ECMA-262 gives it `[[ThisMode]] = lexical`), so the walk passes + * through arrows to the function that actually binds the receiver — the same + * rule `@receiver-owner.this` encodes in the scope queries (#2701). A class + * method never reaches here: parse-worker resolves its owner from the + * enclosing class container first. + */ +export const thisAssignmentOwnerName = (assignment: SyntaxNode): string | null => { + const left = callableAssignmentTarget(assignment); + if (left === null) return null; + if (left.childForFieldName('object')?.type !== 'this') return null; + + for (let anc: SyntaxNode | null = assignment.parent; anc !== null; anc = anc.parent) { + if (anc.type === 'arrow_function') continue; + if (anc.type === 'function_declaration') { + const name = anc.childForFieldName('name'); + return name !== null && name.type === 'identifier' ? name.text : null; + } + // Any other receiver-binding form (function_expression, method_definition, + // generator) owns the `this` but gives this layer no module-scope name to + // point an owner edge at. + if (FUNCTION_NODE_TYPES.has(anc.type) || CLASS_CONTAINER_TYPES.has(anc.type)) return null; + } + return null; +}; + +/** + * True when `node` is a `X.prototype.Y = ` or `this.Y = ` + * assignment — i.e. a callable MEMBER rather than a free function. + * + * Takes the ASSIGNMENT node, because that is what the `@definition.function` + * capture is anchored on and therefore what `provider.labelOverride` receives. + */ +export const isPrototypeMemberAssignmentNode = (node: SyntaxNode): boolean => + prototypeAssignmentOwnerName(node) !== null || isThisMemberAssignmentNode(node); + +/** + * True when `node` is `module.exports = ` (#2723). + * + * The whole module IS the callable, so there is no property to take a name + * from and the caller supplies a file-derived one. A NAMED function expression + * is excluded — its own name is captured directly and is more informative. + */ + +/** + * True when `node` is `module.exports = `, named or anonymous — the + * CommonJS default export, where the whole module IS the callable. + * + * `exports = fn` is deliberately NOT this shape: reassigning the `exports` + * binding does not export anything in CommonJS, it only breaks the alias to + * `module.exports`. + */ +export const isCjsDefaultExportAssignment = (node: SyntaxNode): boolean => { + if (node.type !== 'assignment_expression') return false; + + const right = node.childForFieldName('right'); + if (right === null || !CALLABLE_ASSIGNMENT_VALUE_TYPES.has(right.type)) return false; + + const left = node.childForFieldName('left'); + if (left === null || left.type !== 'member_expression') return false; + + return ( + left.childForFieldName('object')?.text === 'module' && + left.childForFieldName('property')?.text === 'exports' + ); +}; + +/** True when `node` is a `this.Y = ` assignment, at any nesting. */ +export const isThisMemberAssignmentNode = (node: SyntaxNode): boolean => { + const left = callableAssignmentTarget(node); + return left !== null && left.childForFieldName('object')?.type === 'this'; +}; + +/** + * The label the owner named by {@link prototypeAssignmentOwnerName} carries in + * the graph, so the owner edge points at the node that actually exists. + * Returns null when the file declares no such module-scope name. + */ +const prototypeOwnerLabel = (root: SyntaxNode, ownerName: string): 'Class' | 'Function' | null => { + for (const child of root.namedChildren) { + const decl = child.type === 'export_statement' ? child.childForFieldName('declaration') : child; + if (decl === null) continue; + + if (decl.childForFieldName('name')?.text === ownerName) { + if (decl.type === 'class_declaration') return 'Class'; + if (decl.type === 'function_declaration' || decl.type === 'generator_function_declaration') + return 'Function'; + } + + // `var Foo = function () {}` / `const Foo = () => {}` / `const Foo = class {}`. + // The dominant pre-ES6 constructor form, and the population this whole + // change targets. Without it `prototypeOwnerLabel` returned null, the + // member fell back to an UNQUALIFIED `Method::` id, and two + // constructors defining the same member name in one file collapsed onto a + // single node with no owner edges at all (#2729 review F6). + if (!VARIABLE_DECLARATION_NODE_TYPES.has(decl.type)) continue; + for (const declarator of decl.namedChildren) { + if (declarator.type !== 'variable_declarator') continue; + if (declarator.childForFieldName('name')?.text !== ownerName) continue; + const value = declarator.childForFieldName('value'); + if (value === null) continue; + // Only a callable value is claimed. A closure binding reliably emits + // `Function::` (the #2687/#2693 convention), so the owner id + // resolves to a node that exists. A class EXPRESSION or a require()-bound + // value names an owner whose node label this layer cannot predict — + // claim none rather than point an edge at a node that may not exist, + // which is the same defect class this fix exists to remove. + if (CALLABLE_ASSIGNMENT_VALUE_TYPES.has(value.type)) return 'Function'; + } + } + return null; +}; + +/** Declaration nodes carrying `variable_declarator` children (JS/TS). */ +const VARIABLE_DECLARATION_NODE_TYPES: ReadonlySet = new Set([ + 'lexical_declaration', + 'variable_declaration', +]); + /** Convenience wrapper: returns just the class ID string (backward compat). */ export const findEnclosingClassId = (node: SyntaxNode, filePath: string): string | null => { return findEnclosingClassInfo(node, filePath)?.classId ?? null; diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index a33b972bb..cb4fba48b 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -87,6 +87,8 @@ import { getDefinitionNodeFromCaptures, findEnclosingClassInfo, findObjectLiteralBindingInfo, + findMemberAssignmentOwnerInfo, + isCjsDefaultExportAssignment, type EnclosingClassInfo, getLabelFromCaptures, genericFuncName, @@ -106,6 +108,7 @@ import { buildTypeEnv } from '../type-env.js'; import type { ConstructorBinding } from '../type-env.js'; import { detectFrameworkFromAST } from '../framework-detection.js'; import { generateId } from '../../../lib/utils.js'; +import { defaultExportNameCollides } from '../languages/typescript/cjs-export-assignment.js'; import { extractVueScript, extractTemplateComponents, @@ -2139,17 +2142,61 @@ const processFileGroup = ( return deriveDefaultExportHocName(file.path); })(); + // `module.exports = function () {}` (#2723): the whole module is the + // callable. The member-assignment rule captures the LEFT property as the + // name, which here is the literal `exports` — meaningless. Override it: + // a named function expression supplies its own name, and the anonymous + // forms are named after the file by the same convention anonymous + // default exports already use. Takes precedence over `nameNode` for + // exactly that reason. + // + // The derived name is dropped when it COLLIDES with a callable the module + // already declares. `format.js` holding `function format() {}` plus an + // anonymous `module.exports = function () { return format(v); }` merged + // both onto one node, and the inner call to `format` then resolved to + // that merged node — fabricating a self-recursion edge present in no + // source (#2729 review F4). A fabricated edge is worse than a missing + // one: it hands `impact` a caller that does not exist. + const isCjsDefaultExport = + definitionNode !== undefined && isCjsDefaultExportAssignment(definitionNode); + const cjsDefaultExportOwnName = isCjsDefaultExport + ? definitionNode?.childForFieldName('right')?.childForFieldName('name')?.text + : undefined; + // A collision must SUPPRESS the definition outright, not merely decline to + // name it — falling through would let the captured left property name the + // node the literal `exports`, which is both meaningless and the very node + // this feature's own test forbids. + const suppressCjsDefaultExport = (() => { + if (!isCjsDefaultExport || cjsDefaultExportOwnName !== undefined) return false; + const root = (definitionNode as { tree?: { rootNode?: SyntaxNode } }).tree?.rootNode; + if (root === undefined) return false; + return defaultExportNameCollides( + definitionNode!, + root, + deriveDefaultExportHocName(file.path), + ); + })(); + if (suppressCjsDefaultExport) continue; + + const cjsDefaultExportName = isCjsDefaultExport + ? (cjsDefaultExportOwnName ?? deriveDefaultExportHocName(file.path)) + : null; + // Synthesize name for constructors without explicit @name capture (e.g. Swift init) if ( !nameNode && nodeLabel !== 'Constructor' && !extractedClassSymbol && - !defaultExportHocName + !defaultExportHocName && + !cjsDefaultExportName ) continue; const nodeName = - extractedClassSymbol?.name ?? defaultExportHocName ?? (nameNode ? nameNode.text : 'init'); + extractedClassSymbol?.name ?? + defaultExportHocName ?? + cjsDefaultExportName ?? + (nameNode ? nameNode.text : 'init'); // Dedup: variable captures (Const/Static/Variable) may overlap with higher-priority // captures (e.g. `const fn = () => {}` matches both @definition.function and @definition.const). // Multi-name declarations share the same definition node, so include the emitted name. @@ -2266,9 +2313,17 @@ const processFileGroup = ( : null; const enclosingClassId = enclosingClassInfo?.qualifiedClassId ?? enclosingClassInfo?.classId ?? null; + // A Method with no enclosing class container is owned by a NAMED binding + // instead: an object literal (`const service = { load() {} }`) or, since + // the #2723 follow-up, a prototype assignment + // (`Foo.prototype.bar = function () {}`). Both resolve the owner from the + // syntax rather than from an ancestor walk, and both are language-shaped + // helpers behind the provider's own label decision — shared code here + // only asks "does this Method name an owner". const objectLiteralOwnerInfo = !enclosingClassId && nodeLabel === 'Method' && definitionNode - ? findObjectLiteralBindingInfo(definitionNode, file.path) + ? (findMemberAssignmentOwnerInfo(definitionNode, file.path) ?? + findObjectLiteralBindingInfo(definitionNode, file.path)) : null; // #1978: hoisted ABOVE qualifiedName/node-id (load-bearing order) so a @@ -2354,7 +2409,13 @@ const processFileGroup = ( ? nestedCallableQualifiedName(nestedCallablePrefix, definitionNode, nodeName) : enclosingClassInfo ? `${enclosingClassInfo.className}.${nodeName}` - : nodeName; + : // A member whose owner is named by the assignment rather than + // by an enclosing container (`Foo.prototype.bar = …`) qualifies + // by that owner, so two constructors in one file that both + // define `bar` stay distinct nodes. + objectLiteralOwnerInfo?.ownerName !== undefined + ? `${objectLiteralOwnerInfo.ownerName}.${nodeName}` + : nodeName; // Extract method metadata BEFORE generating node ID — parameterCount is needed // to disambiguate overloaded methods via # suffix in the ID. diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index 3cf4e04b3..8a6716849 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -497,7 +497,7 @@ relationships: - IMPORTS: Module imports - EXTENDS: Class inheritance - IMPLEMENTS: Interface implementation - - HAS_METHOD: Class/Struct/Interface owns a Method + - HAS_METHOD: Class/Struct/Interface owns a Method; also a Function acting as a pre-ES6 constructor (prototype assignment) - HAS_PROPERTY: Class/Struct/Interface owns a Property (field) - ACCESSES: Function/Method reads or writes a Property (reason: 'read' or 'write') - METHOD_OVERRIDES: Method overrides another Method (MRO) diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 263b4aa24..78090b415 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -109,13 +109,17 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // and nested-callable caller attribution appends the localIdentity suffix the // definition phase already used. Both are parse-time, so a warm cache would // otherwise replay the old captures and ids verbatim. +// v30/schema v22: CommonJS export capture emission (#2723) — new @definition/@declaration +// captures for exports.X, aliased receivers, module-level `this`, re-export +// forwarding and `module.exports = fn`, plus prototype/`this` Methods. A warm +// cache would otherwise replay the pre-fix captures verbatim. // v20: Java/Kotlin capture side-channels persist package and class-annotation // facts for shared Spring Bean resolution. // v19: Java enum constant bodies emit E$N Class nodes; anonymous naming uses // JLS 13.1 immediate-host chains (#2555). // v18: Worker$N anonymous bodies. v17: callable-value-flow operand identity. // v16: direct callee identity. -const SCHEMA_BUMP = 29; +const SCHEMA_BUMP = 30; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index e3abb9967..f10e679bf 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -533,8 +533,17 @@ export interface RepoMeta { * present nowhere in the source. All of that changes emitted node ids AND edges * on files that did not themselves change, so a v20 index topped up * incrementally keeps serving the old attribution; force a full re-analyze. + * + * v22: CommonJS export forms are indexed (#2723) — `exports.X`/`module.exports.X`, + * aliased receivers, module-level `this`, re-export forwarding, `module.exports = fn`, + * plus prototype/`this` members as Methods with owner edges; and the #2729 review + * fixes that stopped a text-only exports receiver inventing exports inside UMD + * factories and stopped the shadow guard deleting or fabricating call edges. + * These change what is emitted for source whose CONTENT has not changed, so a v21 + * index would keep serving the pre-fix graph for every unchanged CommonJS file — + * the exact "Target not found" symptom #2723 reported. Force a full re-analyze. */ -export const INCREMENTAL_SCHEMA_VERSION = 21; +export const INCREMENTAL_SCHEMA_VERSION = 22; export interface IndexedRepo { repoPath: string; diff --git a/gitnexus/test/integration/cjs-exports-assignment.test.ts b/gitnexus/test/integration/cjs-exports-assignment.test.ts new file mode 100644 index 000000000..68c279c5c --- /dev/null +++ b/gitnexus/test/integration/cjs-exports-assignment.test.ts @@ -0,0 +1,605 @@ +/** + * #2723 — `exports.foo = function () {}` must emit a callable `Function` node. + * + * CommonJS property-assignment exports are the dominant export style in + * pre-ESM Node (Express apps, Firebase Functions). Declared functions were + * indexed; the assignment form was not, so on a CJS codebase the graph held + * the internals and missed the public API. + */ +import { describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; +import { + DIST_WORKER_URL, + distWorkerExists, + parseFilesWithWorkers, +} from '../helpers/worker-parse.js'; + +vi.setConfig({ testTimeout: 90_000 }); + +const labelsFor = async (path: string, content: string, name: string): Promise => { + const { graph } = await parseFilesWithWorkers([{ path, content }]); + return graph.nodes + .filter((node) => node.properties.name === name) + .map((node) => node.label) + .sort(); +}; + +describe('#2723 CommonJS export assignment emits a Function node', () => { + it('exports.foo = function () {}', async () => { + expect( + await labelsFor( + 'src/a.js', + 'exports.areVariablesValid = function (variables) { return !!variables; };\n', + 'areVariablesValid', + ), + ).toEqual(['Function']); + }); + + it('exports.foo = async function () {}', async () => { + expect( + await labelsFor( + 'src/b.js', + 'exports.loadUser = async function (id) { return id; };\n', + 'loadUser', + ), + ).toEqual(['Function']); + }); + + it('exports.foo = (a) => {}', async () => { + expect(await labelsFor('src/c.js', 'exports.toId = (a) => a.id;\n', 'toId')).toEqual([ + 'Function', + ]); + }); + + it('module.exports.foo = function () {}', async () => { + expect( + await labelsFor('src/d.js', 'module.exports.render = function () { return 1; };\n', 'render'), + ).toEqual(['Function']); + }); + + it('module.exports = { foo } re-exports the declared function only once', async () => { + expect( + await labelsFor( + 'src/e.js', + 'function helper() { return 1; }\nmodule.exports = { helper };\n', + 'helper', + ), + ).toEqual(['Function']); + }); + + it('exports.foo = function* () {}', async () => { + expect( + await labelsFor('src/g.js', 'exports.walk = function* () { yield 1; };\n', 'walk'), + ).toEqual(['Function']); + }); + + it('TS parity: exports.foo = function () {}', async () => { + expect( + await labelsFor( + 'src/f.ts', + 'exports.tsExport = function (x: number) { return x; };\n', + 'tsExport', + ), + ).toEqual(['Function']); + }); +}); + +describe('#2723 follow-up: callable member assignments', () => { + const nodesFor = async ( + path: string, + content: string, + ): Promise<{ labels: string[]; ids: string[]; owners: string[] }> => { + const { graph } = await parseFilesWithWorkers([{ path, content }]); + return { + labels: graph.nodes.map((n) => n.label).sort(), + ids: graph.nodes.map((n) => n.id).sort(), + owners: graph.relationships + .filter((r) => r.type === 'HAS_METHOD') + .map((r) => `${r.sourceId} -> ${r.targetId}`) + .sort(), + }; + }; + + it('Foo.prototype.bar = fn is a Method owned by the constructor', async () => { + const { ids, owners } = await nodesFor( + 'src/proto.js', + 'function Foo() {}\nFoo.prototype.bar = function (v) { return v; };\n', + ); + expect(ids).toContain('Method:src/proto.js:Foo.bar'); + expect(owners).toEqual(['Function:src/proto.js:Foo -> Method:src/proto.js:Foo.bar']); + }); + + it('a class owner gets a Class-sourced owner edge', async () => { + const { owners } = await nodesFor( + 'src/protocls.js', + 'class Ctl {}\nCtl.prototype.run = function () { return 1; };\n', + ); + expect(owners).toEqual(['Class:src/protocls.js:Ctl -> Method:src/protocls.js:Ctl.run']); + }); + + // Two constructors defining the same member name must stay distinct nodes; + // an unqualified `Method::bar` would collapse them into one. + it('same-named prototype members on different owners do not collide', async () => { + const { ids } = await nodesFor( + 'src/two.js', + 'function Foo() {}\nFoo.prototype.bar = function () { return 1; };\n' + + 'function Baz() {}\nBaz.prototype.bar = function () { return 2; };\n', + ); + expect(ids).toContain('Method:src/two.js:Foo.bar'); + expect(ids).toContain('Method:src/two.js:Baz.bar'); + }); + + // An owner the file does not declare cannot be resolved to a node, so no + // owner edge is claimed rather than one pointing at a fabricated node. + it('an undeclared prototype owner claims no owner edge', async () => { + const { owners } = await nodesFor( + 'src/ext.js', + 'External.prototype.skipped = function () { return 1; };\n', + ); + expect(owners).toEqual([]); + }); + + it('this.handler = fn in a constructor is a Method owned by it', async () => { + const { owners } = await nodesFor( + 'src/this.js', + 'function Widget() {\n this.handler = function (v) { return v; };\n}\n', + ); + expect(owners).toHaveLength(1); + expect(owners[0]).toMatch(/^Function:src\/this\.js:Widget -> Method:src\/this\.js:Widget\./); + }); + + it('this.cb = fn in a class constructor is owned by the class', async () => { + const { owners } = await nodesFor( + 'src/thiscls.js', + 'class Klass {\n constructor() { this.cb = function () { return 1; }; }\n}\n', + ); + // The class owns its `constructor` AND the `cb` member assigned inside it. + const cbOwners = owners.filter((o) => o.includes('.cb')); + expect(cbOwners).toHaveLength(1); + expect(cbOwners[0]).toMatch(/^Class:src\/thiscls\.js:Klass -> Method:src\/thiscls\.js:Klass\./); + }); + + // The scope declaration for a shadowed CJS export is suppressed, so its graph + // node would be unreachable. With a `class` of the same name the labels + // differ, so it does not even collapse — it lingers as an orphan. + // The member-assignment rule matches ANY identifier receiver so an exports + // alias can be recognised; everything else must be pruned emit-side. Without + // that, every `obj.handler = fn` would emit a top-level `Function`. + // `module.exports = fn` — the whole module is the callable, so there is no + // property to name it after. Anonymous forms take the file-derived name + // (`deriveDefaultExportHocName`), the convention already used for anonymous + // default exports; a named function expression keeps its own name. + it('module.exports = function () {} is named after the file', async () => { + const { ids } = await nodesFor( + 'src/cjsdef.js', + 'module.exports = function (v) { return v; };\n', + ); + expect(ids).toContain('Function:src/cjsdef.js:cjsdef'); + }); + + it('module.exports = (v) => v is named after the file', async () => { + const { ids } = await nodesFor('src/cjsarrow.js', 'module.exports = (v) => v;\n'); + expect(ids).toContain('Function:src/cjsarrow.js:cjsarrow'); + }); + + // The member-assignment rule captures the LEFT property as the name, which + // for this shape is the literal `exports`. That must not survive. + it('a named module.exports function keeps its own name and emits no `exports` node', async () => { + const { ids } = await nodesFor( + 'src/cjsnamed.js', + 'module.exports = function namedFn(v) { return v; };\n', + ); + expect(ids).toContain('Function:src/cjsnamed.js:namedFn'); + expect(ids).not.toContain('Function:src/cjsnamed.js:exports'); + }); + + // Reassigning `exports` does NOT export in CommonJS — it only breaks the + // alias to `module.exports` — so indexing it would invent an export. + // Paired with a positive control in the SAME fixture. Without it this test + // passes trivially: no JS/TS query matches a bare-identifier LHS at all, so + // an empty graph — from a parse failure or an unrelated guard misfire — + // would satisfy it just as well as correct behaviour (#2729 review F9). + it('exports = fn is not treated as a default export', async () => { + const { ids } = await nodesFor( + 'src/rebind.js', + 'exports = function (v) { return v; };\nmodule.exports.ok = function (v) { return v; };\n', + ); + expect(ids).toContain('Function:src/rebind.js:ok'); + expect(ids).not.toContain('Function:src/rebind.js:exports'); + expect(ids).not.toContain('Function:src/rebind.js:rebind'); + }); + + // F4: an anonymous default takes the FILE name. When that collides with a + // callable the module already declares, the two merged onto one node and the + // inner call resolved to itself — fabricating a self-recursion edge present + // in no source. A fabricated edge is worse than a missing one. + it('a default export whose derived name collides emits no merged node', async () => { + const { ids } = await nodesFor( + 'src/format.js', + 'function format(v) { return String(v); }\nmodule.exports = function (v) { return format(v); };\n', + ); + expect(ids).toContain('Function:src/format.js:format'); + expect(ids).not.toContain('Function:src/format.js:exports'); + expect(ids.filter((id) => id.endsWith(':format'))).toHaveLength(1); + }); + + // F6: `var Foo = function(){}` is the dominant pre-ES6 constructor form. + // Owner lookup handled only declarations, so two same-named members + // collapsed onto one unqualified node with no owner edges at all. + it('variable-bound constructors own their prototype methods distinctly', async () => { + const { ids, owners } = await nodesFor( + 'src/varctor.js', + 'var Foo = function () {};\nFoo.prototype.run = function (a) { return a; };\n' + + 'var Baz = function () {};\nBaz.prototype.run = function (a) { return !a; };\n', + ); + expect(ids).toContain('Method:src/varctor.js:Foo.run'); + expect(ids).toContain('Method:src/varctor.js:Baz.run'); + expect(owners).toEqual([ + 'Function:src/varctor.js:Baz -> Method:src/varctor.js:Baz.run', + 'Function:src/varctor.js:Foo -> Method:src/varctor.js:Foo.run', + ]); + }); + + it('a non-exports receiver emits no node', async () => { + const { labels } = await nodesFor( + 'src/plain.js', + 'const obj = {};\nobj.notAnExport = function () { return 1; };\n' + + 'self.alsoNot = () => 1;\nlocalThing.nope = function () { return 2; };\n', + ); + expect(labels.filter((l) => l === 'Function' || l === 'Method')).toEqual([]); + }); + + // Top-level `this` is `undefined` in ESM, so it exports nothing there. This + // gate is what keeps the CJS rule above from mis-indexing every ESM file. + it('module-level this.X = fn in an ESM file is not an export', async () => { + const { ids } = await nodesFor( + 'src/esmmod.js', + "import { helper } from './helper.js';\n" + + 'this.notAnExport = function (v) { return helper(v); };\n' + + 'export const real = 1;\n', + ); + expect(ids).not.toContain('Function:src/esmmod.js:notAnExport'); + }); + + it('a file with no CJS or ESM signal does not claim a this export', async () => { + const { ids } = await nodesFor( + 'src/neither.js', + 'this.ambiguous = function () { return 1; };\n', + ); + expect(ids).not.toContain('Function:src/neither.js:ambiguous'); + }); + + it('a CJS export shadowing a class emits no orphan Function twin', async () => { + const { labels, ids } = await nodesFor( + 'src/twin.js', + 'class Dup { run() { return 1; } }\nexports.Dup = function () { return 2; };\n', + ); + expect(ids).not.toContain('Function:src/twin.js:Dup'); + expect(labels.filter((l) => l === 'Function')).toEqual([]); + }); +}); + +const describeIfWorkerBuilt = distWorkerExists() ? describe : describe.skip; + +describeIfWorkerBuilt('#2723 calls resolve to the CJS-exported function', () => { + /** Names of the symbols that CALL `name`, resolved through the real pipeline. */ + const callersOf = async ( + files: { path: string; content: string }[], + name: string, + inFile?: string, + ): Promise => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-2723-')); + try { + for (const file of files) { + const full = path.join(dir, file.path); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, file.content, 'utf-8'); + } + const { graph } = await runPipelineFromRepo(dir, () => {}, { + workerPoolSize: 1, + workerUrlForTest: DIST_WORKER_URL, + }); + const target = graph.nodes.find( + (n) => + n.properties.name === name && + n.label === 'Function' && + (inFile === undefined || n.id.includes(`:${inFile}:`)), + ); + expect(target).toBeDefined(); + const byId = new Map(graph.nodes.map((n) => [n.id, n])); + return graph.relationships + .filter((rel) => rel.type === 'CALLS' && rel.targetId === target!.id) + .map((rel) => String(byId.get(rel.sourceId)?.properties.name ?? rel.sourceId)) + .sort(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }; + + it('same-file call resolves', async () => { + expect( + await callersOf( + [ + { + path: 'src/validate.js', + content: + 'exports.areVariablesValid = function (v) { return !!v; };\n' + + 'exports.check = function (v) { return exports.areVariablesValid(v); };\n', + }, + ], + 'areVariablesValid', + ), + ).toEqual(['check']); + }); + + it('cross-file require() member call resolves', async () => { + expect( + await callersOf( + [ + { + path: 'src/validate.js', + content: 'exports.areVariablesValid = function (v) { return !!v; };\n', + }, + { + path: 'src/handler.js', + content: + "const validate = require('./validate');\n" + + 'function handle(v) { return validate.areVariablesValid(v); }\n', + }, + ], + 'areVariablesValid', + ), + ).toEqual(['handle']); + }); + + // The graph-node rules accept `generator_function` for this form, so without + // the matching scope declaration the node existed and nothing resolved to it. + it('generator export resolves through both receiver forms', async () => { + const files = [ + { + path: 'src/gen.js', + content: + 'exports.walk = function* () { yield 1; };\n' + + 'module.exports.crawl = function* () { yield 2; };\n', + }, + { + path: 'src/use.js', + content: + "const { walk, crawl } = require('./gen');\n" + + 'function drive() { return [...walk(), ...crawl()]; }\n', + }, + ]; + expect(await callersOf(files, 'walk')).toEqual(['drive']); + expect(await callersOf(files, 'crawl')).toEqual(['drive']); + }); + + // A CJS export assignment must not shadow a same-named `function X(){}` in + // the same file. Registering a second module-scope declaration for `dup` + // makes the name ambiguous and the resolver drops the intra-module edge + // entirely — a silently missing caller, which is worse than the gap #2723 + // set out to close. Verified against base ff86ccf1e, where this edge exists. + it('an exports assignment does not shadow a same-named declared function', async () => { + expect( + await callersOf( + [ + { + path: 'src/collide.js', + content: + 'function dup(v) { return v; }\n' + + 'exports.dup = function (v) { return !v; };\n' + + 'function callIt(v) { return dup(v); }\n', + }, + ], + 'dup', + ), + ).toEqual(['callIt']); + }); + + it('an exports alias resolves like a direct export', async () => { + const files = [ + { + path: 'src/alias.js', + content: + 'const e = exports;\n' + + 'const m = module.exports;\n' + + 'e.aliased = function (v) { return v; };\n' + + 'm.viaModule = (v) => v;\n', + }, + { + path: 'src/aliasuse.js', + content: + "const { aliased, viaModule } = require('./alias');\n" + + 'function drive(v) { return [aliased(v), viaModule(v)]; }\n', + }, + ]; + expect(await callersOf(files, 'aliased')).toEqual(['drive']); + expect(await callersOf(files, 'viaModule')).toEqual(['drive']); + }); + + // In CommonJS, module-level `this` IS `module.exports`. + it('module-level this.X = fn in a CJS file is an export', async () => { + expect( + await callersOf( + [ + { + path: 'src/cjsmod.js', + content: + "const dep = require('./dep');\n" + + 'this.topExport = function (v) { return dep(v); };\n', + }, + { path: 'src/dep.js', content: 'module.exports = function (v) { return v; };\n' }, + { + path: 'src/usecjs.js', + content: + "const { topExport } = require('./cjsmod');\n" + + 'function drive(v) { return topExport(v); }\n', + }, + ], + 'topExport', + ), + ).toEqual(['drive']); + }); + + // `exports.fwd = lib.imported` assigns an existing symbol rather than a + // literal, so no definition rule reaches it. Modelled as a re-export, NOT a + // plain import binding: an import is private to its module, so importers of + // the forwarding module resolved to nothing. + it('a CJS re-export forwards to the original definition', async () => { + const files = [ + { + path: 'src/lib.js', + content: + 'exports.imported = function (v) { return v; };\n' + + 'exports.second = function (v) { return !v; };\n', + }, + { + path: 'src/fwd.js', + content: + "const lib = require('./lib');\n" + + "const { second } = require('./lib');\n" + + 'exports.forwarded = lib.imported;\n' + + 'exports.alsoForwarded = second;\n', + }, + { + path: 'src/usefwd.js', + content: + "const { forwarded, alsoForwarded } = require('./fwd');\n" + + 'function drive(v) { return [forwarded(v), alsoForwarded(v)]; }\n', + }, + ]; + expect(await callersOf(files, 'imported')).toEqual(['drive']); + expect(await callersOf(files, 'second')).toEqual(['drive']); + }); + + // ─── #2729 review regressions ────────────────────────────────────────── + // Each of these failed against the pre-review build. They cover the two root + // causes: a receiver identified by TEXT with no scope lookup, and a shadow + // guard that reached only one of the export forms. + + // F2: the canonical UMD wrapper takes the exports object as a PARAMETER. A + // text match called it a module export, invented a symbol, and deleted the + // factory's real call edges. + it('an `exports` parameter does not hijack the module (UMD factory)', async () => { + expect( + await callersOf( + [ + { path: 'src/other.js', content: 'exports.noop = function (v) { return v; };\n' }, + { + path: 'src/umd.js', + content: + "const other = require('./other');\n" + + '(function (exports) {\n' + + ' function publicApi(v) { return other.noop(v); }\n' + + ' exports.publicApi = function (v) { return publicApi(v); };\n' + + '})(this);\n', + }, + ], + 'noop', + ), + ).toEqual(['publicApi']); + }); + + // F1: `const helper = require('./helper'); exports.helper = fn` resolved an + // importer into the OTHER module's function — an edge in no source. + it('a wrapper export does not resolve into the required module', async () => { + expect( + await callersOf( + [ + { path: 'src/helper.js', content: 'exports.helper = function (v) { return v; };\n' }, + { + path: 'src/wrap.js', + content: + "const helper = require('./helper');\nexports.helper = function (v) { return v; };\n", + }, + { + path: 'src/wrapuse.js', + content: + "const { helper } = require('./wrap');\nfunction driveWrap(v) { return helper(v); }\n", + }, + ], + 'helper', + 'src/wrap.js', + ), + ).toEqual(['driveWrap']); + }); + + // F3 hole A: the alias receiver skipped the shadow guard, dropping a real edge. + it('an aliased export does not shadow a same-named declared function', async () => { + expect( + await callersOf( + [ + { + path: 'src/aliasdup.js', + content: + 'const e = exports;\nfunction dup(v) { return v; }\n' + + 'e.dup = function (v) { return !v; };\nfunction callIt(v) { return dup(v); }\n', + }, + ], + 'dup', + ), + ).toEqual(['callIt']); + }); + + // F3 hole B: module-level `this` never reached the shadow guard at all. + it('a module-level this export does not shadow a declared function', async () => { + expect( + await callersOf( + [ + { + path: 'src/thisdup.js', + content: + "const dep = require('./other2');\nfunction dup2(v) { return v; }\n" + + 'this.dup2 = function (v) { return !v; };\nfunction callIt2(v) { return dup2(v); }\n', + }, + { path: 'src/other2.js', content: 'exports.noop = function (v) { return v; };\n' }, + ], + 'dup2', + ), + ).toEqual(['callIt2']); + }); + + // F8: the guard collected EVERY module-scope name, so an export colliding + // with a plain variable was deleted outright — node and edge both gone. + it('a non-callable variable of the same name does not delete the export', async () => { + expect( + await callersOf( + [ + { + path: 'src/cache.js', + content: + 'let cache = null;\nexports.cache = function (v) { cache = v; return cache; };\n', + }, + { + path: 'src/cacheuse.js', + content: + "const { cache } = require('./cache');\nfunction driveCache(v) { return cache(v); }\n", + }, + ], + 'cache', + ), + ).toEqual(['driveCache']); + }); + + it('cross-file destructured require() call resolves', async () => { + expect( + await callersOf( + [ + { + path: 'src/validate.js', + content: 'exports.areVariablesValid = function (v) { return !!v; };\n', + }, + { + path: 'src/handler.js', + content: + "const { areVariablesValid } = require('./validate');\n" + + 'function handle(v) { return areVariablesValid(v); }\n', + }, + ], + 'areVariablesValid', + ), + ).toEqual(['handle']); + }); +}); diff --git a/gitnexus/test/unit/call-summary-schema-version.test.ts b/gitnexus/test/unit/call-summary-schema-version.test.ts index 5141c8147..65b48dc6f 100644 --- a/gitnexus/test/unit/call-summary-schema-version.test.ts +++ b/gitnexus/test/unit/call-summary-schema-version.test.ts @@ -73,12 +73,12 @@ describe('CALL_SUMMARY relation-type exclusion (U-C1)', () => { }); describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { - it('INCREMENTAL_SCHEMA_VERSION is bumped to 21 (closure bindings are call SOURCES, #2699 part B)', () => { + it('INCREMENTAL_SCHEMA_VERSION is bumped to 22 (CommonJS export indexing, #2723)', () => { // Moves with every bump BY DESIGN — that is the point of pinning it. A // change that alters emitted ids or edges without bumping would otherwise // ship silently, and an existing index would keep serving the old graph // through the reuse gate below. - expect(INCREMENTAL_SCHEMA_VERSION).toBe(21); + expect(INCREMENTAL_SCHEMA_VERSION).toBe(22); }); it('a pre-current stamp fails the `=== INCREMENTAL_SCHEMA_VERSION` reuse gate → forces full re-analyze', () => { @@ -169,6 +169,10 @@ describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { // source. expect(passesReuseGate(20)).toBe(false); // A current-version stamp passes the gate (incremental top-up eligible). - expect(passesReuseGate(21)).toBe(true); + // A pre-v22 (v21) index predates CommonJS export indexing (#2723): every + // unchanged CJS file would keep its pre-fix graph → must NOT reuse. + expect(passesReuseGate(21)).toBe(false); + // The current stamp passes. + expect(passesReuseGate(22)).toBe(true); }); });