Merge pull request #90 from koic/fix_a_false_positive_for_rails_save_…

…bang

[Fix #53] Fix a false positive for `Rails/SaveBang`

.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.

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

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)

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|