diff --git a/gitnexus/src/core/ingestion/languages/javascript/index.ts b/gitnexus/src/core/ingestion/languages/javascript/index.ts index e887bab51..943655084 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/index.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/index.ts @@ -34,11 +34,17 @@ * 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. **`module.exports` / `exports.X`** — CJS export forms are not 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. + * + * `exports.foo = function () {}` / `module.exports.foo = (a) => a` DO + * declare `foo` in the module scope (#2723) — the query block in + * `query.ts` — so importers resolve to them by name. What is still + * unmodeled is the re-export EDGE: `exports.foo = someImported` does not + * forward to the original declaration, and `module.exports = fn` + * (anonymous default) has no name to bind. */ 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..986d34e92 100644 --- a/gitnexus/src/core/ingestion/languages/javascript/query.ts +++ b/gitnexus/src/core/ingestion/languages/javascript/query.ts @@ -152,6 +152,75 @@ 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. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.exports + property: (property_identifier) @declaration.name) + right: (arrow_function) @declaration.function + (#eq? @_cjs.exports "exports")) + +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.exports + property: (property_identifier) @declaration.name) + right: (function_expression) @declaration.function + (#eq? @_cjs.exports "exports")) + +;; Generator parity. The graph-node rules in \`tree-sitter-queries.ts\` accept +;; \`generator_function\` for this form, so without a matching declaration the +;; node existed with nothing resolving to it — the same half-fixed state the +;; block above exists to prevent. \`(generator_function) @scope.function\` is +;; declared near the top of this query, so the anchor still aligns. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.exports + property: (property_identifier) @declaration.name) + right: (generator_function) @declaration.function + (#eq? @_cjs.exports "exports")) + +(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) @declaration.function + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @declaration.name) + right: (function_expression) @declaration.function + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @declaration.name) + right: (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/query.ts b/gitnexus/src/core/ingestion/languages/typescript/query.ts index fa7ff4089..d5e02fac1 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/query.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/query.ts @@ -215,6 +215,62 @@ 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. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.exports + property: (property_identifier) @declaration.name) + right: (arrow_function) @declaration.function + (#eq? @_cjs.exports "exports")) + +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.exports + property: (property_identifier) @declaration.name) + right: (function_expression) @declaration.function + (#eq? @_cjs.exports "exports")) + +;; Generator parity — see the matching note in \`languages/javascript/query.ts\`. +(assignment_expression + left: (member_expression + object: (identifier) @_cjs.exports + property: (property_identifier) @declaration.name) + right: (generator_function) @declaration.function + (#eq? @_cjs.exports "exports")) + +(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) @declaration.function + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @declaration.name) + right: (function_expression) @declaration.function + (#eq? @_cjs.module "module") + (#eq? @_cjs.exports "exports")) + +(assignment_expression + left: (member_expression + object: (member_expression + object: (identifier) @_cjs.module + property: (property_identifier) @_cjs.exports) + property: (property_identifier) @declaration.name) + right: (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 8eb1d8c3d..eebca1bca 100644 --- a/gitnexus/src/core/ingestion/tree-sitter-queries.ts +++ b/gitnexus/src/core/ingestion/tree-sitter-queries.ts @@ -390,6 +390,26 @@ 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.exports + property: (property_identifier) @name) + right: [(function_expression) (arrow_function) (generator_function)] + (#eq? @_cjs.exports "exports")) @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 + ; Constructor parameter properties: constructor(public address: Address) (required_parameter (accessibility_modifier) @@ -540,6 +560,35 @@ 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.exports + property: (property_identifier) @name) + right: [(function_expression) (arrow_function) (generator_function)] + (#eq? @_cjs.exports "exports")) @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 + ; Object-property arrows / function expressions: \`{ addItem: () => ... }\`. ; See TYPESCRIPT_QUERIES for rationale (issue #1166). (pair 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..20cf0ffe6 --- /dev/null +++ b/gitnexus/test/integration/cjs-exports-assignment.test.ts @@ -0,0 +1,197 @@ +/** + * #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']); + }); +}); + +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, + ): 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'); + 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']); + }); + + 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']); + }); +});