Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 871bdee

Browse filesBrowse 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
Copy full SHA for 871bdee

17 files changed

+111-47Lines changed: 111 additions & 47 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/compiler/checker.ts‎

Copy file name to clipboardExpand all lines: src/compiler/checker.ts
+7Lines 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;
Collapse file

‎src/compiler/diagnosticMessages.json‎

Copy file name to clipboardExpand all lines: src/compiler/diagnosticMessages.json
+4Lines 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
}
Collapse file

‎src/compiler/parser.ts‎

Copy file name to clipboardExpand all lines: src/compiler/parser.ts
+7-3Lines 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);
Collapse file

‎src/harness/fourslash.ts‎

Copy file name to clipboardExpand all lines: src/harness/fourslash.ts
+19Lines 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
}
Collapse file

‎tests/baselines/reference/jsxAndTypeAssertion.js‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxAndTypeAssertion.js
+7-4Lines 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+
</></>}</></>}/></></></>;
Collapse file

‎tests/baselines/reference/jsxInvalidEsprimaTestSuite.js‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxInvalidEsprimaTestSuite.js
+1-1Lines 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]
Collapse file

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

Copy file name to clipboard
+7-10Lines 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

Collapse file

‎tests/baselines/reference/jsxParsingError1.js‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxParsingError1.js
+1-2Lines 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}/>;
Collapse file

‎tests/baselines/reference/jsxParsingError1.symbols‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxParsingError1.symbols
+1-1Lines 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

Collapse file

‎tests/baselines/reference/jsxParsingError1.types‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxParsingError1.types
+3-3Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.