diff --git a/src/domain/javascript/javascriptSemanticAnalysis.member_mutations.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.member_mutations.test.ts index f1091165ec..24bc4116f8 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.member_mutations.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.member_mutations.test.ts @@ -164,6 +164,148 @@ describe("JavaScript semantic values after explicit property mutations", () => { }); }); +describe("JavaScript semantic mutations through getter results (#1494)", () => { + it.each([ + [ + "object getter", + "const shared = { value: 1 }; const box = { get value() { return shared; } }; box.value.value = 2; return shared.value;", + ], + [ + "class instance getter", + "const shared = { value: 1 }; class Box { get value() { return shared; } } const box = new Box(); box.value.value = 2; return shared.value;", + ], + [ + "static getter", + "const shared = { value: 1 }; class Box { static get value() { return shared; } } Box.value.value = 2; return shared.value;", + ], + [ + "copied getter result", + "const shared = { value: 1 }; const box = { get value() { return shared; } }; const alias = box.value; alias.value = 2; return shared.value;", + ], + ])("marks shared returned objects uncertain through %s", (_shape, body) => { + expect(resultValue(body)?.status).toBe("unknown"); + }); + + it.each([ + [ + "last duplicate object getter wins", + "const shared = { value: 1 }; const box = { get value() { return shared; }, get value() { return { value: 2 }; } }; box.value.value = 3; return shared.value;", + "literal", + ], + [ + "a later data property replaces an object getter", + "const shared = { value: 1 }; const box = { get value() { return shared; }, value: { value: 2 } }; box.value.value = 3; return shared.value;", + "literal", + ], + [ + "a later object spread replaces an earlier getter", + "const shared = { value: 1 }; const box = { get value() { return shared; }, ...{ value: { value: 2 } } }; box.value.value = 3; return shared.value;", + "literal", + ], + [ + "a dynamic computed getter can replace a static getter", + "const shared = { value: 1 }; const key = 'value'; const box = { get value() { return { value: 2 }; }, get [key]() { return shared; } }; box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "a dynamic instance property can select a class getter", + "const shared = { value: 1 }; const key = getKey(); class Box { get value() { return shared; } } const box = new Box(); box[key].value = 3; return shared.value;", + "unknown", + ], + [ + "a dynamic static property can select a class getter", + "const shared = { value: 1 }; const key = getKey(); class Box { static get value() { return shared; } } Box[key].value = 3; return shared.value;", + "unknown", + ], + [ + "a paired object setter keeps its getter result", + "const shared = { value: 1 }; const box = { get value() { return shared; }, set value(next) {} }; box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "a paired class setter keeps its getter result", + "const shared = { value: 1 }; class Box { get value() { return shared; } set value(next) {} } const box = new Box(); box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "a later class field replaces a getter", + "const shared = { value: 1 }; class Box { get value() { return shared; } value = { value: 2 }; } const box = new Box(); box.value.value = 3; return shared.value;", + "literal", + ], + [ + "a class field shadows a getter even when declared earlier", + "const shared = { value: 1 }; class Box { value = { value: 2 }; get value() { return shared; } } const box = new Box(); box.value.value = 3; return shared.value;", + "literal", + ], + [ + "last duplicate class getter wins", + "const shared = { value: 1 }; class Box { get value() { return shared; } get value() { return { value: 2 }; } } const box = new Box(); box.value.value = 3; return shared.value;", + "literal", + ], + [ + "follows an inherited getter", + "const shared = { value: 1 }; class Base { get value() { return shared; } } class Derived extends Base {} const box = new Derived(); box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "follows every possible conditional superclass getter", + "const shared = { value: 1 }; class Fresh { get value() { return { value: 2 }; } } class Shared { get value() { return shared; } } class Derived extends (flag ? Fresh : Shared) {} const box = new Derived(); box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "includes an unknown computed getter that can shadow a static getter", + "const shared = { value: 1 }; const key = getKey(); class Box { get value() { return { value: 2 }; } get [key]() { return shared; } } const box = new Box(); box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "a later known getter shadows an earlier unknown computed getter", + "const shared = { value: 1 }; const key = getKey(); class Box { get [key]() { return shared; } get value() { return { value: 2 }; } } const box = new Box(); box.value.value = 3; return shared.value;", + "literal", + ], + [ + "follows an aliased class constructor", + "const shared = { value: 1 }; class Box { get value() { return shared; } } const Alias = Box; const box = new Alias(); box.value.value = 3; return shared.value;", + "unknown", + ], + [ + "follows an inline class constructor", + "const shared = { value: 1 }; const box = new (class { get value() { return shared; } })(); box.value.value = 3; return shared.value;", + "unknown", + ], + ])("%s", (_case, body, status) => { + expect(resultValue(body)?.status).toBe(status); + }); + + it("retains conservative unknowns for fresh getter reads and saved results", () => { + expect( + resultValue( + "const box = { get value() { return { x: 1 }; } }; const first = box.value; first.x = 2; return box.value.x;", + )?.status, + ).toBe("unknown"); + expect( + resultValue( + "const box = { get value() { return { x: 1 }; } }; const saved = box.value; saved.x = 2; return saved.x;", + )?.status, + ).toBe("unknown"); + }); + + it("does not invalidate unrelated state through a fresh getter result", () => { + expect( + resultValue( + "const shared = { value: 1 }; const box = { get fresh() { return { value: 2 }; } }; box.fresh.value = 3; return shared.value;", + ), + ).toEqual({ status: "literal", value: 1 }); + }); + + it("keeps a shared value known when its getter is never read", () => { + expect( + resultValue( + "const shared = { value: 1 }; const box = { get value() { return shared; } }; return shared.value;", + ), + ).toEqual({ status: "literal", value: 1 }); + }); +}); + describe("JavaScript semantic values after mutable references escape", () => { it("preserves copied scalar slots while invalidating shared children of an escaped object spread", () => { expect( diff --git a/src/domain/javascript/javascriptSemanticMemberMutations.ts b/src/domain/javascript/javascriptSemanticMemberMutations.ts index ed5d2fb090..5471ee3ab5 100644 --- a/src/domain/javascript/javascriptSemanticMemberMutations.ts +++ b/src/domain/javascript/javascriptSemanticMemberMutations.ts @@ -286,7 +286,7 @@ export function* collectSemanticMemberMutationsSteps( (path[0] === "constructor" && path[1] === "prototype")) ) arrayIterationUnknown = true; - return referencedValues(expression, path, effect); + return referencedValues(expression, path, effect, state); } // Expanding a value below its references depends only on syntax, not on the // escape site. Escaping an object reaches every method's results, so @@ -618,6 +618,7 @@ const referencedValues = ( node: t.Node, path: PropertyPath, effect: ValueEffect, + state: JavaScriptSemanticAnalysisState, ): readonly ReferencedValue[] => { if (effect === "escape" && t.isFunction(node)) return semanticCallableResultExpressions(node).flatMap( @@ -631,6 +632,30 @@ const referencedValues = ( ? objectReferencedValues(node, path) : arrayReferencedValues(node, path); } + if (t.isClass(node)) { + const [key, ...remaining] = path; + if (key === undefined) return []; + return classGetterResults( + node, + key === null ? null : String(key), + true, + remaining, + state, + ); + } + if (t.isNewExpression(node)) { + const [key, ...remaining] = path; + if (key === undefined) return []; + return classCandidates(node.callee, state).flatMap((klass) => + classGetterResults( + klass, + key === null ? null : String(key), + false, + remaining, + state, + ), + ); + } if (t.isAssignmentExpression(node)) return node.operator === "||=" || node.operator === "??=" ? [ @@ -742,11 +767,17 @@ const observableMutationPaths = ( for (const binding of state.bindingsById.values()) { const observer = bindingNodes.get(binding.bindingId); if (observer === undefined) continue; - for (const initializer of binding.initializers) + for (const initializer of binding.initializers) { + const expression = unwrapJavaScriptExpression(initializer.node).node; + // Preserve member paths through class values and local instances so + // getter-result aliases can be resolved at the accessor boundary. + if (t.isClass(expression) || t.isNewExpression(expression)) + (observer.origins ??= new Set()).add("opaque"); link( project(expressionNode(initializer.node), initializer.projection), observer, ); + } // Preserve exact paths where copies, defaults or iteration need their // reference-specific projections; an opaque origin prevents pruning. if (binding.referenceInitializers.length > 0) @@ -797,7 +828,12 @@ const observableMutationPaths = ( observer, ); else - for (const reference of referencedValues(expression, [], "escape")) + for (const reference of referencedValues( + expression, + [], + "escape", + state, + )) link( project(expressionNode(reference.node), reference.path), observer, @@ -842,6 +878,7 @@ const observableMutationPaths = ( origin, [key], "escape", + state, )) { link( project(expressionNode(reference.node), reference.path), @@ -888,6 +925,139 @@ const observableMutationPaths = ( }; }; +const classCandidates = ( + root: t.Node, + state: JavaScriptSemanticAnalysisState, +): readonly (t.ClassDeclaration | t.ClassExpression)[] => { + const pending: t.Node[] = [root]; + const seenNodes = new Set(); + const seenBindings = new Set(); + const classes: (t.ClassDeclaration | t.ClassExpression)[] = []; + for (let cursor = 0; cursor < pending.length; cursor++) { + const node = pending[cursor]; + if (node === undefined) continue; + const expression = unwrapJavaScriptExpression(node).node; + if (seenNodes.has(expression)) continue; + seenNodes.add(expression); + if (t.isClass(expression)) { + classes.push(expression); + } else if (t.isIdentifier(expression)) { + const binding = resolveSemanticBindingState( + state, + expression, + expression.name, + ); + if (binding === undefined || seenBindings.has(binding.bindingId)) + continue; + seenBindings.add(binding.bindingId); + for (const initializer of binding.initializers) + pending.push(initializer.node); + } else if ( + t.isConditionalExpression(expression) || + t.isLogicalExpression(expression) + ) { + pending.push( + t.isConditionalExpression(expression) + ? expression.consequent + : expression.left, + t.isConditionalExpression(expression) + ? expression.alternate + : expression.right, + ); + } else if (t.isSequenceExpression(expression)) { + const last = expression.expressions.at(-1); + if (last !== undefined) pending.push(last); + } + } + return classes; +}; + +const classGetterResults = ( + root: t.ClassDeclaration | t.ClassExpression, + propertyName: string | null, + isStatic: boolean, + path: PropertyPath, + state: JavaScriptSemanticAnalysisState, +): readonly ReferencedValue[] => { + const pending: (t.ClassDeclaration | t.ClassExpression)[] = [root]; + const seen = new Set(); + const possibleResults: ReferencedValue[] = []; + for (let cursor = 0; cursor < pending.length; cursor++) { + const klass = pending[cursor]; + if (klass === undefined || seen.has(klass)) continue; + seen.add(klass); + const indexedMembers: { + readonly member: t.ClassMethod | t.ClassProperty; + readonly index: number; + readonly key: string | null; + }[] = []; + klass.body.body.forEach((member, index) => { + if ( + (t.isClassMethod(member) || t.isClassProperty(member)) && + member.static === isStatic + ) + indexedMembers.push({ + member, + index, + key: semanticStaticPropertyKey(member.key, member.computed), + }); + }); + const matchingMembers = + propertyName === null + ? [] + : indexedMembers.filter(({ key }) => key === propertyName); + const uncertainGetters = indexedMembers.filter( + ({ member, key }) => + t.isClassMethod(member) && + member.kind === "get" && + (propertyName === null || (member.computed && key === null)), + ); + const getters = matchingMembers.filter( + (entry): entry is typeof entry & { member: t.ClassMethod } => + t.isClassMethod(entry.member) && entry.member.kind === "get", + ); + const getter = getters.at(-1); + const getterIndex = getter?.index ?? -1; + const laterDataMember = matchingMembers.some( + ({ member, index }) => + t.isClassProperty(member) || + (t.isClassMethod(member) && + member.kind === "method" && + index > getterIndex), + ); + if (getters.length > 0 && !laterDataMember && getter !== undefined) + possibleResults.push( + ...semanticCallableResultExpressions(getter.member).flatMap( + ({ node }) => (node === null ? [] : [{ node, path }]), + ), + ); + for (const { member: uncertain, index } of uncertainGetters) + if ( + !matchingMembers.some( + ({ member, index: matchingIndex }) => + (t.isClassProperty(member) || + (t.isClassMethod(member) && + (member.kind === "method" || member.kind === "get"))) && + matchingIndex > index, + ) + ) + possibleResults.push( + ...semanticCallableResultExpressions(uncertain).flatMap(({ node }) => + node === null ? [] : [{ node, path }], + ), + ); + // An exact own descriptor suppresses inherited lookup. Dynamic computed + // descriptors may or may not match, so retain possible superclass results. + if ( + matchingMembers.length === 0 && + klass.superClass !== null && + klass.superClass !== undefined + ) + pending.push(...classCandidates(klass.superClass, state)); + } + return possibleResults; +}; + const objectReferencedValues = ( node: t.ObjectExpression, path: PropertyPath, @@ -895,6 +1065,10 @@ const objectReferencedValues = ( const [key, ...remaining] = path; const references: ReferencedValue[] = []; const overwritten = new Set(); + const dataOverwritten = new Set(); + const accessorNames = new Set(); + const getterNames = new Set(); + const setterNames = new Set(); for (const property of [...node.properties].reverse()) { if (t.isSpreadElement(property)) { const selected = key ?? null; @@ -917,31 +1091,100 @@ const objectReferencedValues = ( ...remaining, ], }); + if ( + selected !== null && + typeof selected !== "object" && + t.isObjectExpression(property.argument) && + objectExpressionDefinesKey(property.argument, String(selected)) + ) { + overwritten.add(String(selected)); + dataOverwritten.add(String(selected)); + } continue; } const name = semanticStaticPropertyKey(property.key, property.computed); - if (name !== null && overwritten.has(name)) continue; - if (name === null || semanticPropertyPathKeyMatches(key ?? null, name)) - references.push({ - node: t.isObjectProperty(property) ? property.value : property, - path: remaining, - }); - // A prototype setter does not replace an own property from a spread. + const isGetter = t.isObjectMethod(property) && property.kind === "get"; + const isSetter = t.isObjectMethod(property) && property.kind === "set"; if ( name !== null && - !(t.isObjectMethod(property) && property.kind !== "method") && - !( - t.isObjectProperty(property) && - !property.computed && - !property.shorthand && - name === "__proto__" - ) + (isGetter || isSetter + ? dataOverwritten.has(name) || + (isGetter ? getterNames.has(name) : setterNames.has(name)) + : dataOverwritten.has(name) || accessorNames.has(name)) ) - overwritten.add(name); + continue; + if (name === null || semanticPropertyPathKeyMatches(key ?? null, name)) { + if (t.isObjectMethod(property) && property.kind === "get") { + for (const result of semanticCallableResultExpressions(property)) + if (result.node !== null) + references.push({ node: result.node, path: remaining }); + } else if (t.isObjectMethod(property) && property.kind === "set") { + // A setter contributes no readable reference. A paired getter is + // handled independently when it appears earlier in this object. + } else { + references.push({ + node: t.isObjectProperty(property) ? property.value : property, + path: remaining, + }); + } + } + if (name !== null) { + if (isGetter) { + getterNames.add(name); + accessorNames.add(name); + overwritten.add(name); + } else if (isSetter) { + setterNames.add(name); + accessorNames.add(name); + overwritten.add(name); + } else if ( + !( + t.isObjectProperty(property) && + !property.computed && + !property.shorthand && + name === "__proto__" + ) + ) { + overwritten.add(name); + dataOverwritten.add(name); + } + } } return references; }; +const objectExpressionDefinesKey = ( + root: t.ObjectExpression, + key: string, +): boolean => { + const pending: t.ObjectExpression[] = [root]; + const seen = new Set(); + for (let cursor = 0; cursor < pending.length; cursor++) { + const object = pending[cursor]; + if (object === undefined || seen.has(object)) continue; + seen.add(object); + for (const property of object.properties) { + if (t.isSpreadElement(property)) { + if (t.isObjectExpression(property.argument)) + pending.push(property.argument); + continue; + } + const name = semanticStaticPropertyKey(property.key, property.computed); + if ( + name === key && + !( + t.isObjectProperty(property) && + !property.computed && + !property.shorthand && + name === "__proto__" + ) + ) + return true; + } + } + return false; +}; + const arrayReferencedValues = ( node: t.ArrayExpression, path: PropertyPath, diff --git a/tests/acceptance/applications/javascriptReturnedReferences.test.ts b/tests/acceptance/applications/javascriptReturnedReferences.test.ts index 5bbd36ff61..82b4716387 100644 --- a/tests/acceptance/applications/javascriptReturnedReferences.test.ts +++ b/tests/acceptance/applications/javascriptReturnedReferences.test.ts @@ -42,6 +42,14 @@ const writes = { "const methods = [...[], () => shared]; methods[0]().x = 2;", classMethod: "class Box { get() { return shared; } } const box = new Box(); box.get().x = 2;", + objectGetter: + "const box = { get value() { return shared; } }; box.value.x = 2;", + instanceGetter: + "class Box { get value() { return shared; } } const box = new Box(); box.value.x = 2;", + staticGetter: + "class Box { static get value() { return shared; } } Box.value.x = 2;", + copiedGetter: + "const box = { get value() { return shared; } }; const alias = box.value; alias.x = 2;", staticMethod: "class Box { static get() { return shared; } } Box.get().x = 2;", constructor: @@ -57,6 +65,20 @@ const source = [ ([name, body]) => `export ${name === "asyncAwait" ? "async " : ""}function ${name}(){ const shared = {x: 1}; ${body} return shared.x; }`, ), + "export function plainGetterRead(){ const shared = {x: 1}; const box = {get value() { return shared; }}; box.value; return shared.x; }", + "export function objectGetterLastWins(){ const shared = {x: 1}; const box = {get value(){return shared;}, get value(){return {x: 2};}}; box.value.x = 3; return shared.x; }", + "export function objectDataReplacesGetter(){ const shared = {x: 1}; const box = {get value(){return shared;}, value: {x: 2}}; box.value.x = 3; return shared.x; }", + "export function objectSpreadReplacesGetter(){ const shared = {x: 1}; const box = {get value(){return shared;}, ...{value: {x: 2}}}; box.value.x = 3; return shared.x; }", + "export function dynamicObjectGetter(){ const shared = {x: 1}; const key = 'value'; const box = {get value(){return {x: 2};}, get [key](){return shared;}}; box.value.x = 3; return shared.x; }", + "export function dynamicClassGetter(){ const shared = {x: 1}; const key = getKey(); class Box { get value(){return shared;} } const box = new Box(); box[key].x = 2; return shared.x; }", + "export function dynamicStaticGetter(){ const shared = {x: 1}; const key = getKey(); class Box { static get value(){return shared;} } Box[key].x = 2; return shared.x; }", + "export function inheritedGetter(){ const shared = {x: 1}; class Base { get value(){return shared;} } class Derived extends Base {} new Derived().value.x = 2; return shared.x; }", + "export function conditionalSuperGetter(){ const shared = {x: 1}; class Fresh { get value(){return {x: 2};} } class Shared { get value(){return shared;} } class Derived extends (flag ? Fresh : Shared) {} new Derived().value.x = 2; return shared.x; }", + "export function computedClassGetterOverride(){ const shared = {x: 1}; const key = getKey(); class Box { get value(){return {x: 2};} get [key](){return shared;} } new Box().value.x = 3; return shared.x; }", + "export function aliasedClassGetter(){ const shared = {x: 1}; class Box { get value(){return shared;} } const Alias = Box; new Alias().value.x = 2; return shared.x; }", + "export function inlineClassGetter(){ const shared = {x: 1}; new (class { get value(){return shared;} })().value.x = 2; return shared.x; }", + "export function freshGetterRead(){ const box = {get value(){return {x: 1};}}; const first = box.value; first.x = 2; return box.value.x; }", + "export function savedFreshGetter(){ const box = {get value(){return {x: 1};}}; const saved = box.value; saved.x = 2; return saved.x; }", "export function directSibling(){ const shared = {x: 1}; const parent = {shared, keep: 7}; const get = () => parent.shared; get().x = 2; return {x: shared.x, keep: parent.keep}; }", "export function primitive(){ const n = 1; const get = () => n; const t = get(); return n; }", "export function copy(){ const shared = {x: 1}; const get = () => ({...shared}); get().x = 2; return shared.x; }", @@ -75,6 +97,35 @@ const assertReturns = (fields: JavaScriptReturnFields): void => { expect(field?.state, name).toMatch(/^(unknown|literal)$/); if (field?.state === "literal") expect(field.value, name).toBe(2); } + for (const name of [ + "objectGetter", + "instanceGetter", + "staticGetter", + "copiedGetter", + ]) + expect(fields(name), name).toContainEqual( + expect.objectContaining({ path: "", state: "unknown" }), + ); + for (const name of [ + "plainGetterRead", + "objectGetterLastWins", + "objectDataReplacesGetter", + ]) + expect(fields(name)).toContainEqual( + expect.objectContaining({ path: "", state: "literal", value: 1 }), + ); + for (const name of [ + "inheritedGetter", + "aliasedClassGetter", + "inlineClassGetter", + ]) + expect(fields(name)).toContainEqual( + expect.objectContaining({ path: "", state: "unknown" }), + ); + for (const name of ["freshGetterRead", "savedFreshGetter"]) + expect(fields(name)).toContainEqual( + expect.objectContaining({ path: "", state: "unknown" }), + ); const changed = fields("directSibling").find(({ path }) => path === "/x"); expect(changed?.state).toMatch(/^(unknown|literal)$/); if (changed?.state === "literal") expect(changed.value).toBe(2);