Skip to content
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.

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

Improve error range for ts2657 (jsx expr must have parent element), add code fix for it #37917

Merged
merged 9 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
9 changes: 8 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28742,7 +28742,14 @@ namespace ts {
}
case SyntaxKind.CommaToken:
if (!compilerOptions.allowUnreachableCode && isSideEffectFree(left) && !isEvalNode(right)) {
error(left, Diagnostics.Left_side_of_comma_operator_is_unused_and_has_no_side_effects);
const sf = getSourceFileOfNode(left);
const sourceText = sf.text;
const start = skipTrivia(sourceText, left.pos);
const isInDiag2657 = sf.parseDiagnostics.some(diag => {
if (diag.code !== Diagnostics.JSX_expressions_must_have_one_parent_element.code) return false;
return textSpanContainsPosition(diag, start);
});
if (!isInDiag2657) error(left, Diagnostics.Left_side_of_comma_operator_is_unused_and_has_no_side_effects);
}
return rightType;

Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -5705,6 +5705,14 @@
"category": "Message",
"code": 95118
},
"Wrap JSX in JSX Fragment": {
"category": "Message",
"code": 95119
},
"Wrap all JSX in JSX Fragment": {
"category": "Message",
"code": 95120
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
7 changes: 4 additions & 3 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4503,7 +4503,7 @@ namespace ts {
return finishNode(node);
}

function parseJsxElementOrSelfClosingElementOrFragment(inExpressionContext: boolean): JsxElement | JsxSelfClosingElement | JsxFragment {
function parseJsxElementOrSelfClosingElementOrFragment(inExpressionContext: boolean, topInvalidNodePosition?: number): JsxElement | JsxSelfClosingElement | JsxFragment {
const opening = parseJsxOpeningOrSelfClosingElementOrOpeningFragment(inExpressionContext);
let result: JsxElement | JsxSelfClosingElement | JsxFragment;
if (opening.kind === SyntaxKind.JsxOpeningElement) {
Expand Down Expand Up @@ -4541,15 +4541,16 @@ namespace ts {
// 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) {
const invalidElement = tryParse(() => parseJsxElementOrSelfClosingElementOrFragment(/*inExpressionContext*/ true));
const topBadPos = typeof topInvalidNodePosition === "undefined" ? result.pos : topInvalidNodePosition;
const invalidElement = tryParse(() => parseJsxElementOrSelfClosingElementOrFragment(/*inExpressionContext*/ true, topBadPos));
if (invalidElement) {
parseErrorAtCurrentToken(Diagnostics.JSX_expressions_must_have_one_parent_element);
const badNode = <BinaryExpression>createNode(SyntaxKind.BinaryExpression, result.pos);
badNode.end = invalidElement.end;
badNode.left = result;
badNode.right = invalidElement;
badNode.operatorToken = createMissingNode(SyntaxKind.CommaToken, /*reportAtCurrentPosition*/ false);
badNode.operatorToken.pos = badNode.operatorToken.end = badNode.right.pos;
parseErrorAt(skipTrivia(sourceText, topBadPos), invalidElement.end, Diagnostics.JSX_expressions_must_have_one_parent_element);
Jack-Works marked this conversation as resolved.
Show resolved Hide resolved
return <JsxElement><Node>badNode;
}
}
Expand Down
71 changes: 71 additions & 0 deletions src/services/codefixes/wrapJsxInFragment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/* @internal */
namespace ts.codefix {
const fixID = "wrapJsxInFragment";
const errorCodes = [Diagnostics.JSX_expressions_must_have_one_parent_element.code];
registerCodeFix({
errorCodes,
getCodeActions: context => {
const { jsx } = context.program.getCompilerOptions();
if (jsx !== JsxEmit.React && jsx !== JsxEmit.ReactNative) {
return undefined;
}
const { sourceFile, span } = context;
const node = findNodeToFix(sourceFile, span.start);
if (!node) return undefined;
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, sourceFile, node));
return [createCodeFixAction(fixID, changes, Diagnostics.Wrap_all_JSX_in_JSX_Fragment, fixID, Diagnostics.Wrap_all_JSX_in_JSX_Fragment)];
Jack-Works marked this conversation as resolved.
Show resolved Hide resolved
},
fixIds: [fixID],
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
const node = findNodeToFix(context.sourceFile, diag.start);
if (!node) return undefined;
doChange(changes, context.sourceFile, node);
}),
});

function findNodeToFix(sourceFile: SourceFile, pos: number): BinaryExpression | undefined {
// The error always at 1st token that is "<" in "<a /><a />"
const lessThanToken = getTokenAtPosition(sourceFile, pos);
const firstJsxElementOrOpenElement = lessThanToken.parent;
let binaryExpr = firstJsxElementOrOpenElement.parent;
if (!isBinaryExpression(binaryExpr)) {
// In case the start element is a JsxSelfClosingElement, it the end.
// For JsxOpenElement, find one more parent
binaryExpr = binaryExpr.parent;
if (!isBinaryExpression(binaryExpr)) return undefined;
}
if (!nodeIsMissing(binaryExpr.operatorToken)) return undefined;
return binaryExpr;
}

function doChange(changeTracker: textChanges.ChangeTracker, sf: SourceFile, node: Node) {
const jsx = flattenInvalidBinaryExpr(node);
if (jsx) changeTracker.replaceNode(sf, node, createJsxFragment(createJsxOpeningFragment(), jsx, createJsxJsxClosingFragment()));
}
// The invalid syntax is constructed as
// InvalidJsxTree :: One of
// JsxElement CommaToken InvalidJsxTree
// JsxElement CommaToken JsxElement
function flattenInvalidBinaryExpr(node: Node): JsxChild[] | undefined {
const children: JsxChild[] = [];
let current = node;
while (true) {
if (isBinaryExpression(current) && nodeIsMissing(current.operatorToken) && current.operatorToken.kind === SyntaxKind.CommaToken) {
children.push(<JsxChild>current.left);
if (isJsxChild(current.right)) {
children.push(current.right);
// Indicates the tree has go to the bottom
return children;
}
else if (isBinaryExpression(current.right)) {
current = current.right;
continue;
}
// Unreachable case
else return undefined;
}
// Unreachable case
else return undefined;
}
}
}
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
"codefixes/useDefaultImport.ts",
"codefixes/useBigintLiteral.ts",
"codefixes/fixAddModuleReferTypeMissingTypeof.ts",
"codefixes/wrapJsxInFragment.ts",
"codefixes/convertToMappedObjectType.ts",
"codefixes/removeUnnecessaryAwait.ts",
"codefixes/splitTypeOnlyImport.ts",
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,1): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,17): error TS1005: '{' expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS1005: ';' expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS2304: Cannot find name 'right'.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,41): error TS1382: Unexpected token. Did you mean `{'>'}` or `&gt;`?
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,57): error TS1109: Expression expected.
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expression expected.
Expand Down Expand Up @@ -47,14 +47,14 @@ tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expr
<div><br />7x invalid-js-identifier</div>;

