Skip to content

Commit 89a737c

Browse files
sandersnsandersn
authored
Improve parser recovery for unclosed/mismatched JSX elements (microsoft#43780)
* First draft Everything works, the error messages for unmatched opening elements could still use improvement, plus there is tonnes of unused and ugly code. 1. Make sure the parser can recover from all kinds of unclosed tags. 2. Improve the parse tree for unmatched opening tags. 3. Better errors at some point. * Lots of cleanup * Improve readability of construction/fix lint * improve line-length formatting
1 parent 7901f35 commit 89a737c

14 files changed

+1635
-59
lines changed

‎src/compiler/parser.ts

+67-28
Original file line numberDiff line numberDiff line change
@@ -4865,18 +4865,45 @@ namespace ts {
48654865
return finishNode(factory.createPropertyAccessExpression(expression, parseRightSideOfDot(/*allowIdentifierNames*/ true, /*allowPrivateIdentifiers*/ true)), pos);
48664866
}
48674867

4868-
function parseJsxElementOrSelfClosingElementOrFragment(inExpressionContext: boolean, topInvalidNodePosition?: number): JsxElement | JsxSelfClosingElement | JsxFragment {
4868+
function parseJsxElementOrSelfClosingElementOrFragment(inExpressionContext: boolean, topInvalidNodePosition?: number, openingTag?: JsxOpeningElement | JsxOpeningFragment): JsxElement | JsxSelfClosingElement | JsxFragment {
48694869
const pos = getNodePos();
48704870
const opening = parseJsxOpeningOrSelfClosingElementOrOpeningFragment(inExpressionContext);
48714871
let result: JsxElement | JsxSelfClosingElement | JsxFragment;
48724872
if (opening.kind === SyntaxKind.JsxOpeningElement) {
4873-
const children = parseJsxChildren(opening);
4874-
const closingElement = parseJsxClosingElement(inExpressionContext);
4875-
4876-
if (!tagNamesAreEquivalent(opening.tagName, closingElement.tagName)) {
4877-
parseErrorAtRange(closingElement, Diagnostics.Expected_corresponding_JSX_closing_tag_for_0, getTextOfNodeFromSourceText(sourceText, opening.tagName));
4873+
let children = parseJsxChildren(opening);
4874+
let closingElement: JsxClosingElement;
4875+
4876+
const lastChild: JsxChild | undefined = children[children.length - 1];
4877+
if (lastChild?.kind === SyntaxKind.JsxElement
4878+
&& !tagNamesAreEquivalent(lastChild.openingElement.tagName, lastChild.closingElement.tagName)
4879+
&& tagNamesAreEquivalent(opening.tagName, lastChild.closingElement.tagName)) {
4880+
// when an unclosed JsxOpeningElement incorrectly parses its parent's JsxClosingElement,
4881+
// restructure (<div>(...<span></div>)) --> (<div>(...<span></span>)</div>)
4882+
// (no need to error; the parent will error)
4883+
const end = lastChild.openingElement.end; // newly-created children and closing are both zero-width end/end
4884+
const newLast = finishNode(factory.createJsxElement(
4885+
lastChild.openingElement,
4886+
createNodeArray([], end, end),
4887+
finishNode(factory.createJsxClosingElement(finishNode(factory.createIdentifier(""), end, end)), end, end)),
4888+
lastChild.openingElement.pos,
4889+
end);
4890+
4891+
children = createNodeArray([...children.slice(0, children.length - 1), newLast], children.pos, end);
4892+
closingElement = lastChild.closingElement;
4893+
}
4894+
else {
4895+
closingElement = parseJsxClosingElement(opening, inExpressionContext);
4896+
if (!tagNamesAreEquivalent(opening.tagName, closingElement.tagName)) {
4897+
if (openingTag && isJsxOpeningElement(openingTag) && tagNamesAreEquivalent(closingElement.tagName, openingTag.tagName)) {
4898+
// opening incorrectly matched with its parent's closing -- put error on opening
4899+
parseErrorAtRange(opening.tagName, Diagnostics.JSX_element_0_has_no_corresponding_closing_tag, getTextOfNodeFromSourceText(sourceText, opening.tagName));
4900+
}
4901+
else {
4902+
// other opening/closing mismatches -- put error on closing
4903+
parseErrorAtRange(closingElement.tagName, Diagnostics.Expected_corresponding_JSX_closing_tag_for_0, getTextOfNodeFromSourceText(sourceText, opening.tagName));
4904+
}
4905+
}
48784906
}
4879-
48804907
result = finishNode(factory.createJsxElement(opening, children, closingElement), pos);
48814908
}
48824909
else if (opening.kind === SyntaxKind.JsxOpeningFragment) {
@@ -4941,7 +4968,7 @@ namespace ts {
49414968
case SyntaxKind.OpenBraceToken:
49424969
return parseJsxExpression(/*inExpressionContext*/ false);
49434970
case SyntaxKind.LessThanToken:
4944-
return parseJsxElementOrSelfClosingElementOrFragment(/*inExpressionContext*/ false);
4971+
return parseJsxElementOrSelfClosingElementOrFragment(/*inExpressionContext*/ false, /*topInvalidNodePosition*/ undefined, openingTag);
49454972
default:
49464973
return Debug.assertNever(token);
49474974
}
@@ -4957,6 +4984,13 @@ namespace ts {
49574984
const child = parseJsxChild(openingTag, currentToken = scanner.reScanJsxToken());
49584985
if (!child) break;
49594986
list.push(child);
4987+
if (isJsxOpeningElement(openingTag)
4988+
&& child?.kind === SyntaxKind.JsxElement
4989+
&& !tagNamesAreEquivalent(child.openingElement.tagName, child.closingElement.tagName)
4990+
&& tagNamesAreEquivalent(openingTag.tagName, child.closingElement.tagName)) {
4991+
// stop after parsing a mismatched child like <div>...(<span></div>) in order to reattach the </div> higher
4992+
break;
4993+
}
49604994
}
49614995

49624996
parsingContext = saveParsingContext;
@@ -4978,7 +5012,6 @@ namespace ts {
49785012
scanJsxText();
49795013
return finishNode(factory.createJsxOpeningFragment(), pos);
49805014
}
4981-
49825015
const tagName = parseJsxElementName();
49835016
const typeArguments = (contextFlags & NodeFlags.JavaScriptFile) === 0 ? tryParseTypeArguments() : undefined;
49845017
const attributes = parseJsxAttributes();
@@ -4994,12 +5027,14 @@ namespace ts {
49945027
}
49955028
else {
49965029
parseExpected(SyntaxKind.SlashToken);
4997-
if (inExpressionContext) {
4998-
parseExpected(SyntaxKind.GreaterThanToken);
4999-
}
5000-
else {
5001-
parseExpected(SyntaxKind.GreaterThanToken, /*diagnostic*/ undefined, /*shouldAdvance*/ false);
5002-
scanJsxText();
5030+
if (parseExpected(SyntaxKind.GreaterThanToken, /*diagnostic*/ undefined, /*shouldAdvance*/ false)) {
5031+
// manually advance the scanner in order to look for jsx text inside jsx
5032+
if (inExpressionContext) {
5033+
nextToken();
5034+
}
5035+
else {
5036+
scanJsxText();
5037+
}
50035038
}
50045039
node = factory.createJsxSelfClosingElement(tagName, typeArguments, attributes);
50055040
}
@@ -5077,16 +5112,18 @@ namespace ts {
50775112
return finishNode(factory.createJsxSpreadAttribute(expression), pos);
50785113
}
50795114

5080-
function parseJsxClosingElement(inExpressionContext: boolean): JsxClosingElement {
5115+
function parseJsxClosingElement(open: JsxOpeningElement, inExpressionContext: boolean): JsxClosingElement {
50815116
const pos = getNodePos();
50825117
parseExpected(SyntaxKind.LessThanSlashToken);
50835118
const tagName = parseJsxElementName();
5084-
if (inExpressionContext) {
5085-
parseExpected(SyntaxKind.GreaterThanToken);
5086-
}
5087-
else {
5088-
parseExpected(SyntaxKind.GreaterThanToken, /*diagnostic*/ undefined, /*shouldAdvance*/ false);
5089-
scanJsxText();
5119+
if (parseExpected(SyntaxKind.GreaterThanToken, /*diagnostic*/ undefined, /*shouldAdvance*/ false)) {
5120+
// manually advance the scanner in order to look for jsx text inside jsx
5121+
if (inExpressionContext || !tagNamesAreEquivalent(open.tagName, tagName)) {
5122+
nextToken();
5123+
}
5124+
else {
5125+
scanJsxText();
5126+
}
50905127
}
50915128
return finishNode(factory.createJsxClosingElement(tagName), pos);
50925129
}
@@ -5097,12 +5134,14 @@ namespace ts {
50975134
if (tokenIsIdentifierOrKeyword(token())) {
50985135
parseErrorAtRange(parseJsxElementName(), Diagnostics.Expected_corresponding_closing_tag_for_JSX_fragment);
50995136
}
5100-
if (inExpressionContext) {
5101-
parseExpected(SyntaxKind.GreaterThanToken);
5102-
}
5103-
else {
5104-
parseExpected(SyntaxKind.GreaterThanToken, /*diagnostic*/ undefined, /*shouldAdvance*/ false);
5105-
scanJsxText();
5137+
if (parseExpected(SyntaxKind.GreaterThanToken, /*diagnostic*/ undefined, /*shouldAdvance*/ false)) {
5138+
// manually advance the scanner in order to look for jsx text inside jsx
5139+
if (inExpressionContext) {
5140+
nextToken();
5141+
}
5142+
else {
5143+
scanJsxText();
5144+
}
51065145
}
51075146
return finishNode(factory.createJsxJsxClosingFragment(), pos);
51085147
}

‎src/harness/fourslashImpl.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3177,7 +3177,7 @@ namespace FourSlash {
31773177
for (const markerName in map) {
31783178
this.goToMarker(markerName);
31793179
const actual = this.languageService.getJsxClosingTagAtPosition(this.activeFile.fileName, this.currentCaretPosition);
3180-
assert.deepEqual(actual, map[markerName]);
3180+
assert.deepEqual(actual, map[markerName], markerName);
31813181
}
31823182
}
31833183

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ tests/cases/compiler/a.jsx(1,13): error TS1109: Expression expected.
22
tests/cases/compiler/a.jsx(4,1): error TS2657: JSX expressions must have one parent element.
33
tests/cases/compiler/a.jsx(4,5): error TS1003: Identifier expected.
44
tests/cases/compiler/a.jsx(4,13): error TS1382: Unexpected token. Did you mean `{'>'}` or `&gt;`?
5-
tests/cases/compiler/a.jsx(4,14): error TS17002: Expected corresponding JSX closing tag for 'number'.
5+
tests/cases/compiler/a.jsx(4,16): error TS17002: Expected corresponding JSX closing tag for 'number'.
66
tests/cases/compiler/a.jsx(5,1): error TS2657: JSX expressions must have one parent element.
77
tests/cases/compiler/a.jsx(5,5): error TS1003: Identifier expected.
88
tests/cases/compiler/a.jsx(5,6): error TS17008: JSX element 'number' has no corresponding closing tag.
@@ -23,7 +23,7 @@ tests/cases/compiler/a.jsx(6,1): error TS1005: '</' expected.
2323
!!! error TS1003: Identifier expected.
2424
~
2525
!!! error TS1382: Unexpected token. Did you mean `{'>'}` or `&gt;`?
26-
~~~~~~
26+
~~~
2727
!!! error TS17002: Expected corresponding JSX closing tag for 'number'.
2828
<Foo<number>/>;
2929
~~~~~~~~~~~~~~~

‎tests/baselines/reference/jsxAndTypeAssertion.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ x = <any><any></any>;
3434

3535
x = <foo>hello {<foo>} </foo>};
3636

37-
x = <foo test={<foo>}>hello</foo>}/>
37+
x = <foo test={<foo>}>hello</foo>}/>;
3838

3939
x = <foo test={<foo>}>hello{<foo>}</foo>};
4040

‎tests/baselines/reference/jsxAndTypeAssertion.types

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ x = <foo>hello {<foo>{}} </foo>;
3333
>foo : typeof foo
3434

3535
x = <foo test={<foo>{}}>hello</foo>;
36-
><foo test={<foo>{}}>hello</foo>; : any
36+
><foo test={<foo>{}}>hello</foo> : any
3737
>foo : typeof foo
3838
>test : any
3939
><foo>{}}>hello</foo> : any

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ tests/cases/conformance/jsx/10.tsx(1,13): error TS1005: '>' expected.
88
tests/cases/conformance/jsx/10.tsx(1,14): error TS2304: Cannot find name 'c'.
99
tests/cases/conformance/jsx/10.tsx(1,16): error TS1109: Expression expected.
1010
tests/cases/conformance/jsx/11.tsx(1,2): error TS2304: Cannot find name 'a'.
11-
tests/cases/conformance/jsx/11.tsx(1,8): error TS17002: Expected corresponding JSX closing tag for 'a.b.c'.
11+
tests/cases/conformance/jsx/11.tsx(1,10): error TS17002: Expected corresponding JSX closing tag for 'a.b.c'.
1212
tests/cases/conformance/jsx/12.tsx(1,1): error TS1109: Expression expected.
1313
tests/cases/conformance/jsx/12.tsx(1,2): error TS1109: Expression expected.
1414
tests/cases/conformance/jsx/12.tsx(1,5): error TS1109: Expression expected.
@@ -73,9 +73,9 @@ tests/cases/conformance/jsx/31.tsx(1,4): error TS1003: Identifier expected.
7373
tests/cases/conformance/jsx/4.tsx(1,6): error TS1005: '{' expected.
7474
tests/cases/conformance/jsx/5.tsx(1,2): error TS17008: JSX element 'a' has no corresponding closing tag.
7575
tests/cases/conformance/jsx/5.tsx(1,5): error TS1005: '</' expected.
76-
tests/cases/conformance/jsx/6.tsx(1,4): error TS17002: Expected corresponding JSX closing tag for 'a'.
76+
tests/cases/conformance/jsx/6.tsx(1,6): error TS17002: Expected corresponding JSX closing tag for 'a'.
7777
tests/cases/conformance/jsx/7.tsx(1,13): error TS1002: Unterminated string literal.
78-
tests/cases/conformance/jsx/8.tsx(1,6): error TS17002: Expected corresponding JSX closing tag for 'a:b'.
78+
tests/cases/conformance/jsx/8.tsx(1,8): error TS17002: Expected corresponding JSX closing tag for 'a:b'.
7979
tests/cases/conformance/jsx/9.tsx(1,2): error TS2304: Cannot find name 'a:b'.
8080
tests/cases/conformance/jsx/9.tsx(1,2): error TS2633: JSX property access expressions cannot include JSX namespace names
8181
tests/cases/conformance/jsx/9.tsx(1,10): error TS2304: Cannot find name 'a:b'.
@@ -119,15 +119,15 @@ tests/cases/conformance/jsx/9.tsx(1,10): error TS2304: Cannot find name 'a:b'.
119119
!!! error TS1005: '</' expected.
120120
==== tests/cases/conformance/jsx/6.tsx (1 errors) ====
121121
<a></b>;
122-
~~~~
122+
~
123123
!!! error TS17002: Expected corresponding JSX closing tag for 'a'.
124124
==== tests/cases/conformance/jsx/7.tsx (1 errors) ====
125125
<a foo="bar;
126126

127127
!!! error TS1002: Unterminated string literal.
128128
==== tests/cases/conformance/jsx/8.tsx (1 errors) ====
129129
<a:b></b>;
130-
~~~~
130+
~
131131
!!! error TS17002: Expected corresponding JSX closing tag for 'a:b'.
132132
==== tests/cases/conformance/jsx/9.tsx (3 errors) ====
133133
<a:b.c></a:b.c>;
@@ -155,7 +155,7 @@ tests/cases/conformance/jsx/9.tsx(1,10): error TS2304: Cannot find name 'a:b'.
155155
<a.b.c></a>;
156156
~
157157
!!! error TS2304: Cannot find name 'a'.
158-
~~~~
158+
~
159159
!!! error TS17002: Expected corresponding JSX closing tag for 'a.b.c'.
160160
==== tests/cases/conformance/jsx/12.tsx (6 errors) ====
161161
<.a></.a>;

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

+8-14
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
tests/cases/conformance/jsx/Error1.tsx(2,11): error TS17008: JSX element 'div' has no corresponding closing tag.
2-
tests/cases/conformance/jsx/Error1.tsx(2,21): error TS17002: Expected corresponding JSX closing tag for 'span'.
3-
tests/cases/conformance/jsx/Error1.tsx(3,1): error TS1005: '</' expected.
4-
tests/cases/conformance/jsx/Error2.tsx(1,15): error TS17002: Expected corresponding JSX closing tag for 'div'.
1+
tests/cases/conformance/jsx/Error1.tsx(2,16): error TS17008: JSX element 'span' has no corresponding closing tag.
2+
tests/cases/conformance/jsx/Error2.tsx(1,17): error TS17002: Expected corresponding JSX closing tag for 'div'.
53
tests/cases/conformance/jsx/Error3.tsx(1,11): error TS17008: JSX element 'div' has no corresponding closing tag.
64
tests/cases/conformance/jsx/Error3.tsx(3,1): error TS1005: '</' expected.
75
tests/cases/conformance/jsx/Error4.tsx(1,11): error TS17008: JSX element 'div' has no corresponding closing tag.
8-
tests/cases/conformance/jsx/Error4.tsx(1,20): error TS17002: Expected corresponding JSX closing tag for 'div'.
6+
tests/cases/conformance/jsx/Error4.tsx(1,22): error TS17002: Expected corresponding JSX closing tag for 'div'.
97
tests/cases/conformance/jsx/Error4.tsx(2,1): error TS1005: '</' expected.
108
tests/cases/conformance/jsx/Error5.tsx(1,11): error TS17008: JSX element 'div' has no corresponding closing tag.
119
tests/cases/conformance/jsx/Error5.tsx(1,16): error TS17008: JSX element 'span' has no corresponding closing tag.
@@ -20,19 +18,15 @@ tests/cases/conformance/jsx/Error5.tsx(3,1): error TS1005: '</' expected.
2018
}
2119
}
2220

23-
==== tests/cases/conformance/jsx/Error1.tsx (3 errors) ====
21+
==== tests/cases/conformance/jsx/Error1.tsx (1 errors) ====
2422
// Issue error about missing span closing tag, not missing div closing tag
2523
let x1 = <div><span></div>;
26-
~~~
27-
!!! error TS17008: JSX element 'div' has no corresponding closing tag.
28-
~~~~~~
29-
!!! error TS17002: Expected corresponding JSX closing tag for 'span'.
30-
24+
~~~~
25+
!!! error TS17008: JSX element 'span' has no corresponding closing tag.
3126

32-
!!! error TS1005: '</' expected.
3327
==== tests/cases/conformance/jsx/Error2.tsx (1 errors) ====
3428
let x2 = <div></span>;
35-
~~~~~~~
29+
~~~~
3630
!!! error TS17002: Expected corresponding JSX closing tag for 'div'.
3731

3832

@@ -48,7 +42,7 @@ tests/cases/conformance/jsx/Error5.tsx(3,1): error TS1005: '</' expected.
4842
let x4 = <div><div></span>;
4943
~~~
5044
!!! error TS17008: JSX element 'div' has no corresponding closing tag.
51-
~~~~~~~
45+
~~~~
5246
!!! error TS17002: Expected corresponding JSX closing tag for 'div'.
5347

5448

‎tests/baselines/reference/jsxParsingError2.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ let x5 = <div><span>
3131
//// [file.jsx]
3232
//// [Error1.jsx]
3333
// Issue error about missing span closing tag, not missing div closing tag
34-
var x1 = <div><span></div>;
35-
</>;
34+
var x1 = <div><span></></div>;
3635
//// [Error2.jsx]
3736
var x2 = <div></span>;
3837
//// [Error3.jsx]

‎tests/baselines/reference/jsxParsingError2.types

+3-4
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,12 @@ declare module JSX {
1111
// Issue error about missing span closing tag, not missing div closing tag
1212
let x1 = <div><span></div>;
1313
>x1 : JSX.Element
14-
><div><span></div>; : JSX.Element
14+
><div><span></div> : JSX.Element
1515
>div : any
16-
><span></div> : JSX.Element
16+
><span> : JSX.Element
1717
>span : any
18-
>div : any
19-
2018
> : any
19+
>div : any
2120

2221
=== tests/cases/conformance/jsx/Error2.tsx ===
2322
let x2 = <div></span>;

0 commit comments

Comments
 (0)