Skip to content

Commit feaef9c

Browse files
committed
Improve error message for JSXExpressions that are comma expressions
1 parent 77a76c1 commit feaef9c

File tree

7 files changed

+46
-1
lines changed

7 files changed

+46
-1
lines changed

src/compiler/checker.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19850,6 +19850,7 @@ namespace ts {
1985019850
}
1985119851

1985219852
function checkJsxExpression(node: JsxExpression, checkMode?: CheckMode) {
19853+
checkGrammarJsxExpression(node);
1985319854
if (node.expression) {
1985419855
const type = checkExpression(node.expression, checkMode);
1985519856
if (node.dotDotDotToken && type !== anyType && !isArrayType(type)) {
@@ -31658,6 +31659,12 @@ namespace ts {
3165831659
}
3165931660
}
3166031661

31662+
function checkGrammarJsxExpression(node: JsxExpression) {
31663+
if (node.expression && isCommaSequence(node.expression)) {
31664+
return grammarErrorOnNode(node.expression, Diagnostics.JSX_expressions_may_not_use_the_comma_operator_Did_you_mean_to_write_an_array);
31665+
}
31666+
}
31667+
3166131668
function checkGrammarForInOrForOfStatement(forInOrOfStatement: ForInOrOfStatement): boolean {
3166231669
if (checkGrammarStatementInAmbientContext(forInOrOfStatement)) {
3166331670
return true;

src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4986,5 +4986,9 @@
49864986
"Classes may not have a field named 'constructor'.": {
49874987
"category": "Error",
49884988
"code": 18006
4989+
},
4990+
"JSX expressions may not use the comma operator. Did you mean to write an array?": {
4991+
"category": "Error",
4992+
"code": 18007
49894993
}
49904994
}

src/compiler/parser.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4430,7 +4430,10 @@ 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);

src/harness/fourslash.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,20 @@ namespace FourSlash {
582582
});
583583
}
584584

585+
public verifyErrorExistsAtRange(range: Range, code: number) {
586+
const span = ts.createTextSpanFromRange(range);
587+
const hasMatchingError = ts.some(
588+
this.getDiagnostics(range.fileName),
589+
({ code, start, length }) =>
590+
code === code &&
591+
ts.isNumber(start) && ts.isNumber(length) &&
592+
ts.textSpansEqual(span, { start, length }));
593+
594+
if (!hasMatchingError) {
595+
this.raiseError(`No error with code ${code} found at provided range.`);
596+
}
597+
}
598+
585599
public verifyNumberOfErrorsInCurrentFile(expected: number) {
586600
const errors = this.getDiagnostics(this.activeFile.fileName);
587601
const actual = errors.length;
@@ -3968,6 +3982,10 @@ namespace FourSlashInterface {
39683982
this.state.verifyNoErrors();
39693983
}
39703984

3985+
public errorExistsAtRange(range: FourSlash.Range, code: number) {
3986+
this.state.verifyErrorExistsAtRange(range, code);
3987+
}
3988+
39713989
public numberOfErrorsInCurrentFile(expected: number) {
39723990
this.state.verifyNumberOfErrorsInCurrentFile(expected);
39733991
}

tests/cases/fourslash/fourslash.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ declare namespace FourSlashInterface {
238238
signatureHelp(...options: VerifySignatureHelpOptions[], ): void;
239239
// Checks that there are no compile errors.
240240
noErrors(): void;
241+
errorExistsAtRange(range: Range, code: number): void;
241242
numberOfErrorsInCurrentFile(expected: number): void;
242243
baselineCurrentFileBreakpointLocations(): void;
243244
baselineCurrentFileNameOrDottedNameSpans(): void;

tests/cases/fourslash/jsxExpressionFollowedByIdentifier.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
////declare var React: any;
55
////declare var x: string;
66
////const a = <div>{<div />[|x|]}</div>
7+
////const b = <div x={<div />[|x|]} />
78

89
const range = test.ranges()[0];
910
verify.getSyntacticDiagnostics([{
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
//@Filename: jsxExpressionWithCommaExpression.tsx
4+
//@jsx: react
5+
////declare var React: any;
6+
////declare var x: string;
7+
////const a = <div x={[|x, x|]} />
8+
////const b = <div>{[|x, x|]}</div>
9+
10+
verify.getSyntacticDiagnostics([]);
11+
test.ranges().forEach(range => verify.errorExistsAtRange(range, 18006));

0 commit comments

Comments
 (0)