Make jsx-curly-brace-presence bail out in getting rid of JSX expressions when some chars exist

Author: jackyho112

docs/rules/jsx-curly-brace-presence.md

@@ -122,7 +122,7 @@ will be warned and fixed to:
 <App prop="Hello world">Hello world</App>;
 ```
 
-* If the rule is set to enforce curly braces and the strings have quotes, it will be fixed with double quotes for JSX children and the normal way for JSX attributes.
+* If the rule is set to enforce curly braces and the strings have quotes, it will be fixed with double quotes for JSX children and the normal way for JSX attributes. Also, double quotes will be escaped in the fix.
 
 For example:
 
@@ -137,12 +137,14 @@ will warned and fixed to:
 <App prop={"Hello \"foo\" world"}>{"Hello 'foo' \"bar\" world"}</App>;
 ```
 
-* If the rule is set to get rid of unnecessary curly braces and the strings have escaped characters, it will not warn or fix for JSX children because JSX expressions are necessary in this case. For instance:
+* If the rule is set to get rid of unnecessary curly braces(JSX expression) and there are characters that need to be escaped in its JSX form, such as quote characters, [forbidden JSX text characters](https://facebook.github.io/jsx/), escaped characters and anything that looks like HTML names, the code will not be warned because the fix may make the code less readable.
 
 The following pattern will not be given a warning even if `'never'` is passed.
 
 ```jsx
+<Color text={"\u00a0"} />
 <App>{"Hello \u00b7 world"}</App>;
+<style type="text/css">{'.main { margin-top: 0; }'}</style>;
 ```
 
 ## When Not To Use It

lib/rules/jsx-curly-brace-presence.js

@@ -59,14 +59,39 @@ module.exports = {
       {props: ruleOptions, children: ruleOptions} :
       Object.assign({}, DEFAULT_CONFIG, ruleOptions);
 
-    function containsBackslashForEscaping(rawStringValue) {
+    function containsBackslash(rawStringValue) {
       return rawStringValue.includes('\\');
     }
 
+    function containsHTMLEntity(rawStringValue) {
+      return rawStringValue.match(/&([A-Za-z\d]+);/g);
+    }
+
+    function containsDisallowedJSXTextChars(rawStringValue) {
+      return rawStringValue.match(/{|<|>|}/g);
+    }
+
+    function containsQuoteCharacters(value) {
+      return value.match(/'|"/g);
+    }
+
     function escapeDoubleQuotes(rawStringValue) {
       return rawStringValue.replace(/\\"/g, '"').replace(/"/g, '\\"');
     }
 
+    function escapeBackslashes(rawStringValue) {
+      return rawStringValue.replace(/\\/g, '\\\\');
+    }
+
+    function needToEscapeCharacterForJSX(raw, cooked) {
+      return (
+        containsBackslash(raw) ||
+        containsHTMLEntity(raw) ||
+        containsDisallowedJSXTextChars(raw) ||
+        containsQuoteCharacters(cooked)
+      );
+    }
+
     /**
      * Report and fix an unnecessary curly brace violation on a node
      * @param {ASTNode} node - The AST node with an unnecessary JSX expression
@@ -83,11 +108,10 @@ module.exports = {
 
           let textToReplace;
           if (parentType === 'JSXAttribute') {
-            textToReplace = `"${escapeDoubleQuotes(
-              expressionType === 'TemplateLiteral' ?
-                expression.quasis[0].value.raw :
-                expression.raw.substring(1, expression.raw.length - 1)
-            )}"`;
+            textToReplace = `"${expressionType === 'TemplateLiteral' ?
+              expression.quasis[0].value.raw :
+              expression.raw.substring(1, expression.raw.length - 1)
+            }"`;
           } else {
             textToReplace = expressionType === 'TemplateLiteral' ?
               expression.quasis[0].value.cooked : expression.value;
@@ -103,34 +127,42 @@ module.exports = {
         node: literalNode,
         message: 'Need to wrap this literal in a JSX expression.',
         fix: function(fixer) {
+          // Leave it to the author to fix as it can be fixed by either a
+          // real character or unicode
+          if (containsHTMLEntity(literalNode.raw)) {
+            return null;
+          }
+
           const expression = literalNode.parent.type === 'JSXAttribute' ?
-            `{"${escapeDoubleQuotes(
+            `{"${escapeDoubleQuotes(escapeBackslashes(
               literalNode.raw.substring(1, literalNode.raw.length - 1)
-            )}"}` :
+            ))}"}` :
             `{${JSON.stringify(literalNode.value)}}`;
 
           return fixer.replaceText(literalNode, expression);
         }
       });
     }
 
+    // Bail out if there is any character that needs to be escaped in JSX
+    // because escaping decreases readiblity and the original code may be more
+    // readible anyway or intentional for other specific reasons
     function lintUnnecessaryCurly(JSXExpressionNode) {
       const expression = JSXExpressionNode.expression;
       const expressionType = expression.type;
-      const parentType = JSXExpressionNode.parent.type;
 
       if (
         expressionType === 'Literal' &&
-          typeof expression.value === 'string' && (
-          parentType === 'JSXAttribute' ||
-          !containsBackslashForEscaping(expression.raw))
+          typeof expression.value === 'string' &&
+          !needToEscapeCharacterForJSX(expression.raw, expression.value)
       ) {
         reportUnnecessaryCurly(JSXExpressionNode);
       } else if (
         expressionType === 'TemplateLiteral' &&
-          expression.expressions.length === 0 && (
-          parentType === 'JSXAttribute' ||
-          !containsBackslashForEscaping(expression.quasis[0].value.raw))
+          expression.expressions.length === 0 &&
+          !needToEscapeCharacterForJSX(
+            expression.quasis[0].value.raw, expression.quasis[0].value.cooked
+          )
       ) {
         reportUnnecessaryCurly(JSXExpressionNode);
       }
@@ -170,17 +202,13 @@ module.exports = {
 
     return {
       JSXExpressionContainer: node => {
-        const parent = node.parent;
-
-        if (shouldCheckForUnnecessaryCurly(parent, userConfig)) {
+        if (shouldCheckForUnnecessaryCurly(node.parent, userConfig)) {
           lintUnnecessaryCurly(node);
         }
       },
 
       Literal: node => {
-        const parentType = node.parent.type;
-
-        if (shouldCheckForMissingCurly(parentType, userConfig)) {
+        if (shouldCheckForMissingCurly(node.parent.type, userConfig)) {
           reportMissingCurly(node);
         }
       }

tests/lib/rules/jsx-curly-brace-presence.js

@@ -169,49 +169,53 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
     {
       code: '<MyComponent prop={`bar ${word} foo`}>{`foo ${word}`}</MyComponent>',
       options: ['never']
-    }
-  ],
-
-  invalid: [
+    },
     {
-      code: '<App prop={`foo`} />',
-      output: '<App prop="foo" />',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
+      code: '<MyComponent>{"div { margin-top: 0; }"}</MyComponent>',
+      options: ['never']
     },
     {
-      code: '<App prop={\'foo \\u00b7 bar\'}>foo</App>',
-      output: '<App prop=\"foo \\u00b7 bar\">foo</App>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
+      code: '<MyComponent>{"<Foo />"}</MyComponent>',
+      options: ['never']
     },
     {
-      code: '<App prop={`foo \\n bar`}>foo</App>',
-      output: '<App prop="foo \\n bar">foo</App>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
+      code: '<MyComponent prop={"{ style: true }"}>bar</MyComponent>',
+      options: ['never']
     },
     {
-      code: '<App prop={`foo \\u00b7 bar`}>foo</App>',
-      output: '<App prop="foo \\u00b7 bar">foo</App>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
+      code: '<MyComponent prop={"< style: true >"}>foo</MyComponent>',
+      options: ['never']
     },
     {
-      code: '<App prop={`foo \\\'foo\\\' bar`}>foo</App>',
-      output: '<App prop="foo \\\'foo\\\' bar">foo</App>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
+      code: '<MyComponent prop={"Hello \\u1026 world"}>bar</MyComponent>',
+      options: ['never']
     },
     {
-      code: '<App prop={`foo \\\"foo\\\" bar`}>foo</App>',
-      output: '<App prop="foo \\\"foo\\\" bar">foo</App>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
+      code: '<MyComponent>{"Hello \\u1026 world"}</MyComponent>',
+      options: ['never']
     },
     {
-      code: '<App prop={`foo "foo" bar`}>foo</App>',
-      output: '<App prop="foo \\\"foo\\\" bar">foo</App>',
+      code: '<MyComponent prop={"Hello &middot; world"}>bar</MyComponent>',
+      options: ['never']
+    },
+    {
+      code: '<MyComponent>{"Hello &middot; world"}</MyComponent>',
+      options: ['never']
+    },
+    {
+      code: '<MyComponent>{"Hello \\n world"}</MyComponent>',
+      options: ['never']
+    },
+    {
+      code: ['<a a={"start\\', '\\', 'end"}/>'].join('/n'),
+      options: ['never']
+    }
+  ],
+
+  invalid: [
+    {
+      code: '<App prop={`foo`} />',
+      output: '<App prop="foo" />',
       options: [{props: 'never'}],
       errors: [{message: unnecessaryCurlyMessage}]
     },
@@ -227,12 +231,6 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
       options: [{children: 'never'}],
       errors: [{message: unnecessaryCurlyMessage}]
     },
-    {
-      code: '<App>{`foo "foo" bar`}</App>',
-      output: '<App>foo "foo" bar</App>',
-      options: [{children: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
-    },
     {
       code: '<MyComponent>{\'foo\'}</MyComponent>',
       output: '<MyComponent>foo</MyComponent>',
@@ -255,18 +253,6 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
       options: [{props: 'never'}],
       errors: [{message: unnecessaryCurlyMessage}]
     },
-    {
-      code: '<MyComponent prop={\"bar \'foo\' \"}>foo</MyComponent>',
-      output: '<MyComponent prop=\"bar \'foo\' \">foo</MyComponent>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
-    },
-    {
-      code: '<MyComponent prop={\'bar \"foo\" \'}>foo</MyComponent>',
-      output: '<MyComponent prop=\"bar \\\"foo\\\" \">foo</MyComponent>',
-      options: [{props: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
-    },
     {
       code: '<MyComponent prop=\'bar\'>foo</MyComponent>',
       output: '<MyComponent prop={\"bar\"}>foo</MyComponent>',
@@ -316,11 +302,23 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
       errors: [{message: missingCurlyMessage}]
     },
     {
-      code: '<MyComponent>foo\\nbar</MyComponent>',
-      output: '<MyComponent>{\"foo\\\\nbar\"}</MyComponent>',
+      code: '<MyComponent>foo \\n bar</MyComponent>',
+      output: '<MyComponent>{\"foo \\\\n bar\"}</MyComponent>',
+      options: [{children: 'always'}],
+      errors: [{message: missingCurlyMessage}]
+    },
+    {
+      code: '<MyComponent>foo \\u1234 bar</MyComponent>',
+      output: '<MyComponent>{\"foo \\\\u1234 bar\"}</MyComponent>',
       options: [{children: 'always'}],
       errors: [{message: missingCurlyMessage}]
     },
+    {
+      code: '<MyComponent prop=\'foo \\u1234 bar\' />',
+      output: '<MyComponent prop={"foo \\\\u1234 bar"} />',
+      options: [{props: 'always'}],
+      errors: [{message: missingCurlyMessage}]
+    },
     {
       code: '<MyComponent prop={\'bar\'}>{\'foo\'}</MyComponent>',
       output: '<MyComponent prop=\"bar\">foo</MyComponent>',
@@ -337,30 +335,6 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
         {message: missingCurlyMessage}, {message: missingCurlyMessage}
       ]
     },
-    {
-      code: '<App>{\'foo "bar"\'}</App>',
-      output: '<App>foo \"bar\"</App>',
-      options: [{children: 'never'}],
-      errors: [{message: unnecessaryCurlyMessage}]
-    },
-    {
-      code: '<App>{\"foo \'bar\'\"}</App>',
-      output: '<App>foo \'bar\'</App>',
-      errors: [{message: unnecessaryCurlyMessage}],
-      options: [{children: 'never'}]
-    },
-    {
-      code: '<App prop={\"foo \'bar\' foo \"}>foo</App>',
-      output: '<App prop=\"foo \'bar\' foo \">foo</App>',
-      errors: [{message: unnecessaryCurlyMessage}],
-      options: [{props: 'never'}]
-    },
-    {
-      code: '<App prop={\'foo "bar"\'}>foo</App>',
-      output: '<App prop=\"foo \\\"bar\\\"\">foo</App>',
-      errors: [{message: unnecessaryCurlyMessage}],
-      options: [{props: 'never'}]
-    },
     {
       code: '<App prop={\'foo\'} attr={\" foo \"} />',
       output: '<App prop=\"foo\" attr=\" foo \" />',
@@ -388,6 +362,18 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
       output: '<App prop={\'foo\'} attr={\"bar\"} />',
       errors: [{message: missingCurlyMessage}],
       options: [{props: 'always'}]
+    },
+    {
+      code: '<App prop=\'foo &middot; bar\' />',
+      output: '<App prop=\'foo &middot; bar\' />',
+      errors: [{message: missingCurlyMessage}],
+      options: [{props: 'always'}]
+    },
+    {
+      code: '<App>foo &middot; bar</App>',
+      output: '<App>foo &middot; bar</App>',
+      errors: [{message: missingCurlyMessage}],
+      options: [{children: 'always'}]
     }
   ]
 });