Skip to content

Commit 871bdee

Browse files
andrewbranchandrewbranch
authored
Merge pull request microsoft#31480 from andrewbranch/bug/25487
Fix invalid JSXExpressions having identifier-ish things in their trivia, improve error messages for comma expressions in JSX
2 parents 463da55 + ad9c36e commit 871bdee

17 files changed

+111
-47
lines changed

‎src/compiler/checker.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20033,6 +20033,7 @@ namespace ts {
2003320033
}
2003420034

2003520035
function checkJsxExpression(node: JsxExpression, checkMode?: CheckMode) {
20036+
checkGrammarJsxExpression(node);
2003620037
if (node.expression) {
2003720038
const type = checkExpression(node.expression, checkMode);
2003820039
if (node.dotDotDotToken && type !== anyType && !isArrayType(type)) {
@@ -31912,6 +31913,12 @@ namespace ts {
3191231913
}
3191331914
}
3191431915

31916+
function checkGrammarJsxExpression(node: JsxExpression) {
31917+
if (node.expression && isCommaSequence(node.expression)) {
31918+
return grammarErrorOnNode(node.expression, Diagnostics.JSX_expressions_may_not_use_the_comma_operator_Did_you_mean_to_write_an_array);
31919+
}
31920+
}
31921+
3191531922
function checkGrammarForInOrForOfStatement(forInOrOfStatement: ForInOrOfStatement): boolean {
3191631923
if (checkGrammarStatementInAmbientContext(forInOrOfStatement)) {
3191731924
return true;

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5009,5 +5009,9 @@
50095009
"Classes may not have a field named 'constructor'.": {
50105010
"category": "Error",
50115011
"code": 18006
5012+
},
5013+
"JSX expressions may not use the comma operator. Did you mean to write an array?": {
5014+
"category": "Error",
5015+
"code": 18007
50125016
}
50135017
}

‎src/compiler/parser.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4430,14 +4430,18 @@ namespace ts {
44304430

44314431
if (token() !== SyntaxKind.CloseBraceToken) {
44324432
node.dotDotDotToken = parseOptionalToken(SyntaxKind.DotDotDotToken);
4433-
node.expression = parseAssignmentExpressionOrHigher();
4433+
// Only an AssignmentExpression is valid here per the JSX spec,
4434+
// but we can unambiguously parse a comma sequence and provide
4435+
// a better error message in grammar checking.
4436+
node.expression = parseExpression();
44344437
}
44354438
if (inExpressionContext) {
44364439
parseExpected(SyntaxKind.CloseBraceToken);
44374440
}
44384441
else {
4439-
parseExpected(SyntaxKind.CloseBraceToken, /*message*/ undefined, /*shouldAdvance*/ false);
4440-
scanJsxText();
4442+
if (parseExpected(SyntaxKind.CloseBraceToken, /*message*/ undefined, /*shouldAdvance*/ false)) {
4443+
scanJsxText();
4444+
}
44414445
}
44424446

44434447
return finishNode(node);

‎src/harness/fourslash.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,21 @@ namespace FourSlash {
583583
});
584584
}
585585

586+
public verifyErrorExistsAtRange(range: Range, code: number, expectedMessage?: string) {
587+
const span = ts.createTextSpanFromRange(range);
588+
const hasMatchingError = ts.some(
589+
this.getDiagnostics(range.fileName),
590+
({ code, messageText, start, length }) =>
591+
code === code &&
592+
(!expectedMessage || expectedMessage === messageText) &&
593+
ts.isNumber(start) && ts.isNumber(length) &&
594+
ts.textSpansEqual(span, { start, length }));
595+
596+
if (!hasMatchingError) {
597+
this.raiseError(`No error with code ${code} found at provided range.`);
598+
}
599+
}
600+
586601
public verifyNumberOfErrorsInCurrentFile(expected: number) {
587602
const errors = this.getDiagnostics(this.activeFile.fileName);
588603
const actual = errors.length;
@@ -4009,6 +4024,10 @@ namespace FourSlashInterface {
40094024
this.state.verifyNoErrors();
40104025
}
40114026

4027+
public errorExistsAtRange(range: FourSlash.Range, code: number, message?: string) {
4028+
this.state.verifyErrorExistsAtRange(range, code, message);
4029+
}
4030+
40124031
public numberOfErrorsInCurrentFile(expected: number) {
40134032
this.state.verifyNumberOfErrorsInCurrentFile(expected);
40144033
}

‎tests/baselines/reference/jsxAndTypeAssertion.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,21 @@ var foo = /** @class */ (function () {
2828
return foo;
2929
}());
3030
var x;
31-
x = <any> {test} <any></any> };
31+
x = <any> {test}: <any></any> };
3232

3333
x = <any><any></any>;
3434

35-
x = <foo>hello {<foo>} </foo>}
35+
x = <foo>hello {<foo>} </foo>};
3636

