Skip to content

Commit

Permalink
Update: simplify variable declarator indent handling (fixes #8785) (#…
Browse files Browse the repository at this point in the history
…8801)

This fixes the `indent` rule's VariableDeclarator logic to correctly handle the case where a declaration has more than one declarator, but neither is on the same line as the start of the declaration.

This also updates the variable declarator listener to be slightly more similar to the logic for other nodes. Previously, variable declarators were treated as a special case and handling them involved overwriting some of the previously-declared offsets while exiting the node. Unfortunately, declarators are still a special case, but their logic isn't quite as different now -- the correct behavior is applied when entering the node, like it is for other node types.
  • Loading branch information
not-an-aardvark committed Jun 28, 2017
1 parent 9417818 commit be8d354
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 32 deletions.
73 changes: 41 additions & 32 deletions lib/rules/indent.js
Expand Up @@ -303,6 +303,18 @@ class OffsetStorage {
}
}

/**
* Sets the desired offset of a token, ignoring the usual collapsing behavior.
* **WARNING**: This is usually not what you want to use. See `setDesiredOffset` instead.
* @param {Token} token The token
* @param {Token} offsetFrom The token that `token` should be offset from
* @param {number} offset The desired indent level
* @returns {void}
*/
forceSetDesiredOffset(token, offsetFrom, offset) {
this.desiredOffsets[token.range[0]] = { offset, from: offsetFrom };
}

/**
* Sets the desired offset of multiple tokens
* @param {Token[]} tokens A list of tokens. These tokens should be consecutive.
Expand Down Expand Up @@ -1274,7 +1286,35 @@ module.exports = {
VariableDeclaration(node) {
const variableIndent = options.VariableDeclarator.hasOwnProperty(node.kind) ? options.VariableDeclarator[node.kind] : DEFAULT_VARIABLE_INDENT;

offsets.setDesiredOffsets(getTokensAndComments(node), sourceCode.getFirstToken(node), variableIndent);
if (node.declarations[node.declarations.length - 1].loc.start.line > node.loc.start.line) {

/*
* VariableDeclarator indentation is a bit different from other forms of indentation, in that the
* indentation of an opening bracket sometimes won't match that of a closing bracket. For example,
* the following indentations are correct:
*
* var foo = {
* ok: true
* };
*
* var foo = {
* ok: true,
* },
* bar = 1;
*
* Account for when exiting the AST (after indentations have already been set for the nodes in
* the declaration) by manually increasing the indentation level of the tokens in this declarator
* on the same line as the start of the declaration, provided that there are declarators that
* follow this one.
*/
getTokensAndComments(node).forEach((token, index, tokens) => {
if (index !== 0) {
offsets.forceSetDesiredOffset(token, tokens[0], variableIndent);
}
});
} else {
offsets.setDesiredOffsets(getTokensAndComments(node), sourceCode.getFirstToken(node), variableIndent);
}
const lastToken = sourceCode.getLastToken(node);

if (astUtils.isSemicolonToken(lastToken)) {
Expand All @@ -1294,37 +1334,6 @@ module.exports = {
}
},

"VariableDeclarator:exit"(node) {

/*
* VariableDeclarator indentation is a bit different from other forms of indentation, in that the
* indentation of an opening bracket sometimes won't match that of a closing bracket. For example,
* the following indentations are correct:
*
* var foo = {
* ok: true
* };
*
* var foo = {
* ok: true,
* },
* bar = 1;
*
* Account for when exiting the AST (after indentations have already been set for the nodes in
* the declaration) by manually increasing the indentation level of the tokens in the first declarator if the
* parent declaration has more than one declarator.
*/
if (node.parent.declarations.length > 1 && node.parent.declarations[0] === node && node.init) {
const valueTokens = new Set(getTokensAndComments(node.init));

valueTokens.forEach(token => {
if (!valueTokens.has(offsets.getFirstDependency(token))) {
offsets.increaseOffset(token, options.VariableDeclarator[node.parent.kind]);
}
});
}
},

WhileStatement: node => addBlocklessNodeIndent(node.body),

"*:exit": checkForUnknownNode,
Expand Down
58 changes: 58 additions & 0 deletions tests/lib/rules/indent.js
Expand Up @@ -518,6 +518,64 @@ ruleTester.run("indent", rule, {
`,
options: [2, { VariableDeclarator: 2, SwitchCase: 1 }]
},
{
code: unIndent`
var
x = {
a: 1,
},
y = {
b: 2
}
`
},
{
code: unIndent`
const
x = {
a: 1,
},
y = {
b: 2
}
`
},
{
code: unIndent`
let
x = {
a: 1,
},
y = {
b: 2
}
`
},
{
code: unIndent`
var foo = { a: 1 }, bar = {
b: 2
};
`
},
{
code: unIndent`
var foo = { a: 1 }, bar = {
b: 2
},
baz = {
c: 3
}
`
},
{
code: unIndent`
const {
foo
} = 1,
bar = 2
`
},
{
code: unIndent`
var foo = 1,
Expand Down

0 comments on commit be8d354

Please sign in to comment.