From aa69d6947dd916155d279f144f9875131ab053af Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 26 Apr 2021 22:45:09 +0900 Subject: [PATCH] Fix a false negative for `Rails/SafeNavigation` This PR fixes a false negative for `Rails/SafeNavigation` when using `try!` without receiver. --- CHANGELOG.md | 1 + lib/rubocop/cop/rails/safe_navigation.rb | 9 +++++++-- spec/rubocop/cop/rails/safe_navigation_spec.rb | 6 ++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95e811542c..0f8cbb6067 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ * [#466](https://github.com/rubocop/rubocop-rails/pull/466): Fix a false positive for `Rails/DynamicFindBy` when not inheriting `ApplicationRecord` and without no receiver. ([@koic][]) * [#147](https://github.com/rubocop/rubocop-rails/issues/147): Fix a false positive for `Rails/HasManyOrHasOneDependent` when specifying default `dependent: nil` strategy. ([@koic][]) * [#137](https://github.com/rubocop/rubocop-rails/issues/137): Make `Rails/HasManyOrHasOneDependent` aware of `readonly?` is `true`. ([@koic][]) +* [#474](https://github.com/rubocop/rubocop-rails/pull/474): Fix a false negative for `Rails/SafeNavigation` when using `try!` without receiver. ([@koic][]) ### Changes diff --git a/lib/rubocop/cop/rails/safe_navigation.rb b/lib/rubocop/cop/rails/safe_navigation.rb index 50254a3ca2..1e82928c48 100644 --- a/lib/rubocop/cop/rails/safe_navigation.rb +++ b/lib/rubocop/cop/rails/safe_navigation.rb @@ -44,7 +44,7 @@ class SafeNavigation < Base RESTRICT_ON_SEND = %i[try try!].freeze def_node_matcher :try_call, <<~PATTERN - (send !nil? ${:try :try!} $_ ...) + (send _ ${:try :try!} $_ ...) PATTERN def on_send(node) @@ -64,7 +64,12 @@ def autocorrect(corrector, node) method_node, *params = *node.arguments method = method_node.source[1..-1] - range = range_between(node.loc.dot.begin_pos, node.loc.expression.end_pos) + range = if node.receiver + range_between(node.loc.dot.begin_pos, node.loc.expression.end_pos) + else + corrector.insert_before(node, 'self') + node + end corrector.replace(range, replacement(method, params)) end diff --git a/spec/rubocop/cop/rails/safe_navigation_spec.rb b/spec/rubocop/cop/rails/safe_navigation_spec.rb index 513eb35674..59b5be8f01 100644 --- a/spec/rubocop/cop/rails/safe_navigation_spec.rb +++ b/spec/rubocop/cop/rails/safe_navigation_spec.rb @@ -52,10 +52,6 @@ it_behaves_like 'accepts', 'try! with a method stored as a variable', ['bar = :==', 'foo.try!(baz, bar)'].join("\n") - - it 'accepts usages of try! without receiver' do - expect_no_offenses('try!(:something)') - end end context 'try' do @@ -118,6 +114,8 @@ '[1, 2].try!(:join, ",")', '[1, 2]&.join(",")' it_behaves_like 'autocorrect', 'try! with multiple parameters', '[1, 2].try!(:join, bar, baz)', '[1, 2]&.join(bar, baz)' + it_behaves_like 'autocorrect', 'try! without receiver', + 'try!(:join)', 'self&.join' it_behaves_like 'autocorrect', 'try! with a block', ['[foo, bar].try!(:map) do |e|', ' e.some_method',