Improve error range for ts2657 (jsx expr must have parent element), a…

…dd code fix for it (#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 <[email protected]> * Update src/services/codefixes/wrapJsxInFragment.ts Co-authored-by: Andrew Branch <[email protected]> Co-authored-by: Andrew Branch <[email protected]> Co-authored-by: Andrew Branch <[email protected]>
microsoft · Jun 1, 2020 · 8e290e5 · 8e290e5
1 parent 4f0b81d
commit 8e290e5
Show file tree

Hide file tree

Showing 16 changed files with 152 additions and 60 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -28769,7 +28769,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;
 

diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -5709,6 +5709,14 @@
         "category": "Message",
         "code": 95119
     },
+    "Wrap in JSX fragment": {
+        "category": "Message",
+        "code": 95120
+    },
+    "Wrap all unparented JSX in JSX fragment": {
+        "category": "Message",
+        "code": 95121
+    },
 
     "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
         "category": "Error",

diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -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) {
@@ -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);
                     return <JsxElement><Node>badNode;
                 }
             }

diff --git a/src/services/codefixes/wrapJsxInFragment.ts b/src/services/codefixes/wrapJsxInFragment.ts
@@ -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_in_JSX_fragment, fixID, Diagnostics.Wrap_all_unparented_JSX_in_JSX_fragment)];
+        },
+        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;
+        }
+    }
+}
diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json
@@ -97,6 +97,7 @@
         "codefixes/useDefaultImport.ts",
         "codefixes/useBigintLiteral.ts",
         "codefixes/fixAddModuleReferTypeMissingTypeof.ts",
+        "codefixes/wrapJsxInFragment.ts",
         "codefixes/convertToMappedObjectType.ts",
         "codefixes/removeUnnecessaryAwait.ts",
         "codefixes/splitTypeOnlyImport.ts",

diff --git a/tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt b/tests/baselines/reference/jsxEsprimaFbTestSuite.errors.txt
@@ -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.
@@ -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;`?
                                                             ~

diff --git a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.errors.txt
@@ -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;`?
@@ -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.
@@ -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>;
@@ -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>;

diff --git a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.js b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.js
@@ -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]
@@ -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>;

diff --git a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.symbols b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.symbols
@@ -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 ===

diff --git a/tests/baselines/reference/jsxInvalidEsprimaTestSuite.types b/tests/baselines/reference/jsxInvalidEsprimaTestSuite.types
@@ -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

diff --git a/tests/baselines/reference/tsxErrorRecovery2.errors.txt b/tests/baselines/reference/tsxErrorRecovery2.errors.txt
@@ -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.
+
diff --git a/tests/baselines/reference/tsxErrorRecovery3.errors.txt b/tests/baselines/reference/tsxErrorRecovery3.errors.txt
@@ -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.
+
diff --git a/tests/baselines/reference/tsxFragmentErrors.errors.txt b/tests/baselines/reference/tsxFragmentErrors.errors.txt
@@ -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 {
@@ -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.
 

diff --git a/tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx b/tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx
@@ -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

diff --git a/tests/cases/fourslash/codeFixWrapJsxInFragment.ts b/tests/cases/fourslash/codeFixWrapJsxInFragment.ts
@@ -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);