Rubocop: Layout/IndentFirstArrayElement makes you create quirky code

[mensfeld]

Expected behavior

I expect the previous behavior to be maintained.

This code looks much better:

FILE_NAMES = %w[
               .coditsu/local.yml
               .coditsu/ci.yml
               .coditsu/*.yml
               .coditsu.yml
             ]
             .map { |name| ["../#{name}", name] }
             .tap(&:flatten!)
             .freeze

than

FILE_NAMES = %w[
  .coditsu/local.yml
  .coditsu/ci.yml
  .coditsu/*.yml
  .coditsu.yml
]
             .map { |name| ["../#{name}", name] }
             .tap(&:flatten!)
             .freeze

Actual behavior

Rubocop reports things that shouldn't be considered invalid as the final output format looks really weird.

Steps to reproduce the problem

git clone [email protected]:mensfeld/rubocop-bug.git
bundle
bundle exec rubocop was.rb

Inspecting 1 file
C

Offenses:

was.rb:4:16: C: Layout/IndentFirstArrayElement: Use 2 spaces for indentation in an array, relative to the start of the line where the left square bracket is.
               .coditsu/local.yml
               ^^^^^^^^^^^^^^^^^^
was.rb:8:14: C: Layout/IndentFirstArrayElement: Indent the right bracket the same as the start of the line where the left bracket is.
             ]

git clone [email protected]:mensfeld/rubocop-bug.git
bundle
bundle exec rubocop is.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

RuboCop version

$ [bundle exec] rubocop -V
0.71.0 (using Parser 2.6.3.0, running on ruby 2.6.3 x86_64-linux)

[mensfeld]

I do understand that you guys are busy but could you at least tell me whether this is a new style recommendation or a bug?

[bbatsov]

Hmm, there's there definitely a bug here, but perhaps not the one I first thought of (when you first showed this to me I thought the problem was with the method calls position). Likely the behaviour was changed in this commit https://github.com/rubocop-hq/rubocop/commit/184c2a1e92caa6914fe75a0bcc484c64ab834594#diff-0b8552aed98e2fa61f6cf43358e05933, but someone will have to dig deeper. Both the new and the old indentation of the array elements seems sensible to me, although for multi-line literals I think enforce the new one pretty much everywhere.

[bbatsov]

//cc @maxh

[mensfeld]

although for multi-line literals I think enforce the new one pretty much everywhere.

You mean that as default?

FILE_NAMES = %w[
  .coditsu/local.yml
  .coditsu/ci.yml
  .coditsu/*.yml
  .coditsu.yml
]
             .map { |name| ["../#{name}", name] }
             .tap(&:flatten!)
             .freeze

Can you justify that decision?

[maxh]

Sorry to hear of your trouble. It's possible that some contributions I made to this project impacted the behavior here. Did the behavior previously support what you were looking for? What version did you upgrade from/to where the behavior changed?

In my team's codebase this is autocorrected to:

FILE_NAMES = %w[
  .coditsu/local.yml
  .coditsu/ci.yml
  .coditsu/*.yml
  .coditsu.yml
]
  .map { |name| ["../#{name}", name] }
  .tap(&:flatten!)
  .freeze

Which is what I would expect. Indeed, in JS prettier a similar expression gets autoformatted to:

const FILE_NAMES = [
  ".coditsu/local.yml",
  ".coditsu/ci.yml",
  ".coditsu/.yml",
  ".coditsu.yml",
]
  .map(name => ["../#{name}", name])
  .stringify();

The cop Layout/MultilineMethodCallIndentation is responsible for reducing the indents on the chained method calls. Does enabling that cop address the issue for you?

Alternatively, if you want to preserve the large indentation relative to =, then try changing the EnforcedStyle to align_brackets on IndentFirstArrayElement. It's possible you previously had this configured for the cop's old name (IndentArray) and might just need to update your .rubocop.yml to the new name.

Let us know what works!

[mensfeld]

Hey, thanks for the answer.

Just to clarify:

• I use defaults for Layout/MultilineMethodCallIndentation
  
  
  
  • IndentFirstArrayElement is empty / default as well
  
  
  • IndentArray not present
  
  

I don't use autocorrect as I don't have rubocop installed within my gemset (not relevant to the error).

When I change indentations to this (to give you a full scope):

module Coditsu
  module BuildRunner
    module Config
      class FileFinder
        FILE_NAMES = %w[
          .coditsu/local.yml
          .coditsu/ci.yml
          .coditsu/*.yml
          .coditsu.yml
        ]
          .map { |name| ["../#{name}", name] }
          .tap(&:flatten!)
          .freeze
      end
    end
  end
end

Rubocop still reports following issues:

If you need, I can create a repro case.

Sorry for such a late response btw.

[maxh]

I see. Two suggestions below depending on the behavior you want. The defaults are a bit inconsistent for these two cops, I guess. (It appears this inconsistency is unrelated to my changes FWIW, but potentially worth fixing within the upstream codebase.)

(A) If you want this:

        FILE_NAMES = %w[
          .coditsu/local.yml
          .coditsu/ci.yml
          .coditsu/*.yml
          .coditsu.yml
        ]
          .map { |name| ["../#{name}", name] }
          .tap(&:flatten!)
          .freeze

Then switch to indented style for MultilineMethodCallIndentation:
https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Layout/MultilineMethodCallIndentation

(B) If you want this:

FILE_NAMES = %w[
               .coditsu/local.yml
               .coditsu/ci.yml
               .coditsu/*.yml
               .coditsu.yml
             ]
             .map { |name| ["../#{name}", name] }
             .tap(&:flatten!)
             .freeze

Then switch to align_brackets style for IndentFirstArrayElement:
https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Layout/IndentFirstArrayElement

[mensfeld]

Thank you - so to make it clear, with the default RuboCop settings as it is now, I will be offended either by one or the other, right?

[maxh]

Yep, I think that's correct. I'm not sure how the defaults end up being set to what they are. I think RuboCop overall is more oriented towards configurability + flexibility rather than having perfect defaults out of the box.

Here's our configuration of these cops, fwiw:

Layout/IndentFirstArrayElement:
  Enabled: true
  EnforcedStyle: consistent

Layout/MultilineMethodCallIndentation:
  Enabled: true
  EnforcedStyle: indented

[mensfeld]

Yes I do understand that although I would still consider that as a bug as there's a contradiction here.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[mensfeld]

Why marked as stale :( still waiting on that to be fixed.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale[bot]]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

[marcandre]

Could we have an official statement here (other than StaleBot's)?

I'm not nearly as familiar as I'd like with the many cops we have, but I for one am confused as to this cop's possible settings and its default.

I looked in the ruby-style-guide and couldn't find anything that reflected this default. Actually the style guide says to call most methods with ( except for "dsl" methods; in any case I would like to know what is the justification for the indent being dependent on being part of method call with or without (.

I'm very surprised to not see a setting which is "single 2 space indent from beginning of the line, or the opening '[', your choice, and no need to be consistent across a source file", e.g.

# ok
foo = %i[
        abc
        def
        ghi
      ]

# somewhere else in the same file:
SOME_VERY_LONG_CONSTANT_NAME = %i[
  jkl  # don't want to indent this as it would make for a very long line
  mno 
  pqr
]

# or maybe there are a lot of elements
ANOTHER_VERY_LONG_CONSTANT_NAME = %i[
  all? any? chain chunk chunk_while collect collect_concat count cycle
  detect drop drop_while each each_cons each_entry each_slice
  each_with_index each_with_object entries filter filter_map find
  find_all find_index flat_map grep grep_v group_by inject lazy map
  max max_by min min_by minmax minmax_by none? one? partition reduce
  reject reverse_each select slice_after slice_before slice_when sort
  sort_by sum take take_while tally to_h uniq zip
]


# bad
foo = %i[
            abc
            def
            ghi
        ]

# bad
foo = %i[
    abc
    def
    ghi
]

Or is there such a setting/cop already?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[mensfeld]

Why marked as stale :( still waiting on that to be fixed.