Rubocop: Auto-fix for Style/SafeNavigation chaind with [] operator breaks code

[t-kinoshita]

Auto-fix makes working code to code with error. (After auto-fix, it raises NoMethodError when name is nil)

# before rubocop -a
def some_method
  # Alerts as Style/SafeNavigation
  name && name.split(' ')[0]
end

# after rubocop -a
def some_method
  # Alerts as Lint/SafeNavigationChain
  name&.split(' ')[0]
end

---

Expected behavior

def some_method
  # or, something clearer
  name&.split(' ')&.[](0)
end

Actual behavior

Shown above.

RuboCop version

Include the output of rubocop -V. Here's an example:

$ rubocop -V
0.50.0 (using Parser 2.4.0.0, running on ruby 2.3.4 x86_64-linux)

[ryym]

This also happens with other sign operators such as <.
For example, a && a.b < 3 is corrected to a&.b < 3 and the corrected one raises NoMethodError when a is nil.

[duboff]

Also noticed it does not account for when nil could actually have the opposite effect of false:

# before rubocop -a
def some_method
  # Alerts as Style/SafeNavigation
  raise unless current_user && current_user.id == params[:id]
end

# after rubocop -a
def some_method
  raise unless current_user&.id == params[:id]
end

The first one would not raise if current_user is nil and params[:id] is nil but the latter one presumably would?

[Drenmi]

Fixed in #4749.

[belousovAV]

@Drenmi Sorry. But I think, that this PR don't try fix this bug at all.

[belousovAV]

Now we expect multiple chained autofix only first call
But it raises NoMethodError in this issue and it comments. If we replace all calls with safe navigation - we add more logic that was.
So we can not autofix for multiple chained at all.

[Drenmi]

Hm. This looks like it might be deeper than the previous fix. Reopening.

[rrosenblum]

The code name && name.split(' ')[0] no longer produces an offense. This was fixed via #5419.

[blakewest]

I'm still seeing this issue. See example code below that still produces an offense, but should not, on the latest master (0.52.1).

def unlimited?
  coupon && coupon.duration_in_months.nil?
end

The autofix corrects it to coupon&.duration_in_months.nil?. But this changes the semantics.
If the coupon is nil, then the original code returns nil. But when switched to use coupon&.duration_in_months.nil?, it would return true.

Despite the fact that nil? is implemented on nil, that does not mean it's safe to only use one &.. It seemed like this was addressed in https://github.com/bbatsov/rubocop/pull/5419 , but it seems like it still is there. Anyone else confirm this?

[rrosenblum]

The fix is in master, not 0.52.1. Can you try testing against master?

[Darhazer]

Can't reproduce in 0.54