Configure different mass thresholds for identical & similar issues

README.md

@@ -44,7 +44,12 @@ duplication, try raising the threshold. If you suspect that the engine isn't
 catching enough duplication, try lowering the threshold. The best setting tends
 to differ from language to language.
 
-To adjust this setting, add a `mass_threshold` key with your preferred value for
+You can set thresholds for the two different types of duplication this engine
+reports: blocks that are identical to each other, and blocks that are
+structurally similar but differ in content.
+
+To adjust these thresholds, you can add `identical_mass_threshold` and
+`similar_mass_threshold` keys with your preferred value for
 an enabled language:
 
 ```yaml
@@ -54,11 +59,15 @@ engines:
     config:
       languages:
         ruby:
-          mass_threshold: 20
+          identical_mass_threshold: 20
+          similar_mass_threshold: 30
         javascript:
 ```
 
-Note that you have the update the YAML structure under the `langauges` key to
+If you would like to use the same threshold for both identical & similar issues,
+you can just set the `mass_threshold` key.
+
+Note that you have to update the YAML structure under the `langauges` key to
 the Hash type to support extra configuration.
 
 [codeclimate]: https://codeclimate.com/dashboard

lib/cc/engine/analyzers/analyzer_base.rb

@@ -17,8 +17,15 @@ def files
           file_list.files
         end
 
-        def mass_threshold
-          engine_config.mass_threshold_for(self.class::LANGUAGE) || self.class::DEFAULT_MASS_THRESHOLD
+        def mass_threshold_for_check(type)
+          case type
+          when :identical
+            engine_config.identical_mass_threshold_for(self.class::LANGUAGE) || self.class::DEFAULT_MASS_THRESHOLDS.fetch(type)
+          when :similar
+            engine_config.similar_mass_threshold_for(self.class::LANGUAGE) || self.class::DEFAULT_MASS_THRESHOLDS.fetch(type)
+          else
+            raise ArgumentError.new("#{type} is not a valid check type")
+          end
         end
 
         def base_points

lib/cc/engine/analyzers/engine_config.rb

@@ -18,12 +18,12 @@ def concurrency
           config.fetch("concurrency", 2)
         end
 
-        def mass_threshold_for(language)
-          threshold = fetch_language(language).fetch("mass_threshold", nil)
+        def identical_mass_threshold_for(language)
+          mass_threshold_with_fallback(language, "identical_mass_threshold")
+        end
 
-          if threshold
-            threshold.to_i
-          end
+        def similar_mass_threshold_for(language)
+          mass_threshold_with_fallback(language, "similar_mass_threshold")
         end
 
         def paths_for(language)
@@ -66,6 +66,15 @@ def build_language_config(languages)
             {}
           end
         end
+
+        def mass_threshold_with_fallback(language, key)
+          language_hash = fetch_language(language)
+          threshold = language_hash.fetch(key) do
+            language_hash.fetch("mass_threshold", nil)
+          end
+
+          threshold.to_i if threshold
+        end
       end
     end
   end

lib/cc/engine/analyzers/javascript/main.rb

@@ -15,7 +15,7 @@ class Main < CC::Engine::Analyzers::Base
             "**/*.jsx"
           ]
           LANGUAGE = "javascript"
-          DEFAULT_MASS_THRESHOLD = 40
+          DEFAULT_MASS_THRESHOLDS = {identical: 40, similar: 80}.freeze
           BASE_POINTS = 3000
 
           private

lib/cc/engine/analyzers/php/main.rb

@@ -14,7 +14,7 @@ class Main < CC::Engine::Analyzers::Base
             "**/*.inc",
             "**/*.module"
           ]
-          DEFAULT_MASS_THRESHOLD = 10
+          DEFAULT_MASS_THRESHOLDS = {identical: 10, similar: 20}.freeze
           BASE_POINTS = 4_000
 
           private

lib/cc/engine/analyzers/python/main.rb

@@ -12,7 +12,7 @@ module Python
         class Main < CC::Engine::Analyzers::Base
           LANGUAGE = "python"
           DEFAULT_PATHS = ["**/*.py"]
-          DEFAULT_MASS_THRESHOLD = 40
+          DEFAULT_MASS_THRESHOLDS = {identical: 40, similar: 80}.freeze
           BASE_POINTS = 1000
 
           private

lib/cc/engine/analyzers/reporter.rb

@@ -36,6 +36,7 @@ def process_files
 
         def report
           flay.report(StringIO.new).each do |issue|
+            next unless issue_above_threshold?(issue)
             violation = new_violation(issue)
 
             unless reports.include?(violation.report_name)
@@ -61,7 +62,19 @@ def flay
         attr_reader :engine_config, :language_strategy, :io
 
         def mass_threshold
-          @mass_threshold ||= language_strategy.mass_threshold
+          @mass_threshold ||= [
+            language_strategy.mass_threshold_for_check(:identical),
+            language_strategy.mass_threshold_for_check(:similar),
+          ].min
+        end
+
+        def issue_above_threshold?(issue)
+          base_mass = issue.mass / issue.locations.count
+          if issue.identical?
+            base_mass >= language_strategy.mass_threshold_for_check(:identical)
+          else
+            base_mass >= language_strategy.mass_threshold_for_check(:similar)
+          end
         end
 
         def new_violation(issue)

lib/cc/engine/analyzers/ruby/main.rb

@@ -17,7 +17,7 @@ class Main < CC::Engine::Analyzers::Base
             "**/*.gemspec"
 
           ]
-          DEFAULT_MASS_THRESHOLD = 18
+          DEFAULT_MASS_THRESHOLDS = {identical: 18, similar: 36}.freeze
           BASE_POINTS = 10_000
           TIMEOUT = 10
 

spec/cc/engine/analyzers/engine_config_spec.rb

