Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: add ignoreComments option to indent rule (fixes #9018) #9752

Merged
merged 2 commits into from Dec 28, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion docs/rules/indent.md
Expand Up @@ -84,7 +84,8 @@ This rule has an object option:
* `"ObjectExpression"` (default: 1) enforces indentation level for properties in objects. It can be set to the string `"first"`, indicating that all properties in the object should be aligned with the first property. This can also be set to `"off"` to disable checking for object properties.
* `"ImportDeclaration"` (default: 1) enforces indentation level for import statements. It can be set to the string `"first"`, indicating that all imported members from a module should be aligned with the first member in the list. This can also be set to `"off"` to disable checking for imported module members.
* `"flatTernaryExpressions": true` (`false` by default) requires no indentation for ternary expressions which are nested in other ternary expressions.
* `ignoredNodes` accepts an array of [selectors](/docs/developer-guide/selectors.md). If an AST node is matched by any of the selectors, the indentation of tokens which are direct children of that node will be ignored. This can be used as an escape hatch to relax the rule if you disagree with the indentation that it enforces for a particular syntactic pattern.
* `"ignoredNodes"` accepts an array of [selectors](/docs/developer-guide/selectors.md). If an AST node is matched by any of the selectors, the indentation of tokens which are direct children of that node will be ignored. This can be used as an escape hatch to relax the rule if you disagree with the indentation that it enforces for a particular syntactic pattern.
* `"ignoreComments"` (default: false) can be used when comments do not need to be aligned with nodes on the previous or next line.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should document the existing comment behavior about allowing alignment with the previous/next line. It seems like this description hints at the idea, but I'm not sure if it's explained more thoroughly anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The word "comment" isn't even mentioned in the docs currently, so I would hazard a guess that there is no description of expected comment indentation. If you want me to add that to this pull request, feel free to Request Changes here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¯\_(ツ)_/¯ It's not a big problem, I'm fine with this being merged as-is.


Level of indentation denotes the multiple of the indent specified. Example:

Expand Down Expand Up @@ -641,6 +642,22 @@ bar();
})
```

### ignoreComments

Examples of additional **correct** code for this rule with the `4, { "ignoreComments": true }` option:

```js
/*eslint indent: ["error", 4, { "ignoreComments": true }] */

if (foo) {
doSomething();

// comment intentionally de-indented
doSomethingElse();
}
```


## Compatibility

* **JSHint**: `indent`
Expand Down
12 changes: 11 additions & 1 deletion lib/rules/indent.js
Expand Up @@ -600,6 +600,9 @@ module.exports = {
pattern: ":exit$"
}
}
},
ignoreComments: {
type: "boolean"
}
},
additionalProperties: false
Expand Down Expand Up @@ -638,7 +641,8 @@ module.exports = {
ObjectExpression: 1,
ImportDeclaration: 1,
flatTernaryExpressions: false,
ignoredNodes: []
ignoredNodes: [],
ignoreComments: false
};

if (context.options.length) {
Expand Down Expand Up @@ -1457,6 +1461,12 @@ module.exports = {
},
"Program:exit"() {

// If ignoreComments option is enabled, ignore all comment tokens.
if (options.ignoreComments) {
sourceCode.getAllComments()
.forEach(comment => offsets.ignoreToken(comment));
}

// Invoke the queued offset listeners for the nodes that aren't ignored.
listenerCallQueue
.filter(nodeInfo => !ignoredNodes.has(nodeInfo.node))
Expand Down
31 changes: 31 additions & 0 deletions tests/lib/rules/indent.js
Expand Up @@ -4774,6 +4774,17 @@ ruleTester.run("indent", rule, {
]
`,
options: ["tab", { ArrayExpression: "first", ignoredNodes: ["CallExpression"] }]
},
{
code: unIndent`
if (foo) {
doSomething();

// Intentionally unindented comment
doSomethingElse();
}
`,
options: [4, { ignoreComments: true }]
}
],

Expand Down Expand Up @@ -9199,6 +9210,26 @@ ruleTester.run("indent", rule, {
ignoredNodes: ["ExpressionStatement > CallExpression > FunctionExpression.callee > BlockStatement"]
}],
errors: expectedErrors([3, 4, 0, "Identifier"])
},
{
code: unIndent`
if (foo) {
doSomething();

// Intentionally unindented comment
doSomethingElse();
}
`,
output: unIndent`
if (foo) {
doSomething();

// Intentionally unindented comment
doSomethingElse();
}
`,
options: [4, { ignoreComments: false }],
errors: expectedErrors([4, 4, 0, "Line"])
}
]
});