Skip to content

Commit f1eb989

Browse files
a-tarasyuka-tarasyuk
authored
feat(36908): add 'property overwritten by spread' error for jsx attributes. add related span for conflicting declaration in spread (#37329)
1 parent b0450ae commit f1eb989

10 files changed

+362
-14
lines changed

‎src/compiler/checker.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22684,8 +22684,8 @@ namespace ts {
2268422684
// Grammar checking
2268522685
checkGrammarObjectLiteralExpression(node, inDestructuringPattern);
2268622686

22687-
let propertiesTable: SymbolTable;
22688-
const allPropertiesTable = createSymbolTable();
22687+
const allPropertiesTable = strictNullChecks ? createSymbolTable() : undefined;
22688+
let propertiesTable = createSymbolTable();
2268922689
let propertiesArray: Symbol[] = [];
2269022690
let spread: Type = emptyObjectType;
2269122691

@@ -22701,7 +22701,6 @@ namespace ts {
2270122701
let patternWithComputedProperties = false;
2270222702
let hasComputedStringProperty = false;
2270322703
let hasComputedNumberProperty = false;
22704-
propertiesTable = createSymbolTable();
2270522704

2270622705
let offset = 0;
2270722706
for (let i = 0; i < node.properties.length; i++) {
@@ -22767,7 +22766,7 @@ namespace ts {
2276722766
prop.type = type;
2276822767
prop.target = member;
2276922768
member = prop;
22770-
allPropertiesTable.set(prop.escapedName, prop);
22769+
allPropertiesTable?.set(prop.escapedName, prop);
2277122770
}
2277222771
else if (memberDecl.kind === SyntaxKind.SpreadAssignment) {
2277322772
if (languageVersion < ScriptTarget.ES2015) {
@@ -22785,16 +22784,9 @@ namespace ts {
2278522784
error(memberDecl, Diagnostics.Spread_types_may_only_be_created_from_object_types);
2278622785
return errorType;
2278722786
}
22788-
for (const right of getPropertiesOfType(type)) {
22789-
const rightType = getTypeOfSymbol(right);
22790-
const left = allPropertiesTable.get(right.escapedName);
22791-
if (strictNullChecks &&
22792-
left &&
22793-
!maybeTypeOfKind(rightType, TypeFlags.Nullable)) {
22794-
error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName));
22795-
}
22787+
if (allPropertiesTable) {
22788+
checkSpreadPropOverrides(type, allPropertiesTable, memberDecl);
2279622789
}
22797-
2279822790
spread = getSpreadType(spread, type, node.symbol, objectFlags, inConstContext);
2279922791
offset = i + 1;
2280022792
continue;
@@ -22965,6 +22957,7 @@ namespace ts {
2296522957
*/
2296622958
function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, checkMode: CheckMode | undefined) {
2296722959
const attributes = openingLikeElement.attributes;
22960+
const allAttributesTable = strictNullChecks ? createSymbolTable() : undefined;
2296822961
let attributesTable = createSymbolTable();
2296922962
let spread: Type = emptyJsxObjectType;
2297022963
let hasSpreadAnyType = false;
@@ -22988,6 +22981,7 @@ namespace ts {
2298822981
attributeSymbol.type = exprType;
2298922982
attributeSymbol.target = member;
2299022983
attributesTable.set(attributeSymbol.escapedName, attributeSymbol);
22984+
allAttributesTable?.set(attributeSymbol.escapedName, attributeSymbol);
2299122985
if (attributeDecl.name.escapedText === jsxChildrenPropertyName) {
2299222986
explicitlySpecifyChildrenAttribute = true;
2299322987
}
@@ -23004,6 +22998,9 @@ namespace ts {
2300422998
}
2300522999
if (isValidSpreadType(exprType)) {
2300623000
spread = getSpreadType(spread, exprType, attributes.symbol, objectFlags, /*readonly*/ false);
23001+
if (allAttributesTable) {
23002+
checkSpreadPropOverrides(exprType, allAttributesTable, attributeDecl);
23003+
}
2300723004
}
2300823005
else {
2300923006
typeToIntersect = typeToIntersect ? getIntersectionType([typeToIntersect, exprType]) : exprType;
@@ -23088,6 +23085,16 @@ namespace ts {
2308823085
return childrenTypes;
2308923086
}
2309023087

23088+
function checkSpreadPropOverrides(type: Type, props: SymbolTable, spread: SpreadAssignment | JsxSpreadAttribute) {
23089+
for (const right of getPropertiesOfType(type)) {
23090+
const left = props.get(right.escapedName);
23091+
if (left && !maybeTypeOfKind(getTypeOfSymbol(right), TypeFlags.Nullable)) {
23092+
const diagnostic = error(left.valueDeclaration, Diagnostics._0_is_specified_more_than_once_so_this_usage_will_be_overwritten, unescapeLeadingUnderscores(left.escapedName));
23093+
addRelatedInfo(diagnostic, createDiagnosticForNode(spread, Diagnostics.This_spread_always_overwrites_this_property));
23094+
}
23095+
}
23096+
}
23097+
2309123098
/**
2309223099
* Check attributes property of opening-like element. This function is called during chooseOverload to get call signature of a JSX opening-like element.
2309323100
* (See "checkApplicableSignatureForJsxOpeningLikeElement" for how the function is used)

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2907,6 +2907,10 @@
29072907
"category": "Error",
29082908
"code": 2784
29092909
},
2910+
"This spread always overwrites this property.": {
2911+
"category": "Error",
2912+
"code": 2785
2913+
},
29102914

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

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
error TS2318: Cannot find global type 'CallableFunction'.
2+
error TS2318: Cannot find global type 'NewableFunction'.
3+
tests/cases/conformance/jsx/file.tsx(19,17): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
4+
tests/cases/conformance/jsx/file.tsx(20,17): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
5+
tests/cases/conformance/jsx/file.tsx(20,23): error TS2783: 'b' is specified more than once, so this usage will be overwritten.
6+
tests/cases/conformance/jsx/file.tsx(21,17): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
7+
tests/cases/conformance/jsx/file.tsx(21,23): error TS2783: 'd' is specified more than once, so this usage will be overwritten.
8+
tests/cases/conformance/jsx/file.tsx(22,17): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
9+
tests/cases/conformance/jsx/file.tsx(22,17): error TS2783: 'a' is specified more than once, so this usage will be overwritten.
10+
tests/cases/conformance/jsx/file.tsx(22,23): error TS2783: 'd' is specified more than once, so this usage will be overwritten.
11+
12+
13+
!!! error TS2318: Cannot find global type 'CallableFunction'.
14+
!!! error TS2318: Cannot find global type 'NewableFunction'.
15+
==== tests/cases/conformance/jsx/file.tsx (8 errors) ====
16+
import React = require('react');
17+
18+
interface Props {
19+
a: number;
20+
b: number;
21+
c?: number;
22+
d?: number;
23+
}
24+
25+
26+
const props: Props = { a: 1, b: 1 };
27+
const Foo = (props: Props) => <div>{ props.a }</div>;
28+
29+
// ok
30+
const a1 = <Foo {...props}></Foo>;
31+
const a2 = <Foo d={1} {...props}></Foo>;
32+
33+
// error
34+
const b1 = <Foo a={1} {...props}></Foo>;
35+
~~~~~
36+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
37+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:19:23: This spread always overwrites this property.
38+
const b2 = <Foo a={1} b={2} {...props}></Foo>;
39+
~~~~~
40+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
41+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:20:29: This spread always overwrites this property.
42+
~~~~~
43+
!!! error TS2783: 'b' is specified more than once, so this usage will be overwritten.
44+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:20:29: This spread always overwrites this property.
45+
const b3 = <Foo a={1} d={1} {...props} {...{ d: 1 }}></Foo>;
46+
~~~~~
47+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
48+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:21:29: This spread always overwrites this property.
49+
~~~~~
50+
!!! error TS2783: 'd' is specified more than once, so this usage will be overwritten.
51+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:21:40: This spread always overwrites this property.
52+
const b4 = <Foo a={1} d={1} {...props} {...{ a: 1, d: 1 }}></Foo>;
53+
~~~~~
54+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
55+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:22:29: This spread always overwrites this property.
56+
~~~~~
57+
!!! error TS2783: 'a' is specified more than once, so this usage will be overwritten.
58+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:22:40: This spread always overwrites this property.
59+
~~~~~
60+
!!! error TS2783: 'd' is specified more than once, so this usage will be overwritten.
61+
!!! related TS2785 tests/cases/conformance/jsx/file.tsx:22:40: This spread always overwrites this property.
62+

‎tests/baselines/reference/jsxSpreadOverwritesAttributeStrict.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//// [file.tsx]
2+
import React = require('react');
3+
4+
interface Props {
5+
a: number;
6+
b: number;
7+
c?: number;
8+
d?: number;
9+
}
10+
11+
12+
const props: Props = { a: 1, b: 1 };
13+
const Foo = (props: Props) => <div>{ props.a }</div>;
14+
15+
// ok
16+
const a1 = <Foo {...props}></Foo>;
17+
const a2 = <Foo d={1} {...props}></Foo>;
18+
19+
// error
20+
const b1 = <Foo a={1} {...props}></Foo>;
21+
const b2 = <Foo a={1} b={2} {...props}></Foo>;
22+
const b3 = <Foo a={1} d={1} {...props} {...{ d: 1 }}></Foo>;
23+
const b4 = <Foo a={1} d={1} {...props} {...{ a: 1, d: 1 }}></Foo>;
24+
25+
26+
//// [file.jsx]
27+
"use strict";
28+
exports.__esModule = true;
29+
var React = require("react");
30+
var props = { a: 1, b: 1 };
31+
var Foo = function (props) { return <div>{props.a}</div>; };
32+
// ok
33+
var a1 = <Foo {...props}></Foo>;
34+
var a2 = <Foo d={1} {...props}></Foo>;
35+
// error
36+
var b1 = <Foo a={1} {...props}></Foo>;
37+
var b2 = <Foo a={1} b={2} {...props}></Foo>;
38+
var b3 = <Foo a={1} d={1} {...props} {...{ d: 1 }}></Foo>;
39+
var b4 = <Foo a={1} d={1} {...props} {...{ a: 1, d: 1 }}></Foo>;

‎tests/baselines/reference/jsxSpreadOverwritesAttributeStrict.symbols

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
=== tests/cases/conformance/jsx/file.tsx ===
2+
import React = require('react');
3+
>React : Symbol(React, Decl(file.tsx, 0, 0))
4+
5+
interface Props {
6+
>Props : Symbol(Props, Decl(file.tsx, 0, 32))
7+
8+
a: number;
9+
>a : Symbol(Props.a, Decl(file.tsx, 2, 17))
10+
11+
b: number;
12+
>b : Symbol(Props.b, Decl(file.tsx, 3, 14))
13+
14+
c?: number;
15+
>c : Symbol(Props.c, Decl(file.tsx, 4, 14))
16+
17+
d?: number;
18+
>d : Symbol(Props.d, Decl(file.tsx, 5, 15))
19+
}
20+
21+
22+
const props: Props = { a: 1, b: 1 };
23+
>props : Symbol(props, Decl(file.tsx, 10, 5))
24+
>Props : Symbol(Props, Decl(file.tsx, 0, 32))
25+
>a : Symbol(a, Decl(file.tsx, 10, 22))
26+
>b : Symbol(b, Decl(file.tsx, 10, 28))
27+
28+
const Foo = (props: Props) => <div>{ props.a }</div>;
29+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
30+
>props : Symbol(props, Decl(file.tsx, 11, 13))
31+
>Props : Symbol(Props, Decl(file.tsx, 0, 32))
32+
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2400, 45))
33+
>props.a : Symbol(Props.a, Decl(file.tsx, 2, 17))
34+
>props : Symbol(props, Decl(file.tsx, 11, 13))
35+
>a : Symbol(Props.a, Decl(file.tsx, 2, 17))
36+
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2400, 45))
37+
38+
// ok
39+
const a1 = <Foo {...props}></Foo>;
40+
>a1 : Symbol(a1, Decl(file.tsx, 14, 5))
41+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
42+
>props : Symbol(props, Decl(file.tsx, 10, 5))
43+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
44+
45+
const a2 = <Foo d={1} {...props}></Foo>;
46+
>a2 : Symbol(a2, Decl(file.tsx, 15, 5))
47+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
48+
>d : Symbol(d, Decl(file.tsx, 15, 15))
49+
>props : Symbol(props, Decl(file.tsx, 10, 5))
50+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
51+
52+
// error
53+
const b1 = <Foo a={1} {...props}></Foo>;
54+
>b1 : Symbol(b1, Decl(file.tsx, 18, 5))
55+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
56+
>a : Symbol(a, Decl(file.tsx, 18, 15))
57+
>props : Symbol(props, Decl(file.tsx, 10, 5))
58+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
59+
60+
const b2 = <Foo a={1} b={2} {...props}></Foo>;
61+
>b2 : Symbol(b2, Decl(file.tsx, 19, 5))
62+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
63+
>a : Symbol(a, Decl(file.tsx, 19, 15))
64+
>b : Symbol(b, Decl(file.tsx, 19, 21))
65+
>props : Symbol(props, Decl(file.tsx, 10, 5))
66+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
67+
68+
const b3 = <Foo a={1} d={1} {...props} {...{ d: 1 }}></Foo>;
69+
>b3 : Symbol(b3, Decl(file.tsx, 20, 5))
70+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
71+
>a : Symbol(a, Decl(file.tsx, 20, 15))
72+
>d : Symbol(d, Decl(file.tsx, 20, 21))
73+
>props : Symbol(props, Decl(file.tsx, 10, 5))
74+
>d : Symbol(d, Decl(file.tsx, 20, 44))
75+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
76+
77+
const b4 = <Foo a={1} d={1} {...props} {...{ a: 1, d: 1 }}></Foo>;
78+
>b4 : Symbol(b4, Decl(file.tsx, 21, 5))
79+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
80+
>a : Symbol(a, Decl(file.tsx, 21, 15))
81+
>d : Symbol(d, Decl(file.tsx, 21, 21))
82+
>props : Symbol(props, Decl(file.tsx, 10, 5))
83+
>a : Symbol(a, Decl(file.tsx, 21, 44))
84+
>d : Symbol(d, Decl(file.tsx, 21, 50))
85+
>Foo : Symbol(Foo, Decl(file.tsx, 11, 5))
86+

‎tests/baselines/reference/jsxSpreadOverwritesAttributeStrict.types

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
=== tests/cases/conformance/jsx/file.tsx ===
2+
import React = require('react');
3+
>React : typeof React
4+
5+
interface Props {
6+
a: number;
7+
>a : number
8+
9+
b: number;
10+
>b : number
11+
12+
c?: number;
13+
>c : number | undefined
14+
15+
d?: number;
16+
>d : number | undefined
17+
}
18+
19+
20+
const props: Props = { a: 1, b: 1 };
21+
>props : Props
22+
>{ a: 1, b: 1 } : { a: number; b: number; }
23+
>a : number
24+
>1 : 1
25+
>b : number
26+
>1 : 1
27+
28+
const Foo = (props: Props) => <div>{ props.a }</div>;
29+
>Foo : (props: Props) => JSX.Element
30+
>(props: Props) => <div>{ props.a }</div> : (props: Props) => JSX.Element
31+
>props : Props
32+
><div>{ props.a }</div> : JSX.Element
33+
>div : any
34+
>props.a : number
35+
>props : Props
36+
>a : number
37+
>div : any
38+
39+
// ok
40+
const a1 = <Foo {...props}></Foo>;
41+
>a1 : JSX.Element
42+
><Foo {...props}></Foo> : JSX.Element
43+
>Foo : (props: Props) => JSX.Element
44+
>props : Props
45+
>Foo : (props: Props) => JSX.Element
46+
47+
const a2 = <Foo d={1} {...props}></Foo>;
48+
>a2 : JSX.Element
49+
><Foo d={1} {...props}></Foo> : JSX.Element
50+
>Foo : (props: Props) => JSX.Element
51+
>d : number
52+
>1 : 1
53+
>props : Props
54+
>Foo : (props: Props) => JSX.Element
55+
56+
// error
57+
const b1 = <Foo a={1} {...props}></Foo>;
58+
>b1 : JSX.Element
59+
><Foo a={1} {...props}></Foo> : JSX.Element
60+
>Foo : (props: Props) => JSX.Element
61+
>a : number
62+
>1 : 1
63+
>props : Props
64+
>Foo : (props: Props) => JSX.Element
65+
66+
const b2 = <Foo a={1} b={2} {...props}></Foo>;
67+
>b2 : JSX.Element
68+
><Foo a={1} b={2} {...props}></Foo> : JSX.Element
69+
>Foo : (props: Props) => JSX.Element
70+
>a : number
71+
>1 : 1
72+
>b : number
73+
>2 : 2
74+
>props : Props
75+
>Foo : (props: Props) => JSX.Element
76+
77+
const b3 = <Foo a={1} d={1} {...props} {...{ d: 1 }}></Foo>;
78+
>b3 : JSX.Element
79+
><Foo a={1} d={1} {...props} {...{ d: 1 }}></Foo> : JSX.Element
80+
>Foo : (props: Props) => JSX.Element
81+
>a : number
82+
>1 : 1
83+
>d : number
84+
>1 : 1
85+
>props : Props
86+
>{ d: 1 } : { d: number; }
87+
>d : number
88+
>1 : 1
89+
>Foo : (props: Props) => JSX.Element
90+
91+
const b4 = <Foo a={1} d={1} {...props} {...{ a: 1, d: 1 }}></Foo>;
92+
>b4 : JSX.Element
93+
><Foo a={1} d={1} {...props} {...{ a: 1, d: 1 }}></Foo> : JSX.Element
94+
>Foo : (props: Props) => JSX.Element
95+
>a : number
96+
>1 : 1
97+
>d : number
98+
>1 : 1
99+
>props : Props
100+
>{ a: 1, d: 1 } : { a: number; d: number; }
101+
>a : number
102+
>1 : 1
103+
>d : number
104+
>1 : 1
105+
>Foo : (props: Props) => JSX.Element
106+

0 commit comments

Comments
 (0)