Cop Lint/RedundantSafeNavigation with the autocorrect option fixes the code in a suboptimal way

[ydakuka]

Actual behavior

I have the code:

# frozen_string_literal: true

class HomeController < ApplicationController
  def options
    params[:key]&.to_h || {}
  end
end

I run rubocop with the autocorrect option:

ydakuka@yauhenid:~/Work/project$ brd rubocop -A app/controllers/home_controller.rb
Inspecting 1 file
W

Offenses:

app/controllers/home_controller.rb:5:31: W: [Corrected] Lint/RedundantSafeNavigation: Redundant safe navigation detected.
    params[:key]&.to_h || {}
                              ^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

I got the code:

# frozen_string_literal: true

class HomeController < ApplicationController
  def options
    params[:key].to_h || {}
  end
end

and I run rubocop again:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop app/controllers/home_controller.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

Expected behavior

I expected to receive the following code:

# frozen_string_literal: true

class HomeController < ApplicationController
  def options
    params[:key].to_h
  end
end

RuboCop version

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop -V
1.52.0 (using Parser 3.2.2.1, rubocop-ast 1.28.1, running on ruby 2.7.8) [x86_64-linux]
  - rubocop-capybara 2.18.0
  - rubocop-factory_bot 2.23.1
  - rubocop-performance 1.17.1
  - rubocop-rails 2.19.1
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.22.0
  - rubocop-thread_safety 0.5.1

[koic]

This is an unexpected false positive by Lint/RedundantSafeNavigation. #11915 prevents to false detected. OTOH, It might be possible to handle such detection apart from Lint/RedundantSafeNavigation.

[alex-damian-negru]

There's also param[:key]&.to_i || 42 which is flagged as an offense, despite the safe navigation not being redundant in such cases.

[mingan]

I'm seeing v&.to_f || 0 and super&.to_i || start_date&.year marked (and auto-corrected) after 1.51.0 -> 1.52.0 update despite the safe-navigation being meaningful here.

[rnestler]

There's also param[:key]&.to_i || 42 which is flagged as an offense, despite the safe navigation not being redundant in such cases.

I also hit this with version = request.headers['Api-Version']&.to_i || 1 which would actually break stuff, since it would suddenly return 0 if Api-Version is not set.

[edsu]

I'm seeing this as well with code that previously could return nil or an integer, but now returns 0 instead of nil: presence&.to_i.

[ngan]

There's also param[:key]&.to_i || 42 which is flagged as an offense, despite the safe navigation not being redundant in such cases.

This is also the case for to_s:

foo = something&.to_s || "hello"
# DOES NOT EQUAL...
foo = something.to_s || "hello"

[ngan]

This cop also seems to flag this incorrectly:

foo&.public_send(:bar)

[edsu]

I think this can be closed since #8867 has been resolved?

[bquorning]

I think this can be closed since #8867 has been resolved?

I am confused. #8867 was closed over 2 years ago.

[edsu]

Sorry @bquorning I meant #11915