Style/EmptyElse probably doesn't distinguish between actual nil and empty block

[zenspider]

      @diff = if (RbConfig::CONFIG["host_os"] =~ /mswin|mingw/ &&
                  system("diff.exe", __FILE__, __FILE__)) then
                "diff.exe -u"
              elsif Minitest::Test.maglev? then
                "diff -u"
              elsif system("gdiff", __FILE__, __FILE__)
                "gdiff -u" # solaris and kin suck
              elsif system("diff", __FILE__, __FILE__)
                "diff -u"
              else
                nil
              end unless defined? @diff

[Koronen]

That's correct. This is intentional, see #1444. On the other hand, I can understand if some prefer to be explicit about returning nil in the else case. Maybe this behavior should be configurable?

[bbatsov]

I'm fine with having it be configurable (the other setting would complain
only for truly empty else clauses).

On 27 January 2015 at 22:09, Victor Koronen [email protected]
wrote:

That's correct. This is intentional, see #1444
#1444. On the other hand, I can
understand if some prefer to be explicit about returning nil in the else
case. Maybe this behavior should be configurable?

—
Reply to this email directly or view it on GitHub
#1611 (comment).

[edvardm]

Totally agree with explicit nil being better. That communicates to the reader that else clause was not just forgotten. Returning implicit nil when falling through case statement is hard to understand being intentional.

Other than that, great tool! I especially love all the auto-correct stuff.

[bbatsov]

Btw, there's one more case to consider - a missing else. In such scenarios there's an implicit else returning nil, but some people might prefer to make all such cases explicit...

[edvardm]

That's precisely what I meant. My convention is to even add comment

...
else
   nil # no-op
end

in such cases, to indicate that this not just something that is unhandled, but returning nil in purpose. There are probably better solutions, though.

[bbatsov]

So, it seems to me this cop should support 3 styles:

• warn on implicit nil (both empty else and missing else)
• warn on explicit/redundant nil/else
• warn on on empty else

[edvardm]

+1, should satisfy both the followers of "be explicit" and "meh I know what I'm doing" schools :)