From 42a4c23a3982fe06fb58cecc285dbb59f189d088 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Sat, 31 Oct 2026 21:48:03 -0500 Subject: [PATCH] fix(js,ts): a function inside an object literal's method is a local, not a member A bare call between two functions declared in a method of an object behind `satisfies` or `as const` linked to nothing: 34 calls in react's ESLint rules, where `const rule = {...} satisfies Rule.RuleModule` holds everything in `create`, like `reportProblem(...)` from `visitCallExpression`. 1.6.2 linked them. Regression from f40db4b9 (PR #2363), which never shipped. Traced: the extractor makes a literal's direct function members nodes only when the initializer is the literal itself, so behind `satisfies` the method `create` has no node, and what it declares is qualified under the constant: `rule::reportProblem`. objectLiteralOwnerOf took any function qualified under a holder whose initializer opens with `{` for a member of the literal, and a member is never reached by a bare name. So the shape is wider than the report says: a method of a nested object, a computed key, a function a member's value passes on, and a literal read through `[key]` all lost the same calls, in Vue and Svelte script blocks too. Fix (resolver only): a holder's own text is read once for its `{...}` blocks. A function directly in the literal's braces is a member. One deeper down is a local of the block it is written in: reachable by name from inside that block and from nowhere else, which is tighter than 1.6.2, where such a function answered a bare name from anywhere. No extraction change, so no kernel change. Validation: object-literal tests red on main (16), green here, on both engines. A/B against 4499d007: react +35 (34 calls and 1 reference, all in the two ESLint rules), outline +5 (its oxlint rules); excalidraw, vscode-be, taxonomy, express, vben, flask, gin identical. All 40 read against source. Index CPU: level (see the PR). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- __tests__/js-object-literal-members.test.ts | 81 +++++++++++++++++ src/resolution/name-matcher.ts | 98 ++++++++++++++++----- 3 files changed, 160 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb651f5562..108e0ade1b 100644 --- a/CHANGELOG.md +++ b/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
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. diff --git a/__tests__/js-object-literal-members.test.ts b/__tests__/js-object-literal-members.test.ts index f24270e39f..3a115e7b40 100644 --- a/__tests__/js-object-literal-members.test.ts +++ b/__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': [ + '', + '', + '', + ].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']); + }); }); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 35349d5ddf..35b51e4417 100644 --- a/src/resolution/name-matcher.ts +++ b/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>(); +/** 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>(); /** - * 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>; destructured?: Map>; + /** Per object-literal holder, the `{…}` blocks of its own text (see blockBelowLiteral). */ + holderBlocks?: Map; } const JS_FILE_SCANS = new WeakMap>(); @@ -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);