Repository: eslint/eslint (js)

Issue:
indent rule: ignoreNodes not working as expected in "tab" mode
I have a proposed fix for this, which I’ll send as a pull request. But I don’t think the bug itself has been reported yet, hence the below.

<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).

    This template is for bug reports. If you are reporting a bug, please continue on. If you are here for another reason, please see below:

    1. To propose a new rule: https://eslint.org/docs/developer-guide/contributing/new-rules
    2. To request a change: https://eslint.org/docs/developer-guide/contributing/changes
    3. If you have any questions, please stop by our chatroom: https://gitter.im/eslint/eslint

    Note that leaving sections blank will make it difficult for us to troubleshoot and we may have to close the issue.
-->

**Tell us about your environment**

* **ESLint Version:** 4.8.0
* **Node Version:** 8.1.2
* **npm Version:** 5.4.2

**What parser (default, Babel-ESLint, etc.) are you using?** default

**Please show your full configuration:**

<details>
<summary>Configuration</summary>

<!-- Paste your configuration below: -->
```js
{
    "rules": {
        "indent": ["error", "tab", {
            "ignoredNodes": [ "VariableDeclarator" ]
        }]
    }
}
```

</details>

**What did you do? Please include the actual source code causing the issue.**

<!-- Paste the source code below: -->
```js
/* eslint indent: [ "error", "tab", { "ignoredNodes": [ "VariableDeclarator" ] } ] */
var x = 1,
    y = 2;
```

**What did you expect to happen?**
No error, since VariableDeclarator nodes are ignored.

**What actually happened? Please include the actual, raw output from ESLint.**

```
$ eslint --no-eslintrc dent.js 

/Users/robin/Desktop/dent.js
  3:1  error  Expected indentation of 4 tabs but found 4 spaces  indent

✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.
```

