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 8e290e5

Browse filesBrowse files
Jack-Worksandrewbranch
Jack-Works
and
andrewbranch
authored
Improve error range for ts2657 (jsx expr must have parent element), add code fix for it (microsoft#37917)
* fix: range of ts2657 (jsx expr must have parent) and remove 2695 (LHS expr of comma has no side effects) * feat: add code fix for 2657 * fix: resolve review * chore: hoist a var * chore: add test for skipTrivia * fix: rebase error * Update src/compiler/diagnosticMessages.json Co-authored-by: Andrew Branch <andrewbranch@users.noreply.github.com> * Update src/services/codefixes/wrapJsxInFragment.ts Co-authored-by: Andrew Branch <andrewbranch@users.noreply.github.com> Co-authored-by: Andrew Branch <andrew@wheream.io> Co-authored-by: Andrew Branch <andrewbranch@users.noreply.github.com>
1 parent 4f0b81d commit 8e290e5
Copy full SHA for 8e290e5

16 files changed

+152-60Lines changed: 152 additions & 60 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
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28769,7 +28769,14 @@ namespace ts {
2876928769
}
2877028770
case SyntaxKind.CommaToken:
2877128771
if (!compilerOptions.allowUnreachableCode && isSideEffectFree(left) && !isEvalNode(right)) {
28772-
error(left, Diagnostics.Left_side_of_comma_operator_is_unused_and_has_no_side_effects);
28772+
const sf = getSourceFileOfNode(left);
28773+
const sourceText = sf.text;
28774+
const start = skipTrivia(sourceText, left.pos);
28775+
const isInDiag2657 = sf.parseDiagnostics.some(diag => {
28776+
if (diag.code !== Diagnostics.JSX_expressions_must_have_one_parent_element.code) return false;
28777+
return textSpanContainsPosition(diag, start);
28778+
});
28779+
if (!isInDiag2657) error(left, Diagnostics.Left_side_of_comma_operator_is_unused_and_has_no_side_effects);
2877328780
}
2877428781
return rightType;
2877528782

Collapse file

‎src/compiler/diagnosticMessages.json‎

Copy file name to clipboardExpand all lines: src/compiler/diagnosticMessages.json
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5709,6 +5709,14 @@
57095709
"category": "Message",
57105710
"code": 95119
57115711
},
5712+
"Wrap in JSX fragment": {
5713+
"category": "Message",
5714+
"code": 95120
5715+
},
5716+
"Wrap all unparented JSX in JSX fragment": {
5717+
"category": "Message",
5718+
"code": 95121
5719+
},
57125720

57135721
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
57145722
"category": "Error",
Collapse file

‎src/compiler/parser.ts‎

Copy file name to clipboardExpand all lines: src/compiler/parser.ts
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4503,7 +4503,7 @@ namespace ts {
45034503
return finishNode(node);
45044504
}
45054505

