From 410537667e7c0cdbfce438c875f76b32c32a7b8e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:46:36 +0800 Subject: [PATCH] fix(javascript): retain uncertainty for conditionally skipped writes Own skipped-write interpretation at definition collection, including conditional branches, loop iterations and optional-chain arguments. Remove the narrower initializer-only representation. Fixes #2562 --- .../javascript/javascriptModuleInterop.ts | 2 +- ...SemanticAnalysis.conditional_write.test.ts | 73 ++++++++++++ .../javascript/javascriptSemanticAnalysis.ts | 112 ++++++++++++++---- .../javascript/javascriptSemanticReturns.ts | 2 +- .../javascript/javascriptSemanticState.ts | 5 +- .../javascript/javascriptSemanticValues.ts | 9 +- .../javascript/javascriptSymbolicBindings.ts | 2 +- 7 files changed, 172 insertions(+), 33 deletions(-) create mode 100644 src/domain/javascript/javascriptSemanticAnalysis.conditional_write.test.ts diff --git a/src/domain/javascript/javascriptModuleInterop.ts b/src/domain/javascript/javascriptModuleInterop.ts index 6f8eb9980c..5df0252f81 100644 --- a/src/domain/javascript/javascriptModuleInterop.ts +++ b/src/domain/javascript/javascriptModuleInterop.ts @@ -554,7 +554,7 @@ export const createSemanticModuleInterop = ( converted === undefined || initial.projection.length !== 0 || converted.projection.length !== 0 || - state.conditionalInitializers.has(initial.node) + state.conditionallySkippedExpressions.has(initial.node) ) return undefined; const call = converted.node; diff --git a/src/domain/javascript/javascriptSemanticAnalysis.conditional_write.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.conditional_write.test.ts new file mode 100644 index 0000000000..c7aa8a6c91 --- /dev/null +++ b/src/domain/javascript/javascriptSemanticAnalysis.conditional_write.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js"; + +const bindingValue = (source: string, name: string) => + analyzeJavaScriptSemantics(source).bindings.find( + (binding) => binding.name === name, + )?.value; + +describe("JavaScript semantic analysis: conditionally skipped writes", () => { + it("does not promote a skipped assignment through a later read", () => { + const ir = analyzeJavaScriptSemantics( + "let value; if (flag) value = 1; const observed = value;", + ); + + expect(ir.bindings.find(({ name }) => name === "value")?.value.status).toBe( + "unknown", + ); + expect( + ir.bindings.find(({ name }) => name === "observed")?.value.status, + ).toBe("unknown"); + }); + + it("keeps a conditional initializer uncertain while preserving branch joins", () => { + expect(bindingValue("if (flag) { var value = 1; }", "value")?.status).toBe( + "unknown", + ); + expect( + bindingValue("let value; if (flag) value = 1; else value = 2;", "value"), + ).toMatchObject({ status: "ambiguous" }); + }); + + it("preserves writes that execute on every path and definite writes", () => { + expect(bindingValue("let value; value = 1;", "value")).toEqual({ + status: "literal", + value: 1, + }); + expect( + bindingValue("let value; do { value = 1; } while (flag);", "value"), + ).toEqual({ status: "literal", value: 1 }); + expect(bindingValue("let value; if (value = 1) {}", "value")).toEqual({ + status: "literal", + value: 1, + }); + }); + + it("distinguishes for-loop initializers from potentially skipped updates", () => { + expect( + bindingValue("let value; for (value = 1; false;) {}", "value"), + ).toEqual({ status: "literal", value: 1 }); + expect( + bindingValue("let value; for (; false; value = 1) {}", "value")?.status, + ).toBe("unknown"); + }); + + it("tracks arguments skipped by optional chains in a call callee", () => { + for (const call of [ + "foo?.bar(value = 1)", + "foo?.[key](value = 1)", + "foo?.bar?.(value = 1)", + "getThing?.()(value = 1)", + ]) + expect(bindingValue(`let value; ${call};`, "value")?.status).toBe( + "unknown", + ); + }); + + it("keeps arguments after a grouped optional expression definite", () => { + expect(bindingValue("let value; (foo?.bar)(value = 1);", "value")).toEqual({ + status: "literal", + value: 1, + }); + }); +}); diff --git a/src/domain/javascript/javascriptSemanticAnalysis.ts b/src/domain/javascript/javascriptSemanticAnalysis.ts index e8bbffb487..7698b04d16 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.ts @@ -685,7 +685,7 @@ const createState = ( moduleLinks: [], linkedRequireCalls: new WeakSet(), moduleLinkBindings: new WeakMap(), - conditionalInitializers: new WeakSet(), + conditionallySkippedExpressions: new WeakSet(), requiredModuleEffects: new Map(), ambientBindingWrites: new Map(), ambientWrites: new SemanticPropertyPathCoverage(), @@ -704,6 +704,7 @@ function* collectDefinitionsSteps( const assignments: { readonly node: t.Node; readonly scope: JavaScriptSemanticScopeState; + readonly conditionallySkipped: boolean; }[] = []; const functionBodies: { readonly body: t.BlockStatement; @@ -779,16 +780,23 @@ function* collectDefinitionsSteps( node.init != null && parent !== null && t.isVariableDeclaration(parent, { kind: "var" }) && - hasConditionalInitializer(readAncestors()) + writeMayBeSkipped(node, readAncestors()) ) - state.conditionalInitializers.add(node.init); + state.conditionallySkippedExpressions.add(node.init); if ( t.isAssignmentExpression(node) || t.isUpdateExpression(node) || t.isForOfStatement(node) || t.isForInStatement(node) ) - assignments.push({ node, scope }); + assignments.push({ + node, + scope, + conditionallySkipped: + t.isForOfStatement(node) || + t.isForInStatement(node) || + writeMayBeSkipped(node, readAncestors()), + }); }, exit: (node) => { for (let count = openedScopes.get(node) ?? 0; count > 0; count--) @@ -797,9 +805,12 @@ function* collectDefinitionsSteps( }); // All declarations must exist before resolving writes, including hoisted // functions and local declarations that shadow an outer binding. - for (const [index, { node, scope }] of assignments.entries()) { + for (const [ + index, + { node, scope, conditionallySkipped }, + ] of assignments.entries()) { if (index % 64 === 0) yield; - bindAssignment(node, scope, state); + bindAssignment(node, scope, state, conditionallySkipped); } for (const [index, { body, scope, parameters }] of functionBodies.entries()) { if (index % 64 === 0) yield; @@ -853,29 +864,72 @@ const copyParameterToBody = (input: { } }; -const hasConditionalInitializer = (ancestors: readonly t.Node[]): boolean => { - // A hoisted var can remain undefined when control flow skips its initializer. - // Conditions outside the owning callable/static block do not govern its body. +const writeMayBeSkipped = ( + node: t.Node, + ancestors: readonly t.Node[], +): boolean => { + let child = node; for (let index = ancestors.length - 1; index >= 0; index -= 1) { - const node = ancestors[index]; - if (t.isFunction(node) || t.isProgram(node) || t.isStaticBlock(node)) + const parent = ancestors[index]; + if ( + parent === undefined || + t.isFunction(parent) || + t.isProgram(parent) || + t.isStaticBlock(parent) + ) return false; - if (t.isIfStatement(node) || t.isSwitchCase(node) || t.isTryStatement(node)) - return true; - if (t.isLoop(node)) { - const bodyOrTest = ancestors[index + 1]; - if ( - (t.isForStatement(node) && bodyOrTest === node.init) || - (t.isDoWhileStatement(node) && - (bodyOrTest === node.body || bodyOrTest === node.test)) - ) - continue; - return true; - } + if (isSkippedBranch(parent, child)) return true; + child = parent; } return false; }; +const isSkippedBranch = (parent: t.Node, child: t.Node): boolean => { + if (t.isIfStatement(parent)) + return child === parent.consequent || child === parent.alternate; + if (t.isSwitchCase(parent)) return true; + if (t.isTryStatement(parent)) + return child === parent.block || child === parent.handler; + if (t.isForStatement(parent)) + return child === parent.body || child === parent.update; + if (t.isWhileStatement(parent)) return child === parent.body; + if (t.isDoWhileStatement(parent)) return false; + if (t.isForInStatement(parent) || t.isForOfStatement(parent)) + return child === parent.left || child === parent.body; + if (t.isLogicalExpression(parent)) return child === parent.right; + if (t.isConditionalExpression(parent)) + return child === parent.consequent || child === parent.alternate; + if (t.isOptionalCallExpression(parent) || t.isCallExpression(parent)) + return ( + parent.arguments.includes(child as t.Expression) && + optionalCallArgumentsMayBeSkipped(parent) + ); + if (t.isOptionalMemberExpression(parent)) + return parent.optional && parent.computed && child === parent.property; + return false; +}; + +const optionalCallArgumentsMayBeSkipped = ( + call: t.CallExpression | t.OptionalCallExpression, +): boolean => { + if (t.isOptionalCallExpression(call) && call.optional) return true; + let callee: t.Node = call.callee; + while (true) { + // Parentheses terminate optional-chain short-circuiting. In particular, + // `(foo?.bar)(arg)` evaluates arg before attempting the call. + if (callee.extra?.parenthesized === true) return false; + if (t.isOptionalMemberExpression(callee)) { + if (callee.optional) return true; + callee = callee.object; + } else if (t.isMemberExpression(callee)) callee = callee.object; + else if (t.isOptionalCallExpression(callee)) { + if (callee.optional) return true; + callee = callee.callee; + } else if (t.isCallExpression(callee)) callee = callee.callee; + else return false; + } +}; + const bindOuterDeclaration = ( node: t.Node, scope: JavaScriptSemanticScopeState, @@ -983,6 +1037,7 @@ const bindAssignment = ( node: t.Node, scope: JavaScriptSemanticScopeState, state: JavaScriptSemanticAnalysisState, + conditionallySkipped: boolean, ): void => { if (t.isAssignmentExpression(node)) { const left = unwrapJavaScriptExpression(node.left).node; @@ -996,21 +1051,23 @@ const bindAssignment = ( : node, scope, state, + conditionallySkipped, ); else for (const identifier of semanticWriteTargets(node.left)) if (t.isIdentifier(identifier)) - addAssignment(identifier, node, scope, state); + addAssignment(identifier, node, scope, state, conditionallySkipped); } else if (t.isUpdateExpression(node)) { const argument = unwrapJavaScriptExpression(node.argument).node; - if (t.isIdentifier(argument)) addAssignment(argument, node, scope, state); + if (t.isIdentifier(argument)) + addAssignment(argument, node, scope, state, conditionallySkipped); } else if (t.isForOfStatement(node) || t.isForInStatement(node)) for (const pattern of t.isVariableDeclaration(node.left) ? node.left.declarations.map(({ id }) => id) : [node.left]) { for (const identifier of semanticWriteTargets(pattern)) if (t.isIdentifier(identifier)) - addAssignment(identifier, node, scope, state); + addAssignment(identifier, node, scope, state, conditionallySkipped); if (t.isForOfStatement(node)) // Keep the loop value unknown, but preserve references yielded into // each bound slot so writes/escapes can reach their iterable origins. @@ -1400,6 +1457,7 @@ const addAssignment = ( initializer: t.Node, scope: JavaScriptSemanticScopeState, state: JavaScriptSemanticAnalysisState, + conditionallySkipped: boolean, ): void => { const binding = resolveSemanticBindingFromScope( scope, @@ -1417,4 +1475,6 @@ const addAssignment = ( binding.mutable = true; binding.definitions.push({ kind: "assignment", location: range(identifier) }); binding.initializers.push({ node: initializer, projection: [] }); + if (conditionallySkipped) + state.conditionallySkippedExpressions.add(initializer); }; diff --git a/src/domain/javascript/javascriptSemanticReturns.ts b/src/domain/javascript/javascriptSemanticReturns.ts index 27bb030d01..11aaddc09d 100644 --- a/src/domain/javascript/javascriptSemanticReturns.ts +++ b/src/domain/javascript/javascriptSemanticReturns.ts @@ -155,7 +155,7 @@ const callableIdsForBinding = ( if ( initializer === undefined || initializer.projection.length > 0 || - state.conditionalInitializers.has(initializer.node) + state.conditionallySkippedExpressions.has(initializer.node) ) return []; return callableIdsForNode( diff --git a/src/domain/javascript/javascriptSemanticState.ts b/src/domain/javascript/javascriptSemanticState.ts index 7dac75c975..86183d089a 100644 --- a/src/domain/javascript/javascriptSemanticState.ts +++ b/src/domain/javascript/javascriptSemanticState.ts @@ -92,7 +92,8 @@ export interface JavaScriptSemanticAnalysisState { JavaScriptSemanticModuleLink, JavaScriptSemanticBindingState >; - readonly conditionalInitializers: WeakSet; + /** Initializer expressions whose write can be skipped by enclosing control flow. */ + readonly conditionallySkippedExpressions: WeakSet; /** Unbound identifier writes retain their source sites for capture ordering. */ readonly ambientBindingWrites: Map; /** Source-local writes, tagged by global object or import.meta identity. */ @@ -120,7 +121,7 @@ export const semanticStableInitializer = ( (definition?.kind === "function" || definition?.kind === "class"))) && binding.initializers.length === 1 && initializer !== undefined && - !state.conditionalInitializers.has(initializer.node) + !state.conditionallySkippedExpressions.has(initializer.node) ? initializer : undefined; }; diff --git a/src/domain/javascript/javascriptSemanticValues.ts b/src/domain/javascript/javascriptSemanticValues.ts index 276131f08a..2c4addefd6 100644 --- a/src/domain/javascript/javascriptSemanticValues.ts +++ b/src/domain/javascript/javascriptSemanticValues.ts @@ -360,6 +360,11 @@ const evaluateBinding = ( const initializer = binding.initializers[0]; if (initializer === undefined) return { status: "unknown", reason: "Missing binding initializer." }; + if (context.state.conditionallySkippedExpressions.has(initializer.node)) + return { + status: "unknown", + reason: `Binding ${binding.name} may remain uninitialized.`, + }; const position = context.bindings.size; const callerDependence = context.dependence.lowest; context.dependence.lowest = CALLER_INDEPENDENT; @@ -817,7 +822,7 @@ const provenanceForBindingUncached = ( ({ kind }) => kind === "assignment", ); const conditionallyInitialized = binding.initializers.some(({ node }) => - context.state.conditionalInitializers.has(node), + context.state.conditionallySkippedExpressions.has(node), ); if ( provenance.status !== "module" || @@ -828,7 +833,7 @@ const provenanceForBindingUncached = ( provenance.origins, assigned ? `Binding ${binding.name} may be reassigned.` - : `Binding ${binding.name} is conditionally initialized.`, + : `Binding ${binding.name} may remain uninitialized.`, ); }; diff --git a/src/domain/javascript/javascriptSymbolicBindings.ts b/src/domain/javascript/javascriptSymbolicBindings.ts index 9d771df04b..42f629f097 100644 --- a/src/domain/javascript/javascriptSymbolicBindings.ts +++ b/src/domain/javascript/javascriptSymbolicBindings.ts @@ -557,7 +557,7 @@ export class JavaScriptSymbolicBindings { this.unresolved = true; return symbolicUnknown("initializer not available at this read"); } - if (this.state.conditionalInitializers.has(initializer.node)) + if (this.state.conditionallySkippedExpressions.has(initializer.node)) return symbolicUnknown("conditional initializer"); let value = this.evaluate(initializer.node, env); for (const key of initializer.projection) {