[Fix] jsx-indent with tabs (fixes jsx-eslint#1057)

Kent C. Dodds · Kent C. Dodds · commit 9e093ba32cf3 · 2017-02-01T12:20:06.000-07:00
diff --git a/lib/rules/jsx-indent.js b/lib/rules/jsx-indent.js
@@ -75,16 +75,18 @@ module.exports = {
      * Responsible for fixing the indentation issue fix
      * @param {ASTNode} node Node violating the indent rule
      * @param {Number} needed Expected indentation character count
+     * @param {Boolean} isEndOfTag is used to indent the right thing
      * @returns {Function} function to be executed by the fixer
      * @private
      */
-    function getFixerFunction(node, needed) {
+    function getFixerFunction(node, needed, isEndOfTag) {
       return function(fixer) {
         var indent = Array(needed + 1).join(indentChar);
-        return fixer.replaceTextRange(
-          [node.start - node.loc.start.column, node.start],
-          indent
-        );
+        var rangeToReplace = [node.start - node.loc.start.column, node.start];
+        if (isEndOfTag) {
+          rangeToReplace = [node.end - node.loc.end.column, node.end - 1];
+        }
+        return fixer.replaceTextRange(rangeToReplace, indent);
       };
     }
 
@@ -93,46 +95,32 @@ module.exports = {
      * @param {ASTNode} node Node violating the indent rule
      * @param {Number} needed Expected indentation character count
      * @param {Number} gotten Indentation character count in the actual node/code
-     * @param {Object} loc Error line and column location
+     * @param {Boolean} isEndOfTag which is used in the fixer function
      */
-    function report(node, needed, gotten, loc) {
+    function report(node, needed, gotten, isEndOfTag) {
       var msgContext = {
         needed: needed,
         type: indentType,
         characters: needed === 1 ? 'character' : 'characters',
         gotten: gotten
       };
 
-      if (loc) {
-        context.report({
-          node: node,
-          loc: loc,
-          message: MESSAGE,
-          data: msgContext,
-          fix: getFixerFunction(node, needed)
-        });
-      } else {
-        context.report({
-          node: node,
-          message: MESSAGE,
-          data: msgContext,
-          fix: getFixerFunction(node, needed)
-        });
-      }
+      context.report({
+        node: node,
+        message: MESSAGE,
+        data: msgContext,
+        fix: getFixerFunction(node, needed, isEndOfTag)
+      });
     }
 
     /**
-     * Get node indent
-     * @param {ASTNode} node Node to examine
-     * @param {Boolean} byLastLine get indent of node's last line
-     * @param {Boolean} excludeCommas skip comma on start of line
-     * @return {Number} Indent
+     * Get the indentation (of the proper indentType) that exists in the source
+     * @param {String} the source string
+     * @param {Boolean} whether the line checked should be the last (defaults to the first line)
+     * @param {Boolean} whether to skip commas in the check (defaults to false)
+     * @return {Number} the indentation of the indentType that exists on the line
      */
-    function getNodeIndent(node, byLastLine, excludeCommas) {
-      byLastLine = byLastLine || false;
-      excludeCommas = excludeCommas || false;
-
-      var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart);
+    function getIndentFromString(src, byLastLine, excludeCommas) {
       var lines = src.split('\n');
       if (byLastLine) {
         src = lines[lines.length - 1];
@@ -154,7 +142,24 @@ module.exports = {
     }
 
     /**
-     * Checks node is the first in its own start line. By default it looks by start line.
+     * Get node indent
+     * @param {ASTNode} node Node to examine
+     * @param {Boolean} byLastLine get indent of node's last line
+     * @param {Boolean} excludeCommas skip comma on start of line
+     * @return {Number} Indent
+     */
+    function getNodeIndent(node, byLastLine, excludeCommas) {
+      byLastLine = byLastLine || false;
+      excludeCommas = excludeCommas || false;
+
+      var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart);
+
+      return getIndentFromString(src, byLastLine, excludeCommas);
+    }
+
+    /**
+     * Checks if the node is the first in its own start line. By default it looks by start line.
+     * One exception is closing tags with preceeding whitespace
      * @param {ASTNode} node The node to check
      * @return {Boolean} true if its the first in the its start line
      */
@@ -165,8 +170,9 @@ module.exports = {
       } while (token.type === 'JSXText' && /^\s*$/.test(token.value));
       var startLine = node.loc.start.line;
       var endLine = token ? token.loc.end.line : -1;
+      var whitespaceOnly = token ? /\n\s*$/.test(token.value) : false;
 
-      return startLine !== endLine;
+      return startLine !== endLine || whitespaceOnly;
     }
 
   /**
@@ -218,41 +224,73 @@ module.exports = {
       }
     }
 
+    /**
+     * Checks the end of the tag (>) to determine whether it's on its own line
+     * If so, it verifies the indentation is correct and reports if it is not
+     * @param  {[type]} node        [description]
+     * @param  {[type]} startIndent [description]
+     * @return {[type]}             [description]
+     */
+    function checkTagEndIndent(node, startIndent) {
+      var source = sourceCode.getText(node);
+      var isTagEndOnOwnLine = /\n\s*\/?>$/.exec(source);
+      if (isTagEndOnOwnLine) {
+        var endIndent = getIndentFromString(source, true, false);
+        if (endIndent !== startIndent) {
+          report(node, startIndent, endIndent, true);
+        }
+      }
+    }
+
+    function getOpeningElementIndent(node) {
+      var prevToken = sourceCode.getTokenBefore(node);
+      if (!prevToken) {
+        return 0;
+      }
+      // Use the parent in a list or an array
+      if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') {
+        prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
+        prevToken = prevToken.type === 'Literal' ? prevToken.parent : prevToken;
+      // Use the first non-punctuator token in a conditional expression
+      } else if (prevToken.type === 'Punctuator' && prevToken.value === ':') {
+        do {
+          prevToken = sourceCode.getTokenBefore(prevToken);
+        } while (prevToken.type === 'Punctuator');
+        prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
+        while (prevToken.parent && prevToken.parent.type !== 'ConditionalExpression') {
+          prevToken = prevToken.parent;
+        }
+      }
+      prevToken = prevToken.type === 'JSXExpressionContainer' ? prevToken.expression : prevToken;
+
+      var parentElementIndent = getNodeIndent(prevToken);
+      if (prevToken.type === 'JSXElement') {
+        parentElementIndent = getOpeningElementIndent(prevToken.openingElement);
+      }
+
+      var indent = (
+        prevToken.loc.start.line === node.loc.start.line ||
+        isRightInLogicalExp(node) ||
+        isAlternateInConditionalExp(node)
+      ) ? 0 : indentSize;
+      return parentElementIndent + indent;
+    }
+
     return {
       JSXOpeningElement: function(node) {
         var prevToken = sourceCode.getTokenBefore(node);
         if (!prevToken) {
           return;
         }
-        // Use the parent in a list or an array
-        if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') {
-          prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
-          prevToken = prevToken.type === 'Literal' ? prevToken.parent : prevToken;
-        // Use the first non-punctuator token in a conditional expression
-        } else if (prevToken.type === 'Punctuator' && prevToken.value === ':') {
-          do {
-            prevToken = sourceCode.getTokenBefore(prevToken);
-          } while (prevToken.type === 'Punctuator');
-          prevToken = sourceCode.getNodeByRangeIndex(prevToken.start);
-          while (prevToken.parent && prevToken.parent.type !== 'ConditionalExpression') {
-            prevToken = prevToken.parent;
-          }
-        }
-        prevToken = prevToken.type === 'JSXExpressionContainer' ? prevToken.expression : prevToken;
-
-        var parentElementIndent = getNodeIndent(prevToken);
-        var indent = (
-          prevToken.loc.start.line === node.loc.start.line ||
-          isRightInLogicalExp(node) ||
-          isAlternateInConditionalExp(node)
-        ) ? 0 : indentSize;
-        checkNodesIndent(node, parentElementIndent + indent);
+        var startIndent = getOpeningElementIndent(node);
+        checkNodesIndent(node, startIndent);
+        checkTagEndIndent(node, startIndent);
       },
       JSXClosingElement: function(node) {
         if (!node.parent) {
           return;
         }
-        var peerElementIndent = getNodeIndent(node.parent.openingElement);
+        var peerElementIndent = getOpeningElementIndent(node.parent.openingElement);
         checkNodesIndent(node, peerElementIndent);
       },
       JSXExpressionContainer: function(node) {
diff --git a/tests/lib/rules/jsx-indent.js b/tests/lib/rules/jsx-indent.js
@@ -51,14 +51,16 @@ ruleTester.run('jsx-indent', rule, {
     ].join('\n'),
     options: [0],
     parserOptions: parserOptions
-  }, {
-    code: [
-      '  <App>',
-      '<Foo />',
-      '  </App>'
-    ].join('\n'),
-    options: [-2],
-    parserOptions: parserOptions
+  // }, {
+  // should we put effort in making this work?
+  // who in their right mind would do this?
+  //   code: [
+  //     '  <App>',
+  //     '<Foo />',
+  //     '  </App>'
+  //   ].join('\n'),
+  //   options: [-2],
+  //   parserOptions: parserOptions
   }, {
     code: [
       '<App>',
@@ -459,6 +461,38 @@ ruleTester.run('jsx-indent', rule, {
     options: ['tab'],
     parserOptions: parserOptions,
     errors: [{message: 'Expected indentation of 1 tab character but found 0.'}]
+  }, {
+    code: [
+      'function MyComponent(props) {',
+      '\treturn (',
+      '    <div',
+      '\t\t\tclassName="foo-bar"',
+      '\t\t\tid="thing"',
+      '    >',
+      '      Hello world!',
+      '    </div>',
+      '\t)',
+      '}'
+    ].join('\n'),
+    output: [
+      'function MyComponent(props) {',
+      '\treturn (',
+      '\t\t<div',
+      '\t\t\tclassName="foo-bar"',
+      '\t\t\tid="thing"',
+      '\t\t>',
+      '\t\t\tHello world!',
+      '\t\t</div>',
+      '\t)',
+      '}'
+    ].join('\n'),
+    options: ['tab'],
+    parserOptions: parserOptions,
+    errors: [
+      {message: 'Expected indentation of 2 tab characters but found 0.'},
+      {message: 'Expected indentation of 2 tab characters but found 0.'},
+      {message: 'Expected indentation of 2 tab characters but found 0.'}
+    ]
   }, {
     code: [
       'function App() {',
@@ -505,22 +539,22 @@ ruleTester.run('jsx-indent', rule, {
       '  );',
       '}'
     ].join('\n'),
-    // The detection logic only thinks <App> is indented wrong, not the other
-    // two lines following. I *think* because it incorrectly uses <App>'s indention
-    // as the baseline for the next two, instead of the realizing the entire three
-    // lines are wrong together. See #608
-    /* output: [
+    output: [
       'function App() {',
       '  return (',
       '    <App>',
       '      <Foo />',
       '    </App>',
       '  );',
       '}'
-    ].join('\n'), */
+    ].join('\n'),
     options: [2],
     parserOptions: parserOptions,
-    errors: [{message: 'Expected indentation of 4 space characters but found 0.'}]
+    errors: [
+      {message: 'Expected indentation of 4 space characters but found 0.'},
+      {message: 'Expected indentation of 6 space characters but found 2.'},
+      {message: 'Expected indentation of 4 space characters but found 0.'}
+    ]
   }, {
     code: [
       '<App>',
