From 43271f3cd3533ab2cc20f9a9ab2dd252235a7d2c Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 8 Sep 2026 09:37:20 -0500 Subject: [PATCH] fix(extraction): index CommonJS export assignments as functions (#1675) (#1771) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `exports.getItems = async (req, res) => {…}` and `module.exports.x = function () {…}` — the Express controller style — produced no symbol: the arrow's parent is an assignment, not a declarator, so it stayed anonymous, its calls attributed to the file, and `node`/`callers` answered "Symbol not found" for a route-wired handler. Resolve the name from the export property, mark it exported, in both the wasm walker and the kernel. Co-authored-by: danusha2345 Co-authored-by: Colby McHenry --- CHANGELOG.md | 1 + __tests__/commonjs-exports.test.ts | 71 +++++++++++++++++++++ __tests__/fixtures/kernel-parity/torture.js | 5 ++ codegraph-kernel/src/tsjs/extractors.rs | 36 ++++++++++- src/extraction/tree-sitter.ts | 43 ++++++++++++- 5 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 __tests__/commonjs-exports.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 727f586..bac0770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -213,6 +213,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- CommonJS controllers written as `exports.getItems = async (req, res) => {…}` or `module.exports.x = function () {…}` are now indexed as exported functions, so `node`, `callers` and impact find every Express handler in that style and the calls inside them belong to the handler instead of the file. Re-index JavaScript projects after upgrading. (#1675) - Python parameters annotated with a quoted forward reference — `def f(o: "Alpha")`, or anything under `from __future__ import annotations` — now resolve the methods called on them, the same as the unquoted annotation. Re-index Python projects after upgrading. (#1684) - **A C macro call written with designated initializers no longer swallows every function after it.** Betaflight resets each config struct with `RESET_CONFIG(type, dst, .field = value, …)`, a shape the C grammar cannot parse; past a hundred or so fields its error recovery ran the enclosing function to the end of the file, the next function vanished from the index and every later one was filed under the first, where name matching then treated it as an unreachable closure. The argument list of such a call is now blanked before parsing, offsets kept, so the file's functions come out with their real extents. On that tree 45 functions in `pid.c` alone moved back to top level and their 117 callers resolve at exact-match confidence. Re-index after upgrading. (#1729) - A method called on the result of another call — `d.setdefault(k, []).append(v)`, `make().run()` — no longer produces a call edge to an unrelated top-level function that merely shares the name, in Python and JavaScript/TypeScript. The receiver is kept so the inner call still resolves; the outer method stays unresolved rather than guessed. Re-index after upgrading. (#1683, #1681) diff --git a/__tests__/commonjs-exports.test.ts b/__tests__/commonjs-exports.test.ts new file mode 100644 index 0000000..eaadf22 --- /dev/null +++ b/__tests__/commonjs-exports.test.ts @@ -0,0 +1,71 @@ +/** + * CommonJS export assignments name the function they hold (#1675). + * + * `exports.getItems = async (req, res) => {…}` and `module.exports.x = + * function () {…}` are how Express controllers are commonly written. The + * arrow is anonymous only syntactically — the export property is the name + * every `router.get('/items', getItems)` resolves — so it gets the same + * treatment `const getItems = () => {}` already has: a function node, exported, + * with its calls attributed to it rather than to the file. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; + +beforeAll(async () => { + await initGrammars(); + await loadAllGrammars(); +}); + +const refsFrom = (result: ReturnType, id: string) => + result.unresolvedReferences.filter((r) => r.fromNodeId === id).map((r) => r.referenceName); + +describe('CommonJS export assignments', () => { + it('indexes exports.X / module.exports.X functions as exported function nodes', () => { + const code = ` +const { findItems, removeItem } = require('./db'); + +exports.getItems = async (req, res) => { + res.json(await findItems()); +}; + +module.exports.deleteItem = function (req, res) { + removeItem(req.params.id); + res.end(); +}; + +exports.plain = 42; +module.exports = { legacy: 1 }; +`; + const result = extractFromSource('src/controller.js', code); + const fns = result.nodes.filter((n) => n.kind === 'function'); + expect(fns.map((n) => n.name).sort()).toEqual(['deleteItem', 'getItems']); + + const getItems = fns.find((n) => n.name === 'getItems')!; + const deleteItem = fns.find((n) => n.name === 'deleteItem')!; + expect(getItems.startLine).toBe(4); + expect(getItems.isExported).toBe(true); + expect(deleteItem.isExported).toBe(true); + expect(getItems.isAsync).toBe(true); + + // The handlers' calls are their own, not the file's. + expect(refsFrom(result, getItems.id)).toContain('findItems'); + expect(refsFrom(result, deleteItem.id)).toContain('removeItem'); + const file = result.nodes.find((n) => n.kind === 'file')!; + expect(refsFrom(result, file.id)).not.toContain('findItems'); + expect(refsFrom(result, file.id)).not.toContain('removeItem'); + + // A non-function export is not a function, and nothing is left anonymous. + expect(result.nodes.map((n) => n.name)).not.toContain(''); + }); + + it('leaves other member assignments alone', () => { + const code = ` +const handlers = {}; +handlers.onSave = () => { persist(); }; +app.locals.format = function () { return 1; }; +`; + const result = extractFromSource('src/other.js', code); + expect(result.nodes.filter((n) => n.kind === 'function')).toEqual([]); + }); +}); diff --git a/__tests__/fixtures/kernel-parity/torture.js b/__tests__/fixtures/kernel-parity/torture.js index f1e57a6..54926eb 100644 --- a/__tests__/fixtures/kernel-parity/torture.js +++ b/__tests__/fixtures/kernel-parity/torture.js @@ -74,6 +74,11 @@ export default { }, }; +// --- CommonJS export assignments (#1675) ----------------------------------- +exports.getItems = async (req, res) => { res.json(await findItems()); }; +module.exports.deleteItem = function (req, res) { removeItem(req.params.id); res.end(); }; +exports.plain = 42; +handlers.onSave = () => { persist(); }; // --- call-expression receivers (#1683) ---------------------------------------- function bucketChains(d, k, v) { d.setdefault(k, []).append(v); diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index f3bb881..a7c2f5f 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -20,7 +20,10 @@ impl<'t> Walker<'t> { .unwrap_or_else(|| self.extract_name(node)); // Arrow/function-expression values: resolve the name from the parent - // variable_declarator (`export const useAuth = () => {}`). + // variable_declarator (`export const useAuth = () => {}`), or from a + // CommonJS export assignment (`exports.getItems = async () => {}`, + // #1675). Mirrors TreeSitterExtractor.extractFunction. + let mut common_js_export = false; if name_override.is_none() && name == "" && matches!(node.kind(), "arrow_function" | "function_expression" | "generator_function") @@ -30,6 +33,11 @@ impl<'t> Walker<'t> { if let Some(var_name) = parent.child_by_field_name("name") { name = self.text(var_name).to_string(); } + } else if parent.kind() == "assignment_expression" { + if let Some(export_name) = self.common_js_export_name(parent, node) { + name = export_name; + common_js_export = true; + } } } } @@ -46,7 +54,7 @@ impl<'t> Walker<'t> { docstring: crate::docstring::preceding_docstring(node, self.src), signature: self.signature_of(node), visibility: self.visibility_of(node), - is_exported: Some(self.is_exported(node)), + is_exported: Some(common_js_export || self.is_exported(node)), is_async: Some(self.is_async(node)), is_static: self.is_static(node), ..Extra::default() @@ -65,6 +73,30 @@ impl<'t> Walker<'t> { self.stack.pop(); } + /// The property a CommonJS export assignment binds a function to — + /// `exports.NAME = ` / `module.exports.NAME = ` — or None for + /// any other assignment. The node must be the assignment's whole + /// right-hand side. Mirrors TreeSitterExtractor.commonJsExportName. + fn common_js_export_name(&self, assignment: Node<'t>, value: Node<'t>) -> Option { + let right = assignment.child_by_field_name("right")?; + if right.start_byte() != value.start_byte() || right.end_byte() != value.end_byte() { + return None; + } + let left = assignment.child_by_field_name("left")?; + if left.kind() != "member_expression" { + return None; + } + let object = left.child_by_field_name("object")?; + let property = left.child_by_field_name("property")?; + if property.kind() != "property_identifier" { + return None; + } + if !matches!(self.text(object), "exports" | "module.exports") { + return None; + } + Some(self.text(property).to_string()) + } + // --- reactComponentHoc / extractReactComponentNode (#841) -------------------- /// Some(inner) when the initializer is a recognized component wrapper — diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index c8025d7..a57ec89 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -1590,6 +1590,8 @@ export class TreeSitterExtractor { // — SvelteKit actions). Inline-object arrows reached by the general walker // get no override, so they still fall through to the skip below. let name = nameOverride ?? extractName(node, this.source, this.extractor); + // A CommonJS export assignment names the function it holds — see below. + let commonJsExport = false; // For arrow functions and function expressions assigned to variables, // resolve the name from the parent variable_declarator. // e.g. `export const useAuth = () => { ... }` — the arrow_function node @@ -1605,6 +1607,18 @@ export class TreeSitterExtractor { if (varName) { name = getNodeText(varName, this.source); } + } else if (parent?.type === 'assignment_expression') { + // `exports.getItems = async (req, res) => {…}` / `module.exports.x = + // function () {…}` — the CommonJS controller style. The function is + // anonymous only syntactically: the export property is the name every + // `router.get('/items', getItems)` resolves. Without a node the handler + // is invisible to callers/impact and its calls attribute to the file + // (#1675). Same treatment `const X = () => {}` already gets. + const exportName = this.commonJsExportName(parent, node); + if (exportName) { + name = exportName; + commonJsExport = true; + } } } if (name === '') { @@ -1635,7 +1649,7 @@ export class TreeSitterExtractor { const docstring = getPrecedingDocstring(node, this.source); const signature = this.extractor.getSignature?.(node, this.source); const visibility = this.extractor.getVisibility?.(node); - const isExported = this.extractor.isExported?.(node, this.source); + const isExported = commonJsExport || this.extractor.isExported?.(node, this.source); const isAsync = this.extractor.isAsync?.(node); const isStatic = this.extractor.isStatic?.(node); const returnType = this.extractor.getReturnType?.(node, this.source); @@ -5311,6 +5325,33 @@ export class TreeSitterExtractor { targets.add(target); } + /** + * The property a CommonJS export assignment binds a function to — + * `exports.NAME = ` or `module.exports.NAME = ` — or null for + * any other assignment. JS-family only; the node must be the assignment's + * whole right-hand side. + */ + private commonJsExportName(assignment: SyntaxNode, value: SyntaxNode): string | null { + if ( + this.language !== 'typescript' && + this.language !== 'javascript' && + this.language !== 'tsx' && + this.language !== 'jsx' + ) { + return null; + } + const right = getChildByField(assignment, 'right'); + if (!right || right.startIndex !== value.startIndex || right.endIndex !== value.endIndex) return null; + const left = getChildByField(assignment, 'left'); + if (!left || left.type !== 'member_expression') return null; + const object = getChildByField(left, 'object'); + const property = getChildByField(left, 'property'); + if (!object || !property || property.type !== 'property_identifier') return null; + const objectText = getNodeText(object, this.source); + if (objectText !== 'exports' && objectText !== 'module.exports') return null; + return getNodeText(property, this.source); + } + /** * The declarator name a React handler hook binds an anonymous function to — * `const NAME = useCallback(, [...])` — or null for any other shape.