From fe59472934085503e165bf6b3a23f882680bf392 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 20 Jul 2019 11:03:55 +0900 Subject: [PATCH] [Fix #53] Fix a false positive for `Rails/SaveBang` Fixes #53. This PR fixes a false positive for `Rails/SaveBang` when implicitly return using finder method and creation method connected by `||`. And this PR updates .rubocop_todo.yml with `rubocop --auto_gen_config` to prevent the following offense. ```console % rubocop lib/rubocop/cop/rails/save_bang.rb Inspecting 1 file C Offenses: lib/rubocop/cop/rails/save_bang.rb:100:7: C: Metrics/ClassLength: Class has too many lines. [164/159] class SaveBang < Cop ... ^^^^^^^^^^^^^^^^^^^^ 1 file inspected, 1 offense detected ``` --- .rubocop_todo.yml | 4 ++-- CHANGELOG.md | 4 ++++ lib/rubocop/cop/rails/save_bang.rb | 14 +++++++++++--- spec/rubocop/cop/rails/save_bang_spec.rb | 8 ++++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 688f19617d..101135facc 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-07-20 09:04:51 +0900 using RuboCop version 0.72.0. +# on 2019-07-20 17:51:45 +0900 using RuboCop version 0.72.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -21,7 +21,7 @@ Metrics/AbcSize: # Offense count: 4 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 159 + Max: 164 # Offense count: 23 # Configuration parameters: CountComments, ExcludedMethods. diff --git a/CHANGELOG.md b/CHANGELOG.md index 621c02fc95..117e44d192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### Bug fixes + +* [#53](https://github.com/rubocop-hq/rubocop-rails/issues/53): Fix a false positive for `Rails/SaveBang` when implicitly return using finder method and creation method connected by `||`. ([@koic][]) + ## 2.2.1 (2019-07-13) ### Bug fixes diff --git a/lib/rubocop/cop/rails/save_bang.rb b/lib/rubocop/cop/rails/save_bang.rb index 64927a2113..38a1e4140b 100644 --- a/lib/rubocop/cop/rails/save_bang.rb +++ b/lib/rubocop/cop/rails/save_bang.rb @@ -140,9 +140,9 @@ def check_assignment(assignment) def on_send(node) # rubocop:disable Metrics/CyclomaticComplexity return unless persist_method?(node) return if return_value_assigned?(node) + return if implicit_return?(node) return if check_used_in_conditional(node) return if argument?(node) - return if implicit_return?(node) return if explicit_return?(node) add_offense_for_node(node) @@ -277,10 +277,18 @@ def implicit_return?(node) return false unless cop_config['AllowImplicitReturn'] node = assignable_node(node) - method = node.parent + method, sibling_index = find_method_with_sibling_index(node.parent) return unless method && (method.def_type? || method.block_type?) - method.children.size == node.sibling_index + 1 + method.children.size == node.sibling_index + sibling_index + end + + def find_method_with_sibling_index(node, sibling_index = 1) + return node, sibling_index unless node&.or_type? + + sibling_index += 1 + + find_method_with_sibling_index(node.parent, sibling_index) end def argument?(node) diff --git a/spec/rubocop/cop/rails/save_bang_spec.rb b/spec/rubocop/cop/rails/save_bang_spec.rb index e92c210fc8..ccb92cc4dd 100644 --- a/spec/rubocop/cop/rails/save_bang_spec.rb +++ b/spec/rubocop/cop/rails/save_bang_spec.rb @@ -548,6 +548,14 @@ def whatever if x.persisted?; something; end RUBY end + + it "when using #{method} with `||`" do + expect_no_offenses(<<~RUBY) + def find_or_create(**opts) + find(**opts) || #{method}(**opts) + end + RUBY + end end described_class::CREATE_PERSIST_METHODS.each do |method|