Pull request:
Fix: ignored nodes in indent rule (fixes #9392)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[ ] Documentation update
[X] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

(I shan’t include the bug report template here unless you would especially like me to, since I have already filed the corresponding issue as #9392.)

When a node is ignored by the indent rule, it ought not to matter how it’s indented. But the ignoring of nodes was implemented in such a way that the *type* of indentation (tabs vs spaces) was being checked. For example in "tab" mode, an ignored line indented by four spaces would cause the error “Expected indentation of 4 tabs but found 4 spaces”.

In particular, this is a problem with “tabs for indentation, spaces for alignment” styles, where we want to allow code like:

```js
var x = 1,
    y = 2;
```

where the second line is aligned using four spaces.

This commit marks ignored indents by making them instances of the `IgnoredTokenIndent` class, and explicitly ignores the indentation of such lines.

All tests pass.

**Is there anything you'd like reviewers to focus on?**

Nothing particular comes to mind.


Patch:
diff --git a/README.md b/README.md
index 2f0f1648e..62dc2a657 100644
--- a/README.md
+++ b/README.md
@@ -27,6 +27,8 @@ ESLint is a tool for identifying and reporting on patterns found in ECMAScript/J
 
 ## Installation and Usage
 
+Prerequisites: [Node.js](https://nodejs.org/en/) (>=4.x), npm version 2+.
+
 There are two ways to install ESLint: globally and locally.
 
 ### Local Installation and Usage
diff --git a/lib/rules/indent.js b/lib/rules/indent.js
index 0f6468a7e..d33ed1ab9 100644
--- a/lib/rules/indent.js
+++ b/lib/rules/indent.js
@@ -235,10 +235,12 @@ class OffsetStorage {
     /**
      * @param {TokenInfo} tokenInfo a TokenInfo instance
      * @param {number} indentSize The desired size of each indentation level
+     * @param {string} indentType The indentation character
      */
-    constructor(tokenInfo, indentSize) {
+    constructor(tokenInfo, indentSize, indentType) {
         this._tokenInfo = tokenInfo;
         this._indentSize = indentSize;
+        this._indentType = indentType;
 
         this._tree = new BinarySearchTree();
         this._tree.insert(0, { offset: 0, from: null, force: false });
@@ -408,7 +410,7 @@ class OffsetStorage {
     /**
     * Gets the desired indent of a token
     * @param {Token} token The token
-    * @returns {number} The desired indent of the token
+    * @returns {string} The desired indent of the token
     */
     getDesiredIndent(token) {
         if (!this._desiredIndentCache.has(token)) {
@@ -417,7 +419,10 @@ class OffsetStorage {
 
                 // If the token is ignored, use the actual indent of the token as the desired indent.
                 // This ensures that no errors are reported for this token.
-                this._desiredIndentCache.set(token, this._tokenInfo.getTokenIndent(token).length / this._indentSize);
+                this._desiredIndentCache.set(
+                    token,
+                    this._tokenInfo.getTokenIndent(token)
+                );
             } else if (this._lockedFirstTokens.has(token)) {
                 const firstToken = this._lockedFirstTokens.get(token);
 
@@ -428,7 +433,7 @@ class OffsetStorage {
                     this.getDesiredIndent(this._tokenInfo.getFirstTokenOfLine(firstToken)) +
 
                         // (space between the start of the first element's line and the first element)
-                        (firstToken.loc.start.column - this._tokenInfo.getFirstTokenOfLine(firstToken).loc.start.column) / this._indentSize
+                        this._indentType.repeat(firstToken.loc.start.column - this._tokenInfo.getFirstTokenOfLine(firstToken).loc.start.column)
                 );
             } else {
                 const offsetInfo = this._getOffsetDescriptor(token);
@@ -436,9 +441,12 @@ class OffsetStorage {
                     offsetInfo.from &&
                     offsetInfo.from.loc.start.line === token.loc.start.line &&
                     !offsetInfo.force
-                ) ? 0 : offsetInfo.offset;
+                ) ? 0 : offsetInfo.offset * this._indentSize;
 
-                this._desiredIndentCache.set(token, offset + (offsetInfo.from ? this.getDesiredIndent(offsetInfo.from) : 0));
+                this._desiredIndentCache.set(
+                    token,
+                    (offsetInfo.from ? this.getDesiredIndent(offsetInfo.from) : "") + this._indentType.repeat(offset)
+                );
             }
         }
         return this._desiredIndentCache.get(token);
@@ -655,7 +663,7 @@ module.exports = {
 
         const sourceCode = context.getSourceCode();
         const tokenInfo = new TokenInfo(sourceCode);
-        const offsets = new OffsetStorage(tokenInfo, indentSize);
+        const offsets = new OffsetStorage(tokenInfo, indentSize, indentType === "space" ? " " : "\t");
         const parameterParens = new WeakSet();
 
         /**
@@ -688,27 +696,24 @@ module.exports = {
         /**
          * Reports a given indent violation
          * @param {Token} token Token violating the indent rule
-         * @param {int} neededIndentLevel Expected indentation level
-         * @param {int} gottenSpaces Actual number of indentation spaces for the token
-         * @param {int} gottenTabs Actual number of indentation tabs for the token
+         * @param {string} neededIndent Expected indentation string
          * @returns {void}
          */
-        function report(token, neededIndentLevel) {
+        function report(token, neededIndent) {
             const actualIndent = Array.from(tokenInfo.getTokenIndent(token));
             const numSpaces = actualIndent.filter(char => char === " ").length;
             const numTabs = actualIndent.filter(char => char === "\t").length;
-            const neededChars = neededIndentLevel * indentSize;
 
             context.report({
                 node: token,
-                message: createErrorMessage(neededChars, numSpaces, numTabs),
+                message: createErrorMessage(neededIndent.length, numSpaces, numTabs),
                 loc: {
                     start: { line: token.loc.start.line, column: 0 },
                     end: { line: token.loc.start.line, column: token.loc.start.column }
                 },
                 fix(fixer) {
                     const range = [token.range[0] - token.loc.start.column, token.range[0]];
-                    const newText = (indentType === "space" ? " " : "\t").repeat(neededChars);
+                    const newText = neededIndent;
 
                     return fixer.replaceTextRange(range, newText);
                 }
@@ -718,14 +723,13 @@ module.exports = {
         /**
          * Checks if a token's indentation is correct
          * @param {Token} token Token to examine
-         * @param {int} desiredIndentLevel needed indent level
+         * @param {string} desiredIndent Desired indentation of the string
          * @returns {boolean} `true` if the token's indentation is correct
          */
-        function validateTokenIndent(token, desiredIndentLevel) {
+        function validateTokenIndent(token, desiredIndent) {
             const indentation = tokenInfo.getTokenIndent(token);
-            const expectedChar = indentType === "space" ? " " : "\t";
 
-            return indentation === expectedChar.repeat(desiredIndentLevel * indentSize) ||
+            return indentation === desiredIndent ||
 
                 // To avoid conflicts with no-mixed-spaces-and-tabs, don't report mixed spaces and tabs.
                 indentation.includes(" ") && indentation.includes("\t");
diff --git a/lib/util/source-code.js b/lib/util/source-code.js
index 828eb85cd..d3fed5989 100644
--- a/lib/util/source-code.js
+++ b/lib/util/source-code.js
@@ -25,7 +25,6 @@ const TokenStore = require("../token-store"),
  * @private
  */
 function validate(ast) {
-
     if (!ast.tokens) {
         throw new Error("AST is missing the tokens array.");
     }
@@ -44,34 +43,9 @@ function validate(ast) {
 }
 
 /**
- * Finds a JSDoc comment node in an array of comment nodes.
- * @param {ASTNode[]} comments The array of comment nodes to search.
- * @param {int} line Line number to look around
- * @returns {ASTNode} The node if found, null if not.
- * @private
- */
-function findJSDocComment(comments, line) {
-
-    if (comments) {
-        for (let i = comments.length - 1; i >= 0; i--) {
-            if (comments[i].type === "Block" && comments[i].value.charAt(0) === "*") {
-
-                if (line - comments[i].loc.end.line <= 1) {
-                    return comments[i];
-                }
-                break;
-
-            }
-        }
-    }
-
-    return null;
-}
-
-/**
- * Check to see if its a ES6 export declaration
- * @param {ASTNode} astNode - any node
- * @returns {boolean} whether the given node represents a export declaration
+ * Check to see if its a ES6 export declaration.
+ * @param {ASTNode} astNode An AST node.
+ * @returns {boolean} whether the given node represents an export declaration.
  * @private
  */
 function looksLikeExport(astNode) {
@@ -80,10 +54,11 @@ function looksLikeExport(astNode) {
 }
 
 /**
- * Merges two sorted lists into a larger sorted list in O(n) time
- * @param {Token[]} tokens The list of tokens
- * @param {Token[]} comments The list of comments
- * @returns {Token[]} A sorted list of tokens and comments
+ * Merges two sorted lists into a larger sorted list in O(n) time.
+ * @param {Token[]} tokens The list of tokens.
+ * @param {Token[]} comments The list of comments.
+ * @returns {Token[]} A sorted list of tokens and comments.
+ * @private
  */
 function sortedMerge(tokens, comments) {
     const result = [];
@@ -182,9 +157,9 @@ class SourceCode extends TokenStore {
     }
 
     /**
-     * Split the source code into multiple lines based on the line delimiters
-     * @param {string} text Source code as a string
-     * @returns {string[]} Array of source code lines
+     * Split the source code into multiple lines based on the line delimiters.
+     * @param {string} text Source code as a string.
+     * @returns {string[]} Array of source code lines.
      * @public
      */
     static splitLines(text) {
@@ -197,6 +172,7 @@ class SourceCode extends TokenStore {
      * @param {int=} beforeCount The number of characters before the node to retrieve.
      * @param {int=} afterCount The number of characters after the node to retrieve.
      * @returns {string} The text representing the AST node.
+     * @public
      */
     getText(node, beforeCount, afterCount) {
         if (node) {
@@ -209,6 +185,7 @@ class SourceCode extends TokenStore {
     /**
      * Gets the entire source text split into an array of lines.
      * @returns {Array} The source text as an array of lines.
+     * @public
      */
     getLines() {
         return this.lines;
@@ -217,6 +194,7 @@ class SourceCode extends TokenStore {
     /**
      * Retrieves an array containing all comments in the source code.
      * @returns {ASTNode[]} An array of comment nodes.
+     * @public
      */
     getAllComments() {
         return this.ast.comments;
@@ -225,7 +203,8 @@ class SourceCode extends TokenStore {
     /**
      * Gets all comments for the given node.
      * @param {ASTNode} node The AST node to get the comments for.
-     * @returns {Object} The list of comments indexed by their position.
+     * @returns {Object} An object containing a leading and trailing array
+     *      of comments indexed by their position.
      * @public
      */
     getComments(node) {
@@ -297,48 +276,68 @@ class SourceCode extends TokenStore {
     /**
      * Retrieves the JSDoc comment for a given node.
      * @param {ASTNode} node The AST node to get the comment for.
-     * @returns {ASTNode} The Block comment node containing the JSDoc for the
-     *      given node or null if not found.
+     * @returns {Token|null} The Block comment token containing the JSDoc comment
+     *      for the given node or null if not found.
      * @public
      */
     getJSDocComment(node) {
+
+        /**
+         * Checks for the presence of a JSDoc comment for the given node and returns it.
+         * @param {ASTNode} astNode The AST node to get the comment for.
+         * @returns {Token|null} The Block comment token containing the JSDoc comment
+         *      for the given node or null if not found.
+         * @private
+         */
+        const findJSDocComment = astNode => {
+            const tokenBefore = this.getTokenBefore(astNode, { includeComments: true });
+
+            if (
+                tokenBefore &&
+                astUtils.isCommentToken(tokenBefore) &&
+                tokenBefore.type === "Block" &&
+                tokenBefore.value.charAt(0) === "*" &&
+                astNode.loc.start.line - tokenBefore.loc.end.line <= 1
+            ) {
+                return tokenBefore;
+            }
+
+            return null;
+        };
         let parent = node.parent;
-        const leadingComments = this.getCommentsBefore(node);
 
         switch (node.type) {
             case "ClassDeclaration":
             case "FunctionDeclaration":
-                if (looksLikeExport(parent)) {
-                    return findJSDocComment(this.getCommentsBefore(parent), parent.loc.start.line);
-                }
-                return findJSDocComment(leadingComments, node.loc.start.line);
+                return findJSDocComment(looksLikeExport(parent) ? parent : node);
 
             case "ClassExpression":
-                return findJSDocComment(this.getCommentsBefore(parent.parent), parent.parent.loc.start.line);
+                return findJSDocComment(parent.parent);
 
             case "ArrowFunctionExpression":
             case "FunctionExpression":
                 if (parent.type !== "CallExpression" && parent.type !== "NewExpression") {
-                    let parentLeadingComments = this.getCommentsBefore(parent);
-
-                    while (!parentLeadingComments.length && !/Function/.test(parent.type) && parent.type !== "MethodDefinition" && parent.type !== "Property") {
+                    while (
+                        !this.getCommentsBefore(parent).length &&
+                        !/Function/.test(parent.type) &&
+                        parent.type !== "MethodDefinition" &&
+                        parent.type !== "Property"
+                    ) {
                         parent = parent.parent;
 
                         if (!parent) {
                             break;
                         }
-
-                        parentLeadingComments = this.getCommentsBefore(parent);
                     }
 
-                    return parent && parent.type !== "FunctionDeclaration" && parent.type !== "Program" ? findJSDocComment(parentLeadingComments, parent.loc.start.line) : null;
-                }
-                if (leadingComments.length) {
-                    return findJSDocComment(leadingComments, node.loc.start.line);
+                    if (parent && parent.type !== "FunctionDeclaration" && parent.type !== "Program") {
+                        return findJSDocComment(parent);
+                    }
                 }
 
-            // falls through
+                return findJSDocComment(node);
 
+            // falls through
             default:
                 return null;
         }
@@ -348,6 +347,7 @@ class SourceCode extends TokenStore {
      * Gets the deepest node containing a range index.
      * @param {int} index Range index of the desired node.
      * @returns {ASTNode} The node if found or null if not found.
+     * @public
      */
     getNodeByRangeIndex(index) {
         let result = null,
@@ -381,6 +381,7 @@ class SourceCode extends TokenStore {
      * @param {Token} second The token to check before.
      * @returns {boolean} True if there is only space between tokens, false
      *  if there is anything other than whitespace between tokens.
+     * @public
      */
     isSpaceBetweenTokens(first, second) {
         const text = this.text.slice(first.range[1], second.range[0]);
@@ -389,10 +390,11 @@ class SourceCode extends TokenStore {
     }
 
     /**
-    * Converts a source text index into a (line, column) pair.
-    * @param {number} index The index of a character in a file
-    * @returns {Object} A {line, column} location object with a 0-indexed column
-    */
+     * Converts a source text index into a (line, column) pair.
+     * @param {number} index The index of a character in a file
+     * @returns {Object} A {line, column} location object with a 0-indexed column
+     * @public
+     */
     getLocFromIndex(index) {
         if (typeof index !== "number") {
             throw new TypeError("Expected `index` to be a number.");
@@ -423,12 +425,13 @@ class SourceCode extends TokenStore {
     }
 
     /**
-    * Converts a (line, column) pair into a range index.
-    * @param {Object} loc A line/column location
-    * @param {number} loc.line The line number of the location (1-indexed)
-    * @param {number} loc.column The column number of the location (0-indexed)
-    * @returns {number} The range index of the location in the file.
-    */
+     * Converts a (line, column) pair into a range index.
+     * @param {Object} loc A line/column location
+     * @param {number} loc.line The line number of the location (1-indexed)
+     * @param {number} loc.column The column number of the location (0-indexed)
+     * @returns {number} The range index of the location in the file.
+     * @public
+     */
     getIndexFromLoc(loc) {
         if (typeof loc !== "object" || typeof loc.line !== "number" || typeof loc.column !== "number") {
             throw new TypeError("Expected `loc` to be an object with numeric `line` and `column` properties.");
diff --git a/templates/blogpost.md.ejs b/templates/blogpost.md.ejs
index 0f0a2f752..05261a58f 100644
--- a/templates/blogpost.md.ejs
+++ b/templates/blogpost.md.ejs
@@ -7,7 +7,7 @@ tags:
 ---
 # ESLint v<%= version %> released
 
-We just pushed ESLint v<%- version %>, which is a <%- type %> release upgrade of ESLint. This release <% if (type !== "patch") { %>adds some new features and<% } %>fixes several bugs found in the previous release. <% if (type === "major") { %>This release also has some breaking changes, so please read the following closely. <% } %>
+We just pushed ESLint v<%- version %>, which is a <%- type %> release upgrade of ESLint. This release <% if (type !== "patch") { %>adds some new features and <% } %>fixes several bugs found in the previous release. <% if (type === "major") { %>This release also has some breaking changes, so please read the following closely. <% } %>
 
 <%
 const RULE_REGEX = new RegExp(`\`?\\b(${ruleList.join("|")})\\b\`?`, "g");


Test patch:
diff --git a/tests/lib/rules/indent.js b/tests/lib/rules/indent.js
index 006d7f1dd..0a7718492 100644
--- a/tests/lib/rules/indent.js
+++ b/tests/lib/rules/indent.js
@@ -4744,6 +4744,32 @@ ruleTester.run("indent", rule, {
                 </foo>
             `,
             options: [4, { ignoredNodes: ["JSXOpeningElement"] }]
+        },
+        {
+            code: unIndent`
+                {
+                \tvar x = 1,
+                \t    y = 2;
+                }
+            `,
+            options: ["tab"]
+        },
+        {
+            code: unIndent`
+                var x = 1,
+                    y = 2;
+                var z;
+            `,
+            options: ["tab", { ignoredNodes: ["VariableDeclarator"] }]
+        },
+        {
+            code: unIndent`
+                [
+                    foo(),
+                    bar
+                ]
+            `,
+            options: ["tab", { ArrayExpression: "first", ignoredNodes: ["CallExpression"] }]
         }
     ],