3737
x = <foo test={<foo>}>hello</foo>}/>
3838

39-
x = <foo test={<foo>}>hello{<foo>}</foo>}
39+
x = <foo test={<foo>}>hello{<foo>}</foo>};
4040

4141
x = <foo>x</foo>, x = <foo />;
4242

4343
<foo>{<foo><foo>{/foo/.test(x) ? <foo><foo></foo> : <foo><foo></foo>}</foo>}</foo>
4444
:
45-
}</></>}</></>}/></></></>;
45+
}
46+
47+
48+
</></>}</></>}/></></></>;

‎tests/baselines/reference/jsxInvalidEsprimaTestSuite.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ var x = <div>one</div>, <div>two</div>;
123123
var x = <div>one</div> /* intervening comment */, /* intervening comment */ <div>two</div>;
124124
;
125125
//// [20.jsx]
126-
<a>{"str"}}</a>;
126+
<a>{"str"};}</a>;
127127
//// [21.jsx]
128128
<span className="a" id="b"/>;
129129
//// [22.jsx]

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
tests/cases/conformance/jsx/file.tsx(11,36): error TS1005: '}' expected.
2-
tests/cases/conformance/jsx/file.tsx(11,44): error TS1003: Identifier expected.
3-
tests/cases/conformance/jsx/file.tsx(11,46): error TS1161: Unterminated regular expression literal.
1+
tests/cases/conformance/jsx/file.tsx(11,30): error TS2695: Left side of comma operator is unused and has no side effects.
2+
tests/cases/conformance/jsx/file.tsx(11,30): error TS18007: JSX expressions may not use the comma operator. Did you mean to write an array?
43

54

6-
==== tests/cases/conformance/jsx/file.tsx (3 errors) ====
5+
==== tests/cases/conformance/jsx/file.tsx (2 errors) ====
76
declare module JSX {
87
interface Element { }
98
interface IntrinsicElements {
@@ -15,10 +14,8 @@ tests/cases/conformance/jsx/file.tsx(11,46): error TS1161: Unterminated regular
1514
const class1 = "foo";
1615
const class2 = "bar";
1716
const elem = <div className={class1, class2}/>;
18-
~
19-
!!! error TS1005: '}' expected.
20-
~
21-
!!! error TS1003: Identifier expected.
22-
23-
!!! error TS1161: Unterminated regular expression literal.
17+
~~~~~~
18+
!!! error TS2695: Left side of comma operator is unused and has no side effects.
19+
~~~~~~~~~~~~~~
20+
!!! error TS18007: JSX expressions may not use the comma operator. Did you mean to write an array?
2421

‎tests/baselines/reference/jsxParsingError1.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,4 @@ const elem = <div className={class1, class2}/>;
1616
// This should be a parse error
1717
var class1 = "foo";
1818
var class2 = "bar";
19-
var elem = <div className={class1} class2/>;
20-
/>;;
19+
var elem = <div className={class1, class2}/>;

‎tests/baselines/reference/jsxParsingError1.symbols

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,5 @@ const elem = <div className={class1, class2}/>;
2525
>div : Symbol(JSX.IntrinsicElements, Decl(file.tsx, 1, 22))
2626
>className : Symbol(className, Decl(file.tsx, 10, 17))
2727
>class1 : Symbol(class1, Decl(file.tsx, 8, 5))
28-
>class2 : Symbol(class2, Decl(file.tsx, 10, 36))
28+
>class2 : Symbol(class2, Decl(file.tsx, 9, 5))
2929

‎tests/baselines/reference/jsxParsingError1.types

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ const class2 = "bar";
1818

1919
const elem = <div className={class1, class2}/>;
2020
>elem : JSX.Element
21-
><div className={class1, class2 : JSX.Element
21+
><div className={class1, class2}/> : JSX.Element
2222
>div : any
2323
>className : string
24+
>class1, class2 : "bar"
2425
>class1 : "foo"
25-
>class2 : true
26-
>/>; : RegExp
26+
>class2 : "bar"
2727

0 commit comments

Comments
 (0)