@@ -70,13 +70,26 @@
           }
         }
       })
-
       expect(engine_config.paths_for("elixir")).to be_nil
     end
   end
 
-  describe "mass_threshold_for" do
+  shared_examples "mass_threshold examples" do
     it "returns configured mass threshold as integer" do
+      engine_config = CC::Engine::Analyzers::EngineConfig.new({
+        "config" => {
+          "languages" => {
+            "EliXiR" => {
+              specific_mass_threshold_key => "13"
+            }
+          }
+        }
+      })
+
+      expect(engine_config.send(tested_method, "elixir")).to eq(13)
+    end
+
+    it "returns fallback mass threshold as integer" do
       engine_config = CC::Engine::Analyzers::EngineConfig.new({
         "config" => {
           "languages" => {
@@ -87,7 +100,7 @@
         }
       })
 
-      expect(engine_config.mass_threshold_for("elixir")).to eq(13)
+      expect(engine_config.send(tested_method, "elixir")).to eq(13)
     end
 
     it "returns nil when language is empty" do
@@ -99,7 +112,21 @@
         }
       })
 
-      expect(engine_config.mass_threshold_for("ruby")).to be_nil
+      expect(engine_config.send(tested_method, "ruby")).to be_nil
+    end
+  end
+
+  describe "similar_mass_threshold_for" do
+    include_examples "mass_threshold examples" do
+      let(:specific_mass_threshold_key) { "similar_mass_threshold" }
+      let(:tested_method) { :similar_mass_threshold_for }
+    end
+  end
+
+  describe "identical_mass_threshold_for" do
+    include_examples "mass_threshold examples" do
+      let(:specific_mass_threshold_key) { "identical_mass_threshold" }
+      let(:tested_method) { :identical_mass_threshold_for }
     end
   end
 

spec/cc/engine/analyzers/javascript/main_spec.rb

@@ -61,14 +61,8 @@
   end
 
   def engine_conf
-    CC::Engine::Analyzers::EngineConfig.new({
-      'config' => {
-        'languages' => {
-          'javascript' => {
-            'mass_threshold' => 1
-          }
-        }
-      }
-    })
+    CC::Engine::Analyzers::EngineConfig.new(engine_config_for_language({
+      "mass_threshold" => 1,
+    }))
   end
 end

spec/cc/engine/analyzers/php/main_spec.rb

@@ -56,14 +56,8 @@ def printed_issue
   end
 
   def engine_conf
-    CC::Engine::Analyzers::EngineConfig.new({
-      'config' => {
-        'languages' => {
-          'php' => {
-            'mass_threshold' => 5
-          }
-        }
-      }
-    })
+    CC::Engine::Analyzers::EngineConfig.new(engine_config_for_language({
+      "mass_threshold" => 5,
+    }))
   end
 end

spec/cc/engine/analyzers/python/main_spec.rb

@@ -39,14 +39,8 @@
   end
 
   def engine_conf
-    CC::Engine::Analyzers::EngineConfig.new({
-      "config" => {
-        "languages" => {
-          "python" => {
-            "mass_threshold" => 4
-          }
-        }
-      }
-    })
+    CC::Engine::Analyzers::EngineConfig.new(engine_config_for_language({
+      "mass_threshold" => 4,
+    }))
   end
 end

spec/cc/engine/analyzers/ruby/main_spec.rb

@@ -55,9 +55,50 @@
         expect(run_engine(engine_conf)).to eq("")
       }.to output(/Skipping file/).to_stderr
     end
+
+    it "respects different check type thresholds" do
+      create_source_file("foo.rb", <<-EORUBY)
+        def identical
+          puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
+          puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
+          puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
+        end
+
+        describe 'similar1' do
+          before { subject.type = 'js' }
+
+          it 'returns true' do
+            expect(subject.ruby?).to be true
+          end
+        end
+
+        describe 'similar2' do
+          before { subject.type = 'js' }
+
+          it 'returns true' do
+            expect(subject.js?).to be true
+          end
+        end
+      EORUBY
+
+      config = CC::Engine::Analyzers::EngineConfig.new(engine_config_for_language({
+        "identical_mass_threshold" => 5,
+        "similar_mass_threshold" => 20,
+      }))
+      result = run_engine(config).strip
+      json = JSON.parse(result)
+
+      expect(json["check_name"]).to eq "Identical code"
+      expect(json["location"]).to eq({
+        "path" => "foo.rb",
+        "lines" => { "begin" => 2, "end" => 2 },
+      })
+    end
   end
 
   def engine_conf
-    CC::Engine::Analyzers::EngineConfig.new({})
+    CC::Engine::Analyzers::EngineConfig.new(engine_config_for_language({
+      "mass_threshold" => described_class::DEFAULT_MASS_THRESHOLDS.fetch(:identical),
+    }))
   end
 end

spec/cc/engine/analyzers/sexp_lines_spec.rb

@@ -16,7 +16,7 @@ module CC::Engine::Analyzers
         SOURCE
         flay = Flay.new({
           diff: false,
-          mass: CC::Engine::Analyzers::Ruby::Main::DEFAULT_MASS_THRESHOLD,
+          mass: CC::Engine::Analyzers::Ruby::Main::DEFAULT_MASS_THRESHOLDS.fetch(:identical),
           summary: false,
           verbose: false,
           number: true,

spec/support/helpers/analyzer_spec_helpers.rb

@@ -3,6 +3,16 @@ def create_source_file(path, content)
     File.write(File.join(@code, path), content)
   end
 
+  def engine_config_for_language(config = {})
+    {
+      "config" => {
+        "languages" => {
+          described_class::LANGUAGE => config,
+        },
+      },
+    }
+  end
+
   def run_engine(config = nil)
     io = StringIO.new