From 0e17dc7769f7380591bcc60dd6e116a22bf98c3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Tue, 9 Aug 2022 02:01:50 +0200 Subject: [PATCH] Fixed a false positive related to binding patterns and spread expressions (#49684) * Fixed a false positive related to binding patterns and spread expressions * Improve ancestor lookup when checking if an expression is spread into an object * Fixed ancestor lookup for more node types * Remove equality check for contextual types * Reformat code * Use `isWithinSpreadAssignment` flag + `objectsWithinSpread` cache instead of ancestor traversal * Revert "Use `isWithinSpreadAssignment` flag + `objectsWithinSpread` cache instead of ancestor traversal" This reverts commit be387e3bbf8a5cce2bc4c31fd77b061ea6cf8e0b. * Expand on the existing comment --- src/compiler/checker.ts | 33 ++++++++++---- ...ningObjectExpressionContextualType.symbols | 23 ++++++++++ ...ainingObjectExpressionContextualType.types | 45 +++++++++++++++++++ ...ontainingObjectExpressionContextualType.ts | 15 +++++++ 4 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.symbols create mode 100644 tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.types create mode 100644 tests/cases/compiler/spreadExpressionContainingObjectExpressionContextualType.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2b3cdcb736d..920dbb775d2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -28163,16 +28163,31 @@ namespace ts { // If object literal is contextually typed by the implied type of a binding pattern, augment the result // type with those properties for which the binding pattern specifies a default value. // If the object literal is spread into another object literal, skip this step and let the top-level object - // literal handle it instead. - if (contextualTypeHasPattern && node.parent.kind !== SyntaxKind.SpreadAssignment) { - for (const prop of getPropertiesOfType(contextualType)) { - if (!propertiesTable.get(prop.escapedName) && !getPropertyOfType(spread, prop.escapedName)) { - if (!(prop.flags & SymbolFlags.Optional)) { - error(prop.valueDeclaration || (prop as TransientSymbol).bindingElement, - Diagnostics.Initializer_provides_no_value_for_this_binding_element_and_the_binding_element_has_no_default_value); + // literal handle it instead. Note that this might require full traversal to the root pattern's parent + // as it's the guaranteed to be the common ancestor of the pattern node and the current object node. + // It's not possible to check if the immediate parent node is a spread assignment + // since the type flows in non-obvious ways through conditional expressions, IIFEs and more. + if (contextualTypeHasPattern) { + const rootPatternParent = findAncestor(contextualType.pattern!.parent, n => + n.kind === SyntaxKind.VariableDeclaration || + n.kind === SyntaxKind.BinaryExpression || + n.kind === SyntaxKind.Parameter + ); + const spreadOrOutsideRootObject = findAncestor(node, n => + n === rootPatternParent || + n.kind === SyntaxKind.SpreadAssignment + )!; + + if (spreadOrOutsideRootObject.kind !== SyntaxKind.SpreadAssignment) { + for (const prop of getPropertiesOfType(contextualType)) { + if (!propertiesTable.get(prop.escapedName) && !getPropertyOfType(spread, prop.escapedName)) { + if (!(prop.flags & SymbolFlags.Optional)) { + error(prop.valueDeclaration || (prop as TransientSymbol).bindingElement, + Diagnostics.Initializer_provides_no_value_for_this_binding_element_and_the_binding_element_has_no_default_value); + } + propertiesTable.set(prop.escapedName, prop); + propertiesArray.push(prop); } - propertiesTable.set(prop.escapedName, prop); - propertiesArray.push(prop); } } } diff --git a/tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.symbols b/tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.symbols new file mode 100644 index 00000000000..2b95e9c2fff --- /dev/null +++ b/tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.symbols @@ -0,0 +1,23 @@ +=== tests/cases/compiler/spreadExpressionContainingObjectExpressionContextualType.ts === +// repro #49585 + +const { value } = (() => ({ +>value : Symbol(value, Decl(spreadExpressionContainingObjectExpressionContextualType.ts, 2, 7)) + + value: "", +>value : Symbol(value, Decl(spreadExpressionContainingObjectExpressionContextualType.ts, 2, 27)) + + ...(true ? {} : {}), +}))(); + +// repro 49684#discussion_r920545763 + +const { value2 } = { +>value2 : Symbol(value2, Decl(spreadExpressionContainingObjectExpressionContextualType.ts, 9, 7)) + + value2: "", +>value2 : Symbol(value2, Decl(spreadExpressionContainingObjectExpressionContextualType.ts, 9, 20)) + + ...(() => true ? {} : {})(), +}; + diff --git a/tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.types b/tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.types new file mode 100644 index 00000000000..2e3765e2f69 --- /dev/null +++ b/tests/baselines/reference/spreadExpressionContainingObjectExpressionContextualType.types @@ -0,0 +1,45 @@ +=== tests/cases/compiler/spreadExpressionContainingObjectExpressionContextualType.ts === +// repro #49585 + +const { value } = (() => ({ +>value : string +>(() => ({ value: "", ...(true ? {} : {}),}))() : { value: string; } +>(() => ({ value: "", ...(true ? {} : {}),})) : () => { value: string; } +>() => ({ value: "", ...(true ? {} : {}),}) : () => { value: string; } +>({ value: "", ...(true ? {} : {}),}) : { value: string; } +>{ value: "", ...(true ? {} : {}),} : { value: string; } + + value: "", +>value : string +>"" : "" + + ...(true ? {} : {}), +>(true ? {} : {}) : {} +>true ? {} : {} : {} +>true : true +>{} : {} +>{} : {} + +}))(); + +// repro 49684#discussion_r920545763 + +const { value2 } = { +>value2 : string +>{ value2: "", ...(() => true ? {} : {})(),} : { value2: string; } + + value2: "", +>value2 : string +>"" : "" + + ...(() => true ? {} : {})(), +>(() => true ? {} : {})() : {} +>(() => true ? {} : {}) : () => {} +>() => true ? {} : {} : () => {} +>true ? {} : {} : {} +>true : true +>{} : {} +>{} : {} + +}; + diff --git a/tests/cases/compiler/spreadExpressionContainingObjectExpressionContextualType.ts b/tests/cases/compiler/spreadExpressionContainingObjectExpressionContextualType.ts new file mode 100644 index 00000000000..56c334414d1 --- /dev/null +++ b/tests/cases/compiler/spreadExpressionContainingObjectExpressionContextualType.ts @@ -0,0 +1,15 @@ +// @noEmit: true + +// repro #49585 + +const { value } = (() => ({ + value: "", + ...(true ? {} : {}), +}))(); + +// repro 49684#discussion_r920545763 + +const { value2 } = { + value2: "", + ...(() => true ? {} : {})(), +};