Merge pull request #1799 from le-santos/le-santos/fix-dataclump-analy…

…sis-for-clumps-in-non-consecutive-methods

Fix dataclump analysis for clumps in non consecutive methods

lib/reek/ast/node.rb

@@ -9,6 +9,11 @@ module AST
     # methods to ease the transition from Sexp to AST::Node.
     #
     class Node < ::Parser::AST::Node
+      # Struct representing the rules for the search performed by each_node method.
+      NodeLookupRule = Struct.new(:target_types, :ignoring)
+
+      private_constant :NodeLookupRule
+
       def initialize(type, children = [], options = {})
         @comments = options.fetch(:comments, [])
         super
@@ -62,7 +67,8 @@ def name
       def each_node(target_types, ignoring = [], &blk)
         return enum_for(:each_node, target_types, ignoring) unless blk
 
-        look_for(Array(target_types), ignoring, &blk)
+        lookup_rule = NodeLookupRule.new(Array(target_types), ignoring)
+        look_for(lookup_rule, &blk)
       end
 
       def contains_nested_node?(target_type)
@@ -108,23 +114,23 @@ def condition; end
       protected
 
       # See ".each_node" for documentation.
-      def look_for(target_types, ignoring, &blk)
-        if target_types.include? type
+      def look_for(lookup_rule, &blk)
+        if lookup_rule.target_types.include?(type)
           yield self
-          return if ignoring.include?(type)
+          return if lookup_rule.ignoring.include?(type)
         end
         each_sexp do |elem|
-          elem.look_for_recurse(target_types, ignoring, &blk)
+          elem.look_for_recurse(lookup_rule, &blk)
         end
       end
 
       # See ".each_node" for documentation.
-      def look_for_recurse(target_types, ignoring, &blk)
-        yield self if target_types.include? type
-        return if ignoring.include? type
+      def look_for_recurse(lookup_rule, &blk)
+        yield self if lookup_rule.target_types.include?(type)
+        return if lookup_rule.ignoring.include?(type)
 
         each_sexp do |elem|
-          elem.look_for_recurse(target_types, ignoring, &blk)
+          elem.look_for_recurse(lookup_rule, &blk)
         end
       end
 

lib/reek/smell_detectors/data_clump.rb

@@ -84,7 +84,7 @@ def candidate_methods
       end
 
       def candidate_clumps
-        candidate_methods.each_cons(max_copies + 1).map do |methods|
+        candidate_methods.combination(max_copies + 1).map do |methods|
           common_argument_names_for(methods)
         end.select do |clump|
           clump.length >= min_clump_size

spec/reek/smell_detectors/data_clump_spec.rb

@@ -38,6 +38,25 @@ def india(juliett, kilo); end
       and reek_of(:DataClump, lines: [6, 7, 8], parameters: ['juliett', 'kilo'])
   end
 
+  it 'detects clumps mixed among other methods' do
+    src = <<-RUBY
+      class Alfa
+        def bravo  (echo, foxtrot); end
+        def charlie(echo, foxtrot); end
+        def dummy(param); end
+        def delta  (echo, foxtrot); end
+      end
+    RUBY
+
+    expect(src).to reek_of(:DataClump,
+                           lines:      [2, 3, 5],
+                           context:    'Alfa',
+                           message:    "takes parameters ['echo', 'foxtrot'] to 3 methods",
+                           source:     'string',
+                           parameters: ['echo', 'foxtrot'],
+                           count:      3)
+  end
+
   %w(class module).each do |scope|
     it "does not report parameter sets < 2 for #{scope}" do
       src = <<-RUBY