4506-
function parseJsxElementOrSelfClosingElementOrFragment(inExpressionContext: boolean): JsxElement | JsxSelfClosingElement | JsxFragment {
4506+
function parseJsxElementOrSelfClosingElementOrFragment(inExpressionContext: boolean, topInvalidNodePosition?: number): JsxElement | JsxSelfClosingElement | JsxFragment {
45074507
const opening = parseJsxOpeningOrSelfClosingElementOrOpeningFragment(inExpressionContext);
45084508
let result: JsxElement | JsxSelfClosingElement | JsxFragment;
45094509
if (opening.kind === SyntaxKind.JsxOpeningElement) {
@@ -4541,15 +4541,16 @@ namespace ts {
45414541
// Since JSX elements are invalid < operands anyway, this lookahead parse will only occur in error scenarios
45424542
// of one sort or another.
45434543
if (inExpressionContext && token() === SyntaxKind.LessThanToken) {
4544-
const invalidElement = tryParse(() => parseJsxElementOrSelfClosingElementOrFragment(/*inExpressionContext*/ true));
4544+
const topBadPos = typeof topInvalidNodePosition === "undefined" ? result.pos : topInvalidNodePosition;
4545+
const invalidElement = tryParse(() => parseJsxElementOrSelfClosingElementOrFragment(/*inExpressionContext*/ true, topBadPos));
45454546
if (invalidElement) {
4546-
parseErrorAtCurrentToken(Diagnostics.JSX_expressions_must_have_one_parent_element);
45474547
const badNode = <BinaryExpression>createNode(SyntaxKind.BinaryExpression, result.pos);
45484548
badNode.end = invalidElement.end;
45494549
badNode.left = result;
45504550
badNode.right = invalidElement;
45514551
badNode.operatorToken = createMissingNode(SyntaxKind.CommaToken, /*reportAtCurrentPosition*/ false);
45524552
badNode.operatorToken.pos = badNode.operatorToken.end = badNode.right.pos;
4553+
parseErrorAt(skipTrivia(sourceText, topBadPos), invalidElement.end, Diagnostics.JSX_expressions_must_have_one_parent_element);
45534554
return <JsxElement><Node>badNode;
45544555
}
45554556
}
Collapse file

‎src/services/codefixes/wrapJsxInFragment.ts‎

Copy file name to clipboard
+71Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/* @internal */
2+
namespace ts.codefix {
3+
const fixID = "wrapJsxInFragment";
4+
const errorCodes = [Diagnostics.JSX_expressions_must_have_one_parent_element.code];
5+
registerCodeFix({
6+
errorCodes,
7+
getCodeActions: context => {
8+
const { jsx } = context.program.getCompilerOptions();
9+
if (jsx !== JsxEmit.React && jsx !== JsxEmit.ReactNative) {
10+
return undefined;
11+
}
12+
const { sourceFile, span } = context;
13+
const node = findNodeToFix(sourceFile, span.start);
14+
if (!node) return undefined;
15+
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, sourceFile, node));
16+
return [createCodeFixAction(fixID, changes, Diagnostics.Wrap_in_JSX_fragment, fixID, Diagnostics.Wrap_all_unparented_JSX_in_JSX_fragment)];
17+
},
18+
fixIds: [fixID],
19+
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
20+
const node = findNodeToFix(context.sourceFile, diag.start);
21+
if (!node) return undefined;
22+
doChange(changes, context.sourceFile, node);
23+
}),
24+
});
25+
26+
function findNodeToFix(sourceFile: SourceFile, pos: number): BinaryExpression | undefined {
27+
// The error always at 1st token that is "<" in "<a /><a />"
28+
const lessThanToken = getTokenAtPosition(sourceFile, pos);
29+
const firstJsxElementOrOpenElement = lessThanToken.parent;
30+
let binaryExpr = firstJsxElementOrOpenElement.parent;
31+
if (!isBinaryExpression(binaryExpr)) {
32+
// In case the start element is a JsxSelfClosingElement, it the end.
33+
// For JsxOpenElement, find one more parent
34+
binaryExpr = binaryExpr.parent;
35+
if (!isBinaryExpression(binaryExpr)) return undefined;
36+
}
37+
if (!nodeIsMissing(binaryExpr.operatorToken)) return undefined;
38+
return binaryExpr;
39+
}
40+
41+
function doChange(changeTracker: textChanges.ChangeTracker, sf: SourceFile, node: Node) {
42+
const jsx = flattenInvalidBinaryExpr(node);
43+
if (jsx) changeTracker.replaceNode(sf, node, createJsxFragment(createJsxOpeningFragment(), jsx, createJsxJsxClosingFragment()));
44+
}
45+
// The invalid syntax is constructed as
46+
// InvalidJsxTree :: One of
47+
// JsxElement CommaToken InvalidJsxTree
48+
// JsxElement CommaToken JsxElement
49+
function flattenInvalidBinaryExpr(node: Node): JsxChild[] | undefined {
50+
const children: JsxChild[] = [];
51+
let current = node;
52+
while (true) {
53+
if (isBinaryExpression(current) && nodeIsMissing(current.operatorToken) && current.operatorToken.kind === SyntaxKind.CommaToken) {
54+
children.push(<JsxChild>current.left);
55+
if (isJsxChild(current.right)) {
56+
children.push(current.right);
57+
// Indicates the tree has go to the bottom
58+
return children;
59+
}
60+
else if (isBinaryExpression(current.right)) {
61+
current = current.right;
62+
continue;
63+
}
64+
// Unreachable case
65+
else return undefined;
66+
}
67+
// Unreachable case
68+
else return undefined;
69+
}
70+
}
71+
}
Collapse file

‎src/services/tsconfig.json‎

