Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a false positive for Style/SafeNavigation #8654

Merged
merged 1 commit into from Sep 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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