fix(js,ts): a function exported as exports["name"] is a symbol named after its export

CHANGELOG.md

@@ -131,6 +131,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - Axum routes now link to their handler when rustfmt wraps the `.route(` call so the handler sits on a later line, and when the handler's name also appears elsewhere on the line, as in `delete(handlers::user::delete)` or `.route("/login", post(handlers::user::login))`; Actix's `web::resource(…)` and `.route(…)` registrations get the same fix. A call inside a closure handler is no longer mistaken for a route, and an Actix resource no longer takes the next route's handler as its own. Thanks @mg-mg-mg for the report and @danusha2345. (#2326)
 - In Play projects, a route or a call like `MediaType.parse(...)` now links to a method of the class or object it names, and a call to an overloaded one to the overload its arguments fit. Before, it went to the first method of that name anywhere in the class's file, often a nested or sibling class's. When a Scala and a Java type share a name, as Play's own `RequestAuthority` does, each language's calls now reach their own. Thanks @danusha2345.
 - In JavaScript and TypeScript, the functions written inside a named object literal are now symbols of their own even when the object isn't exported: a plain `const api = { load() {…} }`, an object declared inside an IIFE or a function, and a namespace hung on the page or on another object, like `window.App = { init() {…} }`, `App.utils = {…}` or `dw_page = {…}`. Calls such as `api.load()`, `window.App.init()`, `App.utils.pad()`, a sibling's `this.render()`, and `App.init()` from another script on the page now reach them, the calls made inside a member belong to that member instead of the object, and a bare `init()` is no longer taken for `App.init`, so script-tag apps no longer lose most of their code from `callers` and impact. A member's qualified name now carries its object, exported or not (`api::load`, `window.App::init`), so asking for `App.init` finds it. Re-index JavaScript and TypeScript projects after upgrading. Thanks @tkhoaaa for the report and @danusha2345. (#2300)
+- In JavaScript and TypeScript, a function exported with its name in quotes, like `exports["getItems"] = async (req, res) => {…}` or `module.exports['deleteItem'] = function (req, res) {…}`, is now a symbol named after the export, as `exports.getItems = …` already was. Before, it had no symbol, so nothing could find it or link to it, and its calls counted as the file's own. Re-index JavaScript and TypeScript projects after upgrading. Thanks @johnhldiver-beep.
 - In JavaScript and TypeScript files with Windows (CRLF) line endings, as a Windows checkout usually has them, a store exported on a later line, like `const useStore = create(…)` followed by `export default useStore;`, now has its actions indexed, as it already did with Unix line endings. Before, those actions were missing from the graph on Windows, so their callers and impact came back empty. Re-index JavaScript and TypeScript projects after upgrading.
 - In VB.NET, a name that begins with a keyword, like `SharedCache`, `Dimension`, `PublicKey` or `FriendlyName`, is now indexed whole, and a method like `Sub NewItem()` is no longer taken for a constructor. Before, the keyword was cut off (`Cache`, `ension`, `Key`) or the declaration was lost, and a line in a method that starts with such a name, like `Constants.Reload()` or `ConstVBV.Value = False`, was misread as a declaration, so reads of and calls to these names didn't link. A `Private Protected` member now counts as private instead of public. Re-index VB.NET projects after upgrading.
 - In JavaScript and TypeScript, a store whose name contains a `$`, like `const items$ = create(…)` exported on a later line with `export default items$;`, now has its actions indexed. Before, they could be missing from the graph, for example in a file that also had a syntax error. Re-index JavaScript and TypeScript projects after upgrading.

__tests__/commonjs-exports.test.ts

@@ -8,15 +8,43 @@
  * 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 { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest';
+import * as fs from 'fs';
+import * as path from 'path';
 import { extractFromSource } from '../src/extraction';
 import { initGrammars, loadAllGrammars } from '../src/extraction/grammars';
+import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel';
+import type { ExtractionResult, Language } from '../src/types';
 
 beforeAll(async () => {
   await initGrammars();
   await loadAllGrammars();
 });
 
+const KERNEL_PATH = path.join(
+  __dirname,
+  '..',
+  'codegraph-kernel',
+  'prebuilds',
+  `${process.platform}-${process.arch}`,
+  'codegraph-kernel.node'
+);
+const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1';
+const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const);
+const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const;
+
+function extract(backend: 'kernel' | 'wasm', file: string, source: string, language: Language): ExtractionResult {
+  if (backend === 'wasm') {
+    process.env.CODEGRAPH_KERNEL = '0';
+    return extractFromSource(file, source, language);
+  }
+  delete process.env.CODEGRAPH_KERNEL;
+  process.env.CODEGRAPH_KERNEL_LANGS = 'all';
+  const result = tryKernelExtract(file, source, language);
+  expect(result, `kernel extraction of ${file}`).not.toBeNull();
+  return result!;
+}
+
 const refsFrom = (result: ReturnType<typeof extractFromSource>, id: string) =>
   result.unresolvedReferences.filter((r) => r.fromNodeId === id).map((r) => r.referenceName);
 
