Fix Metrics/CyclomaticComplexity for ||=, &&=, &. and iterating blocks

CHANGELOG.md

@@ -23,6 +23,10 @@
 
 * [#8146](https://github.com/rubocop-hq/rubocop/pull/8146): Use UTC in RuboCop todo file generation. ([@mauro-oto][])
 
+### Changes
+
+* [#8149](https://github.com/rubocop-hq/rubocop/pull/8149): Cop "Metrics/CyclotomicComplexicy" now counts `&.`, `||=`, `&&=` and blocks known to iterate. Default bumped from 6 to 7. ([@marcandre][])
+
 ## 0.85.1 (2020-06-07)
 
 ### Bug fixes

config/default.yml

@@ -1925,7 +1925,7 @@ Metrics/CyclomaticComplexity:
   VersionAdded: '0.25'
   VersionChanged: '0.81'
   IgnoredMethods: []
-  Max: 6
+  Max: 7
 
 Metrics/MethodLength:
   Description: 'Avoid methods longer than 10 lines of code.'

docs/modules/ROOT/pages/cops_metrics.adoc

@@ -166,6 +166,22 @@ else branch does not, since it doesn't add a decision point. The &&
 operator (or keyword and) can be converted to a nested if statement,
 and ||/or is shorthand for a sequence of ifs, so they also add one.
 Loops can be said to have an exit condition, so they add one.
+Blocks that are calls to builtin iteration methods
+(e.g. `ary.map{...}) also add one, otherse are ignored.
+
+  def each_child_node(*types)               # count begins: 1
+    unless block_given?                     # unless: +1
+      return to_enum(__method__, *types)
+
+    children.each do |child|                # each{}: +1
+      next unless child.is_a?(Node)         # unless: +1
+
+      yield child if types.empty? ||        # if: +1, ||: +1
+                     types.include?(child.type)
+    end
+
+    self
+  end                                       # total: 6
 
 === Configurable attributes
 
@@ -177,7 +193,7 @@ Loops can be said to have an exit condition, so they add one.
 | Array
 
 | Max
-| `6`
+| `7`
 | Integer
 |===
 

lib/rubocop.rb

@@ -321,6 +321,7 @@
 require_relative 'rubocop/cop/lint/useless_setter_call'
 require_relative 'rubocop/cop/lint/void'
 
+require_relative 'rubocop/cop/metrics/utils/iterating_block'
 require_relative 'rubocop/cop/metrics/cyclomatic_complexity'
 # relies on cyclomatic_complexity
 require_relative 'rubocop/cop/metrics/utils/abc_size_calculator'

lib/rubocop/cop/layout/hash_alignment.rb

@@ -200,7 +200,7 @@ def on_send(node)
         alias on_super on_send
         alias on_yield on_send
 
-        def on_hash(node)
+        def on_hash(node) # rubocop:todo Metrics/CyclomaticComplexity
           return if ignored_node?(node)
           return if node.pairs.empty? || node.single_line?
 

lib/rubocop/cop/lint/redundant_cop_disable_directive.rb

@@ -135,6 +135,7 @@ def each_already_disabled(line_ranges, disabled_ranges, comments)
           end
         end
 
+        # rubocop:todo Metrics/CyclomaticComplexity
         def find_redundant(comment, offenses, cop, line_range, next_line_range)
           if all_disabled?(comment)
             # If there's a disable all comment followed by a comment
@@ -152,6 +153,7 @@ def find_redundant(comment, offenses, cop, line_range, next_line_range)
             cop if cop_offenses.none? { |o| line_range.cover?(o.line) }
           end
         end
+        # rubocop:enable Metrics/CyclomaticComplexity
 
         def all_disabled?(comment)
           /rubocop\s*:\s*(?:disable|todo)\s+all\b/.match?(comment.text)

lib/rubocop/cop/metrics/cyclomatic_complexity.rb

@@ -13,19 +13,51 @@ module Metrics
       # operator (or keyword and) can be converted to a nested if statement,
       # and ||/or is shorthand for a sequence of ifs, so they also add one.
       # Loops can be said to have an exit condition, so they add one.
+      # Blocks that are calls to builtin iteration methods
+      # (e.g. `ary.map{...}) also add one, otherse are ignored.
+      #
+      #   def each_child_node(*types)               # count begins: 1
+      #     unless block_given?                     # unless: +1
+      #       return to_enum(__method__, *types)
+      #
+      #     children.each do |child|                # each{}: +1
+      #       next unless child.is_a?(Node)         # unless: +1
+      #
+      #       yield child if types.empty? ||        # if: +1, ||: +1
+      #                      types.include?(child.type)
+      #     end
+      #
+      #     self
+      #   end                                       # total: 6
       class CyclomaticComplexity < Cop
         include MethodComplexity
+        include Utils::IteratingBlock
 
         MSG = 'Cyclomatic complexity for %<method>s is too high. ' \
               '[%<complexity>d/%<max>d]'
-        COUNTED_NODES = %i[if while until for
-                           rescue when and or].freeze
+        COUNTED_NODES = %i[if while until for csend block block_pass
+                           rescue when and or or_asgn and_asgn].freeze
 
         private
 
-        def complexity_score_for(_node)
+        def complexity_score_for(node)
+          return 0 if iterating_block?(node) == false
+
           1
         end
+
+        def block_method(node)
+          case node.type
+          when :block
+            node.method_name
+          when :block_pass
+            node.parent.method_name
+          end
+        end
+
+        def count_block?(block)
+          KNOWN_ITERATING_METHODS.include? block.method_name
+        end
       end
     end
   end

lib/rubocop/cop/metrics/utils/abc_size_calculator.rb

@@ -19,7 +19,7 @@ class AbcSizeCalculator
           # > Condition -- a logical/Boolean test, == != <= >= < > else case
           # > default try catch ? and unary conditionals.
           # > http://c2.com/cgi/wiki?AbcMetric
-          CONDITION_NODES = CyclomaticComplexity::COUNTED_NODES.freeze
+          CONDITION_NODES = (CyclomaticComplexity::COUNTED_NODES - %i[block block_pass]).freeze
 
           def self.calculate(node)
             new(node).calculate

lib/rubocop/cop/metrics/utils/iterating_block.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Metrics
+      module Utils
+        # Used to identify iterating blocks like `.map{}` and `.map(&:...)`
+        module IteratingBlock
+          enumerable = %i[
+            all? any? chain chunk chunk_while collect collect_concat count cycle
+            detect drop drop_while each each_cons each_entry each_slice
+            each_with_index each_with_object entries filter filter_map find
+            find_all find_index flat_map grep grep_v group_by inject lazy map
+            max max_by min min_by minmax minmax_by none? one? partition reduce
+            reject reverse_each select slice_after slice_before slice_when sort
+            sort_by sum take take_while tally to_h uniq zip
+          ]
+
+          enumerator = %i[with_index with_object]
+
+          array = %i[
+            bsearch bsearch_index collect! combination d_permutation delete_if
+            each_index keep_if map! permutation product reject! repeat
+            repeated_combination select! sort sort! sort_by sort_by
+          ]
+
+          hash = %i[
+            each_key each_pair each_value fetch fetch_values has_key? merge
+            merge! transform_keys transform_keys! transform_values
+            transform_values!
+          ]
+
+          KNOWN_ITERATING_METHODS = (Set.new(enumerable) + enumerator + array + hash).freeze
+
+          # Returns the name of the method called with a block
+          # if node is a block node, or a block-pass node.
+          def block_method_name(node)
+            case node.type
+            when :block
+              node.method_name
+            when :block_pass
+              node.parent.method_name
+            end
+          end
+
+          # Returns true iff name is a known iterating type (e.g. :each, :transform_values)
+          def iterating_method?(name)
+            KNOWN_ITERATING_METHODS.include? name
+          end
+
+          # Returns nil if node is neither a block node or a block-pass node.
+          # Otherwise returns true/false if method call is a known iterating call
+          def iterating_block?(node)
+            name = block_method_name(node)
+            name && iterating_method?(name)
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/mixin/multiline_expression_indentation.rb

@@ -152,7 +152,7 @@ def indented_keyword_expression(node)
         expression
       end
 
-      def argument_in_method_call(node, kind)
+      def argument_in_method_call(node, kind) # rubocop:todo Metrics/CyclomaticComplexity
         node.each_ancestor(:send, :block).find do |a|
           # If the node is inside a block, it makes no difference if that block
           # is an argument in a method call. It doesn't count.

lib/rubocop/cop/naming/file_name.rb

@@ -126,7 +126,6 @@ def filename_good?(basename)
           basename.match?(regex || SNAKE_CASE)
         end
 
-        # rubocop:disable Metrics/CyclomaticComplexity
         def find_class_or_module(node, namespace)
           return nil unless node
 
@@ -145,7 +144,6 @@ def find_class_or_module(node, namespace)
 
           nil
         end
-        # rubocop:enable Metrics/CyclomaticComplexity
 
         def match_namespace(node, namespace, expected)
           match_partial = partial_matcher!(expected)

lib/rubocop/cop/style/block_delimiters.rb

@@ -252,7 +252,6 @@ def whitespace_after?(range, length = 1)
           /\s/.match?(range.source_buffer.source[range.begin_pos + length, 1])
         end
 
-        # rubocop:disable Metrics/CyclomaticComplexity
         def get_blocks(node, &block)
           case node.type
           when :block
@@ -270,7 +269,6 @@ def get_blocks(node, &block)
             node.each_child_node { |child| get_blocks(child, &block) }
           end
         end
-        # rubocop:enable Metrics/CyclomaticComplexity
 
         def proper_block_style?(node)
           return special_method_proper_block_style?(node) if special_method?(node.method_name)

lib/rubocop/cop/style/conditional_assignment.rb

@@ -67,7 +67,7 @@ def end_with_eq?(sym)
 
         private
 
-        def expand_elsif(node, elsif_branches = [])
+        def expand_elsif(node, elsif_branches = []) # rubocop:todo Metrics/CyclomaticComplexity
           return [] if node.nil? || !node.if_type? || !node.elsif?
 
           elsif_branches << node.if_branch

lib/rubocop/cop/style/identical_conditional_branches.rb

@@ -81,7 +81,7 @@ def on_case(node)
 
         private
 
-        def check_branches(branches)
+        def check_branches(branches) # rubocop:todo Metrics/CyclomaticComplexity
           # return if any branch is empty. An empty branch can be an `if`
           # without an `else` or a branch that contains only comments.
           return if branches.any?(&:nil?)

lib/rubocop/cop/style/if_inside_else.rb

@@ -61,7 +61,7 @@ module Style
       class IfInsideElse < Cop
         MSG = 'Convert `if` nested inside `else` to `elsif`.'
 
-        def on_if(node)
+        def on_if(node) # rubocop:todo Metrics/CyclomaticComplexity
           return if node.ternary? || node.unless?
 
           else_branch = node.else_branch

lib/rubocop/cop/style/sample.rb

@@ -92,7 +92,7 @@ def sample_size_for_two_args(first, second)
           second.int_type? ? second.to_a.first : :unknown
         end
 
-        def range_size(range_node)
+        def range_size(range_node) # rubocop:todo Metrics/CyclomaticComplexity
           vals = range_node.to_a
           return :unknown unless vals.all?(&:int_type?)
 

lib/rubocop/cop/style/semicolon.rb

@@ -39,7 +39,7 @@ def investigate(processed_source)
           check_for_line_terminator_or_opener
         end
 
-        def on_begin(node)
+        def on_begin(node) # rubocop:todo Metrics/CyclomaticComplexity
           return if cop_config['AllowAsExpressionSeparator']
 
           exprs = node.children

spec/rubocop/cop/metrics/cyclomatic_complexity_spec.rb

@@ -150,6 +150,16 @@ def method_name
       RUBY
     end
 
+    it 'registers an offense for &&=' do
+      expect_offense(<<~RUBY)
+        def method_name
+        ^^^^^^^^^^^^^^^ Cyclomatic complexity for method_name is too high. [2/1]
+          foo = nil
+          foo &&= 42
+        end
+      RUBY
+    end
+
     it 'registers an offense for and' do
       expect_offense(<<~RUBY)
         def method_name
@@ -168,6 +178,16 @@ def method_name
       RUBY
     end
 
+    it 'registers an offense for ||=' do
+      expect_offense(<<~RUBY)
+        def method_name
+        ^^^^^^^^^^^^^^^ Cyclomatic complexity for method_name is too high. [2/1]
+          foo = nil
+          foo ||= 42
+        end
+      RUBY
+    end
+
     it 'registers an offense for or' do
       expect_offense(<<~RUBY)
         def method_name
@@ -189,6 +209,16 @@ def method_name
       RUBY
     end
 
+    it 'registers an offense for &.' do
+      expect_offense(<<~RUBY)
+        def method_name
+        ^^^^^^^^^^^^^^^ Cyclomatic complexity for method_name is too high. [2/1]
+          foo = nil
+          foo&.bar
+        end
+      RUBY
+    end
+
     it 'counts only a single method' do
       expect_offense(<<~RUBY)
         def method_name_1
@@ -211,6 +241,41 @@ def method_name_2
         end
       RUBY
     end
+
+    it 'counts enumerating methods with blocks as +1' do
+      expect_offense(<<~RUBY)
+        define_method :method_name do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Cyclomatic complexity for method_name is too high. [3/1]
+          (1..4).map do |i|                            # map: +1
+            i * 2
+          end.each.with_index { |val, i| puts val, i } # each: +0, with_index: +1
+          return treasure.map
+        end
+      RUBY
+    end
+
+    it 'counts enumerating methods with block-pass as +1' do
+      expect_offense(<<~RUBY)
+        define_method :method_name do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Cyclomatic complexity for method_name is too high. [2/1]
+          [].map(&:to_s)
+        end
+      RUBY
+    end
+
+    it 'does not count blocks in general' do
+      expect_no_offenses(<<~RUBY)
+        define_method :method_name do
+          Struct.new(:foo, :bar) do
+            String.class_eval do
+              [42].tap do |answer|
+                foo { bar }
+              end
+            end
+          end
+        end
+      RUBY
+    end
   end
 
   context 'when method is in list of ignored methods' do