diff --git a/docs/javascript-application-workflows.md b/docs/javascript-application-workflows.md index 1e9e7fe193..041015b9c7 100644 --- a/docs/javascript-application-workflows.md +++ b/docs/javascript-application-workflows.md @@ -208,6 +208,13 @@ arrays, and later explicit data properties and methods replace earlier object spread references. A known array prefix does not expose values from a following spread. Accessors, custom or uncertain iteration, unknown computed keys, and unknown spread lengths retain conservative uncertainty. +Writes through an object getter's returned receiver retain that uncertainty +when the object is an array element, nested inside mixed object/array +containers, or assigned to a previously declared identifier. Saving the getter +result in an alias does not make its old field values immutable. Known array +indices include holes; an element after a spread has only a minimum possible +index. Unrelated properties and known array prefixes remain precise, and a +getter read without a write does not invalidate the receiver's fields. Nested callable returns are not assigned to their parent callable. Projected graph observations carry source ranges but never source text. diff --git a/src/domain/javascript/javascriptSemanticAnalysis.getter_receivers.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.getter_receivers.test.ts index 3b83bf4820..c3b38842a6 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.getter_receivers.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.getter_receivers.test.ts @@ -33,6 +33,38 @@ const getterReceivers = [ "nested owner", "const outer = { box: { x: 1, get self() { return this; } } }; outer.box.self.x = 2; return outer.box.x;", ], + [ + "array element", + "const arr = [{ x: 1, get self() { return this; } }]; arr[0].self.x = 2; return arr[0].x;", + ], + [ + "array element after a hole", + "const arr = [, { x: 1, get self() { return this; } }]; arr[1].self.x = 2; return arr[1].x;", + ], + [ + "mixed object and array owners", + "const outer = { rows: [{ box: { x: 1, get self() { return this; } } }] }; outer.rows[0].box.self.x = 2; return outer.rows[0].box.x;", + ], + [ + "assigned owner", + "let box; box = { x: 1, get self() { return this; } }; box.self.x = 2; return box.x;", + ], + [ + "saved array receiver alias", + "const arr = [{ x: 1, get self() { return this; } }]; const saved = arr[0].self; saved.x = 2; return arr[0].x;", + ], + [ + "old receiver captured inside its replacement initializer", + "const old = { x: 1 }; let box = old; box = { old: box, get self() { return this; } }; box.self.old.x = 2; return old.x;", + ], + [ + "array receiver after an unknown spread", + "const arr = [...items, { x: 1, get self() { return this; } }]; arr[index].self.x = 2; return arr[index].x;", + ], + [ + "saved receiver before its array alias is reassigned", + "const original = [{ x: 1, get self() { return this; } }]; let arr = original; const saved = arr[0].self; arr = [{ x: 1 }]; saved.x = 2; return original[0].x;", + ], [ "inherited receiver field", "const shared = { x: 1 }; class A { get v() { return this.s; } } class B extends A { s = shared; } new B().v.x = 2; return shared.x;", @@ -75,7 +107,7 @@ const getterReceivers = [ ], ] as const; -describe("getter receiver mutation aliases (#1665)", () => { +describe("getter receiver mutation aliases (#1665, #2654)", () => { it.each(getterReceivers)( "does not report a stale literal through %s", (_shape, body) => { @@ -103,6 +135,42 @@ describe("getter receiver mutation aliases (#1665)", () => { "unrelated sibling", "const box = { x: 1, y: 1, get self() { return this; } }; box.self.y = 2; return box.x;", ], + [ + "array receiver read without write", + "const arr = [{ x: 1, get self() { return this; } }]; const saved = arr[0].self; return arr[0].x;", + ], + [ + "assigned receiver read without write", + "let box; box = { x: 1, get self() { return this; } }; const saved = box.self; return box.x;", + ], + [ + "unrelated array receiver property", + "const arr = [{ x: 1, y: 1, get self() { return this; } }]; arr[0].self.y = 2; return arr[0].x;", + ], + [ + "unrelated array element", + "const arr = [{ x: 1, get self() { return this; } }, { x: 1 }]; arr[0].self.x = 2; return arr[1].x;", + ], + [ + "array replacement after a receiver was captured", + "const original = [{ x: 1, get self() { return this; } }]; const replacement = [{ x: 1 }]; let arr = original; const saved = arr[0].self; arr = replacement; saved.x = 2; return replacement[0].x;", + ], + [ + "known array prefix before a spread receiver", + "const arr = [{ x: 1 }, ...items, { x: 1, get self() { return this; } }]; arr[1].self.x = 2; return arr[0].x;", + ], + [ + "outer binding shadowed by an assigned receiver", + "let box; box = { x: 1 }; { let box; box = { x: 1, get self() { return this; } }; box.self.x = 2; } return box.x;", + ], + [ + "old object before an unconditional receiver assignment", + "const old = { x: 1 }; let box = old; box = { x: 1, get self() { return this; } }; box.self.x = 2; return old.x;", + ], + [ + "unmodified old receiver captured inside its replacement initializer", + "const old = { x: 1 }; let box = old; box = { x: 1, old: box, get self() { return this; } }; box.self.x = 2; return old.x;", + ], [ "replace getter", "const shared = { x: 1 }; const box = { s: shared, get v() { return this.s; } }; box.v = {}; return shared.x;", diff --git a/src/domain/javascript/javascriptSemanticMemberMutations.ts b/src/domain/javascript/javascriptSemanticMemberMutations.ts index c2f7efba37..35b63c815f 100644 --- a/src/domain/javascript/javascriptSemanticMemberMutations.ts +++ b/src/domain/javascript/javascriptSemanticMemberMutations.ts @@ -18,6 +18,7 @@ import { semanticClassCandidates, semanticClassPropertyReferences, semanticObjectPropertyReferences, + type JavaScriptSemanticObjectReference, } from "./javascriptSemanticObjectReferences.js"; import { semanticIterationSources } from "./javascriptSemanticIterationSources.js"; import { semanticCallableResultExpressions } from "./javascriptSemanticCallableResults.js"; @@ -98,8 +99,20 @@ export function* collectSemanticMemberMutationsSteps( effect: ValueEffect = "write", mutation?: t.Node, originAt: t.Node | undefined = mutation, + ownerInitializer?: t.Node, ): void => { - const pending = [{ node, path, bindings, originAt }]; + const pending: (ReferencedValue & { + readonly bindings: ReadonlySet; + readonly originAt: t.Node | undefined; + })[] = [ + { + node, + path, + bindings, + originAt, + ...(ownerInitializer === undefined ? {} : { ownerInitializer }), + }, + ]; // Visit shallower alias traversals first so their exact prefixes suppress // longer loop paths before those paths enumerate every assignment order. for (let cursor = 0; cursor < pending.length; cursor++) { @@ -132,6 +145,8 @@ export function* collectSemanticMemberMutationsSteps( binding.bindingId, current.originAt?.start, current.originAt?.end, + current.ownerInitializer?.start, + current.ownerInitializer?.end, ]; const lifetime = JSON.stringify([...capture, current.path]); if (current.bindings.has(lifetime)) continue; @@ -165,6 +180,7 @@ export function* collectSemanticMemberMutationsSteps( "escape", mutation, current.originAt, + current.ownerInitializer, ); continue; } @@ -195,6 +211,8 @@ export function* collectSemanticMemberMutationsSteps( mutation?.end, current.originAt?.start, current.originAt?.end, + current.ownerInitializer?.start, + current.ownerInitializer?.end, ]); const previous = selections.get(selectionIdentity); const excludedKeys = @@ -220,6 +238,8 @@ export function* collectSemanticMemberMutationsSteps( mutation?.end, current.originAt?.start, current.originAt?.end, + current.ownerInitializer?.start, + current.ownerInitializer?.end, ]); let covering = coveringPaths.get(pathState); if (covering === undefined) { @@ -235,6 +255,8 @@ export function* collectSemanticMemberMutationsSteps( mutation?.end, current.originAt?.start, current.originAt?.end, + current.ownerInitializer?.start, + current.ownerInitializer?.end, ]); if (recordedEffects.has(identity)) continue; // A recorded write invalidates the binding to a non-primitive value, so @@ -248,10 +270,23 @@ export function* collectSemanticMemberMutationsSteps( value?.status === "literal" || value?.status === "union"; recordedEffects.add(identity); const nested = new Set([...current.bindings, lifetime, captureKey]); + // Returning a literal's receiver reaches that allocation, not the + // binding's older values read while evaluating its initializer. const origins = - current.originAt === undefined - ? binding - : semanticMutationInitializers(binding, current.originAt, parents); + current.ownerInitializer !== undefined + ? { + initializers: binding.initializers.filter( + ({ node }) => node === current.ownerInitializer, + ), + referenceInitializers: [], + } + : current.originAt === undefined + ? binding + : semanticMutationInitializers( + binding, + current.originAt, + parents, + ); if (!primitiveWrite) { if (effect === "escape") binding.escapedPaths.push({ path, node: mutation }); @@ -367,6 +402,8 @@ export function* collectSemanticMemberMutationsSteps( binding === undefined ? null : binding.bindingId, binding === undefined ? node.name : null, pathKey, + value.ownerInitializer?.start, + value.ownerInitializer?.end, ]); if (leafIdentities.has(identity)) continue; leafIdentities.add(identity); @@ -551,9 +588,7 @@ export function* collectSemanticMemberMutationsSteps( clearSemanticValueCaches(state); } -interface ReferencedValue { - readonly node: t.Node; - readonly path: PropertyPath; +interface ReferencedValue extends JavaScriptSemanticObjectReference { readonly iterableFallbackPath?: PropertyPath; } diff --git a/src/domain/javascript/javascriptSemanticObjectReferences.ts b/src/domain/javascript/javascriptSemanticObjectReferences.ts index 387ba09d28..4d1bf3e760 100644 --- a/src/domain/javascript/javascriptSemanticObjectReferences.ts +++ b/src/domain/javascript/javascriptSemanticObjectReferences.ts @@ -14,9 +14,11 @@ import { } from "./javascriptSemanticPropertyPaths.js"; import { compareUnicodeCodePoints } from "../unicodeCodePointOrder.js"; -interface ReferencedValue { +export interface JavaScriptSemanticObjectReference { readonly node: t.Node; readonly path: PropertyPath; + /** The allocation owned by this binding, rather than a read of its prior value. */ + readonly ownerInitializer?: t.Node; } export const semanticClassCandidates = ( @@ -72,10 +74,10 @@ const instanceFieldReferences = ( path: PropertyPath, state: JavaScriptSemanticAnalysisState, ): { - readonly references: readonly ReferencedValue[]; + readonly references: readonly JavaScriptSemanticObjectReference[]; readonly definiteNames: ReadonlySet; } => { - const references: ReferencedValue[] = []; + const references: JavaScriptSemanticObjectReference[] = []; const pending = [{ klass: root, excluded: new Set() }]; const selections = new Map>(); let definiteNames: ReadonlySet | undefined; @@ -208,7 +210,7 @@ export const semanticClassPropertyReferences = ( selection: PropertyPath, state: JavaScriptSemanticAnalysisState, activeGetters: ReadonlyMap = new Map(), -): readonly ReferencedValue[] => { +): readonly JavaScriptSemanticObjectReference[] => { const root = lookup.owner; const receiver = lookup.receiver ?? root; const [key, ...path] = selection; @@ -216,7 +218,7 @@ export const semanticClassPropertyReferences = ( key == null || typeof key === "object" ? null : String(key); const pending: (t.ClassDeclaration | t.ClassExpression)[] = [root]; const seen = new Set(); - const possibleResults: ReferencedValue[] = []; + const possibleResults: JavaScriptSemanticObjectReference[] = []; // Instance fields are own properties, including fields initialized by base // constructors. They take precedence over every prototype descriptor. let instanceFields: ReadonlySet = new Set(); @@ -267,11 +269,11 @@ const objectReceiverReference = ( object: t.ObjectExpression, path: PropertyPath, state: JavaScriptSemanticAnalysisState, -): readonly ReferencedValue[] => { +): readonly JavaScriptSemanticObjectReference[] => { // A literal's enclosing binding owns its slots. Returning the literal // itself would bypass invalidation of that binding's observed properties. let current: t.Node = object; - const projection: (string | number | null)[] = []; + const projection: PropertyPath[number][] = []; while (true) { const parent = state.parentsByNode.get(current); if ( @@ -279,7 +281,43 @@ const objectReceiverReference = ( parent.init === current && t.isIdentifier(parent.id) ) - return [{ node: parent.id, path: [...projection, ...path] }]; + return [ + { + node: parent.id, + path: [...projection, ...path], + ownerInitializer: current, + }, + ]; + if ( + t.isAssignmentExpression(parent, { operator: "=" }) && + parent.right === current && + t.isIdentifier(parent.left) + ) + return [ + { + node: parent.left, + path: [...projection, ...path], + ownerInitializer: current, + }, + ]; + if (t.isArrayExpression(parent)) { + let minimumIndex = 0; + let uncertainIndex = false; + for (const element of parent.elements) { + if (element === current) break; + if (t.isSpreadElement(element)) uncertainIndex = true; + else minimumIndex++; + } + // A spread contributes an unknown number of slots. Retain the known + // prefix without treating the element's syntax position as its index. + projection.unshift( + uncertainIndex + ? { excludedKeys: [], startIndex: minimumIndex } + : minimumIndex, + ); + current = parent; + continue; + } if (t.isObjectProperty(parent) && parent.value === current) { projection.unshift( semanticStaticPropertyKey(parent.key, parent.computed), @@ -312,7 +350,7 @@ const getterResultReferences = ( path: PropertyPath, state: JavaScriptSemanticAnalysisState, activeGetters: ReadonlyMap = new Map(), -): readonly ReferencedValue[] => { +): readonly JavaScriptSemanticObjectReference[] => { const { getter, owner, receiver = owner } = invocation; const previousLength = activeGetters.get(getter); if (previousLength !== undefined && previousLength <= path.length) return []; @@ -323,7 +361,7 @@ const getterResultReferences = ( const resolve = ( node: t.Node, remaining: PropertyPath, - ): readonly ReferencedValue[] => { + ): readonly JavaScriptSemanticObjectReference[] => { const expression = unwrapJavaScriptExpression(node).node; if (t.isThisExpression(expression)) { if (t.isClass(receiver)) { @@ -405,9 +443,9 @@ export const semanticObjectPropertyReferences = ( node: t.ObjectExpression, path: PropertyPath, state: JavaScriptSemanticAnalysisState, -): readonly ReferencedValue[] => { +): readonly JavaScriptSemanticObjectReference[] => { const [key, ...remaining] = path; - const references: ReferencedValue[] = []; + const references: JavaScriptSemanticObjectReference[] = []; const overwritten = new Set(); const dataOverwritten = new Set(); const accessorNames = new Set(); diff --git a/tests/acceptance/applications/getterReceiverMutationsCli.test.ts b/tests/acceptance/applications/getterReceiverMutationsCli.test.ts index a5ba0bfeb3..95b6e4862e 100644 --- a/tests/acceptance/applications/getterReceiverMutationsCli.test.ts +++ b/tests/acceptance/applications/getterReceiverMutationsCli.test.ts @@ -39,6 +39,47 @@ export function superclass() { new B().v.x = 2; return shared.x; } +export function arrayReceiver() { + const arr = [{ x: 1, get self() { return this; } }]; + arr[0].self.x = 2; + return arr[0].x; +} +export function nestedArrayReceiver() { + const outer = { arr: [{ x: 1, get self() { return this; } }] }; + outer.arr[0].self.x = 2; + return outer.arr[0].x; +} +export function assignedReceiver() { + let box; + box = { x: 1, get self() { return this; } }; + box.self.x = 2; + return box.x; +} +export function savedArrayReceiver() { + const arr = [{ x: 1, get self() { return this; } }]; + const saved = arr[0].self; + saved.x = 2; + return arr[0].x; +} +export function arrayReadOnly() { + const arr = [{ x: 1, get self() { return this; } }]; + const saved = arr[0].self; + return arr[0].x; +} +export function replacementKeepsOldObject() { + const old = { x: 1 }; + let box = old; + box = { x: 1, old: box, get self() { return this; } }; + box.self.x = 2; + return old.x; +} +export function replacementMutatesCapturedObject() { + const old = { x: 1 }; + let box = old; + box = { old: box, get self() { return this; } }; + box.self.old.x = 2; + return old.x; +} export function untouched() { const box = { x: 1, y: 1, get self() { return this; } }; box.self.y = 2; @@ -66,13 +107,20 @@ export function untouched() { const { graph } = z .object({ normalized_result: javascriptApplicationAnalysisResultSchema }) .parse(document).normalized_result; - for (const name of [ - "self", - "property", - "privateField", - "superclass", - "untouched", - ]) { + for (const [name, status] of [ + ["self", "unknown"], + ["property", "unknown"], + ["privateField", "unknown"], + ["superclass", "unknown"], + ["arrayReceiver", "unknown"], + ["nestedArrayReceiver", "unknown"], + ["assignedReceiver", "unknown"], + ["savedArrayReceiver", "unknown"], + ["arrayReadOnly", "literal"], + ["replacementKeepsOldObject", "literal"], + ["replacementMutatesCapturedObject", "unknown"], + ["untouched", "literal"], + ] as const) { const projection = graph.nodes .flatMap(({ observations }) => observations) .find( @@ -82,7 +130,16 @@ export function untouched() { ); expect(projection, name).toBeDefined(); expect(projection?.properties.static_return_shapes).toMatchObject([ - { value_status: name === "untouched" ? "literal" : "unknown" }, + { + value_status: status, + fields: [ + { + path: "", + state: status, + ...(status === "literal" ? { value: 1 } : {}), + }, + ], + }, ]); } },