Skip to content

More precise property-overwritten-by-spread errors #37192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13154,7 +13154,7 @@ namespace ts {
* this function should be called in a left folding style, with left = previous result of getSpreadType
* and right = the new element to be spread.
*/
function getSpreadType(left: Type, right: Type, symbol: Symbol | undefined, objectFlags: ObjectFlags, readonly: boolean, isParentTypeNullable?: boolean): Type {
function getSpreadType(left: Type, right: Type, symbol: Symbol | undefined, objectFlags: ObjectFlags, readonly: boolean): Type {
if (left.flags & TypeFlags.Any || right.flags & TypeFlags.Any) {
return anyType;
}
Expand All @@ -13170,16 +13170,16 @@ namespace ts {
if (left.flags & TypeFlags.Union) {
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(left as UnionType, readonly);
if (merged) {
return getSpreadType(merged, right, symbol, objectFlags, readonly, isParentTypeNullable);
return getSpreadType(merged, right, symbol, objectFlags, readonly);
}
return mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly, isParentTypeNullable));
return mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly));
}
if (right.flags & TypeFlags.Union) {
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(right as UnionType, readonly);
if (merged) {
return getSpreadType(left, merged, symbol, objectFlags, readonly, maybeTypeOfKind(right, TypeFlags.Nullable));
return getSpreadType(left, merged, symbol, objectFlags, readonly);
}
return mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly, maybeTypeOfKind(right, TypeFlags.Nullable)));
return mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly));
}
if (right.flags & (TypeFlags.BooleanLike | TypeFlags.NumberLike | TypeFlags.BigIntLike | TypeFlags.StringLike | TypeFlags.EnumLike | TypeFlags.NonPrimitive | TypeFlags.Index)) {
return left;
Expand Down Expand Up @@ -13243,14 +13243,6 @@ namespace ts {
result.nameType = getSymbolLinks(leftProp).nameType;
members.set(leftProp.escapedName, result);
}
else if (strictNullChecks &&
!isParentTypeNullable &&
symbol &&
!isFromSpreadAssignment(leftProp, symbol) &&
isFromSpreadAssignment(rightProp, symbol) &&
!maybeTypeOfKind(rightType, TypeFlags.Nullable)) {
error(leftProp.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(leftProp.escapedName));
}
}
else {
members.set(leftProp.escapedName, getSpreadSymbol(leftProp, readonly));
Expand Down Expand Up @@ -16781,10 +16773,6 @@ namespace ts {
return match === -1 || discriminable.indexOf(/*searchElement*/ true, match + 1) !== -1 ? defaultValue : target.types[match];
}

function isFromSpreadAssignment(prop: Symbol, container: Symbol) {
return prop.valueDeclaration?.parent !== container.valueDeclaration;
}

/**
* A type is 'weak' if it is an object type with at least one optional property
* and no required properties, call/construct signatures or index signatures
Expand Down Expand Up @@ -22545,6 +22533,7 @@ namespace ts {
checkGrammarObjectLiteralExpression(node, inDestructuringPattern);

let propertiesTable: SymbolTable;
const allPropertiesTable = createSymbolTable();
let propertiesArray: Symbol[] = [];
let spread: Type = emptyObjectType;

Expand Down Expand Up @@ -22626,6 +22615,7 @@ namespace ts {
prop.type = type;
prop.target = member;
member = prop;
allPropertiesTable.set(prop.escapedName, prop);
}
else if (memberDecl.kind === SyntaxKind.SpreadAssignment) {
if (languageVersion < ScriptTarget.ES2015) {
Expand All @@ -22643,6 +22633,16 @@ namespace ts {
error(memberDecl, Diagnostics.Spread_types_may_only_be_created_from_object_types);
return errorType;
}
for (const right of getPropertiesOfType(type)) {
const rightType = getTypeOfSymbol(right);
const left = allPropertiesTable.get(right.escapedName);
if (strictNullChecks &&
left &&
!maybeTypeOfKind(rightType, TypeFlags.Nullable)) {
error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName));
}
}

spread = getSpreadType(spread, type, node.symbol, objectFlags, inConstContext);
offset = i + 1;
continue;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/types/spread/objectSpreadSetonlyAccessor.ts(2,34): error TS2783: 'foo' is specified more than once, so this usage will be overwritten.


==== tests/cases/conformance/types/spread/objectSpreadSetonlyAccessor.ts (1 errors) ====
const o1: { foo: number, bar: undefined } = { foo: 1, ... { set bar(_v: number) { } } }
const o2: { foo: undefined } = { foo: 1, ... { set foo(_v: number) { } } }
~~~~~~
!!! error TS2783: 'foo' is specified more than once, so this usage will be overwritten.

Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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(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 (2 errors) ====
==== tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts (3 errors) ====
declare var ab: { a: number, b: number };
declare var abq: { a: number, b?: number };
var unused1 = { b: 1, ...ab } // error
Expand All @@ -23,4 +24,15 @@ tests/cases/conformance/types/spread/spreadOverwritesPropertyStrict.ts(15,14): e
~~~~
!!! error TS2783: 'x' is specified more than once, so this usage will be overwritten.
}
function i(b: boolean, t: { command: string, ok: string }) {
return { command: "hi", ...(b ? t : {}) } // ok
}
function j() {
return { ...{ command: "hi" } , ...{ command: "bye" } } // ok
}
function k(t: { command: string, ok: string }) {
return { command: "hi", ...{ spoiler: true }, spoiler2: true, ...t } // error
~~~~~~~~~~~~~
!!! error TS2783: 'command' is specified more than once, so this usage will be overwritten.
}

18 changes: 18 additions & 0 deletions tests/baselines/reference/spreadOverwritesPropertyStrict.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ function f(obj: { x: number } | undefined) {
function h(obj: { x: number } | { x: string }) {
return { x: 1, ...obj } // error
}
function i(b: boolean, t: { command: string, ok: string }) {
return { command: "hi", ...(b ? t : {}) } // ok
}
function j() {
return { ...{ command: "hi" } , ...{ command: "bye" } } // ok
}
function k(t: { command: string, ok: string }) {
return { command: "hi", ...{ spoiler: true }, spoiler2: true, ...t } // error
}


//// [spreadOverwritesPropertyStrict.js]
Expand Down Expand Up @@ -44,3 +53,12 @@ function f(obj) {
function h(obj) {
return __assign({ x: 1 }, obj); // error
}
function i(b, t) {
return __assign({ command: "hi" }, (b ? t : {})); // ok
}
function j() {
return __assign({ command: "hi" }, { command: "bye" }); // ok
}
function k(t) {
return __assign(__assign(__assign({ command: "hi" }, { spoiler: true }), { spoiler2: true }), t); // error
}
31 changes: 31 additions & 0 deletions tests/baselines/reference/spreadOverwritesPropertyStrict.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,35 @@ function h(obj: { x: number } | { x: string }) {
>x : Symbol(x, Decl(spreadOverwritesPropertyStrict.ts, 14, 12))
>obj : Symbol(obj, Decl(spreadOverwritesPropertyStrict.ts, 13, 11))
}
function i(b: boolean, t: { command: string, ok: string }) {
>i : Symbol(i, Decl(spreadOverwritesPropertyStrict.ts, 15, 1))
>b : Symbol(b, Decl(spreadOverwritesPropertyStrict.ts, 16, 11))
>t : Symbol(t, Decl(spreadOverwritesPropertyStrict.ts, 16, 22))
>command : Symbol(command, Decl(spreadOverwritesPropertyStrict.ts, 16, 27))
>ok : Symbol(ok, Decl(spreadOverwritesPropertyStrict.ts, 16, 44))

return { command: "hi", ...(b ? t : {}) } // ok
>command : Symbol(command, Decl(spreadOverwritesPropertyStrict.ts, 17, 12))
>b : Symbol(b, Decl(spreadOverwritesPropertyStrict.ts, 16, 11))
>t : Symbol(t, Decl(spreadOverwritesPropertyStrict.ts, 16, 22))
}
function j() {
>j : Symbol(j, Decl(spreadOverwritesPropertyStrict.ts, 18, 1))

return { ...{ command: "hi" } , ...{ command: "bye" } } // ok
>command : Symbol(command, Decl(spreadOverwritesPropertyStrict.ts, 20, 17))
>command : Symbol(command, Decl(spreadOverwritesPropertyStrict.ts, 20, 40))
}
function k(t: { command: string, ok: string }) {
>k : Symbol(k, Decl(spreadOverwritesPropertyStrict.ts, 21, 1))
>t : Symbol(t, Decl(spreadOverwritesPropertyStrict.ts, 22, 11))
>command : Symbol(command, Decl(spreadOverwritesPropertyStrict.ts, 22, 15))
>ok : Symbol(ok, Decl(spreadOverwritesPropertyStrict.ts, 22, 32))

return { command: "hi", ...{ spoiler: true }, spoiler2: true, ...t } // error
>command : Symbol(command, Decl(spreadOverwritesPropertyStrict.ts, 23, 12))
>spoiler : Symbol(spoiler, Decl(spreadOverwritesPropertyStrict.ts, 23, 32))
>spoiler2 : Symbol(spoiler2, Decl(spreadOverwritesPropertyStrict.ts, 23, 49))
>t : Symbol(t, Decl(spreadOverwritesPropertyStrict.ts, 22, 11))
}

46 changes: 46 additions & 0 deletions tests/baselines/reference/spreadOverwritesPropertyStrict.types
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,50 @@ function h(obj: { x: number } | { x: string }) {
>1 : 1
>obj : { x: number; } | { x: string; }
}
function i(b: boolean, t: { command: string, ok: string }) {
>i : (b: boolean, t: { command: string; ok: string; }) => { command: string; ok: string; } | { command: string; }
>b : boolean
>t : { command: string; ok: string; }
>command : string
>ok : string

return { command: "hi", ...(b ? t : {}) } // ok
>{ command: "hi", ...(b ? t : {}) } : { command: string; ok: string; } | { command: string; }
>command : string
>"hi" : "hi"
>(b ? t : {}) : { command: string; ok: string; } | {}
>b ? t : {} : { command: string; ok: string; } | {}
>b : boolean
>t : { command: string; ok: string; }
>{} : {}
}
function j() {
>j : () => { command: string; }

return { ...{ command: "hi" } , ...{ command: "bye" } } // ok
>{ ...{ command: "hi" } , ...{ command: "bye" } } : { command: string; }
>{ command: "hi" } : { command: string; }
>command : string
>"hi" : "hi"
>{ command: "bye" } : { command: string; }
>command : string
>"bye" : "bye"
}
function k(t: { command: string, ok: string }) {
>k : (t: { command: string; ok: string; }) => { command: string; ok: string; spoiler2: boolean; spoiler: boolean; }
>t : { command: string; ok: string; }
>command : string
>ok : string

return { command: "hi", ...{ spoiler: true }, spoiler2: true, ...t } // error
>{ command: "hi", ...{ spoiler: true }, spoiler2: true, ...t } : { command: string; ok: string; spoiler2: boolean; spoiler: boolean; }
>command : string
>"hi" : "hi"
>{ spoiler: true } : { spoiler: boolean; }
>spoiler : boolean
>true : true
>spoiler2 : boolean
>true : true
>t : { command: string; ok: string; }
}

Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,12 @@ function f(obj: { x: number } | undefined) {
function h(obj: { x: number } | { x: string }) {
return { x: 1, ...obj } // error
}
function i(b: boolean, t: { command: string, ok: string }) {
return { command: "hi", ...(b ? t : {}) } // ok
}
function j() {
return { ...{ command: "hi" } , ...{ command: "bye" } } // ok
}
function k(t: { command: string, ok: string }) {
return { command: "hi", ...{ spoiler: true }, spoiler2: true, ...t } // error
}