fix(js,ts): a function inside an object literal's method is a local, not a member

CHANGELOG.md

@@ -191,7 +191,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 Spring projects, a route now links to the method its mapping annotation is written on when that method has no access modifier, like `@GetMapping List<Article> list()` in a package-private controller, and a method-level `@RequestMapping(value = "/feed", method = GET)` on such a method now makes a route. Before, such a handler was skipped, so its route linked the next method declared `public`, `private` or `protected`, or nothing, and the handler looked unused. An overloaded handler under a tall stack of annotations, like `@Operation` and `@ApiResponses` above `@GetMapping`, now links the overload it annotates rather than its neighbour. Re-index Spring projects after upgrading. Thanks @Liurchen.
-- 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, 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) A function declared inside a method of such an object, nested in it or behind `as const` or `satisfies`, is a local of that method, and calls from beside it link as before.
 - 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.

__tests__/js-object-literal-members.test.ts

@@ -317,4 +317,85 @@ describe.each(['native', 'wasm'].filter((backend) => backend === 'wasm' || kerne
     expect(callees(g, 'handlers::onSave')).toEqual(['persist']);
     expect(callees(g, 'save')).toEqual(['handlers::onSave']);
   });
+
+  // The extractor gives a method of these literals no node, so what the method
+  // declares is qualified under the holder as a member would be (`rule::visit`),
+  // and a bare call of it, between neighbours in that body, was taken for a call
+  // of a member by its name alone.
+  it.each([
+    ['satisfies', 'const rule = {\n  create(context) {\n    function report() {}\n    function visit() { report(); }\n    return { Program: visit };\n  },\n} satisfies Rule;\n'],
+    ['as const', 'const rule = {\n  create(context) {\n    function report() {}\n    function visit() { report(); }\n    return { Program: visit };\n  },\n} as const;\n'],
+    ['a nested object', 'const rule = {\n  rules: {\n    create(context) {\n      function report() {}\n      function visit() { report(); }\n      return { Program: visit };\n    },\n  },\n};\n'],
+    ['a computed key', 'const key = "create";\nconst rule = {\n  plain() {},\n  [key](context) {\n    function report() {}\n    function visit() { report(); }\n    return { Program: visit };\n  },\n};\n'],
+    ['a callback a member passes on', 'declare function wrap(f: unknown): unknown;\nconst rule = {\n  create: wrap(function (context) {\n    function report() {}\n    function visit() { report(); }\n    return { Program: visit };\n  }),\n};\n'],
+    ['a property read off the literal', 'const key = "create";\nconst rule = {\n  create() {\n    function report() {}\n    function visit() { report(); }\n    return { Program: visit };\n  },\n}[key];\n'],
+  ])('reaches a function declared beside the caller in a method of %s', async (_shape, body) => {
+    const g = await index({ 'rule.ts': `${body}export default rule;\n` });
+    expect(callees(g, 'rule::visit')).toEqual(['rule::report']);
+    expect(g.getOutgoingEdges(node(g, 'rule').id).filter((e) => e.kind === 'references')
+      .map((e) => g.getNode(e.target)!.qualifiedName)).toContain('rule::visit');
+  });
+
+  it('reaches a function declared in a method of a literal in a component', async () => {
+    const g = await index({
+      'Panel.vue': [
+        '<template><p>Don\'t {{ msg }}</p></template>',
+        '<script setup lang="ts">',
+        'const rule = {',
+        '  create() {',
+        '    function report() {}',
+        '    function visit() { report(); }',
+        '    return visit;',
+        '  },',
+        '} satisfies Rule;',
+        '</script>',
+        '',
+      ].join('\n'),
+    });
+    expect(callees(g, 'rule::visit')).toEqual(['rule::report']);
+  });
+
+  it('keeps such a function to the block that declares it', async () => {
+    const g = await index({
+      'rule.ts': [
+        'const rule = {',
+        '  create() {',
+        '    function report() {}',
+        '    return report();',
+        '  },',
+        '  check() { return report(); },',
+        '} satisfies Rule;',
+        'export function outside() { return report(); }',
+        'export default rule;',
+        '',
+      ].join('\n'),
+      'other.ts': 'export function elsewhere() { return report(); }\n',
+    });
+    const target = node(g, 'rule::report');
+    const callers = g.getIncomingEdges(target.id).filter((e) => e.kind === 'calls');
+    expect(callers.map((e) => e.line)).toEqual([4]);
+    expect(callees(g, 'outside')).toEqual([]);
+    expect(callees(g, 'elsewhere')).toEqual([]);
+  });
+
+  it('still reaches a member only through its object, a literal nested in a function included', async () => {
+    const g = await index({
+      'app.ts': [
+        'const App = {',
+        '  init() { return 1; },',
+        '  run() {',
+        '    function inner() { return 2; }',
+        '    return inner();',
+        '  },',
+        '};',
+        'export function bare() { return init(); }',
+        'export function through() { return App.init(); }',
+        'export default App;',
+        '',
+      ].join('\n'),
+    });
+    expect(callees(g, 'bare')).toEqual([]);
+    expect(callees(g, 'through')).toEqual(['App::init']);
+    expect(callees(g, 'App::run')).toEqual(['App::run::inner']);
+  });
 });

