Rubocop: Inside if/else/end, rubocop complains about missing guard clause instead of extraneous return

[gregnavis]

Rubocop behaves oddly on:

def positive?(number)
  if number > 0
    return true
  else
    false
  end
end

Expected behavior

Rubocop complains about unnecessary return statement.

Actual behavior

Rubocop complains about missing guard clause.

Steps to reproduce the problem

Save the ruby snippet above in test.rb and run rubocop. You'll see:

$ rubocop test.rb 
Inspecting 1 file
C

Offenses:

test.rb:2:3: C: Use a guard clause instead of wrapping the code inside a conditional expression.
  if number > 0
  ^^

1 file inspected, 1 offense detected

RuboCop version

Include the output of rubocop -V:

$ bundle exec rubocop -V
0.37.2 (using Parser 2.3.0.6, running on ruby 2.3.0 x86_64-linux)

[segiddins]

RuboCop wants:

def positive?(number)
  return true if number > 0
  false
end

[alexdowad]

This can be closed.

[gregnavis]

@segiddins That's exactly the problem. Please notice that rubocop is also fine after I remove the return statement.

What I'm saying is that I find it odd that

def positive?(number)
  return true if number > 0
  false
end

over

def positive?(number)
  if number > 0
   true
  else
    false
  end
end

[alexdowad]

Why don't you just write:

def positive?(number)
  number > 0
end

[gregnavis]

Numbers, comparison, true and false were just an example. Sorry for not making this clear.

In general, I think this:

def some_method(argument)
  if condition?(argument)
    return operation1(argument)
  else
    operation2(argument)
  end
end

should be turned into

def some_method(argument)
  if condition?(argument)
    operation1(argument)
  else
    operation2(argument)
  end
end

instead of

def some_method(argument)
  return operation1(argument) if condition?(argument)
  operation2(argument)
end

[lumeet]

I think @gregnavis's point is valid. Using the default configuration, both of the above mentioned alternatives are equally good. It's just that Style/RedundantReturn doesn't currently look into if-branches. If it did, we'd have two offenses. I don't know which one of them would have precedence when auto-correcting, though.

[gregnavis]

Any thoughts? If you think that

def some_method(argument)
  if condition?(argument)
    operation1(argument)
  else
    operation2(argument)
  end
end

should be preferred then I might be able to spend time on contributing a patch.

[mikegee]

If you think that [if/else/end] should be preferred then I might be able to spend time on contributing a patch.

No, disable the Style/GuardClause cop in your Rubocop config.

[gregnavis]

@mikegee, I'm sorry for not being clear. My point was _being able to continue using Style/GuardClause as it's useful in many cases_. It simply isn't too smart when it sees:

def some_method(argument)
  if condition?(argument)
    return operation1(argument)
  else
    operation2(argument)
  end
end