Skip to content

Error when property is specified more than once via a spread #36727

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 12 commits into from
Feb 11, 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
24 changes: 18 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13032,7 +13032,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): Type {
function getSpreadType(left: Type, right: Type, symbol: Symbol | undefined, objectFlags: ObjectFlags, readonly: boolean, isParentTypeNullable?: boolean): Type {
if (left.flags & TypeFlags.Any || right.flags & TypeFlags.Any) {
return anyType;
}
Expand All @@ -13048,16 +13048,16 @@ namespace ts {
if (left.flags & TypeFlags.Union) {
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(left as UnionType, readonly);
if (merged) {
return getSpreadType(merged, right, symbol, objectFlags, readonly);
return getSpreadType(merged, right, symbol, objectFlags, readonly, isParentTypeNullable);
}
return mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly));
return mapType(left, t => getSpreadType(t, right, symbol, objectFlags, readonly, isParentTypeNullable));
}
if (right.flags & TypeFlags.Union) {
const merged = tryMergeUnionOfObjectTypeAndEmptyObject(right as UnionType, readonly);
if (merged) {
return getSpreadType(left, merged, symbol, objectFlags, readonly);
return getSpreadType(left, merged, symbol, objectFlags, readonly, maybeTypeOfKind(right, TypeFlags.Nullable));
}
return mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly));
return mapType(right, t => getSpreadType(left, t, symbol, objectFlags, readonly, maybeTypeOfKind(right, TypeFlags.Nullable)));
}
if (right.flags & (TypeFlags.BooleanLike | TypeFlags.NumberLike | TypeFlags.BigIntLike | TypeFlags.StringLike | TypeFlags.EnumLike | TypeFlags.NonPrimitive | TypeFlags.Index)) {
return left;
Expand Down Expand Up @@ -13121,6 +13121,14 @@ 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 @@ -16620,6 +16628,10 @@ 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 @@ -25178,7 +25190,7 @@ namespace ts {
if (node.arguments.length === 1) {
const text = getSourceFileOfNode(node).text;
if (isLineBreak(text.charCodeAt(skipTrivia(text, node.expression.end, /* stopAfterLineBreak */ true) - 1))) {
relatedInformation = createDiagnosticForNode(node.expression, Diagnostics.It_is_highly_likely_that_you_are_missing_a_semicolon);
relatedInformation = createDiagnosticForNode(node.expression, Diagnostics.Are_you_missing_a_semicolon);
}
}
invocationError(node.expression, apparentType, SignatureKind.Call, relatedInformation);
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2683,7 +2683,7 @@
"category": "Error",
"code": 2733
},
"It is highly likely that you are missing a semicolon.": {
"Are you missing a semicolon?": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“$20 says you’re missing a semicolon”

"category": "Error",
"code": 2734
},
Expand Down Expand Up @@ -2879,6 +2879,10 @@
"category": "Message",
"code": 2782
},
"'{0}' is specified more than once, so this usage will be overwritten.": {
"category": "Error",
"code": 2783
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,20 @@ tests/cases/compiler/betterErrorForAccidentalCall.ts(13,1): error TS2349: This e
~~~~~
!!! error TS2349: This expression is not callable.
!!! error TS2349: Type 'String' has no call signatures.
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:7:1: It is highly likely that you are missing a semicolon.
!!! related TS2734 tests/cases/compiler/betterErrorForAccidentalCall.ts:7:1: Are you missing a semicolon?
(1 as number).toString();

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

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

4 changes: 2 additions & 2 deletions tests/baselines/reference/checkJsxChildrenProperty13.types
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ class Button extends React.Component<ButtonProp, any> {
>(<InnerButton {...this.props} children="hi"> <div>Hello World</div> </InnerButton>) : JSX.Element
><InnerButton {...this.props} children="hi"> <div>Hello World</div> </InnerButton> : JSX.Element
>InnerButton : typeof InnerButton
>this.props : ButtonProp & { children?: React.ReactNode; }
>this.props : ButtonProp & { children?: string | number | boolean | {} | React.ReactElement<any> | (string | number | boolean | any[] | React.ReactElement<any>)[] | undefined; }
>this : this
>props : ButtonProp & { children?: React.ReactNode; }
>props : ButtonProp & { children?: string | number | boolean | {} | React.ReactElement<any> | (string | number | boolean | any[] | React.ReactElement<any>)[] | undefined; }
>children : string

<div>Hello World</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,5 @@ tests/cases/conformance/jsx/file.tsx(49,6): error TS2746: This JSX tag's 'childr
!!! error TS2746: This JSX tag's 'children' prop expects a single child of type 'string | Element', but multiple children were provided.
<div> My Div </div>
<div> My Div </div>
</Comp>;
</Comp>;

3 changes: 2 additions & 1 deletion tests/baselines/reference/checkJsxChildrenProperty2.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ let k5 =
<Comp a={10} b="hi" >
<div> My Div </div>
<div> My Div </div>
</Comp>;
</Comp>;


//// [file.jsx]
"use strict";
Expand Down
24 changes: 2 additions & 22 deletions tests/baselines/reference/objectSpread.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,14 @@ let addAfter: { a: number, b: string, c: boolean } =
{ ...o, c: false }
let addBefore: { a: number, b: string, c: boolean } =
{ c: false, ...o }
// Note: ignore still changes the order that properties are printed
let ignore: { a: number, b: string } =
{ b: 'ignored', ...o }
let override: { a: number, b: string } =
{ ...o, b: 'override' }
let nested: { a: number, b: boolean, c: string } =
{ ...{ a: 3, ...{ b: false, c: 'overriden' } }, c: 'whatever' }
let combined: { a: number, b: string, c: boolean } =
{ ...o, ...o2 }
let combinedBefore: { a: number, b: string, c: boolean } =
{ b: 'ok', ...o, ...o2 }
let combinedMid: { a: number, b: string, c: boolean } =
{ ...o, b: 'ok', ...o2 }
let combinedAfter: { a: number, b: string, c: boolean } =
{ ...o, ...o2, b: 'ok' }
let combinedNested: { a: number, b: boolean, c: string, d: string } =
{ ...{ a: 4, ...{ b: false, c: 'overriden' } }, d: 'actually new', ...{ a: 5, d: 'maybe new' } }
let combinedNestedChangeType: { a: number, b: boolean, c: number } =
{ ...{ a: 1, ...{ b: false, c: 'overriden' } }, c: -1 }
let propertyNested: { a: { a: number, b: string } } =
Expand Down Expand Up @@ -91,8 +82,6 @@ cplus.plus();
// new field's type conflicting with existing field is OK
let changeTypeAfter: { a: string, b: string } =
{ ...o, a: 'wrong type?' }
let changeTypeBefore: { a: number, b: string } =
{ a: 'wrong type?', ...o };
let changeTypeBoth: { a: string, b: number } =
{ ...o, ...swap };

Expand All @@ -109,8 +98,6 @@ function container(
// computed property
let computedFirst: { a: number, b: string, "before everything": number } =
{ ['before everything']: 12, ...o, b: 'yes' }
let computedMiddle: { a: number, b: string, c: boolean, "in the middle": number } =
{ ...o, ['in the middle']: 13, b: 'maybe?', ...o2 }
let computedAfter: { a: number, b: string, "at the end": number } =
{ ...o, b: 'yeah', ['at the end']: 14 }
}
Expand Down Expand Up @@ -173,15 +160,10 @@ var o2 = { b: 'yes', c: true };
var swap = { a: 'yes', b: -1 };
var addAfter = __assign(__assign({}, o), { c: false });
var addBefore = __assign({ c: false }, o);
// Note: ignore still changes the order that properties are printed
var ignore = __assign({ b: 'ignored' }, o);
var override = __assign(__assign({}, o), { b: 'override' });
var nested = __assign(__assign({}, __assign({ a: 3 }, { b: false, c: 'overriden' })), { c: 'whatever' });
var combined = __assign(__assign({}, o), o2);
var combinedBefore = __assign(__assign({ b: 'ok' }, o), o2);
var combinedMid = __assign(__assign(__assign({}, o), { b: 'ok' }), o2);
var combinedAfter = __assign(__assign(__assign({}, o), o2), { b: 'ok' });
var combinedNested = __assign(__assign(__assign({}, __assign({ a: 4 }, { b: false, c: 'overriden' })), { d: 'actually new' }), { a: 5, d: 'maybe new' });
var combinedNestedChangeType = __assign(__assign({}, __assign({ a: 1 }, { b: false, c: 'overriden' })), { c: -1 });
var propertyNested = { a: __assign({}, o) };
// accessors don't copy the descriptor
Expand Down Expand Up @@ -231,18 +213,16 @@ var cplus = __assign(__assign({}, c), { plus: function () { return this.p + 1; }
cplus.plus();
// new field's type conflicting with existing field is OK
var changeTypeAfter = __assign(__assign({}, o), { a: 'wrong type?' });
var changeTypeBefore = __assign({ a: 'wrong type?' }, o);
var changeTypeBoth = __assign(__assign({}, o), swap);
// optional
function container(definiteBoolean, definiteString, optionalString, optionalNumber) {
var _a, _b, _c;
var _a, _b;
var optionalUnionStops = __assign(__assign(__assign({}, definiteBoolean), definiteString), optionalNumber);
var optionalUnionDuplicates = __assign(__assign(__assign(__assign({}, definiteBoolean), definiteString), optionalString), optionalNumber);
var allOptional = __assign(__assign({}, optionalString), optionalNumber);
// computed property
var computedFirst = __assign(__assign((_a = {}, _a['before everything'] = 12, _a), o), { b: 'yes' });
var computedMiddle = __assign(__assign(__assign({}, o), (_b = {}, _b['in the middle'] = 13, _b.b = 'maybe?', _b)), o2);
var computedAfter = __assign(__assign({}, o), (_c = { b: 'yeah' }, _c['at the end'] = 14, _c));
var computedAfter = __assign(__assign({}, o), (_b = { b: 'yeah' }, _b['at the end'] = 14, _b));
}
// shortcut syntax
var a = 12;
Expand Down
Loading