qpixel
qpixel copied to clipboard
Decide on a hash syntax rule and apply it
Rubocop can enforce a particular style of hash syntax. Can we decide on what to enforce, record it here, and then gradually change the codebase to match? At that point we would be able to switch on the lint in order to keep that syntax enforced consistently in future.
Related: #1652 (enables Style/HashSyntax in either mode until we decide which one to use)
Yes that's what triggered me raising this issue. You saw this and commented before I had finished my review that links to it... Not sure if you can see that review since Art already posted an approving review so the pull request was merged before I finished my review. Let me know if not - although I only requested a very minor change.
For those who don't know, there are two main ways of writing a hash in Ruby:
{ foo: 'bar', abc: 123 } # which I'll call JS-style
{ :foo => 'bar', :abc => 123 } # which is usually known as old-style or "rockets" after the =>
Generally I find the former, JS-style, to be clearer (particularly for newcomers to Ruby) and more concise. The only exception to that is when not using symbols as hash keys (i.e. when using strings or numbers), in which case we have to use { 'foo' => 'bar', 'abc' => 123 }.
Any objections to standardising on JS-style?
I prefer JS-style too. Would standardising on it mean we need to work around those cases it doesn't support, or does standardising mean avoiding those cases altogether? I'm pretty happy either way.
The other question is whether we enforce the shorthand style:
{foo:, abc:}
rather than
{foo: foo, abc: abc}
I would hope (but haven't researched) that rubocop is smart enough to recognise when we have to use rocket syntax because of the key type.
As for shorthand - I didn't even know that was possible! I think I would lean towards full declarations, i.e. no shorthand.
I would hope (but haven't researched) that rubocop is smart enough to recognise when we have to use rocket syntax because of the key type.
Yes (just looked it up), we can specify either the default
EnforcedStyle: ruby19
or
EnforcedStyle: ruby19_no_mixed_keys
The first enforces JS-style everywhere except for keys that need rockets, and the second enforces JS-style when no rockets are needed, but all rockets when at least one is needed (so it's either no rockets or all rockets).
So either way we can still use rockets when needed.
As for shorthand - I didn't even know that was possible! I think I would lean towards full declarations, i.e. no shorthand.
For that we could specify
EnforcedShorthandSyntax: never
This will complain about any use of the shorthand.
ruby19_no_mixed_keys makes the most sense to me given that we all seem to agree that the ruby19 syntax is preferable. No mixed keys seems like a no-brainer here as being consistent is almost always the best play (and it's definitely not one of the rare cases it's not).