@@ -69,3 +97,71 @@ app.locals.format = function () { return 1; };
     expect(result.nodes.filter((n) => n.kind === 'function')).toEqual([]);
   });
 });
+
+describe('CommonJS export assignments with the name in quotes', () => {
+  // `exports["getItems"] = …` is the same export as `exports.getItems = …`,
+  // but only the dotted form named its function: the bracket form left the
+  // handler without a node, so its calls belonged to the file and nothing
+  // could reach it.
+  let savedEnv: Record<string, string | undefined> = {};
+  beforeEach(() => {
+    savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]]));
+    resetKernelForTests();
+  });
+  afterEach(() => {
+    for (const k of ENV_KEYS) {
+      if (savedEnv[k] === undefined) delete process.env[k];
+      else process.env[k] = savedEnv[k];
+    }
+    resetKernelForTests();
+  });
+
+  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["not-a-name"] = () => { skipped(); };
+exports[key] = () => { computed(); };
+exports[\`tpl\`] = () => { template(); };
+handlers["onSave"] = () => { persist(); };
+`;
+
+  const summary = (result: ExtractionResult) =>
+    result.nodes
+      .filter((n) => n.kind === 'function')
+      .map((n) => `${n.name}@${n.startLine} exported=${n.isExported} async=${n.isAsync}`)
+      .sort();
+
+  it.each(backends)('names the function an identifier in quotes exports: %s', (backend) => {
+    for (const [file, language] of [['src/controller.js', 'javascript'], ['src/controller.ts', 'typescript']] as const) {
+      const result = extract(backend, file, code, language);
+      expect(summary(result), file).toEqual(['deleteItem@8 exported=true async=false', 'getItems@4 exported=true async=true']);
+
+      const fn = (name: string) => result.nodes.find((n) => n.kind === 'function' && n.name === name)!;
+      expect(refsFrom(result, fn('getItems').id)).toContain('findItems');
+      expect(refsFrom(result, fn('deleteItem').id)).toContain('removeItem');
+      const fileNode = result.nodes.find((n) => n.kind === 'file')!;
+      expect(refsFrom(result, fileNode.id)).not.toContain('findItems');
+      expect(refsFrom(result, fileNode.id)).not.toContain('removeItem');
+    }
+  });
+
+  it.runIf(kernelAvailable)('reads the same in both engines', () => {
+    for (const [file, language] of [['src/controller.js', 'javascript'], ['src/controller.ts', 'typescript']] as const) {
+      const kernel = extract('kernel', file, code, language);
+      const wasm = extract('wasm', file, code, language);
+      const nodes = (r: ExtractionResult) => r.nodes.map((n) => `${n.kind} ${n.qualifiedName} ${n.startLine}-${n.endLine} ${n.isExported}`).sort();
+      const refs = (r: ExtractionResult) => r.unresolvedReferences.map((x) => `${x.referenceKind} ${x.referenceName} ${x.line}:${x.column} ${x.fromNodeId}`).sort();
+      expect(nodes(kernel), file).toEqual(nodes(wasm));
+      expect(refs(kernel), file).toEqual(refs(wasm));
+    }
+  });
+});

codegraph-kernel/src/tsjs/extractors.rs

@@ -138,27 +138,57 @@ impl<'t> Walker<'t> {
     }
 
     /// 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.
+    /// `exports.NAME = <node>` / `module.exports.NAME = <node>`, or the same
+    /// with the name in quotes, `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" {
+        if !matches!(left.kind(), "member_expression" | "subscript_expression") {
             return None;
         }
         let object = left.child_by_field_name("object")?;
+        if !matches!(self.text(object), "exports" | "module.exports") {
+            return None;
+        }
+        if left.kind() == "subscript_expression" {
+            return self.quoted_identifier(left.child_by_field_name("index")?).map(str::to_string);
+        }
         let property = left.child_by_field_name("property")?;
         if property.kind() != "property_identifier" {
             return None;
         }
