Просмотр исходного кода

fix(extraction): index CommonJS export assignments as functions (#1675) (#1771)

`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 <ewidusoc498@gmail.com>
Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 6 часов назад
Родитель
Сommit
43271f3cd3

+ 1 - 0
CHANGELOG.md

@@ -213,6 +213,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 
 #### Symbols, tests and the viewer
 #### 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)
 - 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 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)
 - 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)

+ 71 - 0
__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<typeof extractFromSource>, 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('<anonymous>');
+  });
+
+  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([]);
+  });
+});

+ 5 - 0
__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) ----------------------------------------
 // --- call-expression receivers (#1683) ----------------------------------------
 function bucketChains(d, k, v) {
 function bucketChains(d, k, v) {
   d.setdefault(k, []).append(v);
   d.setdefault(k, []).append(v);

+ 34 - 2
codegraph-kernel/src/tsjs/extractors.rs

@@ -20,7 +20,10 @@ impl<'t> Walker<'t> {
             .unwrap_or_else(|| self.extract_name(node));
             .unwrap_or_else(|| self.extract_name(node));
 
 
         // Arrow/function-expression values: resolve the name from the parent
         // 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()
         if name_override.is_none()
             && name == "<anonymous>"
             && name == "<anonymous>"
             && matches!(node.kind(), "arrow_function" | "function_expression" | "generator_function")
             && 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") {
                     if let Some(var_name) = parent.child_by_field_name("name") {
                         name = self.text(var_name).to_string();
                         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),
             docstring: crate::docstring::preceding_docstring(node, self.src),
             signature: self.signature_of(node),
             signature: self.signature_of(node),
             visibility: self.visibility_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_async: Some(self.is_async(node)),
             is_static: self.is_static(node),
             is_static: self.is_static(node),
             ..Extra::default()
             ..Extra::default()
@@ -65,6 +73,30 @@ impl<'t> Walker<'t> {
         self.stack.pop();
         self.stack.pop();
     }
     }
 
 
+    /// The property a CommonJS export assignment binds a function to —
+    /// `exports.NAME = <node>` / `module.exports.NAME = <node>` — 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<String> {
+        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) --------------------
     // --- reactComponentHoc / extractReactComponentNode (#841) --------------------
 
 
     /// Some(inner) when the initializer is a recognized component wrapper —
     /// Some(inner) when the initializer is a recognized component wrapper —

+ 42 - 1
src/extraction/tree-sitter.ts

@@ -1590,6 +1590,8 @@ export class TreeSitterExtractor {
     // — SvelteKit actions). Inline-object arrows reached by the general walker
     // — SvelteKit actions). Inline-object arrows reached by the general walker
     // get no override, so they still fall through to the <anonymous> skip below.
     // get no override, so they still fall through to the <anonymous> skip below.
     let name = nameOverride ?? extractName(node, this.source, this.extractor);
     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,
     // For arrow functions and function expressions assigned to variables,
     // resolve the name from the parent variable_declarator.
     // resolve the name from the parent variable_declarator.
     // e.g. `export const useAuth = () => { ... }` — the arrow_function node
     // e.g. `export const useAuth = () => { ... }` — the arrow_function node
@@ -1605,6 +1607,18 @@ export class TreeSitterExtractor {
         if (varName) {
         if (varName) {
           name = getNodeText(varName, this.source);
           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 === '<anonymous>') {
     if (name === '<anonymous>') {
@@ -1635,7 +1649,7 @@ export class TreeSitterExtractor {
     const docstring = getPrecedingDocstring(node, this.source);
     const docstring = getPrecedingDocstring(node, this.source);
     const signature = this.extractor.getSignature?.(node, this.source);
     const signature = this.extractor.getSignature?.(node, this.source);
     const visibility = this.extractor.getVisibility?.(node);
     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 isAsync = this.extractor.isAsync?.(node);
     const isStatic = this.extractor.isStatic?.(node);
     const isStatic = this.extractor.isStatic?.(node);
     const returnType = this.extractor.getReturnType?.(node, this.source);
     const returnType = this.extractor.getReturnType?.(node, this.source);
@@ -5311,6 +5325,33 @@ export class TreeSitterExtractor {
     targets.add(target);
     targets.add(target);
   }
   }
 
 
+  /**
+   * The property a CommonJS export assignment binds a function to —
+   * `exports.NAME = <node>` or `module.exports.NAME = <node>` — 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 —
    * The declarator name a React handler hook binds an anonymous function to —
    * `const NAME = useCallback(<node>, [...])` — or null for any other shape.
    * `const NAME = useCallback(<node>, [...])` — or null for any other shape.