Skip to content

Commit a772c26

Browse files
authored
Error when property is specified more than once via a spread (#36727)
* add tests but not baselines or fixes * Update original change Still probably wrong; probably doesn't even compile beacuse I'm just typing on my laptop. * fix error code ok * notes to self * Error: property is specified more than once via spread * make jsx tests stricter * update semicolon error message * use ?. because it is great * use maybeTypeOfKind in new code * restore jsx error * add tests
1 parent 348c4dd commit a772c26

26 files changed

+1080
-602
lines changed

src/compiler/checker.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13032,7 +13032,7 @@ namespace ts {
1303213032
* this function should be called in a left folding style, with left = previous result of getSpreadType
1303313033
* and right = the new element to be spread.
1303413034
*/
13035-
function getSpreadType(left: Type, right: Type, symbol: Symbol | undefined, objectFlags: ObjectFlags, readonly: boolean): Type {
13035+
function getSpreadType(left: Type, right: Type, symbol: Symbol | undefined, objectFlags: ObjectFlags, readonly: boolean, isParentTypeNullable?: boolean): Type {
1303613036
if (left.flags & TypeFlags.Any || right.flags & TypeFlags.Any) {
1303713037
return anyType;
1303813038
}
@@ -13048,16 +13048,16 @@ namespace ts {
1304813048
if (left.flags & TypeFlags.Union) {
1304913049
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(left as UnionType, readonly);
1305013050
if (merged) {
13051-
return getSpreadType(merged, right, symbol, objectFlags, readonly);
13051+
return getSpreadType(merged, right, symbol, objectFlags, readonly, isParentTypeNullable);
1305213052
}
13053-
return mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly));
13053+
return mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly, isParentTypeNullable));
1305413054
}
1305513055
if (right.flags & TypeFlags.Union) {
1305613056
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(right as UnionType, readonly);
1305713057
if (merged) {
13058-
return getSpreadType(left, merged, symbol, objectFlags, readonly);
13058+
return getSpreadType(left, merged, symbol, objectFlags, readonly, maybeTypeOfKind(right, TypeFlags.Nullable));
1305913059
}
13060-
return mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly));
13060+
return mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly, maybeTypeOfKind(right, TypeFlags.Nullable)));
1306113061
}
1306213062
if (right.flags & (TypeFlags.BooleanLike | TypeFlags.NumberLike | TypeFlags.BigIntLike | TypeFlags.StringLike | TypeFlags.EnumLike | TypeFlags.NonPrimitive | TypeFlags.Index)) {
1306313063
return left;
@@ -13121,6 +13121,14 @@ namespace ts {
1312113121
result.nameType = getSymbolLinks(leftProp).nameType;
1312213122
members.set(leftProp.escapedName, result);
1312313123
}
13124+
else if (strictNullChecks &&
13125+
!isParentTypeNullable &&
13126+
symbol &&
13127+
!isFromSpreadAssignment(leftProp, symbol) &&
13128+
isFromSpreadAssignment(rightProp, symbol) &&
13129+
!maybeTypeOfKind(rightType, TypeFlags.Nullable)) {
13130+
error(leftProp.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(leftProp.escapedName));
13131+
}
1312413132
}
1312513133
else {
1312613134
members.set(leftProp.escapedName, getSpreadSymbol(leftProp, readonly));
@@ -16620,6 +16628,10 @@ namespace ts {
1662016628
return match === -1 || discriminable.indexOf(/*searchElement*/ true, match + 1) !== -1 ? defaultValue : target.types[match];
1662116629
}
1662216630

16631+
function isFromSpreadAssignment(prop: Symbol, container: Symbol) {
16632+
return prop.valueDeclaration?.parent !== container.valueDeclaration;
16633+
}
16634+
1662316635
/**
1662416636
* A type is 'weak' if it is an object type with at least one optional property
1662516637
* and no required properties, call/construct signatures or index signatures
@@ -25178,7 +25190,7 @@ namespace ts {
2517825190
if (node.arguments.length === 1) {
2517925191
const text = getSourceFileOfNode(node).text;
2518025192
if (isLineBreak(text.charCodeAt(skipTrivia(text, node.expression.end, /* stopAfterLineBreak */ true) - 1))) {
25181-
relatedInformation = createDiagnosticForNode(node.expression, Diagnostics.It_is_highly_likely_that_you_are_missing_a_semicolon);
25193+
relatedInformation = createDiagnosticForNode(node.expression, Diagnostics.Are_you_missing_a_semicolon);
2518225194
}
2518325195
}
2518425196
invocationError(node.expression, apparentType, SignatureKind.Call, relatedInformation);

