Merge pull request #1458 from jackyho112/make-jsx-curly-brace-presence-not-care-about-some-cases

[jsx-curly-brace-presence] Bail out in warning JSX expression when some chars exist

Author: ljharb

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 entity 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

@@ -1,6 +1,7 @@
 /**
  * @fileoverview Enforce curly braces or disallow unnecessary curly brace in JSX
  * @author Jacky Ho
+ * @author Simon Lydell
  */
 'use strict';
 
@@ -59,18 +60,45 @@ module.exports = {
       {props: ruleOptions, children: ruleOptions} :
       Object.assign({}, DEFAULT_CONFIG, ruleOptions);
 
-    function containsBackslashForEscaping(rawStringValue) {
+    function containsLineTerminators(rawStringValue) {
+      return /[\n\r\u2028\u2029]/.test(rawStringValue);
+    }
+
+    function containsBackslash(rawStringValue) {
       return rawStringValue.includes('\\');
     }
 
+    function containsHTMLEntity(rawStringValue) {
+      return /&[A-Za-z\d#]+;/.test(rawStringValue);
+    }
+
+    function containsDisallowedJSXTextChars(rawStringValue) {
+      return /[{<>}]/.test(rawStringValue);
+    }
+
+    function containsQuoteCharacters(value) {
+      return /['"]/.test(value);
+    }
+
     function escapeDoubleQuotes(rawStringValue) {
       return rawStringValue.replace(/\\"/g, '"').replace(/"/g, '\\"');
     }
 
+    function escapeBackslashes(rawStringValue) {
+      return rawStringValue.replace(/\\/g, '\\\\');
+    }
+
+    function needToEscapeCharacterForJSX(raw) {
+      return (
+        containsBackslash(raw) ||
+        containsHTMLEntity(raw) ||
+        containsDisallowedJSXTextChars(raw)
+      );
+    }
+
     /**
      * Report and fix an unnecessary curly brace violation on a node
      * @param {ASTNode} node - The AST node with an unnecessary JSX expression
-     * @param {String} text - The text to replace the unnecessary JSX expression
      */
     function reportUnnecessaryCurly(JSXExpressionNode) {
       context.report({
@@ -83,11 +111,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 +130,51 @@ module.exports = {
         node: literalNode,
         message: 'Need to wrap this literal in a JSX expression.',
         fix: function(fixer) {
+          // If a HTML entity name is found, bail out because it can be fixed
+          // by either using the real character or the unicode equivalent.
+          // If it contains any line terminator character, bail out as well.
+          if (
+            containsHTMLEntity(literalNode.raw) ||
+            containsLineTerminators(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) && (
+          parentType === 'JSXElement' ||
+          !containsQuoteCharacters(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) && (
+          parentType === 'JSXElement' ||
+          !containsQuoteCharacters(expression.quasis[0].value.cooked)
+        )
       ) {
         reportUnnecessaryCurly(JSXExpressionNode);
       }
@@ -170,17 +214,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);
         }
       }