From 6c93e2c5d00693387fa56389dfdf15d1bfd2d1e9 Mon Sep 17 00:00:00 2001
From: Marc-Andre Lafortune <github@marc-andre.ca>
Date: Thu, 17 Sep 2020 12:21:38 -0400
Subject: [PATCH] `Metrics/AbcSize`: Fix assignment counts for various cases.

`var += ...` was counted twice
`obj.method += ...` was not counted
`Const ||= ...` was crashing [#8742]
---
 CHANGELOG.md                                  |  1 +
 .../cop/metrics/utils/abc_size_calculator.rb  | 35 ++++++++----
 .../metrics/utils/abc_size_calculator_spec.rb | 53 +++++++++++++++++++
 3 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1a72ed8fc2a..747554606d9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
 * [#8730](https://github.com/rubocop-hq/rubocop/issues/8730): Fix an error for `Lint/UselessTimes` when there is a blank line in the method definition. ([@koic][])
 * [#8740](https://github.com/rubocop-hq/rubocop/issues/8740): Fix a false positive for `Style/HashAsLastArrayItem` when the hash is in an implicit array. ([@dvandersluis][])
 * [#8739](https://github.com/rubocop-hq/rubocop/issues/8739): Fix an error for `Lint/UselessTimes` when using empty block argument. ([@koic][])
+* [#8742](https://github.com/rubocop-hq/rubocop/issues/8742): Fix some assignment counts for `Metrics/AbcSize`. ([@marcandre][])
 
 ### Changes
 
diff --git a/lib/rubocop/cop/metrics/utils/abc_size_calculator.rb b/lib/rubocop/cop/metrics/utils/abc_size_calculator.rb
index 3f409647a5d..79ed3c8eceb 100644
--- a/lib/rubocop/cop/metrics/utils/abc_size_calculator.rb
+++ b/lib/rubocop/cop/metrics/utils/abc_size_calculator.rb
@@ -81,21 +81,37 @@ def else_branch?(node)
           private
 
           def assignment?(node)
+            return compound_assignment(node) if node.masgn_type? || node.shorthand_asgn?
+
             node.for_type? ||
-              node.op_asgn_type? ||
               (node.respond_to?(:setter_method?) && node.setter_method?) ||
-              (simple_assignment?(node) && capturing_variable?(node.children.first))
+              simple_assignment?(node) ||
+              argument?(node)
           end
 
-          def simple_assignment?(node)
-            return false if node.masgn_type?
+          def compound_assignment(node)
+            # Methods setter can not be detected for multiple assignments
+            # and shorthand assigns, so we'll count them here instead
+            children = node.masgn_type? ? node.children[0].children : node.children
 
-            if node.equals_asgn?
-              reset_on_lvasgn(node) if node.lvasgn_type?
-              return true
+            will_be_miscounted = children.count do |child|
+              child.respond_to?(:setter_method?) &&
+                !child.setter_method?
             end
+            @assignment += will_be_miscounted
+
+            false
+          end
 
-            argument?(node)
+          def simple_assignment?(node)
+            if !node.equals_asgn?
+              false
+            elsif node.lvasgn_type?
+              reset_on_lvasgn(node)
+              capturing_variable?(node.children.first)
+            else
+              true
+            end
           end
 
           def capturing_variable?(name)
@@ -106,9 +122,8 @@ def branch?(node)
             BRANCH_NODES.include?(node.type)
           end
 
-          # TODO: move to rubocop-ast
           def argument?(node)
-            ARGUMENT_TYPES.include?(node.type)
+            ARGUMENT_TYPES.include?(node.type) && capturing_variable?(node.children.first)
           end
 
           def condition?(node)
diff --git a/spec/rubocop/cop/metrics/utils/abc_size_calculator_spec.rb b/spec/rubocop/cop/metrics/utils/abc_size_calculator_spec.rb
index 788eb2e50ba..b69453f7d13 100644
--- a/spec/rubocop/cop/metrics/utils/abc_size_calculator_spec.rb
+++ b/spec/rubocop/cop/metrics/utils/abc_size_calculator_spec.rb
@@ -16,6 +16,27 @@ def method_name
       it { is_expected.to eq '<0, 3, 0>' }
     end
 
+    context 'with +=' do
+      let(:source) { <<~RUBY }
+        def method_name
+          x = nil
+          x += 1
+        end
+      RUBY
+
+      it { is_expected.to eq '<2, 0, 0>' }
+    end
+
+    context 'with += for setters' do
+      let(:source) { <<~RUBY }
+        def method_name
+          foo.bar += 1
+        end
+      RUBY
+
+      it { is_expected.to eq '<1, 2, 0>' }
+    end
+
     context 'with ||=' do
       let(:source) { <<~RUBY }
         def method_name
@@ -27,6 +48,16 @@ def method_name
       it { is_expected.to eq '<2, 0, 1>' }
     end
 
+    context 'with ||= on a constant' do
+      let(:source) { <<~RUBY }
+        def method_name
+          self::FooModule ||= Mod
+        end
+      RUBY
+
+      it { is_expected.to eq '<1, 0, 1>' }
+    end
+
     context 'with &&=' do
       let(:source) { <<~RUBY }
         def method_name
@@ -142,6 +173,28 @@ def method_name
       it { is_expected.to eq '<3, 1, 0>' }
     end
 
+    context 'multiple assignment with method setters' do
+      let(:source) { <<~RUBY }
+        def method_name
+          self.a, foo.b, bar[42] = nil
+        end
+      RUBY
+
+      it { is_expected.to eq '<3, 5, 0>' }
+    end
+
+    context 'equivalent to multiple assignment with method setters' do
+      let(:source) { <<~RUBY }
+        def method_name
+          self.a = nil    # 1,  1, 0
+          foo.b = nil     # 1, +2, 0
+          bar[42] = nil   # 1, +2, 0
+        end
+      RUBY
+
+      it { is_expected.to eq '<3, 5, 0>' }
+    end
+
     context 'if and arithmetic operations' do
       let(:source) { <<~RUBY }
         def method_name