<LeftRight left=<a /> right=<b>monkeys /> gorillas</b> />;
~~~~~~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~~~~~~~~~~~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
~
!!! error TS1005: '{' expected.
~~~~~
!!! error TS2304: Cannot find name 'right'.
!!! error TS1005: ';' expected.
~~~~~
!!! error TS2657: JSX expressions must have one parent element.
!!! error TS2304: Cannot find name 'right'.
~
!!! error TS1382: Unexpected token. Did you mean `{'>'}` or `&gt;`?
~
Expand Down
27 changes: 11 additions & 16 deletions tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ tests/cases/conformance/jsx/16.tsx(1,2): error TS17008: JSX element 'a' has no c
tests/cases/conformance/jsx/16.tsx(1,10): error TS1005: '</' expected.
tests/cases/conformance/jsx/17.tsx(1,2): error TS17008: JSX element 'a' has no corresponding closing tag.
tests/cases/conformance/jsx/17.tsx(1,10): error TS1005: '</' expected.
tests/cases/conformance/jsx/18.tsx(1,9): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/18.tsx(1,37): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/19.tsx(1,9): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/19.tsx(1,64): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/18.tsx(1,30): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/19.tsx(1,9): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/2.tsx(1,3): error TS1003: Identifier expected.
tests/cases/conformance/jsx/20.tsx(1,10): error TS1005: '}' expected.
tests/cases/conformance/jsx/20.tsx(1,11): error TS1381: Unexpected token. Did you mean `{'}'}` or `&rbrace;`?
Expand All @@ -61,7 +59,7 @@ tests/cases/conformance/jsx/27.tsx(1,5): error TS1382: Unexpected token. Did you
tests/cases/conformance/jsx/28.tsx(1,2): error TS17008: JSX element 'a' has no corresponding closing tag.
tests/cases/conformance/jsx/28.tsx(1,6): error TS1005: '{' expected.
tests/cases/conformance/jsx/28.tsx(2,1): error TS1005: '</' expected.
tests/cases/conformance/jsx/29.tsx(1,1): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/29.tsx(1,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/29.tsx(1,6): error TS1005: '{' expected.
tests/cases/conformance/jsx/29.tsx(1,7): error TS1003: Identifier expected.
tests/cases/conformance/jsx/29.tsx(2,1): error TS1005: '</' expected.
Expand Down Expand Up @@ -228,17 +226,13 @@ tests/cases/conformance/jsx/9.tsx(1,16): error TS1109: Expression expected.
!!! error TS17008: JSX element 'a' has no corresponding closing tag.

!!! error TS1005: '</' expected.
==== tests/cases/conformance/jsx/18.tsx (2 errors) ====
var x = <div>one</div><div>two</div>;;
~~~~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~
==== tests/cases/conformance/jsx/18.tsx (1 errors) ====
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
==== tests/cases/conformance/jsx/19.tsx (2 errors) ====
==== tests/cases/conformance/jsx/19.tsx (1 errors) ====
var x = <div>one</div> /* intervening comment */ <div>two</div>;;
~~~~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
==== tests/cases/conformance/jsx/20.tsx (2 errors) ====
<a>{"str";}</a>;
Expand Down Expand Up @@ -313,14 +307,15 @@ tests/cases/conformance/jsx/9.tsx(1,16): error TS1109: Expression expected.
!!! error TS1005: '</' expected.
==== tests/cases/conformance/jsx/29.tsx (4 errors) ====
<a b=<}>;
~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~~~~~~~~~
~
!!! error TS1005: '{' expected.
~
!!! error TS1003: Identifier expected.


