Update: Make indent report lines with mixed spaces/tabs (fixes #4274) (#7076)

Author: not-an-aardvark

lib/rules/indent.js

@@ -121,8 +121,6 @@ module.exports = {
     },
 
     create(context) {
-
-        const MESSAGE = "Expected indentation of {{needed}} {{type}} {{characters}} but found {{gotten}}.";
         const DEFAULT_VARIABLE_INDENT = 1;
         const DEFAULT_PARAMETER_INDENT = null; // For backwards compatibility, don't check parameter indentation unless specified in the config
         const DEFAULT_FUNCTION_BODY_INDENT = 1;
@@ -192,108 +190,85 @@ module.exports = {
             }
         }
 
-        const indentPattern = {
-            normal: indentType === "space" ? /^ +/ : /^\t+/,
-            excludeCommas: indentType === "space" ? /^[ ,]+/ : /^[\t,]+/
-        };
-
         const caseIndentStore = {};
 
         /**
-         * Reports a given indent violation and properly pluralizes the message
+         * Creates an error message for a line, given the expected/actual indentation.
+         * @param {int} expectedAmount The expected amount of indentation characters for this line
+         * @param {int} actualSpaces The actual number of indentation spaces that were found on this line
+         * @param {int} actualTabs The actual number of indentation tabs that were found on this line
+         * @returns {string} An error message for this line
+         */
+        function createErrorMessage(expectedAmount, actualSpaces, actualTabs) {
+            const expectedStatement = `${expectedAmount} ${indentType}${expectedAmount === 1 ? "" : "s"}`; // e.g. "2 tabs"
+            const foundSpacesWord = `space${actualSpaces === 1 ? "" : "s"}`; // e.g. "space"
+            const foundTabsWord = `tab${actualTabs === 1 ? "" : "s"}`; // e.g. "tabs"
+            let foundStatement;
+
+            if (actualSpaces > 0 && actualTabs > 0) {
+                foundStatement = `${actualSpaces} ${foundSpacesWord} and ${actualTabs} ${foundTabsWord}`; // e.g. "1 space and 2 tabs"
+            } else if (actualSpaces > 0) {
+
+                // Abbreviate the message if the expected indentation is also spaces.
+                // e.g. 'Expected 4 spaces but found 2' rather than 'Expected 4 spaces but found 2 spaces'
+                foundStatement = indentType === "space" ? actualSpaces : `${actualSpaces} ${foundSpacesWord}`;
+            } else if (actualTabs > 0) {
+                foundStatement = indentType === "tab" ? actualTabs : `${actualTabs} ${foundTabsWord}`;
+            } else {
+                foundStatement = "0";
+            }
+
+            return `Expected indentation of ${expectedStatement} but found ${foundStatement}.`;
+        }
+
+        /**
+         * Reports a given indent violation
          * @param {ASTNode} node Node violating the indent rule
          * @param {int} needed Expected indentation character count
-         * @param {int} gotten Indentation character count in the actual node/code
+         * @param {int} gottenSpaces Indentation space count in the actual node/code
+         * @param {int} gottenTabs Indentation tab count in the actual node/code
          * @param {Object=} loc Error line and column location
          * @param {boolean} isLastNodeCheck Is the error for last node check
          * @returns {void}
          */
-        function report(node, needed, gotten, loc, isLastNodeCheck) {
-            const msgContext = {
-                needed,
-                type: indentType,
-                characters: needed === 1 ? "character" : "characters",
-                gotten
-            };
-            const indentChar = indentType === "space" ? " " : "\t";
-
-            /**
-             * Responsible for fixing the indentation issue fix
-             * @returns {Function} function to be executed by the fixer
-             * @private
-             */
-            function getFixerFunction() {
-                let rangeToFix = [];
-
-                if (needed > gotten) {
-                    const spaces = indentChar.repeat(needed - gotten);
-
-                    if (isLastNodeCheck === true) {
-                        rangeToFix = [
-                            node.range[1] - 1,
-                            node.range[1] - 1
-                        ];
-                    } else {
-                        rangeToFix = [
-                            node.range[0],
-                            node.range[0]
-                        ];
-                    }
+        function report(node, needed, gottenSpaces, gottenTabs, loc, isLastNodeCheck) {
 
-                    return function(fixer) {
-                        return fixer.insertTextBeforeRange(rangeToFix, spaces);
-                    };
-                } else {
-                    if (isLastNodeCheck === true) {
-                        rangeToFix = [
-                            node.range[1] - (gotten - needed) - 1,
-                            node.range[1] - 1
-                        ];
-                    } else {
-                        rangeToFix = [
-                            node.range[0] - (gotten - needed),
-                            node.range[0]
-                        ];
-                    }
+            const desiredIndent = (indentType === "space" ? " " : "\t").repeat(needed);
 
-                    return function(fixer) {
-                        return fixer.removeRange(rangeToFix);
-                    };
-                }
-            }
+            const textRange = isLastNodeCheck
+                ? [node.range[1] - gottenSpaces - gottenTabs - 1, node.range[1] - 1]
+                : [node.range[0] - gottenSpaces - gottenTabs, node.range[0]];
 
-            if (loc) {
-                context.report({
-                    node,
-                    loc,
-                    message: MESSAGE,
-                    data: msgContext,
-                    fix: getFixerFunction()
-                });
-            } else {
-                context.report({
-                    node,
-                    message: MESSAGE,
-                    data: msgContext,
-                    fix: getFixerFunction()
-                });
-            }
+            context.report({
+                node,
+                loc,
+                message: createErrorMessage(needed, gottenSpaces, gottenTabs),
+                fix: fixer => fixer.replaceTextRange(textRange, desiredIndent)
+            });
         }
 
         /**
          * Get the actual indent of node
          * @param {ASTNode|Token} node Node to examine
          * @param {boolean} [byLastLine=false] get indent of node's last line
          * @param {boolean} [excludeCommas=false] skip comma on start of line
-         * @returns {int} Indent
+         * @returns {Object} The node's indent. Contains keys `space` and `tab`, representing the indent of each character. Also
+         contains keys `goodChar` and `badChar`, where `goodChar` is the amount of the user's desired indentation character, and
+         `badChar` is the amount of the other indentation character.
          */
-        function getNodeIndent(node, byLastLine, excludeCommas) {
+        function getNodeIndent(node, byLastLine) {
             const token = byLastLine ? sourceCode.getLastToken(node) : sourceCode.getFirstToken(node);
-            const src = sourceCode.getText(token, token.loc.start.column);
-            const regExp = excludeCommas ? indentPattern.excludeCommas : indentPattern.normal;
-            const indent = regExp.exec(src);
-
-            return indent ? indent[0].length : 0;
+            const srcCharsBeforeNode = sourceCode.getText(token, token.loc.start.column).split("");
+            const indentChars = srcCharsBeforeNode.slice(0, srcCharsBeforeNode.findIndex(char => char !== " " && char !== "\t"));
+            const spaces = indentChars.filter(char => char === " ").length;
+            const tabs = indentChars.filter(char => char === "\t").length;
+
+            return {
+                space: spaces,
+                tab: tabs,
+                goodChar: indentType === "space" ? spaces : tabs,
+                badChar: indentType === "space" ? tabs : spaces
+            };
         }
 
         /**
@@ -313,27 +288,29 @@ module.exports = {
         /**
          * Check indent for node
          * @param {ASTNode} node Node to check
-         * @param {int} indent needed indent
+         * @param {int} neededIndent needed indent
          * @param {boolean} [excludeCommas=false] skip comma on start of line
          * @returns {void}
          */
-        function checkNodeIndent(node, indent, excludeCommas) {
-            const nodeIndent = getNodeIndent(node, false, excludeCommas);
+        function checkNodeIndent(node, neededIndent) {
+            const actualIndent = getNodeIndent(node, false);
 
             if (
-                node.type !== "ArrayExpression" && node.type !== "ObjectExpression" &&
-                nodeIndent !== indent && isNodeFirstInLine(node)
+                node.type !== "ArrayExpression" &&
+                node.type !== "ObjectExpression" &&
+                (actualIndent.goodChar !== neededIndent || actualIndent.badChar !== 0) &&
+                isNodeFirstInLine(node)
             ) {
-                report(node, indent, nodeIndent);
+                report(node, neededIndent, actualIndent.space, actualIndent.tab);
             }
 
             if (node.type === "IfStatement" && node.alternate) {
                 const elseToken = sourceCode.getTokenBefore(node.alternate);
 
-                checkNodeIndent(elseToken, indent, excludeCommas);
+                checkNodeIndent(elseToken, neededIndent);
 
                 if (!isNodeFirstInLine(node.alternate)) {
-                    checkNodeIndent(node.alternate, indent, excludeCommas);
+                    checkNodeIndent(node.alternate, neededIndent);
                 }
             }
         }
@@ -345,8 +322,8 @@ module.exports = {
          * @param {boolean} [excludeCommas=false] skip comma on start of line
          * @returns {void}
          */
-        function checkNodesIndent(nodes, indent, excludeCommas) {
-            nodes.forEach(node => checkNodeIndent(node, indent, excludeCommas));
+        function checkNodesIndent(nodes, indent) {
+            nodes.forEach(node => checkNodeIndent(node, indent));
         }
 
         /**
@@ -359,11 +336,12 @@ module.exports = {
             const lastToken = sourceCode.getLastToken(node);
             const endIndent = getNodeIndent(lastToken, true);
 
-            if (endIndent !== lastLineIndent && isNodeFirstInLine(node, true)) {
+            if ((endIndent.goodChar !== lastLineIndent || endIndent.badChar !== 0) && isNodeFirstInLine(node, true)) {
                 report(
                     node,
                     lastLineIndent,
-                    endIndent,
+                    endIndent.space,
+                    endIndent.tab,
                     { line: lastToken.loc.start.line, column: lastToken.loc.start.column },
                     true
                 );
@@ -379,11 +357,12 @@ module.exports = {
         function checkFirstNodeLineIndent(node, firstLineIndent) {
             const startIndent = getNodeIndent(node, false);
 
-            if (startIndent !== firstLineIndent && isNodeFirstInLine(node)) {
+            if ((startIndent.goodChar !== firstLineIndent || startIndent.badChar !== 0) && isNodeFirstInLine(node)) {
                 report(
                     node,
                     firstLineIndent,
-                    startIndent,
+                    startIndent.space,
+                    startIndent.tab,
                     { line: node.loc.start.line, column: node.loc.start.column }
                 );
             }
@@ -526,25 +505,25 @@ module.exports = {
                 calleeNode.parent.type === "ArrayExpression")) {
 
                 // If function is part of array or object, comma can be put at left
-                indent = getNodeIndent(calleeNode, false, false);
+                indent = getNodeIndent(calleeNode, false, false).goodChar;
             } else {
 
                 // If function is standalone, simple calculate indent
-                indent = getNodeIndent(calleeNode);
+                indent = getNodeIndent(calleeNode).goodChar;
             }
 
             if (calleeNode.parent.type === "CallExpression") {
                 const calleeParent = calleeNode.parent;
 
                 if (calleeNode.type !== "FunctionExpression" && calleeNode.type !== "ArrowFunctionExpression") {
                     if (calleeParent && calleeParent.loc.start.line < node.loc.start.line) {
-                        indent = getNodeIndent(calleeParent);
+                        indent = getNodeIndent(calleeParent).goodChar;
                     }
                 } else {
                     if (isArgBeforeCalleeNodeMultiline(calleeNode) &&
                         calleeParent.callee.loc.start.line === calleeParent.callee.loc.end.line &&
                         !isNodeFirstInLine(calleeNode)) {
-                        indent = getNodeIndent(calleeParent);
+                        indent = getNodeIndent(calleeParent).goodChar;
                     }
                 }
             }
@@ -644,7 +623,7 @@ module.exports = {
                         effectiveParent = parent.parent;
                     }
                 }
-                nodeIndent = getNodeIndent(effectiveParent);
+                nodeIndent = getNodeIndent(effectiveParent).goodChar;
                 if (parentVarNode && parentVarNode.loc.start.line !== node.loc.start.line) {
                     if (parent.type !== "VariableDeclarator" || parentVarNode === parentVarNode.parent.declarations[0]) {
                         if (parent.type === "VariableDeclarator" && parentVarNode.loc.start.line === effectiveParent.loc.start.line) {
@@ -668,7 +647,7 @@ module.exports = {
 
                 checkFirstNodeLineIndent(node, nodeIndent);
             } else {
-                nodeIndent = getNodeIndent(node);
+                nodeIndent = getNodeIndent(node).goodChar;
                 elementsIndent = nodeIndent + indentSize;
             }
 
@@ -680,8 +659,7 @@ module.exports = {
                 elementsIndent += indentSize * options.VariableDeclarator[parentVarNode.parent.kind];
             }
 
-            // Comma can be placed before property name
-            checkNodesIndent(elements, elementsIndent, true);
+            checkNodesIndent(elements, elementsIndent);
 
             if (elements.length > 0) {
 
@@ -737,9 +715,9 @@ module.exports = {
             ];
 
             if (node.parent && statementsWithProperties.indexOf(node.parent.type) !== -1 && isNodeBodyBlock(node)) {
-                indent = getNodeIndent(node.parent);
+                indent = getNodeIndent(node.parent).goodChar;
             } else {
-                indent = getNodeIndent(node);
+                indent = getNodeIndent(node).goodChar;
             }
 
             if (node.type === "IfStatement" && node.consequent.type !== "BlockStatement") {
@@ -785,13 +763,12 @@ module.exports = {
          */
         function checkIndentInVariableDeclarations(node) {
             const elements = filterOutSameLineVars(node);
-            const nodeIndent = getNodeIndent(node);
+            const nodeIndent = getNodeIndent(node).goodChar;
             const lastElement = elements[elements.length - 1];
 
             const elementsIndent = nodeIndent + indentSize * options.VariableDeclarator[node.kind];
 
-            // Comma can be placed before declaration
-            checkNodesIndent(elements, elementsIndent, true);
+            checkNodesIndent(elements, elementsIndent);
 
             // Only check the last line if there is any token after the last item
             if (sourceCode.getLastToken(node).loc.end.line <= lastElement.loc.end.line) {
@@ -803,7 +780,7 @@ module.exports = {
             if (tokenBeforeLastElement.value === ",") {
 
                 // Special case for comma-first syntax where the semicolon is indented
-                checkLastNodeLineIndent(node, getNodeIndent(tokenBeforeLastElement));
+                checkLastNodeLineIndent(node, getNodeIndent(tokenBeforeLastElement).goodChar);
             } else {
                 checkLastNodeLineIndent(node, elementsIndent - indentSize);
             }
@@ -835,7 +812,7 @@ module.exports = {
                 return caseIndentStore[switchNode.loc.start.line];
             } else {
                 if (typeof switchIndent === "undefined") {
-                    switchIndent = getNodeIndent(switchNode);
+                    switchIndent = getNodeIndent(switchNode).goodChar;
                 }
 
                 if (switchNode.cases.length > 0 && options.SwitchCase === 0) {
@@ -854,7 +831,7 @@ module.exports = {
                 if (node.body.length > 0) {
 
                     // Root nodes should have no indent
-                    checkNodesIndent(node.body, getNodeIndent(node));
+                    checkNodesIndent(node.body, getNodeIndent(node).goodChar);
                 }
             },
 
@@ -913,7 +890,7 @@ module.exports = {
                     return;
                 }
 
-                const propertyIndent = getNodeIndent(node) + indentSize * options.MemberExpression;
+                const propertyIndent = getNodeIndent(node).goodChar + indentSize * options.MemberExpression;
 
                 const checkNodes = [node.property];
 
@@ -929,7 +906,7 @@ module.exports = {
             SwitchStatement(node) {
 
                 // Switch is not a 'BlockStatement'
-                const switchIndent = getNodeIndent(node);
+                const switchIndent = getNodeIndent(node).goodChar;
                 const caseIndent = expectedCaseIndent(node, switchIndent);
 
                 checkNodesIndent(node.cases, caseIndent);