-        if !matches!(self.text(object), "exports" | "module.exports") {
+        Some(self.text(property).to_string())
+    }
+
+    /// The name a string literal spells when it is a plain identifier: `'svc'`
+    /// or `"getItems"` → the text between the quotes. None for anything else,
+    /// including a string with an escape or a character no identifier can
+    /// hold. Mirrors quotedIdentifier in tree-sitter.ts.
+    fn quoted_identifier(&self, node: Node<'t>) -> Option<&'t str> {
+        if node.kind() != "string" {
             return None;
         }
-        Some(self.text(property).to_string())
+        let text = self.text(node);
+        let quoted = text.len() >= 2
+            && (text.starts_with('\'') || text.starts_with('"'))
+            && text.ends_with(&text[..1]);
+        if !quoted {
+            return None;
+        }
+        let key = &text[1..text.len() - 1];
+        let mut chars = key.chars();
+        let first = chars.next()?;
+        if !(first.is_ascii_alphabetic() || first == '_' || first == '$')
+            || !chars.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '$')
+        {
+            return None;
+        }
+        Some(key)
     }
 
     // --- reactComponentHoc / extractReactComponentNode (#841) --------------------
@@ -1775,22 +1805,7 @@ impl<'t> Walker<'t> {
         }
         let object = peel_receiver(receiver.child_by_field_name("object")?);
         if object.kind() == "this" {
-            let index = receiver.child_by_field_name("index")?;
-            if index.kind() != "string" {
-                return None;
-            }
-            let text = self.text(index);
-            let quoted = text.len() >= 2
-                && (text.starts_with('\'') || text.starts_with('"'))
-                && text.ends_with(&text[..1]);
-            let key = if quoted { &text[1..text.len() - 1] } else { return None };
-            let mut chars = key.chars();
-            let first = chars.next()?;
-            if !(first.is_ascii_alphabetic() || first == '_' || first == '$')
-                || !chars.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '$')
-            {
-                return None;
-            }
+            let key = self.quoted_identifier(receiver.child_by_field_name("index")?)?;
             return Some(format!("this.{key}"));
         }
         self.this_field_of(object).map(|field| format!("this.{field}[]"))

src/extraction/tree-sitter.ts

@@ -704,6 +704,17 @@ function tsJsCalleeName(func: SyntaxNode, source: string): { name: string; at: S
   return /^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+$/.test(chain) ? { name: chain, at: cur } : null;
 }
 
+/**
+ * The name a JS/TS string literal spells when it is a plain identifier: `'svc'`
+ * or `"getItems"` → the text between the quotes. Null for anything else,
+ * including a string with an escape or a character no identifier can hold.
+ * Mirrored in the kernel's quoted_identifier.
+ */
+function quotedIdentifier(node: SyntaxNode | null, source: string): string | null {
+  if (node?.type !== 'string') return null;
+  return /^(['"])([A-Za-z_$][\w$]*)\1$/.exec(getNodeText(node, source))?.[2] ?? null;
+}
+
 /**
  * A TS/JS receiver that subscripts the enclosing class's object: an element
  * of a field, `this.<field>[k]` → `this.<field>[]`, or a field named by a
@@ -716,8 +727,7 @@ function tsThisSubscriptReceiver(node: SyntaxNode, source: string): string | nul
   if (!objectNode) return null;
   const object = peelTsJsReceiver(objectNode);
   if (object.type === 'this') {
-    const index = getChildByField(node, 'index');
-    const key = index?.type === 'string' ? /^(['"])([A-Za-z_$][\w$]*)\1$/.exec(getNodeText(index, source))?.[2] : undefined;
+    const key = quotedIdentifier(getChildByField(node, 'index'), source);
     return key ? `this.${key}` : null;
   }
   if (object.type !== 'member_expression' || getChildByField(object, 'object')?.type !== 'this') return null;
@@ -7305,9 +7315,10 @@ export class TreeSitterExtractor {
 
   /**
    * 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.
+   * `exports.NAME = <node>` or `module.exports.NAME = <node>`, or the same
+   * with the name in quotes, `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 (
@@ -7321,12 +7332,14 @@ export class TreeSitterExtractor {
     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;
+    if (!left || (left.type !== 'member_expression' && left.type !== 'subscript_expression')) return null;
     const object = getChildByField(left, 'object');
-    const property = getChildByField(left, 'property');
-    if (!object || !property || property.type !== 'property_identifier') return null;
+    if (!object) return null;
     const objectText = getNodeText(object, this.source);
     if (objectText !== 'exports' && objectText !== 'module.exports') return null;
+    if (left.type === 'subscript_expression') return quotedIdentifier(getChildByField(left, 'index'), this.source);
+    const property = getChildByField(left, 'property');
+    if (!property || property.type !== 'property_identifier') return null;
     return getNodeText(property, this.source);
   }