False positive Lint/SafeNavigationConsistency on .nil? check

[michniewicz]

Expected behavior

Lint/SafeNavigationConsistency should at least not autocorrect .nil? calls.

Actual behavior

Lint/SafeNavigationConsistency replaces .nil? call with safe navigation like &.nil?

Steps to reproduce the problem

say you have a sample code like this:

class Foo
  def bar
    return true if a.nil? || a&.whatever?
  end
end

you get a warning:

app/test.rb:5:20: W: Lint/SafeNavigationConsistency: Ensure that safe navigation is used consistently inside of && and ||. return true if a.nil? || a&.whatever?

if you run rubocop with autocorrect, the code is replaced with:

return true if a&.nil? || a&.whatever?

in that case the code is broken, as if a == nil, the code returns nil instead of true.

RuboCop version

$ rubocop -V
0.55.0 (using Parser 2.5.1.0, running on ruby 2.5.1 x86_64-linux)

[hashicco]

2018年4月30日(月) 21:13 Konstantin Mikhnievych <[email protected]>:

…

Expected behavior Lint/SafeNavigationConsistency should at least not autocorrect .nil? calls. Actual behavior Lint/SafeNavigationConsistency replaces .nil? call with safe navigation like &.nil? Steps to reproduce the problem say you have a sample code like this: class Foo def bar return true if a.nil? || a&.whatever? end end you get a warning: app/test.rb:5:20: W: Lint/SafeNavigationConsistency: Ensure that safe navigation is used consistently inside of && and ||. return true if a.nil? || a&.whatever? if you run rubocop with autocorrect, the code is replaced with: return true if a&.nil? || a&.whatever? in that case the code is broken, as if a == nil, the code returns nil instead of true. RuboCop version $ rubocop -V 0.55.0 (using Parser 2.5.1.0, running on ruby 2.5.1 x86_64-linux) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#5840>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHpqZrVuMVGEv_E3RBRu5oprzauSRgw5ks5ttv_xgaJpZM4TsfmL> .

[rrosenblum]

@michniewicz thanks for the bug report. This is a bug. Like the other safe navigation cops, I probably need to ignore methods that nil responds to entirely.