Indent rule false positive violation for VariableDeclarator on named export

[gonzoyumo]

What version of ESLint are you using?
2.11.1

What parser (default, Babel-ESLint, etc.) are you using?
default

Please show your full configuration:

"indent" : ["error", 2, {
  "SwitchCase": 0,
  "VariableDeclarator": { "var": 2, "let": 2, "const": 3 }
}]

What did you do? Please include the actual source code causing the issue.

export const MAX_POSTS           = 10,
             MAX_PUBLISHED_POSTS = 5,
             MAX_TEAM_MEMBERS    = 9,
             MAX_TEAM_PARTNERS   = 5;

What did you expect to happen?
No errors from the indent rule

What actually happened?
The indent rule reports violations as it doesn't take the export keyword into account.

A quick workaround I found for my usage is to add more spaces to the indent count in the checkIndentInVariableDeclarations method from indent.js rule file:

// If variable declaration is exported, add 7 spaces to indent count for the "export " keyword
if (node.parent.type.indexOf("Export") === 0) {
  elementsIndent += 7;
}

I'm brand new to ESLint so I didn't had time to dig into its core concepts. This is a quick n dirty fix that only works for my particular setup, it may need some more knowledge to handle any possible cases.

Thanks

[vitorbal]

Hi @gonzoyumo! thank you for the issue. Unfortunately, I'm not sure I understand why you would expect no indentation errors in this case.

From your config file, you have 2-space indentation. The VariableDeclarator option takes a multiple of the indent, so the indentation for const declarations would be 2 * 3 = 6 spaces.

So the correct indentation would be like the following:

export const MAX_POSTS    = 10,
      MAX_PUBLISHED_POSTS = 5,
      MAX_TEAM_MEMBERS    = 9,
      MAX_TEAM_PARTNERS   = 5;

I hope I haven't misunderstood your issue, if not, I think the error being reported is correct and this is not a bug. Let me know what you think!

[mysticatea]

Yeah, @vitorbal is correct. "VariableDeclarator": { "var": 2, "let": 2, "const": 3 } is not meaning alignment to the same kind element of the previous line. So, I think this request is an enhancement.

[gonzoyumo]

Hi,

sorry for the confusion, its actually more about an alignment issue but it's the indent rule that reports error.

This indeed works as expected when the declaration is not exported:

const MAX_POSTS           = 10,
      MAX_PUBLISHED_POSTS = 5,
      MAX_TEAM_MEMBERS    = 9,
      MAX_TEAM_PARTNERS   = 5;

Here alignments looks correct and indentation is respected.

But when I export a declaration, I want the items to be aligned the same, so I need the indent to be "greater" to obtain desired result:

export const MAX_POSTS           = 10,
             MAX_PUBLISHED_POSTS = 5,
             MAX_TEAM_MEMBERS    = 9,
             MAX_TEAM_PARTNERS   = 5;

I hope it's clear enough now :)

[vitorbal]

Hm, as was mentioned, the option takes only a multiple of the indentation configuration you have set. I guess one way of achieving what you want would be to have a separate option to configure the "export" statements:

"VariableDeclarator": { "var": 2, "let": 2, "const": 3, "export": 5 }

I guess we need some feedback from more people to decide if this option should be part of the core.
@eslint/eslint-team any thoughts?

[nzakas]

That would be one way of handling it. The indent rule is so complicated, though, that I'm not sure how practical it would be to add. @BYK @gyandeeps

[BYK]

May be we should just change the options and say align: true or something so we don't need this?

[nzakas]

@BYK you know my answer to that question. :)

[platinumazure]

The multiplier we currently use has been shown to be grossly outmoded. Even if it is a breaking change, we should seriously consider fixing the design.

[ilyavolodin]

I actually agree with @BYK we should deprecate var, let and const options and add align option instead. We don't need to remove schema, but just make it either or schema like we did for couple of other rules.

[nzakas]

@ilyavolodin I also agree, but I dont think we should do this as a one-off. I've asked @BYK to put together a proposal for how the options on this rule should change (of course, anyone else is welcome to do the same). This is really our most complicated rule, so I'd prefer to see a well-reasoned, complete proposal for how the options should change rather than slapping on options for individual pain points.

[mysticatea]

Maybe related to #6052.
That proposal is using "first", it's meaning that align 2nd line and later to the first parameter.
For consistency, it needs to be rethought by this issue.

[kaicataldo]

I'm closing this issues because it looks like consensus couldn't be reached. While we wish we could accommodate all requests, we have limited resources and do need to prioritize. We've found that issues failing to reach consensus after 21 days tend to never reach consensus, and as a team have decided to close such issues. This doesn't mean the idea isn't interesting or valuable, only that it's not something the team can commit to at the moment.

[kaicataldo]

Sorry, I'm going to reopen this for now until the indent rule rewrite is sorted out.

[not-an-aardvark]

Re-closing this, as it's not handled by the indent rewrite and it looks like there's insufficient interest for the team to pursue this.