Skip to content

Commit

Permalink
[Fix rubocop#12271] Fix a false positive for `Lint/RedundantSafeNavig…
Browse files Browse the repository at this point in the history
…ation`

Fixes rubocop#12271.

This PR fixes a false positive for `Lint/RedundantSafeNavigation`
when using snake case constant receiver.

Camel case is used for naming classes and modules, while snake case is used for all other constants.
This naming conforms with `Naming/ConstantName` cop.

Since constants that turn into classes or modules are normally not `nil`, they will continue to be detected.
However, this PR will update to allow safe navigation for constants in snake case.

This change resolves both issue rubocop/rubocop-rails#1104 and rubocop#12271.
  • Loading branch information
koic committed Oct 13, 2023
1 parent 03cdf04 commit 2c1ee21
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12271](https://github.com/rubocop/rubocop/issues/12271): Fix a false positive for `Lint/RedundantSafeNavigation` when using snake case constant receiver. ([@koic][])
14 changes: 9 additions & 5 deletions lib/rubocop/cop/lint/redundant_safe_navigation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ module RuboCop
module Cop
module Lint
# Checks for redundant safe navigation calls.
# Use cases where a constant is `nil` are rare and an offense is detected
# when the receiver is a constant.
# Use cases where a constant, named in camel case for classes and modules is `nil` are rare,
# and an offense is not detected when the receiver is a snake case constant.
#
# For all receivers, the `instance_of?`, `kind_of?`, `is_a?`, `eql?`, `respond_to?`,
# and `equal?` methods are checked by default.
Expand All @@ -26,7 +26,7 @@ module Lint
#
# @example
# # bad
# Const&.do_something
# CamelCaseConst&.do_something
#
# # bad
# do_something if attrs&.respond_to?(:[])
Expand All @@ -40,7 +40,7 @@ module Lint
# end
#
# # good
# Const.do_something
# CamelCaseConst.do_something
#
# # good
# while node.is_a?(BeginNode)
Expand All @@ -67,20 +67,24 @@ class RedundantSafeNavigation < Base

NIL_SPECIFIC_METHODS = (nil.methods - Object.new.methods).to_set.freeze

SNAKE_CASE = /\A[[:digit:][:upper:]_]+\z/.freeze

# @!method respond_to_nil_specific_method?(node)
def_node_matcher :respond_to_nil_specific_method?, <<~PATTERN
(csend _ :respond_to? (sym %NIL_SPECIFIC_METHODS))
PATTERN

# rubocop:disable Metrics/AbcSize
def on_csend(node)
unless node.receiver.const_type?
unless node.receiver.const_type? && !node.receiver.source.match?(SNAKE_CASE)
return unless check?(node) && allowed_method?(node.method_name)
return if respond_to_nil_specific_method?(node)
end

range = range_between(node.loc.dot.begin_pos, node.source_range.end_pos)
add_offense(range) { |corrector| corrector.replace(node.loc.dot, '.') }
end
# rubocop:enable Metrics/AbcSize

private

Expand Down
21 changes: 17 additions & 4 deletions spec/rubocop/cop/lint/redundant_safe_navigation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,27 @@
RSpec.describe RuboCop::Cop::Lint::RedundantSafeNavigation, :config do
let(:cop_config) { { 'AllowedMethods' => %w[respond_to?] } }

it 'registers an offense and corrects when `&.` is used for const receiver' do
it 'registers an offense and corrects when `&.` is used for camel case const receiver' do
expect_offense(<<~RUBY)
Foo&.do_something
^^^^^^^^^^^^^^ Redundant safe navigation detected.
Const&.do_something
^^^^^^^^^^^^^^ Redundant safe navigation detected.
ConstName&.do_something
^^^^^^^^^^^^^^ Redundant safe navigation detected.
Const_name&.do_something # It is treated as camel case, similar to the `Naming/ConstantName` cop.
^^^^^^^^^^^^^^ Redundant safe navigation detected.
RUBY

expect_correction(<<~RUBY)
Foo.do_something
Const.do_something
ConstName.do_something
Const_name.do_something # It is treated as camel case, similar to the `Naming/ConstantName` cop.
RUBY
end

it 'does not register an offense and corrects when `&.` is used for snake case const receiver' do
expect_no_offenses(<<~RUBY)
CONST&.do_something
CONST_NAME&.do_something
RUBY
end

Expand Down

0 comments on commit 2c1ee21

Please sign in to comment.