Enable Hash shorthand syntax - accept both new and old syntax

[lucianghinda]

This enables both type of hash syntax:

#bad 
{foo: , bar: bar}

#bad
{foo: , bar: baz}

#good
{foo:, bar:} # shorthand syntax

#good
{foo: foo, bar: baz} # old/currnet syntax

Source: https://docs.rubocop.org/rubocop/cops_style.html#stylehashsyntax

See my arguments for this 👇

[lucianghinda]

Arguments for Hash Shorthand Syntax

Here are some arguments for using the shorthand syntax for hashes and with it what is called method punning, and I will try to express this in some steps argument.

Hash shorthand syntax

Here are some arguments for hash shorthand syntax

When working with hash, it sometimes happens to write boilerplate code:

client_id = get_client_id(app)
response_type = get_response_from(request)
redirect_url = # ...

params = {
  client_id: client_id, 
  response_type: response_type,
  redirect_url: redirect_url
}

This makes the code longer, as sometimes specifying keys and values like this will make the line longer. Thus we break it on multiple lines, adding to longer diffs when changing.

Citing Matz from "Treating Code as an Essay":

Because there is a definite cost involved in scanning code with the human eye, programs should ideally contain no unnecessary information

Thus it is easier to read the following:

client_id = get_client_id(app)
response_type = get_response_from(request)
redirect_url = # ...

params = { client_id:, response_type:, redirect_url: }

What before spanned five lines is now contained in one single line.

One can make the case that to read the second piece of code you have to know how the hash shorthand syntax works; thus it needs extra information. While this is a valid argument, it is also a very general argument: the same can be said about using map shorthand notation.

Method call punning

Accepting hash shorthand syntax opens the path to another nice feature of Ruby 3.1

Method punning means to call a method that has keyword arguments in the same way as the hash shorthand syntax:

def response(client_id:, response_type:, redirect_url:)
end

client_id = get_client_id(app)
response_type = get_response_from(request)
redirect_url = # ...

# 👇 method can be called omitting the keyword args values 
response(client_id:, response_type:, redirect_url:)

Arguments for method call punning

1. Using keyword arguments for methods definitions is a way to remove the dependency on the order of arguments
2. The tendency is to use keyword arguments when there are more than (or equal to) 2 arguments for a method
3. Thus, we are starting to write some kind of boilerplate code

Examples:

class SocialMediaService
  def initialize(data)
    @data = data
    @client = Twitter::Client.new
  end

  def run
    height, width = get_resolution(@data)
    media_key = get_media_key(@data)
    tweet = @data.fetch(:body)
    author = get_author_id

    @client.tweet(
      tweet: tweet,
      media_key: media_key,
      height: height,
      width: width,
      author: author
    )
  end
end

4. We go back to brevity; the code can be made shorter and span fewer lines with a method called punning:

class SocialMediaService
  def initialize(data)
    @data = data
    @client = Twitter::Client.new
  end

  def run
    height, width = get_resolution(@data)
    media_key = get_media_key(@data)
    tweet = @data.fetch(:body)
    author = get_author_id

    @client.tweet(tweet:, media_key:, height:, width:, author:)
  end
end

Rubocop rules

Rubocop defines a couple of options for the Shorthand Syntax. I think probably there are three that can be of interest:

• always
• either
• consistent

I read the conversation on 192, and it is not clear to me from there what the conclusion was about if we want or not to use the new syntax. What I got from there was that we don't want to enable the always mode as that will generate either a big bang update (with auto-correct) or a lot of messages.

I propose to use consistent for now. This will allow us to make the transition smooth. New code can use the new shorthand syntax, and the old code can remain as it is until it will be fixed step by step. This will also help make us comfortable with the new code. As we review new code having this syntax, we will get more comfortable with it.

[lucianghinda]

@cookpad/global-web-devs what do you think about this?

[sikachu]

I didn't know you can now do this. Nice feature.

👍 from me.

[Bodacious]

Thanks @lucianghinda

I wanted to raise a general discussion about this feature a few months ago.
https://ckpd.slack.com/archives/C03HKCP86N8/p1657034167233149

My view is that we should embrace all features of the language. I don't think we should be auto-correcting old instances though, as we have a lot of them, and this will add unnecessary noise to PRs.

[lucianghinda]

Thank you @sikachu for the feedback. I am glad that you are on board with this

[lucianghinda]

Thank you @Bodacious for the reply.

I read your previous PR about enabling this behaviour that inspired me to open this new PR and now read the Slack conversation.

I saw there that the discussion focused in the end on changing the old code. This is why my proposal is to adopt consistent which will allow us to write new code with the new syntax and keep the old code as it is. Thus there is no need for a big bang change.

[tejanium]

As mentioned in #192 (review) as well. I'm 101% for this 👍

[ollieh-m]

Thanks @lucianghinda 👍

I was confused by the explanation of consistent here in rubocop:

always - forces use of the 3.1 syntax (e.g. {foo:})
consistent - like "always", but will avoid mixing styles in a single hash

I think that's meant to say "like 'either', but will avoid mixing styles in a single hash". It's not like "always" at all, unless I'm missing something, because {foo: foo, bar: baz} is still allowed 😅.

[lucianghinda]

@ollieh-m
Thank you for the feedback.

I proposed a small fix for the Rubocop documentation that was not merged.

Now it says consistent is like either, but it does not allow mixing styles.