Rubocop, safe navigation, and negation

1.3k Views Asked by At

I've got constructs like the following that Rubocop complains about that I'm trying to appease

unless foo && !foo.empty? ...

(item exists and is not empty)

and

bar && !bar.positive?

(item exists and is not positive)

In both cases, Rubocop throws a Style/SafeNavigation and says I should use &. instead. However, there does not seem to be a way to write it.

In both cases part of the problem is that there is no concise way of expressing the opposite function - there is no "not empty" for strings, and no "negative or zero" for numbers. Writing bar&.negative? returns the opposite result for zero, for example.

In the other case, writing unless !foo&.empty? has Rubocop complain about unless+a negation, but again there's no way to rewrite it as an if foo&. with any operation that exists.

(btw this is straight Ruby, no Rails, so I don't have blank? and present?)

2

There are 2 best solutions below

0
On

This is an oversight in RuboCop, because the suggested correction can not be guaranteed to have the same semantics. It will be fixed in the upcoming 0.50.1 patch, and the cop will ignore cases with negation.

In the meanwhile, if you're dealing with conditions where your variable could be nil, one option is to use the fact that nil responds to the coercion methods. Example, assuming foo is either an array or nil:

# With nil check
unless foo && !foo.empty?

# With array coercion
if foo.to_a.empty?
0
On

This:

next unless foo && !foo.empty?

Could be replaced with:

next if foo.nil? || foo.empty?

And similarly, this:

next unless bar && !bar.positive?

Could be replaced with:

next if bar.nil? || bar.positive?

The real code smell here, in my opinion, was the next unless ... not. The double negative indicates that there's probably a cleaner way to write it.