Skip to content

Commit

Permalink
Merge pull request #8654 from koic/fix_false_positive_for_safe_naviga…
Browse files Browse the repository at this point in the history
…tion

Fix a false positive for `Style/SafeNavigation`
  • Loading branch information
koic committed Sep 6, 2020
2 parents dfdf8f9 + 35ebbf9 commit 3e304ba
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@
* [#8572](https://github.com/rubocop-hq/rubocop/issues/8572): Fix a false positive for `Style/RedundantParentheses` when parentheses are used like method argument parentheses. ([@koic][])
* [#8653](https://github.com/rubocop-hq/rubocop/pull/8653): Fix a false positive for `Layout/DefEndAlignment` when using refinements and `private def`. ([@koic][])
* [#8655](https://github.com/rubocop-hq/rubocop/pull/8655): Fix a false positive for `Style/ClassAndModuleChildren` when using cbase class name. ([@koic][])
* [#8654](https://github.com/rubocop-hq/rubocop/pull/8654): Fix a false positive for `Style/SafeNavigation` when checking `foo&.empty?` in a conditional. ([@koic][])

### Changes

Expand Down
4 changes: 4 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -8653,6 +8653,10 @@ foo && foo.nil? # method that `nil` responds to
foo && foo < bar
foo < bar if foo
# When checking `foo&.empty?` in a conditional, `foo` being `nil` will actually
# do the opposite of what the author intends.
foo && foo.empty?
# This could start returning `nil` as well as the return of the method
foo.nil? || foo.bar
!foo || foo.bar
Expand Down
5 changes: 5 additions & 0 deletions lib/rubocop/cop/style/safe_navigation.rb
Expand Up @@ -49,6 +49,10 @@ module Style
# foo && foo < bar
# foo < bar if foo
#
# # When checking `foo&.empty?` in a conditional, `foo` being `nil` will actually
# # do the opposite of what the author intends.
# foo && foo.empty?
#
# # This could start returning `nil` as well as the return of the method
# foo.nil? || foo.bar
# !foo || foo.bar
Expand Down Expand Up @@ -104,6 +108,7 @@ def check_node(node)
# chain greater than 2
return if chain_size(method_chain, method) > 1
return if unsafe_method_used?(method_chain, method)
return if method_chain.method?(:empty?)

add_offense(node) do |corrector|
autocorrect(corrector, node)
Expand Down
18 changes: 18 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Expand Up @@ -1581,6 +1581,24 @@ def self.some_method(foo, bar: 1)
RUBY
end

it 'does not crash Lint/SafeNavigationWithEmpty and offenses and accepts Style/SafeNavigation ' \
'when checking `foo&.empty?` in a conditional' do
create_file('example.rb', <<~RUBY)
do_something if ENV['VERSION'] && ENV['VERSION'].empty?
RUBY
expect(
cli.run(
[
'--auto-correct',
'--only', 'Lint/SafeNavigationWithEmpty,Style/SafeNavigation'
]
)
).to eq(0)
expect(IO.read('example.rb')).to eq(<<~RUBY)
do_something if ENV['VERSION'] && ENV['VERSION'].empty?
RUBY
end

it 'corrects TrailingCommaIn(Array|Hash)Literal and ' \
'Multiline(Array|Hash)BraceLayout offenses' do
create_file('.rubocop.yml', <<~YAML)
Expand Down
4 changes: 4 additions & 0 deletions spec/rubocop/cop/style/safe_navigation_spec.rb
Expand Up @@ -90,6 +90,10 @@
expect_no_offenses('foo && foo.bar !~ /baz/')
end

it 'allows an object check before a method call that is used with `empty?`' do
expect_no_offenses('foo && foo.empty?')
end

it 'allows an object check before a method call that is used in ' \
'a spaceship comparison' do
expect_no_offenses('foo && foo.bar <=> baz')
Expand Down

0 comments on commit 3e304ba

Please sign in to comment.