Skip to content

Commit 61ccc49

Browse files
ahejlsbergahejlsberg
authored
Fix check for overwritten properties in object spreads (#44696)
* Fix check for overwritten properties in object spreads * Accept new baselines * Add tests * Accept new baselines
1 parent 22637a2 commit 61ccc49

12 files changed

Lines changed: 616 additions & 24 deletions

‎src/compiler/checker.ts‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15368,17 +15368,20 @@ namespace ts {
1536815368
return isEmptyObjectType(type) || !!(type.flags & (TypeFlags.Null | TypeFlags.Undefined | TypeFlags.BooleanLike | TypeFlags.NumberLike | TypeFlags.BigIntLike | TypeFlags.StringLike | TypeFlags.EnumLike | TypeFlags.NonPrimitive | TypeFlags.Index));
1536915369
}
1537015370

15371-
function tryMergeUnionOfObjectTypeAndEmptyObject(type: UnionType, readonly: boolean): Type | undefined {
15372-
if (every(type.types, isEmptyObjectTypeOrSpreadsIntoEmptyObject)) {
15373-
return find(type.types, isEmptyObjectType) || emptyObjectType;
15371+
function tryMergeUnionOfObjectTypeAndEmptyObject(type: Type, readonly: boolean): Type {
15372+
if (!(type.flags & TypeFlags.Union)) {
15373+
return type;
15374+
}
15375+
if (every((type as UnionType).types, isEmptyObjectTypeOrSpreadsIntoEmptyObject)) {
15376+
return find((type as UnionType).types, isEmptyObjectType) || emptyObjectType;
1537415377
}
15375-
const firstType = find(type.types, t => !isEmptyObjectTypeOrSpreadsIntoEmptyObject(t));
15378+
const firstType = find((type as UnionType).types, t => !isEmptyObjectTypeOrSpreadsIntoEmptyObject(t));
1537615379
if (!firstType) {
15377-
return undefined;
15380+
return type;
1537815381
}
15379-
const secondType = firstType && find(type.types, t => t !== firstType && !isEmptyObjectTypeOrSpreadsIntoEmptyObject(t));
15382+
const secondType = find((type as UnionType).types, t => t !== firstType && !isEmptyObjectTypeOrSpreadsIntoEmptyObject(t));
1538015383
if (secondType) {
15381-
return undefined;
15384+
return type;
1538215385
}
1538315386
return getAnonymousPartialType(firstType);
1538415387

@@ -15424,20 +15427,14 @@ namespace ts {
1542415427
if (right.flags & TypeFlags.Never) {
1542515428
return left;
1542615429
}
15430+
left = tryMergeUnionOfObjectTypeAndEmptyObject(left, readonly);
1542715431
if (left.flags & TypeFlags.Union) {
15428-
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(left as UnionType, readonly);
15429-
if (merged) {
15430-
return getSpreadType(merged, right, symbol, objectFlags, readonly);
15431-
}
1543215432
return checkCrossProductUnion([left, right])
1543315433
? mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly))
1543415434
: errorType;
1543515435
}
15436+
right = tryMergeUnionOfObjectTypeAndEmptyObject(right, readonly);
1543615437
if (right.flags & TypeFlags.Union) {
15437-
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(right as UnionType, readonly);
15438-
if (merged) {
15439-
return getSpreadType(left, merged, symbol, objectFlags, readonly);
15440-
}
1544115438
return checkCrossProductUnion([left, right])
1544215439
? mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly))
1544315440
: errorType;
@@ -15487,7 +15484,7 @@ namespace ts {
1548715484
const declarations = concatenate(leftProp.declarations, rightProp.declarations);
1548815485
const flags = SymbolFlags.Property | (leftProp.flags & SymbolFlags.Optional);
1548915486
const result = createSymbol(flags, leftProp.escapedName);
15490-
result.type = getUnionType([getTypeOfSymbol(leftProp), getTypeWithFacts(rightType, TypeFacts.NEUndefined)]);
15487+
result.type = getUnionType([getTypeOfSymbol(leftProp), removeMissingOrUndefinedType(rightType)]);
1549115488
result.leftSpread = leftProp;
1549215489
result.rightSpread = rightProp;
1549315490
result.declarations = declarations;
@@ -26310,14 +26307,15 @@ namespace ts {
2631026307
}
2631126308
const type = getReducedType(checkExpression(memberDecl.expression));
2631226309
if (isValidSpreadType(type)) {
26310+
const mergedType = tryMergeUnionOfObjectTypeAndEmptyObject(type, inConstContext);
2631326311
if (allPropertiesTable) {
26314-
checkSpreadPropOverrides(type, allPropertiesTable, memberDecl);
26312+
checkSpreadPropOverrides(mergedType, allPropertiesTable, memberDecl);
2631526313
}
2631626314
offset = propertiesArray.length;
2631726315
if (spread === errorType) {
2631826316
continue;
2631926317
}
26320-
spread = getSpreadType(spread, type, node.symbol, objectFlags, inConstContext);
26318+
spread = getSpreadType(spread, mergedType, node.symbol, objectFlags, inConstContext);
2632126319
}
2632226320
else {
2632326321
error(memberDecl, Diagnostics.Spread_types_may_only_be_created_from_object_types);
@@ -26632,11 +26630,12 @@ namespace ts {
2663226630

2663326631
function checkSpreadPropOverrides(type: Type, props: SymbolTable, spread: SpreadAssignment | JsxSpreadAttribute) {
2663426632
for (const right of getPropertiesOfType(type)) {
26635-
const left = props.get(right.escapedName);
26636-
const rightType = getTypeOfSymbol(right);
26637-
if (left && !maybeTypeOfKind(rightType, TypeFlags.Nullable) && !(maybeTypeOfKind(rightType, TypeFlags.AnyOrUnknown) && right.flags & SymbolFlags.Optional)) {
26638-
const diagnostic = error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName));
26639-
addRelatedInfo(diagnostic, createDiagnosticForNode(spread, Diagnostics.This_spread_always_overwrites_this_property));
26633+
if (!(right.flags & SymbolFlags.Optional)) {
26634+
const left = props.get(right.escapedName);
26635+
if (left) {
26636+
const diagnostic = error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName));
26637+
addRelatedInfo(diagnostic, createDiagnosticForNode(spread, Diagnostics.This_spread_always_overwrites_this_property));
26638+
}
2664026639
}
2664126640
}
2664226641
}

‎tests/baselines/reference/spreadDuplicate.errors.txt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
tests/cases/conformance/types/spread/spreadDuplicate.ts(10,12): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
2+
tests/cases/conformance/types/spread/spreadDuplicate.ts(12,12): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
3+
4+
5+
==== tests/cases/conformance/types/spread/spreadDuplicate.ts (2 errors) ====
6+
// Repro from #44438
7+
8+
declare let a: { a: string };
9+
declare let b: { a?: string };
10+
declare let c: { a: string | undefined };
11+
declare let d: { a?: string | undefined };
12+
13+
declare let t: boolean;
14+
15+
let a1 = { a: 123, ...a }; // string (Error)
16+
~~~~~~
17+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
18+
!!! related TS2785 tests/cases/conformance/types/spread/spreadDuplicate.ts:10:20: This spread always overwrites this property.
19+
let b1 = { a: 123, ...b }; // string | number
20+
let c1 = { a: 123, ...c }; // string | undefined (Error)
21+
~~~~~~
22+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
23+
!!! related TS2785 tests/cases/conformance/types/spread/spreadDuplicate.ts:12:20: This spread always overwrites this property.
24+
let d1 = { a: 123, ...d }; // string | number
25+
26+
let a2 = { a: 123, ...(t ? a : {}) }; // string | number
27+
let b2 = { a: 123, ...(t ? b : {}) }; // string | number
28+
let c2 = { a: 123, ...(t ? c : {}) }; // string | number
29+
let d2 = { a: 123, ...(t ? d : {}) }; // string | number
30+

‎tests/baselines/reference/spreadDuplicate.js‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
//// [spreadDuplicate.ts]
2+
// Repro from #44438
3+
4+
declare let a: { a: string };
5+
declare let b: { a?: string };
6+
declare let c: { a: string | undefined };
7+
declare let d: { a?: string | undefined };
8+
9+
declare let t: boolean;
10+
11+
let a1 = { a: 123, ...a }; // string (Error)
12+
let b1 = { a: 123, ...b }; // string | number
13+
let c1 = { a: 123, ...c }; // string | undefined (Error)
14+
let d1 = { a: 123, ...d }; // string | number
15+
16+
let a2 = { a: 123, ...(t ? a : {}) }; // string | number
17+
let b2 = { a: 123, ...(t ? b : {}) }; // string | number
18+
let c2 = { a: 123, ...(t ? c : {}) }; // string | number
19+
let d2 = { a: 123, ...(t ? d : {}) }; // string | number
20+
21+
22+
//// [spreadDuplicate.js]
23+
"use strict";
24+
// Repro from #44438
25+
var __assign = (this && this.__assign) || function () {
26+
__assign = Object.assign || function(t) {
27+
for (var s, i = 1, n = arguments.length; i < n; i++) {
28+
s = arguments[i];
29+
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
30+
t[p] = s[p];
31+
}
32+
return t;
33+
};
34+
return __assign.apply(this, arguments);
35+
};
36+
var a1 = __assign({ a: 123 }, a); // string (Error)
37+
var b1 = __assign({ a: 123 }, b); // string | number
38+
var c1 = __assign({ a: 123 }, c); // string | undefined (Error)
39+
var d1 = __assign({ a: 123 }, d); // string | number
40+
var a2 = __assign({ a: 123 }, (t ? a : {})); // string | number
41+
var b2 = __assign({ a: 123 }, (t ? b : {})); // string | number
42+
var c2 = __assign({ a: 123 }, (t ? c : {})); // string | number
43+
var d2 = __assign({ a: 123 }, (t ? d : {})); // string | number
44+
45+
46+
//// [spreadDuplicate.d.ts]
47+
declare let a: {
48+
a: string;
49+
};
50+
declare let b: {
51+
a?: string;
52+
};
53+
declare let c: {
54+
a: string | undefined;
55+
};
56+
declare let d: {
57+
a?: string | undefined;
58+
};
59+
declare let t: boolean;
60+
declare let a1: {
61+
a: string;
62+
};
63+
declare let b1: {
64+
a: string | number;
65+
};
66+
declare let c1: {
67+
a: string | undefined;
68+
};
69+
declare let d1: {
70+
a: string | number;
71+
};
72+
declare let a2: {
73+
a: string | number;
74+
};
75+
declare let b2: {
76+
a: string | number;
77+
};
78+
declare let c2: {
79+
a: string | number;
80+
};
81+
declare let d2: {
82+
a: string | number;
83+
};

‎tests/baselines/reference/spreadDuplicate.symbols‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
=== tests/cases/conformance/types/spread/spreadDuplicate.ts ===
2+
// Repro from #44438
3+
4+
declare let a: { a: string };
5+
>a : Symbol(a, Decl(spreadDuplicate.ts, 2, 11))
6+
>a : Symbol(a, Decl(spreadDuplicate.ts, 2, 16))
7+
8+
declare let b: { a?: string };
9+
>b : Symbol(b, Decl(spreadDuplicate.ts, 3, 11))
10+
>a : Symbol(a, Decl(spreadDuplicate.ts, 3, 16))
11+
12+
declare let c: { a: string | undefined };
13+
>c : Symbol(c, Decl(spreadDuplicate.ts, 4, 11))
14+
>a : Symbol(a, Decl(spreadDuplicate.ts, 4, 16))
15+
16+
declare let d: { a?: string | undefined };
17+
>d : Symbol(d, Decl(spreadDuplicate.ts, 5, 11))
18+
>a : Symbol(a, Decl(spreadDuplicate.ts, 5, 16))
19+
20+
declare let t: boolean;
21+
>t : Symbol(t, Decl(spreadDuplicate.ts, 7, 11))
22+
23+
let a1 = { a: 123, ...a }; // string (Error)
24+
>a1 : Symbol(a1, Decl(spreadDuplicate.ts, 9, 3))
25+
>a : Symbol(a, Decl(spreadDuplicate.ts, 9, 10))
26+
>a : Symbol(a, Decl(spreadDuplicate.ts, 2, 11))
27+
28+
let b1 = { a: 123, ...b }; // string | number
29+
>b1 : Symbol(b1, Decl(spreadDuplicate.ts, 10, 3))
30+
>a : Symbol(a, Decl(spreadDuplicate.ts, 10, 10))
31+
>b : Symbol(b, Decl(spreadDuplicate.ts, 3, 11))
32+
33+
let c1 = { a: 123, ...c }; // string | undefined (Error)
34+
>c1 : Symbol(c1, Decl(spreadDuplicate.ts, 11, 3))
35+
>a : Symbol(a, Decl(spreadDuplicate.ts, 11, 10))
36+
>c : Symbol(c, Decl(spreadDuplicate.ts, 4, 11))
37+
38+
let d1 = { a: 123, ...d }; // string | number
39+
>d1 : Symbol(d1, Decl(spreadDuplicate.ts, 12, 3))
40+
>a : Symbol(a, Decl(spreadDuplicate.ts, 12, 10))
41+
>d : Symbol(d, Decl(spreadDuplicate.ts, 5, 11))
42+
43+
let a2 = { a: 123, ...(t ? a : {}) }; // string | number
44+
>a2 : Symbol(a2, Decl(spreadDuplicate.ts, 14, 3))
45+
>a : Symbol(a, Decl(spreadDuplicate.ts, 14, 10))
46+
>t : Symbol(t, Decl(spreadDuplicate.ts, 7, 11))
47+
>a : Symbol(a, Decl(spreadDuplicate.ts, 2, 11))
48+
49+
let b2 = { a: 123, ...(t ? b : {}) }; // string | number
50+
>b2 : Symbol(b2, Decl(spreadDuplicate.ts, 15, 3))
51+
>a : Symbol(a, Decl(spreadDuplicate.ts, 15, 10))
52+
>t : Symbol(t, Decl(spreadDuplicate.ts, 7, 11))
53+
>b : Symbol(b, Decl(spreadDuplicate.ts, 3, 11))
54+
55+
let c2 = { a: 123, ...(t ? c : {}) }; // string | number
56+
>c2 : Symbol(c2, Decl(spreadDuplicate.ts, 16, 3))
57+
>a : Symbol(a, Decl(spreadDuplicate.ts, 16, 10))
58+
>t : Symbol(t, Decl(spreadDuplicate.ts, 7, 11))
59+
>c : Symbol(c, Decl(spreadDuplicate.ts, 4, 11))
60+
61+
let d2 = { a: 123, ...(t ? d : {}) }; // string | number
62+
>d2 : Symbol(d2, Decl(spreadDuplicate.ts, 17, 3))
63+
>a : Symbol(a, Decl(spreadDuplicate.ts, 17, 10))
64+
>t : Symbol(t, Decl(spreadDuplicate.ts, 7, 11))
65+
>d : Symbol(d, Decl(spreadDuplicate.ts, 5, 11))
66+

‎tests/baselines/reference/spreadDuplicate.types‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
=== tests/cases/conformance/types/spread/spreadDuplicate.ts ===
2+
// Repro from #44438
3+
4+
declare let a: { a: string };
5+
>a : { a: string; }
6+
>a : string
7+
8+
declare let b: { a?: string };
9+
>b : { a?: string | undefined; }
10+
>a : string | undefined
11+
12+
declare let c: { a: string | undefined };
13+
>c : { a: string | undefined; }
14+
>a : string | undefined
15+
16+
declare let d: { a?: string | undefined };
17+
>d : { a?: string | undefined; }
18+
>a : string | undefined
19+
20+
declare let t: boolean;
21+
>t : boolean
22+
23+
let a1 = { a: 123, ...a }; // string (Error)
24+
>a1 : { a: string; }
25+
>{ a: 123, ...a } : { a: string; }
26+
>a : number
27+
>123 : 123
28+
>a : { a: string; }
29+
30+
let b1 = { a: 123, ...b }; // string | number
31+
>b1 : { a: string | number; }
32+
>{ a: 123, ...b } : { a: string | number; }
33+
>a : number
34+
>123 : 123
35+
>b : { a?: string | undefined; }
36+
37+
let c1 = { a: 123, ...c }; // string | undefined (Error)
38+
>c1 : { a: string | undefined; }
39+
>{ a: 123, ...c } : { a: string | undefined; }
40+
>a : number
41+
>123 : 123
42+
>c : { a: string | undefined; }
43+
44+
let d1 = { a: 123, ...d }; // string | number
45+
>d1 : { a: string | number; }
46+
>{ a: 123, ...d } : { a: string | number; }
47+
>a : number
48+
>123 : 123
49+
>d : { a?: string | undefined; }
50+
51+
let a2 = { a: 123, ...(t ? a : {}) }; // string | number
52+
>a2 : { a: string | number; }
53+
>{ a: 123, ...(t ? a : {}) } : { a: string | number; }
54+
>a : number
55+
>123 : 123
56+
>(t ? a : {}) : { a: string; } | {}
57+
>t ? a : {} : { a: string; } | {}
58+
>t : boolean
59+
>a : { a: string; }
60+
>{} : {}
61+
62+
let b2 = { a: 123, ...(t ? b : {}) }; // string | number
63+
>b2 : { a: string | number; }
64+
>{ a: 123, ...(t ? b : {}) } : { a: string | number; }
65+
>a : number
66+
>123 : 123
67+
>(t ? b : {}) : { a?: string | undefined; }
68+
>t ? b : {} : { a?: string | undefined; }
69+
>t : boolean
70+
>b : { a?: string | undefined; }
71+
>{} : {}
72+
73+
let c2 = { a: 123, ...(t ? c : {}) }; // string | number
74+
>c2 : { a: string | number; }
75+
>{ a: 123, ...(t ? c : {}) } : { a: string | number; }
76+
>a : number
77+
>123 : 123
78+
>(t ? c : {}) : { a: string | undefined; } | {}
79+
>t ? c : {} : { a: string | undefined; } | {}
80+
>t : boolean
81+
>c : { a: string | undefined; }
82+
>{} : {}
83+
84+
let d2 = { a: 123, ...(t ? d : {}) }; // string | number
85+
>d2 : { a: string | number; }
86+
>{ a: 123, ...(t ? d : {}) } : { a: string | number; }
87+
>a : number
88+
>123 : 123
89+
>(t ? d : {}) : { a?: string | undefined; }
90+
>t ? d : {} : { a?: string | undefined; }
91+
>t : boolean
92+
>d : { a?: string | undefined; }
93+
>{} : {}
94+

0 commit comments

Comments
 (0)