!!! error TS2657: JSX expressions must have one parent element.

!!! error TS1005: '</' expected.
==== tests/cases/conformance/jsx/30.tsx (1 errors) ====
<a>}</a>;
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/jsxInvalidEsprimaTestSuite.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ declare var React: any;
//// [17.tsx]
<a b={}>;
//// [18.tsx]
var x = <div>one</div><div>two</div>;;
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
//// [19.tsx]
var x = <div>one</div> /* intervening comment */ <div>two</div>;;
//// [20.tsx]
Expand Down Expand Up @@ -117,7 +117,7 @@ a['foo'] > ;
//// [17.jsx]
<a b=>;</>;
//// [18.jsx]
var x = <div>one</div>, <div>two</div>;
var x = /* Leading trivia */ <div>one</div>, <div>two</div>;
;
//// [19.jsx]
var x = <div>one</div> /* intervening comment */, /* intervening comment */ <div>two</div>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ No type information for this code.=== tests/cases/conformance/jsx/17.tsx ===
>b : Symbol(b, Decl(17.tsx, 0, 2))

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

=== tests/cases/conformance/jsx/19.tsx ===
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsxInvalidEsprimaTestSuite.types
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ declare var React: any;
> : any

=== tests/cases/conformance/jsx/18.tsx ===
var x = <div>one</div><div>two</div>;;
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
>x : any
><div>one</div><div>two</div> : any
><div>one</div> : any
Expand Down
23 changes: 9 additions & 14 deletions tests/baselines/reference/tsxErrorRecovery2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
tests/cases/conformance/jsx/file1.tsx(3,1): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/file1.tsx(5,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/file2.tsx(1,9): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/file2.tsx(2,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/file1.tsx(3,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/file2.tsx(1,9): error TS2657: JSX expressions must have one parent element.


==== tests/cases/conformance/jsx/file1.tsx (2 errors) ====
==== tests/cases/conformance/jsx/file1.tsx (1 errors) ====
declare namespace JSX { interface Element { } }

<div></div>
~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
<div></div>


~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
==== tests/cases/conformance/jsx/file2.tsx (2 errors) ====
var x = <div></div><div></div>
~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.


!!! 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.

23 changes: 9 additions & 14 deletions tests/baselines/reference/tsxErrorRecovery3.errors.txt
Original file line number Diff line number Diff line change
@@ -1,35 +1,30 @@
tests/cases/conformance/jsx/file1.tsx(3,1): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/file1.tsx(3,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/file1.tsx(3,2): error TS2304: Cannot find name 'React'.
tests/cases/conformance/jsx/file1.tsx(4,2): error TS2304: Cannot find name 'React'.
tests/cases/conformance/jsx/file1.tsx(5,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/file2.tsx(1,9): error TS2695: Left side of comma operator is unused and has no side effects.
tests/cases/conformance/jsx/file2.tsx(1,9): 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 (4 errors) ====
==== tests/cases/conformance/jsx/file1.tsx (3 errors) ====
declare namespace JSX { interface Element { } }

<div></div>
~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~~~
!!! error TS2304: Cannot find name 'React'.
<div></div>
~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
~~~
!!! error TS2304: Cannot find name 'React'.


!!! error TS2657: JSX expressions must have one parent element.
==== tests/cases/conformance/jsx/file2.tsx (4 errors) ====
==== tests/cases/conformance/jsx/file2.tsx (3 errors) ====
var x = <div></div><div></div>
~~~~~~~~~~~
!!! error TS2695: Left side of comma operator is unused and has no side effects.
~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
~~~
!!! error TS2304: Cannot find name 'React'.
~~~
!!! error TS2304: Cannot find name 'React'.


!!! error TS2657: JSX expressions must have one parent element.

7 changes: 6 additions & 1 deletion tests/baselines/reference/tsxFragmentErrors.errors.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
tests/cases/conformance/jsx/file.tsx(9,1): error TS2657: JSX expressions must have one parent element.
tests/cases/conformance/jsx/file.tsx(9,7): error TS17015: Expected corresponding closing tag for JSX fragment.
tests/cases/conformance/jsx/file.tsx(9,11): error TS17014: JSX fragment has no corresponding closing tag.
tests/cases/conformance/jsx/file.tsx(11,17): error TS1005: '</' expected.


==== tests/cases/conformance/jsx/file.tsx (3 errors) ====
==== tests/cases/conformance/jsx/file.tsx (4 errors) ====
declare module JSX {
interface Element { }
interface IntrinsicElements {
Expand All @@ -13,12 +14,16 @@ tests/cases/conformance/jsx/file.tsx(11,17): error TS1005: '</' expected.
declare var React: any;

<>hi</div> // Error
~~~~~~~~~~~~~~~~~~~
~~~
!!! error TS17015: Expected corresponding closing tag for JSX fragment.
~~~~~~~~~



<>eof // Error
~~~~~~~~~~~~~~~~
!!! error TS2657: JSX expressions must have one parent element.
~~
!!! error TS17014: JSX fragment has no corresponding closing tag.

Expand Down
2 changes: 1 addition & 1 deletion tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ declare var React: any;
// @filename: 17.tsx
<a b={}>;
// @filename: 18.tsx
var x = <div>one</div><div>two</div>;;
var x = /* Leading trivia */ <div>one</div><div>two</div>;;
// @filename: 19.tsx
var x = <div>one</div> /* intervening comment */ <div>two</div>;;
// @filename: 20.tsx
Expand Down
7 changes: 7 additions & 0 deletions tests/cases/fourslash/codeFixWrapJsxInFragment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path='fourslash.ts' />

// @jsx: react
// @Filename: /a.tsx
////[|<a /><a />|]

verify.rangeAfterCodeFix(`<><a /><a /></>`, /*includeWhiteSpace*/false, /*errorCode*/ undefined, /*index*/ 0);
Loading