rewrite the fixer to fixes more cases

lib/rules/jsx-no-useless-fragment.js

@@ -12,13 +12,35 @@ function isJSXText(node) {
   return !!node && (node.type === 'JSXText' || node.type === 'Literal');
 }
 
+/**
+ * @param {ASTNode} node
+ * @returns {boolean}
+ */
+function isJSXTextOrJSXCurly(node) {
+  return isJSXText(node) || node.type === 'JSXExpressionContainer';
+}
+
 /**
  * @param {string} text
  */
 function isOnlyWhitespace(text) {
   return text.trim().length === 0;
 }
 
+/**
+ * @param {string} text
+ * @returns {string}
+ */
+function trimLikeReact(text) {
+  const leadingSpaces = /^\s*/.exec(text)[0];
+  const trailingSpaces = /\s*$/.exec(text)[0];
+
+  const start = leadingSpaces.includes('\n') ? leadingSpaces.length : 0;
+  const end = trailingSpaces.includes('\n') ? text.length - trailingSpaces.length : text.length;
+
+  return text.slice(start, end);
+}
+
 /**
  * Test if node is like `<Fragment key={_}>_</Fragment>`
  * @param {JSXElement} node
@@ -93,47 +115,42 @@ module.exports = {
     }
 
     /**
-     * Avoid fixing cases that involve tricky whitespaces changes like:
-     * ```jsx
-     * <div>
-     *   pine<>
-     *     apple
-     *   </>
-     * </div>
-     * ```
-     * Give up fixing if one neighboring node is JSXText and is not only whitespaces
-     * @param {JSXElement|JSXFragment} node
+     * @param {ASTNode} node
      * @returns {boolean}
      */
-    function isSafeToFix(node) {
-      if (!node.parent.children) {
-        return false;
-      }
-
-      const i = node.parent.children.indexOf(node);
-      const previousChild = node.parent.children[i - 1];
-      const nextChild = node.parent.children[i + 1];
+    function canFix(node) {
+      // Fragments that are child elements can always be fixed
+      if (!(node.parent.type === 'JSXElement' || node.parent.type === 'JSXFragment')) {
+        // const a = <></>
+        if (node.children.length === 0) {
+          return false;
+        }
 
-      if (
-        (isJSXText(previousChild) && !isOnlyWhitespace(previousChild.value)) ||
-        (isJSXText(nextChild) && !isOnlyWhitespace(nextChild.value))
-      ) {
-        return false;
+        // const a = <>cat {meow}</>
+        if (node.children.some(isJSXTextOrJSXCurly)) {
+          return false;
+        }
       }
 
       return true;
     }
 
-    function fix(node, fixer) {
-      return node.type === 'JSXFragment' ?
-        [
-          fixer.remove(node.openingFragment),
-          fixer.remove(node.closingFragment)
-        ] :
-        [
-          fixer.remove(node.openingElement),
-          fixer.remove(node.closingElement)
-        ];
+    /**
+     * @param {ASTNode} node
+     * @returns {((fixer: object) => object) | undefined}
+     */
+    function getFix(node) {
+      if (!canFix(node)) {
+        return undefined;
+      }
+
+      return function fix(fixer) {
+        const opener = node.type === 'JSXFragment' ? node.openingFragment : node.openingElement;
+        const closer = node.type === 'JSXFragment' ? node.closingFragment : node.closingElement;
+        const childrenText = context.getSourceCode().getText().slice(opener.range[1], closer.range[0]);
+
+        return fixer.replaceText(node, trimLikeReact(childrenText));
+      };
     }
 
     function checkNode(node) {
@@ -145,15 +162,15 @@ module.exports = {
         context.report({
           node,
           messageId: 'NeedsMoreChidren',
-          fix: isSafeToFix(node) ? (fixer => fix(node, fixer)) : null
+          fix: getFix(node)
         });
       }
 
       if (isChildOfHtmlElement(node)) {
         context.report({
           node,
           messageId: 'ChildOfHtmlElement',
-          fix: isSafeToFix(node) ? (fixer => fix(node, fixer)) : null
+          fix: getFix(node)
         });
       }
     }

tests/lib/rules/jsx-no-useless-fragment.js

@@ -75,11 +75,29 @@ ruleTester.run('jsx-no-uselses-fragment', rule, {
       parser: parsers.BABEL_ESLINT
     },
     {
-      code: '<><div/></>',
+      code: '<p>moo<>foo</></p>',
+      output: '<p>moofoo</p>',
+      errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}],
+      parser: parsers.BABEL_ESLINT
+    },
+    {
+      code: '<>{meow}</>',
       output: null,
       errors: [{messageId: 'NeedsMoreChidren'}],
       parser: parsers.BABEL_ESLINT
     },
+    {
+      code: '<p><>{meow}</></p>',
+      output: '<p>{meow}</p>',
+      errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}],
+      parser: parsers.BABEL_ESLINT
+    },
+    {
+      code: '<><div/></>',
+      output: '<div/>',
+      errors: [{messageId: 'NeedsMoreChidren'}],
+      parser: parsers.BABEL_ESLINT
+    },
     {
       code: '<Fragment />',
       errors: [{messageId: 'NeedsMoreChidren'}]
@@ -151,20 +169,27 @@ ruleTester.run('jsx-no-uselses-fragment', rule, {
       errors: [{messageId: 'ChildOfHtmlElement'}]
     },
     {
+      // whitepace tricky case
       code: `
-        // not safe to fix
         <section>
           git<>
             <b>hub</b>.
           </>
+
+          git<> <b>hub</b></>
         </section>`,
-      output: null,
-      errors: [{messageId: 'ChildOfHtmlElement'}],
+      output: `
+        <section>
+          git<b>hub</b>.
+
+          git <b>hub</b>
+        </section>`,
+      errors: [{messageId: 'ChildOfHtmlElement'}, {messageId: 'ChildOfHtmlElement'}],
       parser: parsers.BABEL_ESLINT
     },
     {
       code: '<div>a <>{""}{""}</> a</div>',
-      output: null,
+      output: '<div>a {""}{""} a</div>',
       errors: [{messageId: 'ChildOfHtmlElement'}],
       parser: parsers.BABEL_ESLINT
     }