src/resolution/name-matcher.ts

@@ -1894,6 +1894,7 @@ function lexicalScopeOf(candidate: Node, context: ResolutionContext): { start: n
     scope = { start: container.startLine, end: container.endLine };
     break;
   }
+  scope ??= literalPlaceOf(candidate, context)?.block ?? null;
   memo.set(candidate.id, scope);
   return scope;
 }
@@ -14305,20 +14306,27 @@ export function objectLiteralMemberBinding(
   return objectLiteralProperty(container, member, context)?.binding ?? null;
 }
 
+/** A holder's own text from its first column to its last: comments blanked, and in `code` string contents too. */
+function holderExtent(container: Node, context: ResolutionContext): { lines: string[]; extent: string; code: string } | null {
+  const lines = context.getFileLines?.(container.filePath) ?? context.readFile(container.filePath)?.split('\n');
+  if (!lines) return null;
+  const extentLines = lines.slice(container.startLine - 1, container.endLine);
+  if (!extentLines.length) return null;
+  extentLines[extentLines.length - 1] = extentLines[extentLines.length - 1]!.slice(0, container.endColumn);
+  extentLines[0] = extentLines[0]!.slice(container.startColumn);
+  const extent = stripCommentsForRegex(extentLines.join('\n'), 'typescript');
+  return { lines, extent, code: blankStringContents(extent) };
+}
+
 /** The last own property wins; an unknown spread/computed key invalidates earlier evidence. */
 function objectLiteralProperty(
   container: Node,
   member: string,
   context: ResolutionContext,
 ): { binding: string | null; contains: (node: Node) => boolean } | null | undefined {
-  const lines = context.getFileLines?.(container.filePath) ?? context.readFile(container.filePath)?.split('\n');
-  if (!lines) return undefined;
-  const extentLines = lines.slice(container.startLine - 1, container.endLine);
-  if (!extentLines.length) return undefined;
-  extentLines[extentLines.length - 1] = extentLines[extentLines.length - 1]!.slice(0, container.endColumn);
-  extentLines[0] = extentLines[0]!.slice(container.startColumn);
-  const extent = stripCommentsForRegex(extentLines.join('\n'), 'typescript');
-  const code = blankStringContents(extent);
+  const read = holderExtent(container, context);
+  if (!read) return undefined;
+  const { lines, extent, code } = read;
   // Start at THIS declarator, including its columns, never a sibling on the same line.
   const open = /^[^=]*=\s*(?:(?:Object\.(?:freeze|seal)\s*)?\(\s*)*\{/.exec(code);
   if (!open) return undefined;
@@ -14471,27 +14479,75 @@ function isObjectLiteralOwner(n: Node): boolean {
   return (n.kind === 'constant' || n.kind === 'variable') && JS_FAMILY.has(n.language) && /^=\s*\{/.test(n.signature ?? '');
 }
 
-const OBJECT_OWNER_OF = new WeakMap<ResolutionContext, Map<string, Node | null>>();
+/** A function qualified under an object literal's holder: the holder, and the block it is written in when that is no member's place. */
+interface LiteralPlace {
+  holder: Node;
+  block: { start: number; end: number } | null;
+}
+
+const LITERAL_PLACES = new WeakMap<ResolutionContext, Map<string, LiteralPlace | null>>();
 
 /**
- * The object literal a function node is a member of — its qualified parent,
- * an owner in its file whose extent holds it — or null for every other
- * function: a declaration, a helper nested in a function, a function inside
- * an initializer that is not an object literal (a module IIFE's own helpers).
+ * Where a function qualified under an object literal's holder is written, or
+ * null for every other function: a declaration, a helper nested in a function
+ * with a node of its own, a function inside an initializer that is not an
+ * object literal (a module IIFE's own helpers).
  */
-function objectLiteralOwnerOf(n: Node, context: ResolutionContext): Node | null {
+function literalPlaceOf(n: Node, context: ResolutionContext): LiteralPlace | null {
   if (n.kind !== 'function' || !JS_FAMILY.has(n.language)) return null;
   const cut = n.qualifiedName.lastIndexOf('::');
   if (cut <= 0) return null;
-  let memo = OBJECT_OWNER_OF.get(context);
-  if (!memo) OBJECT_OWNER_OF.set(context, (memo = new Map()));
+  let memo = LITERAL_PLACES.get(context);
+  if (!memo) LITERAL_PLACES.set(context, (memo = new Map()));
   const hit = memo.get(n.id);
   if (hit !== undefined) return hit;
   const parent = n.qualifiedName.slice(0, cut);
-  const owner = context.getNodesByQualifiedName(parent).find((o) =>
+  const holder = context.getNodesByQualifiedName(parent).find((o) =>
     o.filePath === n.filePath && o.id !== n.id && isObjectLiteralOwner(o) && rangeWithin(n, o)) ?? null;
-  memo.set(n.id, owner);
-  return owner;
+  const place = holder && { holder, block: blockBelowLiteral(n, holder, context) };
+  memo.set(n.id, place);
+  return place;
+}
+
+/**
+ * The lines of the block `n` is written in, when that lies below the literal
+ * `holder` holds, and null for a member, which sits in the literal's own
+ * braces. Only a literal's direct members have nodes under it in their own
+ * right. A function deeper down — in a method of a nested object, under a
+ * computed key, in a callback a member's value passes on — is qualified under
+ * the holder too, because nothing between them has a node, and so is one in a
+ * literal behind `satisfies` or `as const`, whose methods have none. Each is
+ * an ordinary local of the block it is written in, named bare there. The
+ * holder's own text is read, not the file's: a component's template and styles
+ * are not code.
+ */
+function blockBelowLiteral(n: Node, holder: Node, context: ResolutionContext): { start: number; end: number } | null {
+  const scan = jsFileScan(holder.filePath, context);
+  const blocks = (scan.holderBlocks ??= new Map());
+  let index = blocks.get(holder.id);
+  if (index === undefined) {
+    const code = holderExtent(holder, context)?.code;
+    index = code === undefined ? null : readJsBlocks(code);
+    blocks.set(holder.id, index);
+  }
+  if (!index) return null;
+  const firstLine = n.startLine === holder.startLine;
+  const at = jsOffset(index, n.startLine - holder.startLine + 1, n.startColumn - (firstLine ? holder.startColumn : 0));
+  const block = jsBlockAt(index, at);
+  if (!block || jsBlockAt(index, block[0]) === null) return null;
+  const { lineStarts } = index;
+  const lineOf = (offset: number): number => {
+    let line = lineStarts.length - 1;
+    while (line > 0 && lineStarts[line]! > offset) line--;
+    return holder.startLine + line;
+  };
+  return { start: lineOf(block[0]), end: lineOf(block[1]) };
+}
+
+/** The object literal a function node is a member of, or null: see literalPlaceOf and blockBelowLiteral. */
+function objectLiteralOwnerOf(n: Node, context: ResolutionContext): Node | null {
+  const place = literalPlaceOf(n, context);
+  return place && !place.block ? place.holder : null;
 }
 
 /** The node a qualified name's parent names, in `n`'s file and holding it. */
@@ -14570,6 +14626,8 @@ interface JsFileScan {
   blocks?: JsBlockIndex | null;
   bindings?: Map<string, Array<[number, number]>>;
   destructured?: Map<string, Set<string>>;
+  /** Per object-literal holder, the `{…}` blocks of its own text (see blockBelowLiteral). */
+  holderBlocks?: Map<string, JsBlockIndex | null>;
 }
 
 const JS_FILE_SCANS = new WeakMap<ResolutionContext, Map<string, JsFileScan>>();
@@ -21293,7 +21351,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
   SCALA_IMPORTS.delete(context);
   ESM_EXPORT_LISTS.delete(context);
   JS_FILE_SCANS.delete(context);
-  OBJECT_OWNER_OF.delete(context);
+  LITERAL_PLACES.delete(context);
   THIS_CALLERS.delete(context);
   GLOBAL_HOLDERS.delete(context);
   CLASSIC_SCRIPTS.delete(context);