Copy file name to clipboardExpand all lines: src/services/tsconfig.json
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
"codefixes/useDefaultImport.ts",
9898
"codefixes/useBigintLiteral.ts",
9999
"codefixes/fixAddModuleReferTypeMissingTypeof.ts",
100+
"codefixes/wrapJsxInFragment.ts",
100101
"codefixes/convertToMappedObjectType.ts",
101102
"codefixes/removeUnnecessaryAwait.ts",
102103
"codefixes/splitTypeOnlyImport.ts",
Collapse file

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

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,1): error TS2695: Left side of comma operator is unused and has no side effects.
1+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,1): error TS2657: JSX expressions must have one parent element.
22
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,17): error TS1005: '{' expected.
3+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS1005: ';' expected.
34
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS2304: Cannot find name 'right'.
4-
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS2657: JSX expressions must have one parent element.
55
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,41): error TS1382: Unexpected token. Did you mean `{'>'}` or `&gt;`?
66
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,57): error TS1109: Expression expected.
77
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expression expected.
@@ -47,14 +47,14 @@ tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expr
4747
<div><br />7x invalid-js-identifier</div>;
4848

4949
<LeftRight left=<a /> right=<b>monkeys /> gorillas</b> />;
50-
~~~~~~~~~~~~~~~~
51-
!!! error TS2695: Left side of comma operator is unused and has no side effects.
50+
~~~~~~~~~~~~~~~~~~~~~
51+
!!! error TS2657: JSX expressions must have one parent element.
5252
~
5353
!!! error TS1005: '{' expected.
5454
~~~~~
55-
!!! error TS2304: Cannot find name 'right'.
55+
!!! error TS1005: ';' expected.
5656
~~~~~
57-
!!! error TS2657: JSX expressions must have one parent element.
57+
!!! error TS2304: Cannot find name 'right'.
5858
~
5959
!!! error TS1382: Unexpected token. Did you mean `{'>'}` or `&gt;`?
6060
~
Collapse file

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

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt
+11-16Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,8 @@ tests/cases/conformance/jsx/16.tsx(1,2): error TS17008: JSX element 'a' has no c
3333
tests/cases/conformance/jsx/16.tsx(1,10): error TS1005: '</' expected.
3434
tests/cases/conformance/jsx/17.tsx(1,2): error TS17008: JSX element 'a' has no corresponding closing tag.
3535
tests/cases/conformance/jsx/17.tsx(1,10): error TS1005: '</' expected.
36-
tests/cases/conformance/jsx/18.tsx(1,9): error TS2695: Left side of comma operator is unused and has no side effects.
37-
tests/cases/conformance/jsx/18.tsx(1,37): error TS2657: JSX expressions must have one parent element.
38-
tests/cases/conformance/jsx/19.tsx(1,9): error TS2695: Left side of comma operator is unused and has no side effects.
39-
tests/cases/conformance/jsx/19.tsx(1,64): error TS2657: JSX expressions must have one parent element.
36+
tests/cases/conformance/jsx/18.tsx(1,30): error TS2657: JSX expressions must have one parent element.
37+
tests/cases/conformance/jsx/19.tsx(1,9): error TS2657: JSX expressions must have one parent element.
4038
tests/cases/conformance/jsx/2.tsx(1,3): error TS1003: Identifier expected.
4139
tests/cases/conformance/jsx/20.tsx(1,10): error TS1005: '}' expected.
4240
tests/cases/conformance/jsx/20.tsx(1,11): error TS1381: Unexpected token. Did you mean `{'}'}` or `&rbrace;`?
@@ -61,7 +59,7 @@ tests/cases/conformance/jsx/27.tsx(1,5): error TS1382: Unexpected token. Did you
6159
tests/cases/conformance/jsx/28.tsx(1,2): error TS17008: JSX element 'a' has no corresponding closing tag.
6260
tests/cases/conformance/jsx/28.tsx(1,6): error TS1005: '{' expected.
6361
tests/cases/conformance/jsx/28.tsx(2,1): error TS1005: '</' expected.
64-
tests/cases/conformance/jsx/29.tsx(1,1): error TS2695: Left side of comma operator is unused and has no side effects.
62+
tests/cases/conformance/jsx/29.tsx(1,1): error TS2657: JSX expressions must have one parent element.
6563
tests/cases/conformance/jsx/29.tsx(1,6): error TS1005: '{' expected.
6664
tests/cases/conformance/jsx/29.tsx(1,7): error TS1003: Identifier expected.
6765
tests/cases/conformance/jsx/29.tsx(2,1): error TS1005: '</' expected.
@@ -228,17 +226,13 @@ tests/cases/conformance/jsx/9.tsx(1,16): error TS1109: Expression expected.
228226
!!! error TS17008: JSX element 'a' has no corresponding closing tag.
229227

