Skip to content

Commit

Permalink
Fix: ignored nodes in indent rule (fixes eslint#9392)
Browse files Browse the repository at this point in the history
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:

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 ignoring the indentation
of such lines.

All tests pass.

Fixes eslint#9392.
  • Loading branch information
robinhouston committed Oct 5, 2017
1 parent ee99876 commit 8af1f44
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
24 changes: 19 additions & 5 deletions lib/rules/indent.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,15 @@ class OffsetStorage {
}
}

/**
* Check whether a token is being ignored.
* @param {Token} token The token
* @returns {boolean} `true` if the token is being ignored
*/
isTokenIgnored(token) {
return this._ignoredTokens.has(token);
}

/**
* Gets the first token that the given token's indentation is dependent on
* @param {Token} token The token
Expand Down Expand Up @@ -718,11 +727,16 @@ module.exports = {
/**
* Checks if a token's indentation is correct
* @param {Token} token Token to examine
* @param {int} desiredIndentLevel needed indent level
* @param {Token} referenceToken Token to use as a reference for the desired indentation
* @returns {boolean} `true` if the token's indentation is correct
*/
function validateTokenIndent(token, desiredIndentLevel) {
function validateTokenIndent(token, referenceToken) {
if (offsets.isTokenIgnored(token)) {
return true;
}

const indentation = tokenInfo.getTokenIndent(token);
const desiredIndentLevel = offsets.getDesiredIndent(referenceToken);
const expectedChar = indentType === "space" ? " " : "\t";

return indentation === expectedChar.repeat(desiredIndentLevel * indentSize) ||
Expand Down Expand Up @@ -1485,7 +1499,7 @@ module.exports = {
}

// If the token matches the expected expected indentation, don't report it.
if (validateTokenIndent(firstTokenOfLine, offsets.getDesiredIndent(firstTokenOfLine))) {
if (validateTokenIndent(firstTokenOfLine, firstTokenOfLine)) {
return;
}

Expand All @@ -1495,8 +1509,8 @@ module.exports = {

// If a comment matches the expected indentation of the token immediately before or after, don't report it.
if (
tokenBefore && validateTokenIndent(firstTokenOfLine, offsets.getDesiredIndent(tokenBefore)) ||
tokenAfter && validateTokenIndent(firstTokenOfLine, offsets.getDesiredIndent(tokenAfter))
tokenBefore && validateTokenIndent(firstTokenOfLine, tokenBefore) ||
tokenAfter && validateTokenIndent(firstTokenOfLine, tokenAfter)
) {
return;
}
Expand Down
17 changes: 17 additions & 0 deletions tests/lib/rules/indent.js
Original file line number Diff line number Diff line change
Expand Up @@ -4744,6 +4744,23 @@ 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"] }]
}
],

Expand Down

0 comments on commit 8af1f44

Please sign in to comment.