src/compiler/diagnosticMessages.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2691,7 +2691,7 @@
26912691
"category": "Error",
26922692
"code": 2733
26932693
},
2694-
"It is highly likely that you are missing a semicolon.": {
2694+
"Are you missing a semicolon?": {
26952695
"category": "Error",
26962696
"code": 2734
26972697
},
@@ -2887,6 +2887,10 @@
28872887
"category": "Message",
28882888
"code": 2782
28892889
},
2890+
"'{0}' is specified more than once, so this usage will be overwritten.": {
2891+
"category": "Error",
2892+
"code": 2783
2893+
},
28902894

28912895
"Import declaration '{0}' is using private name '{1}'.": {
28922896
"category": "Error",

tests/baselines/reference/betterErrorForAccidentalCall.errors.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,20 @@ tests/cases/compiler/betterErrorForAccidentalCall.ts(13,1): error TS2349: This e
2727
~~~~~
2828
!!! error TS2349: This expression is not callable.
2929
!!! error TS2349: Type 'String' has no call signatures.
30-
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:7:1: It is highly likely that you are missing a semicolon.
30+
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:7:1: Are you missing a semicolon?
3131
(1 as number).toString();
3232

3333
foo()
3434
~~~~~
3535
!!! error TS2349: This expression is not callable.
3636
!!! error TS2349: Type 'String' has no call signatures.
37-
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:10:1: It is highly likely that you are missing a semicolon.
37+
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:10:1: Are you missing a semicolon?
3838
(1 + 2).toString();
3939

4040
foo()
4141
~~~~~
4242
!!! error TS2349: This expression is not callable.
4343
!!! error TS2349: Type 'String' has no call signatures.
44-
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:13:1: It is highly likely that you are missing a semicolon.
44+
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:13:1: Are you missing a semicolon?
4545
(<number>1).toString();
4646

tests/baselines/reference/checkJsxChildrenProperty13.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ class Button extends React.Component<ButtonProp, any> {
2727
>(<InnerButton {...this.props} children="hi"> <div>Hello World</div> </InnerButton>) : JSX.Element
2828
><InnerButton {...this.props} children="hi"> <div>Hello World</div> </InnerButton> : JSX.Element
2929
>InnerButton : typeof InnerButton
30-
>this.props : ButtonProp & { children?: React.ReactNode; }
30+
>this.props : ButtonProp & { children?: string | number | boolean | {} | React.ReactElement<any> | (string | number | boolean | any[] | React.ReactElement<any>)[] | undefined; }
3131
>this : this
32-
>props : ButtonProp & { children?: React.ReactNode; }
32+
>props : ButtonProp & { children?: string | number | boolean | {} | React.ReactElement<any> | (string | number | boolean | any[] | React.ReactElement<any>)[] | undefined; }
3333
>children : string
3434

3535
<div>Hello World</div>

tests/baselines/reference/checkJsxChildrenProperty2.errors.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,5 @@ tests/cases/conformance/jsx/file.tsx(49,6): error TS2746: This JSX tag's 'childr
7171
!!! error TS2746: This JSX tag's 'children' prop expects a single child of type 'string | Element', but multiple children were provided.
7272
<div> My Div </div>
7373
<div> My Div </div>
74-
</Comp>;
74+
</Comp>;
75+

tests/baselines/reference/checkJsxChildrenProperty2.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ let k5 =
5050
<Comp a={10} b="hi" >
5151
<div> My Div </div>
5252
<div> My Div </div>
53-
</Comp>;
53+
</Comp>;
54+
5455

5556
//// [file.jsx]
5657
"use strict";

tests/baselines/reference/objectSpread.js

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,14 @@ let addAfter: { a: number, b: string, c: boolean } =
77
{ ...o, c: false }
88
let addBefore: { a: number, b: string, c: boolean } =
99
{ c: false, ...o }
10-
// Note: ignore still changes the order that properties are printed
11-
let ignore: { a: number, b: string } =
12-
{ b: 'ignored', ...o }
1310
let override: { a: number, b: string } =
1411
{ ...o, b: 'override' }
1512
let nested: { a: number, b: boolean, c: string } =
1613
{ ...{ a: 3, ...{ b: false, c: 'overriden' } }, c: 'whatever' }
1714
let combined: { a: number, b: string, c: boolean } =
1815
{ ...o, ...o2 }
19-
let combinedBefore: { a: number, b: string, c: boolean } =
20-
{ b: 'ok', ...o, ...o2 }
21-
let combinedMid: { a: number, b: string, c: boolean } =
22-
{ ...o, b: 'ok', ...o2 }
2316
let combinedAfter: { a: number, b: string, c: boolean } =
2417
{ ...o, ...o2, b: 'ok' }
25-
let combinedNested: { a: number, b: boolean, c: string, d: string } =
26-
{ ...{ a: 4, ...{ b: false, c: 'overriden' } }, d: 'actually new', ...{ a: 5, d: 'maybe new' } }
2718
let combinedNestedChangeType: { a: number, b: boolean, c: number } =
2819
{ ...{ a: 1, ...{ b: false, c: 'overriden' } }, c: -1 }
2920
let propertyNested: { a: { a: number, b: string } } =
@@ -91,8 +82,6 @@ cplus.plus();
9182
// new field's type conflicting with existing field is OK
9283
let changeTypeAfter: { a: string, b: string } =
9384
{ ...o, a: 'wrong type?' }
94-
let changeTypeBefore: { a: number, b: string } =
95-
{ a: 'wrong type?', ...o };
9685
let changeTypeBoth: { a: string, b: number } =
9786
{ ...o, ...swap };
9887

@@ -109,8 +98,6 @@ function container(
10998
// computed property
11099
let computedFirst: { a: number, b: string, "before everything": number } =
111100
{ ['before everything']: 12, ...o, b: 'yes' }
112-
let computedMiddle: { a: number, b: string, c: boolean, "in the middle": number } =
113-
{ ...o, ['in the middle']: 13, b: 'maybe?', ...o2 }
114101
let computedAfter: { a: number, b: string, "at the end": number } =
115102
{ ...o, b: 'yeah', ['at the end']: 14 }
116103
}
@@ -173,15 +160,10 @@ var o2 = { b: 'yes', c: true };
173160
var swap = { a: 'yes', b: -1 };
174161
var addAfter = __assign(__assign({}, o), { c: false });
175162
var addBefore = __assign({ c: false }, o);
176-
// Note: ignore still changes the order that properties are printed
177-
var ignore = __assign({ b: 'ignored' }, o);
178163
var override = __assign(__assign({}, o), { b: 'override' });
179164
var nested = __assign(__assign({}, __assign({ a: 3 }, { b: false, c: 'overriden' })), { c: 'whatever' });
180165
var combined = __assign(__assign({}, o), o2);
181-
var combinedBefore = __assign(__assign({ b: 'ok' }, o), o2);
182-
var combinedMid = __assign(__assign(__assign({}, o), { b: 'ok' }), o2);
183166
var combinedAfter = __assign(__assign(__assign({}, o), o2), { b: 'ok' });
184-
var combinedNested = __assign(__assign(__assign({}, __assign({ a: 4 }, { b: false, c: 'overriden' })), { d: 'actually new' }), { a: 5, d: 'maybe new' });
185167
var combinedNestedChangeType = __assign(__assign({}, __assign({ a: 1 }, { b: false, c: 'overriden' })), { c: -1 });
186168
var propertyNested = { a: __assign({}, o) };
187169
// accessors don't copy the descriptor
@@ -231,18 +213,16 @@ var cplus = __assign(__assign({}, c), { plus: function () { return this.p + 1; }
231213
cplus.plus();
232214
// new field's type conflicting with existing field is OK
233215
var changeTypeAfter = __assign(__assign({}, o), { a: 'wrong type?' });
234-
var changeTypeBefore = __assign({ a: 'wrong type?' }, o);
235216
var changeTypeBoth = __assign(__assign({}, o), swap);
236217
// optional
237218
function container(definiteBoolean, definiteString, optionalString, optionalNumber) {
238-
var _a, _b, _c;
219+
var _a, _b;
239220
var optionalUnionStops = __assign(__assign(__assign({}, definiteBoolean), definiteString), optionalNumber);
240221
var optionalUnionDuplicates = __assign(__assign(__assign(__assign({}, definiteBoolean), definiteString), optionalString), optionalNumber);
241222
var allOptional = __assign(__assign({}, optionalString), optionalNumber);
242223
// computed property
243224
var computedFirst = __assign(__assign((_a = {}, _a['before everything'] = 12, _a), o), { b: 'yes' });
244-
var computedMiddle = __assign(__assign(__assign({}, o), (_b = {}, _b['in the middle'] = 13, _b.b = 'maybe?', _b)), o2);
245-
var computedAfter = __assign(__assign({}, o), (_c = { b: 'yeah' }, _c['at the end'] = 14, _c));
225+
var computedAfter = __assign(__assign({}, o), (_b = { b: 'yeah' }, _b['at the end'] = 14, _b));
246226
}
247227
// shortcut syntax
248228
var a = 12;

0 commit comments

Comments
 (0)