From cc005d1d72ffe1d7867e8583f405c752cfcdcc1c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 9 Feb 2022 08:32:35 -0800 Subject: [PATCH] Make spread overwrite error more lenient. Fixes #46463, but I don't thik it's a good fix. See https://github.com/microsoft/TypeScript/issues/46463#issuecomment-1033956377 for discussion. --- src/compiler/checker.ts | 11 ++++----- .../reference/spreadDuplicate.errors.txt | 13 ++++++---- tests/baselines/reference/spreadDuplicate.js | 15 ++++++++++++ .../reference/spreadDuplicate.symbols | 16 +++++++++++++ .../baselines/reference/spreadDuplicate.types | 24 +++++++++++++++++++ .../reference/spreadDuplicateExact.errors.txt | 13 +++++++++- .../reference/spreadDuplicateExact.js | 15 ++++++++++++ .../reference/spreadDuplicateExact.symbols | 16 +++++++++++++ .../reference/spreadDuplicateExact.types | 24 +++++++++++++++++++ .../spreadOverwritesPropertyStrict.errors.txt | 6 +---- .../types/spread/spreadDuplicate.ts | 7 ++++++ .../types/spread/spreadDuplicateExact.ts | 7 ++++++ 12 files changed, 150 insertions(+), 17 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ecac36fafdc..670e23de865 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27593,12 +27593,11 @@ namespace ts { function checkSpreadPropOverrides(type: Type, props: SymbolTable, spread: SpreadAssignment | JsxSpreadAttribute) { for (const right of getPropertiesOfType(type)) { - if (!(right.flags & SymbolFlags.Optional)) { - const left = props.get(right.escapedName); - if (left) { - const diagnostic = error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName)); - addRelatedInfo(diagnostic, createDiagnosticForNode(spread, Diagnostics.This_spread_always_overwrites_this_property)); - } + const rightType = getTypeOfSymbol(right); + const left = props.get(right.escapedName); + if (left && (exactOptionalPropertyTypes || !maybeTypeOfKind(rightType, TypeFlags.Nullable)) && !(right.flags & SymbolFlags.Optional)) { + const diagnostic = error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName)); + addRelatedInfo(diagnostic, createDiagnosticForNode(spread, Diagnostics.This_spread_always_overwrites_this_property)); } } } diff --git a/tests/baselines/reference/spreadDuplicate.errors.txt b/tests/baselines/reference/spreadDuplicate.errors.txt index f3e7db01596..8fb53e25498 100644 --- a/tests/baselines/reference/spreadDuplicate.errors.txt +++ b/tests/baselines/reference/spreadDuplicate.errors.txt @@ -1,8 +1,7 @@ tests/cases/conformance/types/spread/spreadDuplicate.ts(10,12): error TS2783: 'a' is specified more than once, so this usage will be overwritten. -tests/cases/conformance/types/spread/spreadDuplicate.ts(12,12): error TS2783: 'a' is specified more than once, so this usage will be overwritten. -==== tests/cases/conformance/types/spread/spreadDuplicate.ts (2 errors) ==== +==== tests/cases/conformance/types/spread/spreadDuplicate.ts (1 errors) ==== // Repro from #44438 declare let a: { a: string }; @@ -18,13 +17,17 @@ tests/cases/conformance/types/spread/spreadDuplicate.ts(12,12): error TS2783: 'a !!! related TS2785 tests/cases/conformance/types/spread/spreadDuplicate.ts:10:20: This spread always overwrites this property. let b1 = { a: 123, ...b }; // string | number let c1 = { a: 123, ...c }; // string | undefined (Error) - ~~~~~~ -!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten. -!!! related TS2785 tests/cases/conformance/types/spread/spreadDuplicate.ts:12:20: This spread always overwrites this property. let d1 = { a: 123, ...d }; // string | number let a2 = { a: 123, ...(t ? a : {}) }; // string | number let b2 = { a: 123, ...(t ? b : {}) }; // string | number let c2 = { a: 123, ...(t ? c : {}) }; // string | number let d2 = { a: 123, ...(t ? d : {}) }; // string | number + + // from #46463 + declare const conditional: boolean; + const example = { + color: "red", // OK + ...(conditional ? { color: "green" } : { size: "large" }) + } \ No newline at end of file diff --git a/tests/baselines/reference/spreadDuplicate.js b/tests/baselines/reference/spreadDuplicate.js index 70e659052dc..c6adcfaa33a 100644 --- a/tests/baselines/reference/spreadDuplicate.js +++ b/tests/baselines/reference/spreadDuplicate.js @@ -17,6 +17,13 @@ let a2 = { a: 123, ...(t ? a : {}) }; // string | number let b2 = { a: 123, ...(t ? b : {}) }; // string | number let c2 = { a: 123, ...(t ? c : {}) }; // string | number let d2 = { a: 123, ...(t ? d : {}) }; // string | number + +// from #46463 +declare const conditional: boolean; +const example = { + color: "red", // OK + ...(conditional ? { color: "green" } : { size: "large" }) +} //// [spreadDuplicate.js] @@ -41,6 +48,7 @@ var a2 = __assign({ a: 123 }, (t ? a : {})); // string | number var b2 = __assign({ a: 123 }, (t ? b : {})); // string | number var c2 = __assign({ a: 123 }, (t ? c : {})); // string | number var d2 = __assign({ a: 123 }, (t ? d : {})); // string | number +var example = __assign({ color: "red" }, (conditional ? { color: "green" } : { size: "large" })); //// [spreadDuplicate.d.ts] @@ -81,3 +89,10 @@ declare let c2: { declare let d2: { a: string | number; }; +declare const conditional: boolean; +declare const example: { + color: string; +} | { + size: string; + color: string; +}; diff --git a/tests/baselines/reference/spreadDuplicate.symbols b/tests/baselines/reference/spreadDuplicate.symbols index 96ce507f772..f19d1490d15 100644 --- a/tests/baselines/reference/spreadDuplicate.symbols +++ b/tests/baselines/reference/spreadDuplicate.symbols @@ -64,3 +64,19 @@ let d2 = { a: 123, ...(t ? d : {}) }; // string | number >t : Symbol(t, Decl(spreadDuplicate.ts, 7, 11)) >d : Symbol(d, Decl(spreadDuplicate.ts, 5, 11)) +// from #46463 +declare const conditional: boolean; +>conditional : Symbol(conditional, Decl(spreadDuplicate.ts, 20, 13)) + +const example = { +>example : Symbol(example, Decl(spreadDuplicate.ts, 21, 5)) + + color: "red", // OK +>color : Symbol(color, Decl(spreadDuplicate.ts, 21, 17)) + + ...(conditional ? { color: "green" } : { size: "large" }) +>conditional : Symbol(conditional, Decl(spreadDuplicate.ts, 20, 13)) +>color : Symbol(color, Decl(spreadDuplicate.ts, 23, 23)) +>size : Symbol(size, Decl(spreadDuplicate.ts, 23, 44)) +} + diff --git a/tests/baselines/reference/spreadDuplicate.types b/tests/baselines/reference/spreadDuplicate.types index c9fd44e7bf6..a9c34e28507 100644 --- a/tests/baselines/reference/spreadDuplicate.types +++ b/tests/baselines/reference/spreadDuplicate.types @@ -92,3 +92,27 @@ let d2 = { a: 123, ...(t ? d : {}) }; // string | number >d : { a?: string | undefined; } >{} : {} +// from #46463 +declare const conditional: boolean; +>conditional : boolean + +const example = { +>example : { color: string; } | { size: string; color: string; } +>{ color: "red", // OK ...(conditional ? { color: "green" } : { size: "large" })} : { color: string; } | { size: string; color: string; } + + color: "red", // OK +>color : string +>"red" : "red" + + ...(conditional ? { color: "green" } : { size: "large" }) +>(conditional ? { color: "green" } : { size: "large" }) : { color: string; } | { size: string; } +>conditional ? { color: "green" } : { size: "large" } : { color: string; } | { size: string; } +>conditional : boolean +>{ color: "green" } : { color: string; } +>color : string +>"green" : "green" +>{ size: "large" } : { size: string; } +>size : string +>"large" : "large" +} + diff --git a/tests/baselines/reference/spreadDuplicateExact.errors.txt b/tests/baselines/reference/spreadDuplicateExact.errors.txt index 5cba7dac813..945a4064d43 100644 --- a/tests/baselines/reference/spreadDuplicateExact.errors.txt +++ b/tests/baselines/reference/spreadDuplicateExact.errors.txt @@ -1,8 +1,9 @@ tests/cases/conformance/types/spread/spreadDuplicateExact.ts(10,12): error TS2783: 'a' is specified more than once, so this usage will be overwritten. tests/cases/conformance/types/spread/spreadDuplicateExact.ts(12,12): error TS2783: 'a' is specified more than once, so this usage will be overwritten. +tests/cases/conformance/types/spread/spreadDuplicateExact.ts(23,5): error TS2783: 'color' is specified more than once, so this usage will be overwritten. -==== tests/cases/conformance/types/spread/spreadDuplicateExact.ts (2 errors) ==== +==== tests/cases/conformance/types/spread/spreadDuplicateExact.ts (3 errors) ==== // Repro from #44438 declare let a: { a: string }; @@ -27,4 +28,14 @@ tests/cases/conformance/types/spread/spreadDuplicateExact.ts(12,12): error TS278 let b2 = { a: 123, ...(t ? b : {}) }; // string | number let c2 = { a: 123, ...(t ? c : {}) }; // string | number | undefined let d2 = { a: 123, ...(t ? d : {}) }; // string | number | undefined + + // from #46463 + declare const conditional: boolean; + const example = { + color: "red", // error + ~~~~~~~~~~~~ +!!! error TS2783: 'color' is specified more than once, so this usage will be overwritten. +!!! related TS2785 tests/cases/conformance/types/spread/spreadDuplicateExact.ts:24:5: This spread always overwrites this property. + ...(conditional ? { color: "green" } : { size: "large" }) + } \ No newline at end of file diff --git a/tests/baselines/reference/spreadDuplicateExact.js b/tests/baselines/reference/spreadDuplicateExact.js index febbff29f93..f3e114bdc3f 100644 --- a/tests/baselines/reference/spreadDuplicateExact.js +++ b/tests/baselines/reference/spreadDuplicateExact.js @@ -17,6 +17,13 @@ let a2 = { a: 123, ...(t ? a : {}) }; // string | number let b2 = { a: 123, ...(t ? b : {}) }; // string | number let c2 = { a: 123, ...(t ? c : {}) }; // string | number | undefined let d2 = { a: 123, ...(t ? d : {}) }; // string | number | undefined + +// from #46463 +declare const conditional: boolean; +const example = { + color: "red", // error + ...(conditional ? { color: "green" } : { size: "large" }) +} //// [spreadDuplicateExact.js] @@ -41,6 +48,7 @@ var a2 = __assign({ a: 123 }, (t ? a : {})); // string | number var b2 = __assign({ a: 123 }, (t ? b : {})); // string | number var c2 = __assign({ a: 123 }, (t ? c : {})); // string | number | undefined var d2 = __assign({ a: 123 }, (t ? d : {})); // string | number | undefined +var example = __assign({ color: "red" }, (conditional ? { color: "green" } : { size: "large" })); //// [spreadDuplicateExact.d.ts] @@ -81,3 +89,10 @@ declare let c2: { declare let d2: { a: string | number | undefined; }; +declare const conditional: boolean; +declare const example: { + color: string; +} | { + size: string; + color: string; +}; diff --git a/tests/baselines/reference/spreadDuplicateExact.symbols b/tests/baselines/reference/spreadDuplicateExact.symbols index f61e1f242b0..885796b2b8b 100644 --- a/tests/baselines/reference/spreadDuplicateExact.symbols +++ b/tests/baselines/reference/spreadDuplicateExact.symbols @@ -64,3 +64,19 @@ let d2 = { a: 123, ...(t ? d : {}) }; // string | number | undefined >t : Symbol(t, Decl(spreadDuplicateExact.ts, 7, 11)) >d : Symbol(d, Decl(spreadDuplicateExact.ts, 5, 11)) +// from #46463 +declare const conditional: boolean; +>conditional : Symbol(conditional, Decl(spreadDuplicateExact.ts, 20, 13)) + +const example = { +>example : Symbol(example, Decl(spreadDuplicateExact.ts, 21, 5)) + + color: "red", // error +>color : Symbol(color, Decl(spreadDuplicateExact.ts, 21, 17)) + + ...(conditional ? { color: "green" } : { size: "large" }) +>conditional : Symbol(conditional, Decl(spreadDuplicateExact.ts, 20, 13)) +>color : Symbol(color, Decl(spreadDuplicateExact.ts, 23, 23)) +>size : Symbol(size, Decl(spreadDuplicateExact.ts, 23, 44)) +} + diff --git a/tests/baselines/reference/spreadDuplicateExact.types b/tests/baselines/reference/spreadDuplicateExact.types index a4b11003747..bba9bc7753f 100644 --- a/tests/baselines/reference/spreadDuplicateExact.types +++ b/tests/baselines/reference/spreadDuplicateExact.types @@ -92,3 +92,27 @@ let d2 = { a: 123, ...(t ? d : {}) }; // string | number | undefined >d : { a?: string | undefined; } >{} : {} +// from #46463 +declare const conditional: boolean; +>conditional : boolean + +const example = { +>example : { color: string; } | { size: string; color: string; } +>{ color: "red", // error ...(conditional ? { color: "green" } : { size: "large" })} : { color: string; } | { size: string; color: string; } + + color: "red", // error +>color : string +>"red" : "red" + + ...(conditional ? { color: "green" } : { size: "large" }) +>(conditional ? { color: "green" } : { size: "large" }) : { color: string; } | { size: string; } +>conditional ? { color: "green" } : { size: "large" } : { color: string; } | { size: string; } +>conditional : boolean +>{ color: "green" } : { color: string; } +>color : string +>"green" : "green" +>{ size: "large" } : { size: string; } +>size : string +>"large" : "large" +} + diff --git a/tests/baselines/reference/spreadOverwritesPropertyStrict.errors.txt b/tests/baselines/reference/spreadOverwritesPropertyStrict.errors.txt index 448e31384e3..354e7d667a5 100644 --- a/tests/baselines/reference/spreadOverwritesPropertyStrict.errors.txt +++ b/tests/baselines/reference/spreadOverwritesPropertyStrict.errors.txt @@ -1,11 +1,10 @@ tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(3,17): error TS2783: 'b' is specified more than once, so this usage will be overwritten. -tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(9,14): error TS2783: 'x' is specified more than once, so this usage will be overwritten. tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(15,14): error TS2783: 'x' is specified more than once, so this usage will be overwritten. tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(24,14): error TS2783: 'command' is specified more than once, so this usage will be overwritten. tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(28,14): error TS2783: 'a' is specified more than once, so this usage will be overwritten. -==== tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts (5 errors) ==== +==== tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts (4 errors) ==== declare var ab: { a: number, b: number }; declare var abq: { a: number, b?: number }; var unused1 = { b: 1, ...ab } // error @@ -18,9 +17,6 @@ tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(28,14): e var unused5 = { ...abq, b: 1 } // ok function g(obj: { x: number | undefined }) { return { x: 1, ...obj }; // ok, obj might have x: undefined - ~~~~ -!!! error TS2783: 'x' is specified more than once, so this usage will be overwritten. -!!! related TS2785 tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts:9:20: This spread always overwrites this property. } function f(obj: { x: number } | undefined) { return { x: 1, ...obj }; // ok, obj might be undefined diff --git a/tests/cases/conformance/types/spread/spreadDuplicate.ts b/tests/cases/conformance/types/spread/spreadDuplicate.ts index f27a2529f01..5e530befdd3 100644 --- a/tests/cases/conformance/types/spread/spreadDuplicate.ts +++ b/tests/cases/conformance/types/spread/spreadDuplicate.ts @@ -19,3 +19,10 @@ let a2 = { a: 123, ...(t ? a : {}) }; // string | number let b2 = { a: 123, ...(t ? b : {}) }; // string | number let c2 = { a: 123, ...(t ? c : {}) }; // string | number let d2 = { a: 123, ...(t ? d : {}) }; // string | number + +// from #46463 +declare const conditional: boolean; +const example = { + color: "red", // OK + ...(conditional ? { color: "green" } : { size: "large" }) +} diff --git a/tests/cases/conformance/types/spread/spreadDuplicateExact.ts b/tests/cases/conformance/types/spread/spreadDuplicateExact.ts index e395af4ce51..eb181671942 100644 --- a/tests/cases/conformance/types/spread/spreadDuplicateExact.ts +++ b/tests/cases/conformance/types/spread/spreadDuplicateExact.ts @@ -20,3 +20,10 @@ let a2 = { a: 123, ...(t ? a : {}) }; // string | number let b2 = { a: 123, ...(t ? b : {}) }; // string | number let c2 = { a: 123, ...(t ? c : {}) }; // string | number | undefined let d2 = { a: 123, ...(t ? d : {}) }; // string | number | undefined + +// from #46463 +declare const conditional: boolean; +const example = { + color: "red", // error + ...(conditional ? { color: "green" } : { size: "large" }) +}