Skip to content

Better error recovery for adjacent JSX elements in expression positions #5295

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.

Sign up for GitHub

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 1 commit into from
Oct 20, 2015
Merged

Better error recovery for adjacent JSX elements in expression positions #5295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 0 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7510,9 +7510,6 @@ namespace ts {
case SyntaxKind.JsxSelfClosingElement:
checkJsxSelfClosingElement(<JsxSelfClosingElement>child);
break;
default:
// No checks for JSX Text
Debug.assert(child.kind === SyntaxKind.JsxText);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,10 @@
"category": "Error",
"code": 2656
},
"JSX expressions must have one parent element": {
"category": "Error",
"code": 2657
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
28 changes: 26 additions & 2 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3454,19 +3454,43 @@ namespace ts {

function parseJsxElementOrSelfClosingElement(inExpressionContext: boolean): JsxElement | JsxSelfClosingElement {
let opening = parseJsxOpeningOrSelfClosingElement(inExpressionContext);
let result: JsxElement | JsxSelfClosingElement;
if (opening.kind === SyntaxKind.JsxOpeningElement) {
let node = <JsxElement>createNode(SyntaxKind.JsxElement, opening.pos);
node.openingElement = opening;

node.children = parseJsxChildren(node.openingElement.tagName);
node.closingElement = parseJsxClosingElement(inExpressionContext);
return finishNode(node);
result = finishNode(node);
}
else {
Debug.assert(opening.kind === SyntaxKind.JsxSelfClosingElement);
// Nothing else to do for self-closing elements
return <JsxSelfClosingElement>opening;
result = <JsxSelfClosingElement>opening;
}

// If the user writes the invalid code '<div></div><div></div>' in an expression context (i.e. not wrapped in
// an enclosing tag), we'll naively try to parse ^ this as a 'less than' operator and the remainder of the tag
// as garbage, which will cause the formatter to badly mangle the JSX. Perform a speculative parse of a JSX
// element if we see a < token so that we can wrap it in a synthetic binary expression so the formatter
// does less damage and we can report a better error.
// Since JSX elements are invalid < operands anyway, this lookahead parse will only occur in error scenarios
// of one sort or another.
if (inExpressionContext && token === SyntaxKind.LessThanToken) {
let invalidElement = tryParse(() => parseJsxElementOrSelfClosingElement(/*inExpressionContext*/true));
if (invalidElement) {
parseErrorAtCurrentToken(Diagnostics.JSX_expressions_must_have_one_parent_element);
let badNode = <BinaryExpression>createNode(SyntaxKind.BinaryExpression, result.pos);
badNode.end = invalidElement.end;
badNode.left = result;
badNode.right = invalidElement;
badNode.operatorToken = createMissingNode(SyntaxKind.CommaToken, /*reportAtCurrentPosition*/ false, /*diagnosticMessage*/ undefined);
badNode.operatorToken.pos = badNode.operatorToken.end = badNode.right.pos;
return <JsxElement><Node>badNode;
}
}

return result;
}

function parseJsxText(): JsxText {
Expand Down
9 changes: 3 additions & 6 deletions tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,29): error TS1005: '{'
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,57): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,1): error TS1003: Identifier expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,6): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,12): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,12): error TS2657: JSX expressions must have one parent element


==== tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx (8 errors) ====
==== tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx (7 errors) ====
declare var React: any;
declare var 日本語;
declare var AbC_def;
Expand Down Expand Up @@ -62,10 +61,8 @@ tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,12): error TS1109: Expr
<a.b></a.b>;
~
!!! error TS1003: Identifier expected.
~~
!!! error TS1109: Expression expected.
~
!!! error TS1109: Expression expected.
!!! error TS2657: JSX expressions must have one parent element

<a.b.c></a.b.c>;

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/jsxEsprimaFbTestSuite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ baz
<div>@test content</div>;
<div><br />7x invalid-js-identifier</div>;
<LeftRight left={<a />} right={<b>monkeys /> gorillas</b> / > }/>
< a.b > ;
a.b > ;
,
Copy link
Contributor

Choose a reason for hiding this comment

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

is this , comes from synthesized missing node?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, I didn't realize that we print missing nodes. In this case I'm not sure that user would expect to see it - this is our internal recovery strategy and I don' think it should leak outside. Maybe we should not emit missing nodes (since they are ...missed in the source code)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is consistent with what we've done for a while. For example if you write

var x = a b;

we emit

var x = a, b;

As I've said before, I don't think emitting in the presence of syntax errors is a rational thing to do, but since we're doing it anyway, I think the code we emit should at least be syntactically valid (even if it makes guesses at the intent).

<a.b></a.b>;
<a.b.c></a.b.c>;
(<div />) < x;
<div {...props}/>;
Expand Down
30 changes: 6 additions & 24 deletions tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,11 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(6,6): error TS2304: C
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(6,9): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(6,10): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(7,1): error TS1003: Identifier expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(7,2): error TS2304: Cannot find name 'a'.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(7,4): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(8,4): error TS17002: Expected corresponding JSX closing tag for 'a'.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(9,13): error TS1002: Unterminated string literal.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,1): error TS1003: Identifier expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,2): error TS2304: Cannot find name 'a'.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,3): error TS1005: ';' expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,4): error TS2304: Cannot find name 'b'.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,6): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,8): error TS2304: Cannot find name 'b'.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,10): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,6): error TS17002: Expected corresponding JSX closing tag for 'a'.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(10,10): error TS2657: JSX expressions must have one parent element
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(11,3): error TS1003: Identifier expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(11,5): error TS1003: Identifier expected.
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(11,11): error TS1005: '>' expected.
Expand Down Expand Up @@ -71,7 +65,7 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,4): error TS1003:
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002: Expected corresponding JSX closing tag for 'a'.


==== tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx (71 errors) ====
==== tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx (65 errors) ====
declare var React: any;

</>;
Expand Down Expand Up @@ -107,10 +101,6 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002
<a>;
~
!!! error TS1003: Identifier expected.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS1109: Expression expected.
<a></b>;
~~~~
!!! error TS17002: Expected corresponding JSX closing tag for 'a'.
Expand All @@ -120,18 +110,10 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002
<a:b></b>;
~
!!! error TS1003: Identifier expected.
~
!!! error TS2304: Cannot find name 'a'.
~
!!! error TS1005: ';' expected.
~
!!! error TS2304: Cannot find name 'b'.
~~
!!! error TS1109: Expression expected.
~
!!! error TS2304: Cannot find name 'b'.
~~~~
!!! error TS17002: Expected corresponding JSX closing tag for 'a'.
~
!!! error TS1109: Expression expected.
!!! error TS2657: JSX expressions must have one parent element
<a:b.c></a:b.c>;
~
!!! error TS1003: Identifier expected.
Expand Down
10 changes: 4 additions & 6 deletions tests/baselines/reference/jsxInvalidEsprimaTestSuite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,10 @@ var x = <div>one</div> /* intervening comment */ <div>two</div>;;
< ;
a / > ;
<a b={d / > }/>
< a > ;
<a></b>;
<a foo="bar;/>
< a;
b > ;
b > ;
,
<a>;
<a></b>;
<a foo="bar;/>a:b></b>;
<a b c></a>;
b.c > ;
<a.b c></a.b>;
Expand Down
18 changes: 18 additions & 0 deletions tests/baselines/reference/tsxErrorRecovery2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
tests/cases/conformance/jsx/file1.tsx(6,1): error TS2657: JSX expressions must have one parent element
tests/cases/conformance/jsx/file2.tsx(2,1): error TS2657: JSX expressions must have one parent element


==== tests/cases/conformance/jsx/file1.tsx (1 errors) ====

declare namespace JSX { interface Element { } }

<div></div>
<div></div>


!!! error TS2657: JSX expressions must have one parent element
==== tests/cases/conformance/jsx/file2.tsx (1 errors) ====
var x = <div></div><div></div>


!!! error TS2657: JSX expressions must have one parent element
19 changes: 19 additions & 0 deletions tests/baselines/reference/tsxErrorRecovery2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/jsx/tsxErrorRecovery2.tsx] ////

//// [file1.tsx]

declare namespace JSX { interface Element { } }

<div></div>
<div></div>

//// [file2.tsx]
var x = <div></div><div></div>


//// [file1.jsx]
<div></div>
,
<div></div>;
//// [file2.jsx]
var x = <div></div>, <div></div>;
30 changes: 30 additions & 0 deletions tests/baselines/reference/tsxErrorRecovery3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
tests/cases/conformance/jsx/file1.tsx(4,2): error TS2304: Cannot find name 'React'.
tests/cases/conformance/jsx/file1.tsx(5,2): error TS2304: Cannot find name 'React'.
tests/cases/conformance/jsx/file1.tsx(6,1): error TS2657: JSX expressions must have one parent element
tests/cases/conformance/jsx/file2.tsx(1,10): error TS2304: Cannot find name 'React'.
tests/cases/conformance/jsx/file2.tsx(1,21): error TS2304: Cannot find name 'React'.
tests/cases/conformance/jsx/file2.tsx(2,1): error TS2657: JSX expressions must have one parent element


==== tests/cases/conformance/jsx/file1.tsx (3 errors) ====

declare namespace JSX { interface Element { } }

<div></div>
~~~
!!! error TS2304: Cannot find name 'React'.
<div></div>
~~~
!!! error TS2304: Cannot find name 'React'.


!!! error TS2657: JSX expressions must have one parent element
==== tests/cases/conformance/jsx/file2.tsx (3 errors) ====
var x = <div></div><div></div>
~~~
!!! error TS2304: Cannot find name 'React'.
~~~
!!! error TS2304: Cannot find name 'React'.


!!! error TS2657: JSX expressions must have one parent element
19 changes: 19 additions & 0 deletions tests/baselines/reference/tsxErrorRecovery3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/jsx/tsxErrorRecovery3.tsx] ////

//// [file1.tsx]

declare namespace JSX { interface Element { } }

<div></div>
<div></div>

//// [file2.tsx]
var x = <div></div><div></div>


//// [file1.js]
React.createElement("div", null)
,
React.createElement("div", null);
//// [file2.js]
var x = React.createElement("div", null), React.createElement("div", null);
10 changes: 10 additions & 0 deletions tests/cases/conformance/jsx/tsxErrorRecovery2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//@jsx: preserve

//@filename: file1.tsx
declare namespace JSX { interface Element { } }

<div></div>
<div></div>

//@filename: file2.tsx
var x = <div></div><div></div>
10 changes: 10 additions & 0 deletions tests/cases/conformance/jsx/tsxErrorRecovery3.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//@jsx: react

//@filename: file1.tsx
declare namespace JSX { interface Element { } }

<div></div>
<div></div>

//@filename: file2.tsx
var x = <div></div><div></div>