Version 3.9.0 broke multiline array indentation

[cyrus-and]

Tell us about your environment

• ESLint Version: v3.9.0
• Node Version: v7.0.0-pre
• npm Version: 3.10.8

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

Please show your full configuration:

{
    "rules": {
        "indent": [
            "error",
            4
        ]
    }
}

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

var array = [1,
             2,
             3];

What did you expect to happen?

This was considered valid before this release. It would be great to have the first option for arrays too.

What actually happened? Please include the actual, raw output from ESLint.

2:14  error  Expected indentation of 4 spaces but found 13  indent
3:14  error  Expected indentation of 4 spaces but found 13  indent

[not-an-aardvark]

Thanks for the suggestion. I agree that this would be a useful option.

Previously, the indentation of array elements was not checked at all in these cases, so the change in v3.9.0 was a bugfix for that case. But it seems reasonable to have an option to customize this.

[not-an-aardvark]

@ilyavolodin: I saw that you added the "regression"/"patch candidate" labels. However, I disagree that this is a regression; the bugfix is working as intended, which in this case means that more errors are reported. If we added an additional option, it would be semver-minor, not semver-patch.

[ianlyons]

I'm experiencing something similar, and it seems that this is, indeed, a regression. I would expect the following to be valid:

Actual code causing the error:

const fn = function(events) {
  return IfThen(
    'If in review go to review',
    [{
      type: conditionTypes.FIELD_RESPONSE,
    }],
    [{
      property: 'property'
    }].concat(events)
  );
};

Actual error:

  73:5  error  Expected indentation of 2 spaces but found 4  indent

The key here, interestingly, is the concat method being called on the third argument, the second array... when removed, it no longer triggers as invalid.

[not-an-aardvark]

@ianlyons Hmm, that part does seem like unintended behavior. Would you mind creating a new issue for it so that we can track it separately?

[ianlyons]

@not-an-aardvark will do.

Edit: done - #7484.

[ilyavolodin]

@not-an-aardvark Fair enough.

[YarnSphere]

Note: version 3.9.0 also broke multi-line object indentation.

My two cents on this issue:

Indentation checking is disabled by default on function parameters and call expression arguments. Why must it be enabled by default on arrays/objects? The change broke a lot of my code (where broke reads as: ESLint now complains about it).

I agree that the option to indent array elements and object properties should be added, possibly with a first option, but with consistency in mind, it should probably be disabled by default.

[not-an-aardvark]

I'll champion this proposal.

[not-an-aardvark]

@eslint/eslint-team We need one more +1 to accept this issue; any takers?

@nunocastromartins Ideally, I think nothing would be disabled by default. The user shouldn't have to specify ten different config options just to check indentation.

[ilyavolodin]

@not-an-aardvark We have 3 + champion (me, @platinumazure and @mysticatea). So marking as accepted.

[jacksonrayhamilton]

I know I'm late to the game on this, but having just noticed this, I agree with @nunocastromartins. This was effectively a breaking change and should have been denoted as such with semver.