Rubocop: [Case statement] Disallow "then" keyword for multi-line "when" clauses

[madejejej]

In a case statement we can use the then keyword to have the whole when clause in a single line, eg.

case temperature
when (-30..0) then puts "Freezing!"
when (1..10) then puts "Cold"
when (11..20) then puts "Not bad"
when (21..28) then puts "Warm"
when (29..40) then puts "Quite hot"
else
  puts "Not on my scale"
end 

We can rewrite this case statement by putting all the puts calls in separate lines:

case temperature
when (-30..0) then
  puts "Freezing!"
when (1..10) then 
  puts "Cold"
when (11..20) then 
  puts "Not bad"
when (21..28) then 
  puts "Warm"
when (29..40) then 
  puts "Quite hot"
else
  puts "Not on my scale"
end 

In this case, the then keyword is optional. I don't see any use cases when you would want to leave it there as it just generates more noise. Rubocop should complain about this and the correct version should be:

case temperature
when (-30..0)
  puts "Freezing!"
when (1..10) 
  puts "Cold"
when (11..20) 
  puts "Not bad"
when (21..28) 
  puts "Warm"
when (29..40) 
  puts "Quite hot"
else
  puts "Not on my scale"
end 

[blackst0ne]

case temperature
when (-30..0) then puts "Freezing!"
when (1..10)  then puts "Cold"
when (11..20) then puts "Not bad"
when (21..28) then puts "Warm"
when (29..40) then puts "Quite hot"
else
  puts "Not on my scale"
end

This is easier to read than the multi-line variant, imo.

[okuramasafumi]

I think I'm going to tackle this issue as my first patch to RuboCop.

IMO there should be an option to choose whether they prefer then (one-line) style or multiline style.
Also, I've heard some users prefer semicolon over then, so should we provide an option for that as well?

[madejejej]

@okuramasafumi that's great!

Here's what I think:

1. Multiline then syntax should always be banned
2. One-line then syntax should be optional - I agree it's sometimes easier to read @blackst0ne and I don't have anything against it
3. Multiline syntax should always be allowed - as sometimes you want to use more than one line in the when clause. It won't end up well with the one-line syntax.
4. I'm not sure about the semicolons though. I'd prefer not to allow them (at least by default)

So the following code should be OK be default:

case temperature
when (-30..0) then puts "Freezing!"
when (1..10)  then puts "Cold"
when (11..20) then puts "Not bad"
when (21..28) then puts "Warm"
when (29..40) then puts "Quite hot"
else
  puts "Not on my scale"
end

The following piece of code should always be banned (then keyword is not needed)

case temperature
when (-30..0) then
  puts "Freezing!"
when (1..10) then 
  puts "Cold"
when (11..20) then 
  puts "Not bad"
when (21..28) then 
  puts "Warm"
when (29..40) then 
  puts "Quite hot"
else
  puts "Not on my scale"
end 

And the following piece of code should always be allowed:

case temperature
when (-30..0)
  puts "Freezing!"
when (1..10) 
  puts "Cold"
when (11..20) 
  puts "Not bad"
when (21..28) 
  puts "Warm"
when (29..40) 
  puts "Quite hot"
else
  puts "Not on my scale"
end 

Do you agree with this?

[okuramasafumi]

@madejejej I agree!

[okuramasafumi]

I'm implementing this cop and thinking about the name of the cop.
I came up with RedundantThen cop but it might be too broad. RedundantWhenThen is more specific but sounds weird.
My basic idea is that in multiline case statement then is redundant.
@bbatsov What do you think?

[mikegee]

This code gets a complaint from Style/MultilineIfThen:

if cond then
  doit
end

So, maybe Style/MultilineWhenThen for consistency?

[okuramasafumi]

@mikegee Style/MultilineWhenThen sounds reasonable.
I hope we can merge two cops about then into one cop, such as RedundantThen.

[koic]

This feature has been implemented by #7114.