From 81afb043f781676a15f0ce511c7784180e2224f7 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Sun, 25 Oct 2026 05:25:54 -0700 Subject: [PATCH] fix(javascript): evaluate each short-circuit left operand once (#7126) Logical branch selection evaluated the left operand to decide which branches stay reachable, and value evaluation then evaluated the selected left branch again. In a left-nested `a || b || c ...` chain every level repeated that, so semantic analysis doubled per operand and a 24-operand chain exhausted the per-source deadline. The selection now carries the left operand's value with the branch, and value evaluation reuses it. Provenance still walks the branch nodes. Results are unchanged. Fixes #7115 Co-authored-by: Claude Opus 5.5 --- ...scriptSemanticAnalysis.scope_depth.test.ts | 102 +++++++++++++++++- .../javascript/javascriptSemanticValues.ts | 36 +++++-- .../cli/javascriptLogicalChainScaling.test.ts | 52 +++++++++ 3 files changed, 178 insertions(+), 12 deletions(-) create mode 100644 tests/boundary/cli/javascriptLogicalChainScaling.test.ts diff --git a/src/domain/javascript/javascriptSemanticAnalysis.scope_depth.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.scope_depth.test.ts index 89eaf31cf5..cf751f11ad 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.scope_depth.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.scope_depth.test.ts @@ -1,8 +1,14 @@ +import * as t from "@babel/types"; import { expect, it, vi } from "vitest"; -import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js"; +import { + analyzeJavaScriptSemantics, + analyzeParsedJavaScriptSemantics, +} from "./javascriptSemanticAnalysis.js"; import { onlyBinding } from "./javascriptSemanticAnalysis.fixture.js"; import type { JavaScriptSemanticIr } from "./javascriptSemanticIr.js"; +import { traverseJavaScriptAst } from "./javascriptSemanticTraversal.js"; +import { parseJavaScriptSource } from "./javascriptSourceParser.js"; /** Count AST ancestry lookups made while analyzing one source. */ const ancestryReads = ( @@ -145,3 +151,97 @@ it.each([ expect(larger).toBeLessThan(8 * smaller); }, ); + +/** Count reads of short-circuit left operands while analyzing one source. */ +const logicalOperandReads = ( + source: string, + inspect: (ir: JavaScriptSemanticIr) => void, +): number => { + const file = parseJavaScriptSource(source); + if (file === null) throw new Error("fixture must parse"); + let reads = 0; + traverseJavaScriptAst(file, { + enter: (node) => { + if (!t.isLogicalExpression(node)) return; + const left = node.left; + Object.defineProperty(node, "left", { + enumerable: true, + configurable: true, + get: () => { + reads++; + return left; + }, + }); + }, + }); + reads = 0; + inspect(analyzeParsedJavaScriptSemantics(file)); + return reads; +}; + +/** A left-nested chain `o0 op1 o1 op2 o2 ...` as written in tokenizers. */ +const logicalChain = ( + length: number, + operand: (index: number) => string, + operator: (index: number) => string, +): string => + Array.from({ length: length - 1 }, (unused, index) => index + 1).reduce( + (chain, index) => `(${chain}) ${operator(index)} ${operand(index)}`, + operand(0), + ); + +const call = (index: number): string => `f${String(index)}(s)`; + +it.each([ + ["||", call, () => "||"], + ["&&", (index: number) => `s.k${String(index)}`, () => "&&"], + ["??", call, () => "??"], + [ + "mixed ||/&&/?? and conditional", + (index: number) => + index % 4 === 0 + ? `(s.k ? ${call(index)} : s.k${String(index)})` + : call(index), + (index: number) => ["||", "&&", "??"][index % 3] ?? "||", + ], +])( + "reads each operand of a long %s chain a bounded number of times", + (_, operand, operator) => { + // Logical branch selection evaluates the left operand. Evaluating it again + // as a value branch at every level doubles the work per operand. + const reads = (length: number): number => + logicalOperandReads( + `function next(s){ var tok=${logicalChain(length, operand, operator)}; return tok; } module.exports=next;`, + (ir) => { + expect(ir.coverage).toEqual({ status: "complete", omittedCount: 0 }); + expect(onlyBinding(ir, "tok").value.status).not.toBe("literal"); + }, + ); + + const smaller = reads(8); + const larger = reads(16); + expect(smaller).toBeGreaterThan(0); + // Doubling per operand would multiply the reads by 256. + expect(larger).toBeLessThan(4 * smaller); + }, +); + +it("keeps established logical operands across a long chain", () => { + // Known-falsy operands select the right branch at every level; the value and + // provenance must still come from the one reachable operand. + const ir = analyzeJavaScriptSemantics( + `const tok = ${'false || null || 0 || "" || '.repeat(10)}require("electron").app;`, + ); + expect(ir.coverage).toEqual({ status: "complete", omittedCount: 0 }); + expect(onlyBinding(ir, "tok").provenance).toEqual({ + status: "module", + origins: [ + { + specifier: "electron", + importedPath: ["app"], + resolutionMode: "require", + }, + ], + reason: null, + }); +}); diff --git a/src/domain/javascript/javascriptSemanticValues.ts b/src/domain/javascript/javascriptSemanticValues.ts index 73dcbca690..e2fe0f63b0 100644 --- a/src/domain/javascript/javascriptSemanticValues.ts +++ b/src/domain/javascript/javascriptSemanticValues.ts @@ -721,8 +721,10 @@ const evaluateExpressionValue = ( if (t.isTemplateLiteral(node)) return evaluateTemplate(node, context); if (t.isConditionalExpression(node) || t.isLogicalExpression(node)) return mergeValues( - semanticSelectedBranches(node, context).map((branch) => - evaluateExpression(branch, nestedContext(context)), + semanticSelectedBranches(node, context).map( + (branch) => + branch.value ?? + evaluateExpression(branch.node, nestedContext(context)), ), ); if (t.isObjectExpression(node)) return evaluateObject(node, context); @@ -774,27 +776,39 @@ const evaluateTemplate = ( return primitiveSet(candidates); }; +/** + * A branch that may produce the expression's value. A logical left operand + * carries the value its selection already evaluated, so value consumers reuse + * it instead of re-evaluating that operand; re-evaluating it at every level of + * a left-nested `a || b || c` chain doubles the work per operand. + */ +interface SemanticSelectedBranch { + readonly node: t.Node; + readonly value?: JavaScriptSemanticValue; +} + /** Value and provenance selection consume the same established branch facts. */ const semanticSelectedBranches = ( node: t.ConditionalExpression | t.LogicalExpression, context: EvaluationContext, -): readonly t.Node[] => { +): readonly SemanticSelectedBranch[] => { if (t.isConditionalExpression(node)) { const test = evaluateExpression(node.test, nestedContext(context)); const choice = semanticCondition(test, Boolean); return choice === undefined - ? [node.consequent, node.alternate] - : [choice ? node.consequent : node.alternate]; + ? [{ node: node.consequent }, { node: node.alternate }] + : [{ node: choice ? node.consequent : node.alternate }]; } - const left = evaluateExpression(node.left, nestedContext(context)); + const value = evaluateExpression(node.left, nestedContext(context)); + const left = { node: node.left, value }; const choice = logicalRightOperand( node.operator, - semanticCondition(left, Boolean), - semanticCondition(left, (value) => value === null, false), + semanticCondition(value, Boolean), + semanticCondition(value, (candidate) => candidate === null, false), ); return choice === undefined - ? [node.left, node.right] - : [choice ? node.right : node.left]; + ? [left, { node: node.right }] + : [choice ? { node: node.right } : left]; }; /** All retained alternatives must agree before discarding a branch. */ @@ -1320,7 +1334,7 @@ const provenanceForExpression = ( } if (t.isConditionalExpression(node) || t.isLogicalExpression(node)) { const candidates = semanticSelectedBranches(node, context).map((branch) => - provenanceForExpression(branch, nestedContext(context)), + provenanceForExpression(branch.node, nestedContext(context)), ); const origins = uniqueSemanticOrigins( candidates.flatMap((candidate) => candidate.origins), diff --git a/tests/boundary/cli/javascriptLogicalChainScaling.test.ts b/tests/boundary/cli/javascriptLogicalChainScaling.test.ts new file mode 100644 index 0000000000..23ad49c42a --- /dev/null +++ b/tests/boundary/cli/javascriptLogicalChainScaling.test.ts @@ -0,0 +1,52 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; +import { z } from "zod"; + +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +// Hand-written tokenizers dispatch through long short-circuit chains; the +// CodeMirror 5 pug mode's `nextToken` has 38 operands. Work that doubled per +// operand could not finish such a chain within any per-source deadline. +const operands = 38; + +cliTest( + "analyzes a long short-circuit dispatch chain within the source deadline", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-js-logical-chain-"); + const chain = Array.from( + { length: operands }, + (unused, index) => `f${String(index)}(s)`, + ).join(" || "); + await writeFile( + join(root, "index.js"), + `function next(s) {\n var tok = ${chain};\n return tok;\n}\nmodule.exports = next;\n`, + ); + await writeFile( + join(root, "package.json"), + '{"name":"chain","version":"1.0.0","main":"index.js"}\n', + ); + const output = await cli.run({ + arguments: [ + "analyze-javascript-application", + root, + "--analysis-timeout-ms", + "15000", + "--json", + ], + }); + expect(output.exitCode).toBe(0); + const result = z + .object({ normalized_result: javascriptApplicationAnalysisResultSchema }) + .parse(output.json).normalized_result; + expect(result.statistics).toMatchObject({ + parsed_javascript_files: 1, + parse_failures: 0, + }); + expect( + result.semantic_graph.nodes.filter(({ kind }) => kind === "call-site"), + ).toHaveLength(operands); + }, +);