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 #6915] Fix false positive in Style/SafeNavigation with a modifier condition safe guarding a logic jump keyword #6959

Merged
merged 1 commit into from Apr 24, 2019
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 @@ -22,6 +22,7 @@
* [#6860](https://github.com/rubocop-hq/rubocop/issues/6860): Prevent auto-correct conflict of `Style/InverseMethods` and `Style/Not`. ([@hoshinotsuyoshi][])
* [#6935](https://github.com/rubocop-hq/rubocop/issues/6935): `Layout/AccessModifierIndentation` should ignore access modifiers that apply to specific methods. ([@deivid-rodriguez][])
* [#6956](https://github.com/rubocop-hq/rubocop/issues/6956): Prevent auto-correct confliction of `Lint/Lambda` and `Lint/UnusedBlockArgument`. ([@koic][])
* [#6915](https://github.com/rubocop-hq/rubocop/issues/6915): Fix false positive in `Style/SafeNavigation` when a modifier if is safe guarding a method call being passed to `break`, `fail`, `next`, `raise`, `return`, `throw`, and `yield`. ([@rrosenblum][])

### Changes

Expand Down
3 changes: 3 additions & 0 deletions lib/rubocop/cop/style/safe_navigation.rb
Expand Up @@ -65,6 +65,8 @@ class SafeNavigation < Cop

MSG = 'Use safe navigation (`&.`) instead of checking if an object ' \
'exists before calling the method.'.freeze
LOGIC_JUMP_KEYWORDS = %i[break fail next raise
return throw yield].freeze

minimum_target_ruby_version 2.3

Expand Down Expand Up @@ -149,6 +151,7 @@ def extract_parts_from_if(node)

checked_variable, matching_receiver, method =
extract_common_parts(receiver, variable)
matching_receiver = nil if LOGIC_JUMP_KEYWORDS.include?(receiver.type)
[checked_variable, matching_receiver, receiver, method]
end

Expand Down
19 changes: 19 additions & 0 deletions spec/rubocop/cop/style/safe_navigation_spec.rb
Expand Up @@ -151,6 +151,25 @@

shared_examples 'all variable types' do |variable|
context 'modifier if' do
shared_examples 'safe guarding logical break keywords' do |keyword|
it "allows a method call being passed to #{keyword} safe guarded " \
'by an object check' do
expect_no_offenses(<<-RUBY.strip_indent)
something.each do
#{keyword} #{variable}.bar if #{variable}
end
RUBY
end
end

it_behaves_like 'safe guarding logical break keywords', 'break'
it_behaves_like 'safe guarding logical break keywords', 'fail'
it_behaves_like 'safe guarding logical break keywords', 'next'
it_behaves_like 'safe guarding logical break keywords', 'raise'
it_behaves_like 'safe guarding logical break keywords', 'return'
it_behaves_like 'safe guarding logical break keywords', 'throw'
it_behaves_like 'safe guarding logical break keywords', 'yield'

it 'registers an offense for a method call that nil responds to ' \
'safe guarded by an object check' do
inspect_source("#{variable}.to_i if #{variable}")
Expand Down