Rubocop: Feature request: Layout/AlignHash option to allow for `key` and `table` styles to both be permissible

[mattbrictson]

Is your feature request related to a problem? Please describe.

The changes to Layout/AlignHash in 0.60 mean that you can no longer mix the key and table style without one of them being reported as a violation. For example:

key_style = {
  key: :value,
  another: :value,
  yet_another: :value
}
table_style = {
  key:         :value,
  another:     :value
  yet_another: :value
}

Before 0.60, if you chose the key style, both hashes would be allowed, because in both cases the keys are left-aligned.

However starting with 0.60, RuboCop strictly enforces the whitespace to the right of the keys. If you set the enforced style to key, the second hash will get marked as a violation. If you choose table, then the first will be a violation.

I believe that there is room for both styles in a single codebase. For small hashes, key is appropriate. But for large hashes the table style is much easier to read. Take a look at the changes introduced by 0.60 to RuboCop's own codebase. I think many people would agree that readability has been decreased.

Describe the solution you'd like

I am open to ideas. A naive solution would be a key_or_table option that allows for both styles of whitespace to be accepted. I.e.

Layout/AlignHash:
  EnforcedColonStyle: key_or_table
  EnforcedHashRocketStyle: key_or_table

Describe alternatives you've considered

Currently the only way to mix key and table styles in the same codebase is to turn off AlignHash altogether. This is a big step backwards, though, because now even blatantly bad alignments will be allowed without any violations, like this:

{
  key: :value,
    another:   :value
}

Or I could set the enforced style to key, and then use a # rubocop:disable comment around every hash where I want to use table syntax. That is a bit tedious, and again, it disables alignment checking completely. To my knowledge there is not a way to change the enforced style with a # rubocop comment; you can only disable the cop.

[bquorning]

I believe that there is room for both styles in a single codebase. […] Take a look at the changes introduced by 0.60 to RuboCop's own codebase. I think many people would agree that readability has been decreased.

I agree 100%, and was about to open an issue myself.

What I noticed after seeing the changes to this repository, and two others where I tried upgrading to rubocop v0.60.0 was: it seems that the table style is often used for “literal” hashes, while the key style is used when a hash makes part of method arguments. If I have time, I will try modifying AlignHash to enforce this style, so I can count the number of offenses in these three repositories I checked.

[bbatsov]

I expected a ticket like this, as the change was somewhat controversial. :) Maybe we can add something which behaved like the old style, but I'm not sure about the naming there (or the exact behaviour it should have).

[thomthom]

Fully agree with the original post. I had to disable AlignHash on my own projects because readability of the hashes got reduced in many cases by forcing one or the other option.

What I found useful of this cop was that the key alignment was consistent.
Having value alignedment check would be nice if it is able to accept that values are consistently aligned either by 1. one space after key or left aligned with the rest of the values.

Via the AST that should be possible, right? After having collected the Hash literal, one could query the source ranges for key and value and match them against these criterias?

Not sure what the naming should be. key_or_tablesounds very accurate. While flexibleis much more ambiguous (would require looking up the docs to recall its meaning.)

[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!

[mvz]

@bquorning have you been able to find the time to do your experiment?

[bquorning]

I did a small experiment, but as I recall, the results were inconclusive.

[stoivo]

I think I have created a PR where you can allow both key and table at the same time. Take a look at #6649.

The rubocop yaml will look like:
https://github.com/rubocop-hq/rubocop/pull/6649/files#diff-11a0d7906801d9dea0eccb85667ad811

[mattbrictson]

Closing as fixed by #6649