230228
!!! error TS1005: '</' expected.
231-
==== tests/cases/conformance/jsx/18.tsx (2 errors) ====
232-
var x = <div>one</div><div>two</div>;;
233-
~~~~~~~~~~~~~~
234-
!!! error TS2695: Left side of comma operator is unused and has no side effects.
235-
~
229+
==== tests/cases/conformance/jsx/18.tsx (1 errors) ====
230+
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
231+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
236232
!!! error TS2657: JSX expressions must have one parent element.
237-
==== tests/cases/conformance/jsx/19.tsx (2 errors) ====
233+
==== tests/cases/conformance/jsx/19.tsx (1 errors) ====
238234
var x = <div>one</div> /* intervening comment */ <div>two</div>;;
239-
~~~~~~~~~~~~~~
240-
!!! error TS2695: Left side of comma operator is unused and has no side effects.
241-
~
235+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
242236
!!! error TS2657: JSX expressions must have one parent element.
243237
==== tests/cases/conformance/jsx/20.tsx (2 errors) ====
244238
<a>{"str";}</a>;
@@ -313,14 +307,15 @@ tests/cases/conformance/jsx/9.tsx(1,16): error TS1109: Expression expected.
313307
!!! error TS1005: '</' expected.
314308
==== tests/cases/conformance/jsx/29.tsx (4 errors) ====
315309
<a b=<}>;
316-
~~~~~
317-
!!! error TS2695: Left side of comma operator is unused and has no side effects.
310+
~~~~~~~~~
318311
~
319312
!!! error TS1005: '{' expected.
320313
~
321314
!!! error TS1003: Identifier expected.
322315

323316

317+
!!! error TS2657: JSX expressions must have one parent element.
318+
324319
!!! error TS1005: '</' expected.
325320
==== tests/cases/conformance/jsx/30.tsx (1 errors) ====
326321
<a>}</a>;
Collapse file

‎tests/baselines/reference/jsxInvalidEsprimaTestSuite.js‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxInvalidEsprimaTestSuite.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ declare var React: any;
3737
//// [17.tsx]
3838
<a b={}>;
3939
//// [18.tsx]
40-
var x = <div>one</div><div>two</div>;;
40+
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
4141
//// [19.tsx]
4242
var x = <div>one</div> /* intervening comment */ <div>two</div>;;
4343
//// [20.tsx]
@@ -117,7 +117,7 @@ a['foo'] > ;
117117
//// [17.jsx]
118118
<a b=>;</>;
119119
//// [18.jsx]
120-
var x = <div>one</div>, <div>two</div>;
120+
var x = /* Leading trivia */ <div>one</div>, <div>two</div>;
121121
;
122122
//// [19.jsx]
123123
var x = <div>one</div> /* intervening comment */, /* intervening comment */ <div>two</div>;
Collapse file

‎tests/baselines/reference/jsxInvalidEsprimaTestSuite.symbols‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxInvalidEsprimaTestSuite.symbols
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ No type information for this code.=== tests/cases/conformance/jsx/17.tsx ===
5050
>b : Symbol(b, Decl(17.tsx, 0, 2))
5151

5252
=== tests/cases/conformance/jsx/18.tsx ===
53-
var x = <div>one</div><div>two</div>;;
53+
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
5454
>x : Symbol(x, Decl(18.tsx, 0, 3), Decl(19.tsx, 0, 3))
5555

5656
=== tests/cases/conformance/jsx/19.tsx ===
Collapse file

‎tests/baselines/reference/jsxInvalidEsprimaTestSuite.types‎

Copy file name to clipboardExpand all lines: tests/baselines/reference/jsxInvalidEsprimaTestSuite.types
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ declare var React: any;
159159
> : any
160160

161161
=== tests/cases/conformance/jsx/18.tsx ===
162-
var x = <div>one</div><div>two</div>;;
162+
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
163163
>x : any
164164
><div>one</div><div>two</div> : any
165165
><div>one</div> : any

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.