Rubocop: Rubocop undecided between Style/GuardClause and Style/MultilineIfModifier

Created on 21 Sep 2017  路  4Comments  路  Source: rubocop-hq/rubocop

Expected behavior

Rubocop be okay with the code below

def foo                                                                          
  if @f1.nil? && @f2.nil? && @f3.nil?                                            
    raise ['Some long error message that for some reason we are not succint',    
           'that spans multiple lines'].join(' ')                                
  end                                                                            
end

Actual behavior

Instead Rubocop fails with offense as I should use a guard clause. However the code neither fits in 80-chars nor is a single liner.

Note that the cop does not trigger outside a function, so if you put the if "naked" in the file it does not trigger.

If I follow its advise then the other cop kicks in (correctly).

Steps to reproduce the problem

Put the snippet above on a file and run.

RuboCop version

$ rubocop -V
0.48.1 (using Parser 2.4.0.0, running on ruby 2.3.3 x86_64-linux)
bug stale

All 4 comments

Hm. I agree Style/GuardClause should probably ignore this since the raise is multiline.

Another option for you in this particular case is to use a Heredoc, which is a good choice for multiline strings, as it's easier to read and change than concatenating arrays of string fragments:

raise <<~ERROR if foo?
  Some long error message
  that spans multiple lines.
ERROR

@nelsonjr Actually, what the Style/GuardClause cop wants is this:

def foo
  return unless @f1.nil? && @f2.nil? && @f3.nil?

  raise ['Some long error message that for some reason we are not succint',
         'that spans multiple lines'].join(' ')
end

or if we disregard what Style/IfUnlessModifier has to say, this is fine too:

def foo
  unless @f1.nil? && @f2.nil? && @f3.nil?
    return
  end
  raise ['Some long error message that for some reason we are not succint',
         'that spans multiple lines'].join(' ')
end

It's all a bit confusing. It's not really about multi-line. By guard clause we mean that we end a method early under some condition. But since raise ends the method too, it's easy to misunderstand the offense, and maybe the cop should not report an offense when the body is a raise.

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!

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

Was this page helpful?
0 / 5 - 0 ratings