[Fix] jsx-curly-spacing:

- Fix comment nodes being deleted when fixed - Fix problem when last token spans multiple lines
jsx-eslint · Nov 22, 2017 · adf5d81 · adf5d81
1 parent 7aaec03
commit adf5d81
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 11 deletions.
diff --git a/lib/rules/jsx-curly-spacing.js b/lib/rules/jsx-curly-spacing.js
@@ -148,7 +148,33 @@ module.exports = {
      * @returns {boolean} Whether or not there is a newline between the tokens.
      */
     function isMultiline(left, right) {
-      return left.loc.start.line !== right.loc.start.line;
+      return left.loc.end.line !== right.loc.start.line;
+    }
+
+    /**
+     * Trims text of whitespace between two ranges
+     * @param {Fixer} fixer - the eslint fixer object
+     * @param {Location} fromLoc - the start location
+     * @param {Location} toLoc - the end location
+     * @param {string} mode - either 'start' or 'end'
+     * @param {string=} spacing - a spacing value that will optionally add a space to the removed text
+     * @returns {Object|*|{range, text}}
+     */
+    function fixByTrimmingWhitespace(fixer, fromLoc, toLoc, mode, spacing) {
+      let replacementText = sourceCode.text.slice(fromLoc, toLoc);
+      if (mode === 'start') {
+        replacementText = replacementText.replace(/^\s+/gm, '');
+      } else {
+        replacementText = replacementText.replace(/\s+$/gm, '');
+      }
+      if (spacing === SPACING.always) {
+        if (mode === 'start') {
+          replacementText += ' ';
+        } else {
+          replacementText = ` ${replacementText}`;
+        }
+      }
+      return fixer.replaceTextRange([fromLoc, toLoc], replacementText);
     }
 
     /**
@@ -164,7 +190,7 @@ module.exports = {
         message: `There should be no newline after '${token.value}'`,
         fix: function(fixer) {
           const nextToken = sourceCode.getTokenAfter(token);
-          return fixer.replaceTextRange([token.range[1], nextToken.range[0]], spacing === SPACING.always ? ' ' : '');
+          return fixByTrimmingWhitespace(fixer, token.range[1], nextToken.range[0], 'start', spacing);
         }
       });
     }
@@ -182,7 +208,7 @@ module.exports = {
         message: `There should be no newline before '${token.value}'`,
         fix: function(fixer) {
           const previousToken = sourceCode.getTokenBefore(token);
-          return fixer.replaceTextRange([previousToken.range[1], token.range[0]], spacing === SPACING.always ? ' ' : '');
+          return fixByTrimmingWhitespace(fixer, previousToken.range[1], token.range[0], 'end', spacing);
         }
       });
     }
@@ -200,10 +226,7 @@ module.exports = {
         message: `There should be no space after '${token.value}'`,
         fix: function(fixer) {
           const nextToken = sourceCode.getTokenAfter(token);
-          const nextNode = sourceCode.getNodeByRangeIndex(nextToken.range[0]);
-          const leadingComments = sourceCode.getComments(nextNode).leading;
-          const rangeEndRef = leadingComments.length ? leadingComments[0] : nextToken;
-          return fixer.removeRange([token.range[1], rangeEndRef.range[0]]);
+          return fixByTrimmingWhitespace(fixer, token.range[1], nextToken.range[0], 'start');
         }
       });
     }
@@ -221,10 +244,7 @@ module.exports = {
         message: `There should be no space before '${token.value}'`,
         fix: function(fixer) {
           const previousToken = sourceCode.getTokenBefore(token);
-          const previousNode = sourceCode.getNodeByRangeIndex(previousToken.range[0]);
-          const trailingComments = sourceCode.getComments(previousNode).trailing;
-          const rangeStartRef = trailingComments.length ? trailingComments[trailingComments.length - 1] : previousToken;
-          return fixer.removeRange([rangeStartRef.range[1], token.range[0]]);
+          return fixByTrimmingWhitespace(fixer, previousToken.range[1], token.range[0], 'end');
         }
       });
     }

diff --git a/tests/lib/rules/jsx-curly-spacing.js b/tests/lib/rules/jsx-curly-spacing.js
@@ -671,6 +671,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
   }, {
     code: '<App foo={ bar }>{bar}</App>',
     options: ['always']
+  }, {
+    code: [
+      '<App>{`',
+      'text',
+      '`}</App>'
+    ].join('\n'),
+    options: [{children: {when: 'never', allowMultiline: false}}]
   }],
 
   invalid: [{
@@ -2118,5 +2125,60 @@ ruleTester.run('jsx-curly-spacing', rule, {
     }, {
       message: 'A space is required before \'}\''
     }]
+  }, {
+    code: '<App>{/*comment*/ }</App>',
+    output: '<App>{/*comment*/}</App>',
+    options: [{children: {when: 'never'}}],
+    errors: [{
+      message: 'There should be no space before \'}\''
+    }]
+  }, {
+    code: '<App>{ /*comment*/}</App>',
+    output: '<App>{/*comment*/}</App>',
+    options: [{children: {when: 'never'}}],
+    errors: [{
+      message: 'There should be no space after \'{\''
+    }]
+  }, {
+    code: '<App>{/*comment*/}</App>',
+    output: '<App>{ /*comment*/ }</App>',
+    options: [{children: {when: 'always'}}],
+    errors: [{
+      message: 'A space is required after \'{\''
+    }, {
+      message: 'A space is required before \'}\''
+    }]
+  }, {
+    code: [
+      '<App>',
+      '{/*comment*/',
+      '}',
+      '</App>'
+    ].join('\n'),
+    output: [
+      '<App>',
+      '{/*comment*/}',
+      '</App>'
+    ].join('\n'),
+    options: [{children: {when: 'never', allowMultiline: false}}],
+    errors: [{
+      message: 'There should be no newline before \'}\''
+    }]
+  }, {
+    code: [
+      '<App>',
+      '{',
+      '/*comment*/}',
+      '</App>'
+    ].join('\n'),
+    output: [
+      '<App>',
+      '{/*comment*/}',
+      '</App>'
+    ].join('\n'),
+    options: [{children: {when: 'never', allowMultiline: false}}],
+    errors: [{
+      message: 'There should be no newline after \'{\''
+    }]
   }]
 });