Update: give a node at least the indentation of its parent (fixes #9995…

…) (#10054) This updates the `indent` rule to ensure that the tokens in a non-ignored node are always indented at least as much as the first token of the node. This fixes an issue where some tokens would not have enough indentation if their node's listener did not explicitly give them an offset.
eslint · Mar 6, 2018 · 2e68be6 · 2e68be6
1 parent 72ca5b3
commit 2e68be6
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 28 deletions.
diff --git a/lib/rules/indent.js b/lib/rules/indent.js
@@ -997,6 +997,8 @@ module.exports = {
             return !node || node.loc.start.line === token.loc.start.line;
         }
 
+        const ignoredNodeFirstTokens = new Set();
+
         const baseOffsetListeners = {
             "ArrayExpression, ArrayPattern"(node) {
                 const openingBracket = sourceCode.getFirstToken(node);
@@ -1027,15 +1029,6 @@ module.exports = {
                     addElementListIndent(node.params, openingParen, closingParen, options.FunctionExpression.parameters);
                 }
                 addBlocklessNodeIndent(node.body);
-
-                let arrowToken;
-
-                if (node.params.length) {
-                    arrowToken = sourceCode.getTokenAfter(node.params[node.params.length - 1], astUtils.isArrowToken);
-                } else {
-                    arrowToken = sourceCode.getFirstToken(node, astUtils.isArrowToken);
-                }
-                offsets.setDesiredOffset(arrowToken, sourceCode.getFirstToken(node), 0);
             },
 
             AssignmentExpression(node) {
@@ -1145,9 +1138,6 @@ module.exports = {
                          */
                         offsets.setDesiredOffset(firstAlternateToken, firstToken, 1);
                     }
-
-                    offsets.setDesiredOffsets([questionMarkToken.range[1], colonToken.range[0]], firstConsequentToken, 0);
-                    offsets.setDesiredOffsets([colonToken.range[1], node.range[1]], firstAlternateToken, 0);
                 }
             },
 
@@ -1289,20 +1279,9 @@ module.exports = {
             SwitchStatement(node) {
                 const openingCurly = sourceCode.getTokenAfter(node.discriminant, astUtils.isOpeningBraceToken);
                 const closingCurly = sourceCode.getLastToken(node);
-                const caseKeywords = node.cases.map(switchCase => sourceCode.getFirstToken(switchCase));
 
                 offsets.setDesiredOffsets([openingCurly.range[1], closingCurly.range[0]], openingCurly, options.SwitchCase);
 
-                node.cases.forEach((switchCase, index) => {
-                    const caseKeyword = caseKeywords[index];
-
-                    if (!(switchCase.consequent.length === 1 && switchCase.consequent[0].type === "BlockStatement")) {
-                        const tokenAfterCurrentCase = index === node.cases.length - 1 ? closingCurly : caseKeywords[index + 1];
-
-                        offsets.setDesiredOffsets([caseKeyword.range[1], tokenAfterCurrentCase.range[0]], caseKeyword, 1);
-                    }
-                });
-
                 if (node.cases.length) {
                     sourceCode.getTokensBetween(
                         node.cases[node.cases.length - 1],
@@ -1312,6 +1291,15 @@ module.exports = {
                 }
             },
 
+            SwitchCase(node) {
+                if (!(node.consequent.length === 1 && node.consequent[0].type === "BlockStatement")) {
+                    const caseKeyword = sourceCode.getFirstToken(node);
+                    const tokenAfterCurrentCase = sourceCode.getTokenAfter(node);
+
+                    offsets.setDesiredOffsets([caseKeyword.range[1], tokenAfterCurrentCase.range[0]], caseKeyword, 1);
+                }
+            },
+
             TemplateLiteral(node) {
                 node.expressions.forEach((expression, index) => {
                     const previousQuasi = node.quasis[index];
@@ -1402,7 +1390,6 @@ module.exports = {
                 const firstToken = sourceCode.getFirstToken(node);
 
                 offsets.setDesiredOffsets(node.name.range, firstToken, 1);
-                offsets.setDesiredOffset(sourceCode.getLastToken(node), firstToken, 0);
             },
 
             JSXExpressionContainer(node) {
@@ -1414,7 +1401,15 @@ module.exports = {
                     openingCurly,
                     1
                 );
-                offsets.setDesiredOffset(closingCurly, openingCurly, 0);
+            },
+
+            "*"(node) {
+                const firstToken = sourceCode.getFirstToken(node);
+
+                // Ensure that the children of every node are indented at least as much as the first token.
+                if (firstToken && !ignoredNodeFirstTokens.has(firstToken)) {
+                    offsets.setDesiredOffsets(node.range, firstToken, 0);
+                }
             }
         };
 
@@ -1423,7 +1418,8 @@ module.exports = {
         /*
          * To ignore the indentation of a node:
          * 1. Don't call the node's listener when entering it (if it has a listener)
-         * 2. Call `ignoreNode` on the node sometime after exiting it and before validating offsets.
+         * 2. Don't set any offsets against the first token of the node.
+         * 3. Call `ignoreNode` on the node sometime after exiting it and before validating offsets.
          */
         const offsetListeners = lodash.mapValues(
             baseOffsetListeners,
@@ -1451,7 +1447,16 @@ module.exports = {
 
         // For each ignored node selector, set up a listener to collect it into the `ignoredNodes` set.
         const ignoredNodes = new Set();
-        const addToIgnoredNodes = ignoredNodes.add.bind(ignoredNodes);
+
+        /**
+         * Ignores a node
+         * @param {ASTNode} node The node to ignore
+         * @returns {void}
+         */
+        function addToIgnoredNodes(node) {
+            ignoredNodes.add(node);
+            ignoredNodeFirstTokens.add(sourceCode.getFirstToken(node));
+        }
 
         const ignoredNodeListeners = options.ignoredNodes.reduce(
             (listeners, ignoredSelector) => Object.assign(listeners, { [ignoredSelector]: addToIgnoredNodes }),
@@ -1474,7 +1479,7 @@ module.exports = {
 
                     // If a node's type is nonstandard, we can't tell how its children should be offset, so ignore it.
                     if (!KNOWN_NODES.has(node.type)) {
-                        ignoredNodes.add(node);
+                        addToIgnoredNodes(node);
                     }
                 },
                 "Program:exit"() {

diff --git a/tests/lib/rules/indent.js b/tests/lib/rules/indent.js
@@ -2302,6 +2302,17 @@ ruleTester.run("indent", rule, {
             `,
             options: [4]
         },
+        unIndent`
+            foo &&
+                !bar(
+                )
+        `,
+        unIndent`
+            foo &&
+                ![].map(() => {
+                    bar();
+                })
+        `,
         {
             code: unIndent`
                 foo =
@@ -7549,6 +7560,34 @@ ruleTester.run("indent", rule, {
             options: [4],
             errors: expectedErrors([2, 4, 8, "Identifier"])
         },
+        {
+            code: unIndent`
+                foo &&
+                    !bar(
+                )
+            `,
+            output: unIndent`
+                foo &&
+                    !bar(
+                    )
+            `,
+            errors: expectedErrors([3, 4, 0, "Punctuator"])
+        },
+        {
+            code: unIndent`
+                foo &&
+                    ![].map(() => {
+                    bar();
+                })
+            `,
+            output: unIndent`
+                foo &&
+                    ![].map(() => {
+                        bar();
+                    })
+            `,
+            errors: expectedErrors([[3, 8, 4, "Identifier"], [4, 4, 0, "Punctuator"]])
+        },
         {
             code: unIndent`
                 [