Merge pull request #67 from codeclimate/abh-points-ruby-only

Tune RUBY remediation points to match Classic

Author: Ashley Baldwin-Hunter

lib/cc/engine/analyzers/analyzer_base.rb

@@ -35,8 +35,8 @@ def mass_threshold
           engine_config.mass_threshold_for(self.class::LANGUAGE) || self.class::DEFAULT_MASS_THRESHOLD
         end
 
-        def base_points
-          self.class::BASE_POINTS
+        def calculate_points(issue)
+          self.class::BASE_POINTS * issue.mass
         end
 
         private

lib/cc/engine/analyzers/reporter.rb

@@ -60,19 +60,15 @@ def flay
 
         attr_reader :engine_config, :language_strategy, :io
 
-        def mass_threshold
-          @mass_threshold ||= language_strategy.mass_threshold
-        end
-
         def new_violation(issue)
           hashes = flay.hashes[issue.structural_hash]
-          Violation.new(language_strategy.base_points, issue, hashes)
+          Violation.new(language_strategy, issue, hashes)
         end
 
         def flay_options
           {
             diff: false,
-            mass: mass_threshold,
+            mass: language_strategy.mass_threshold,
             summary: false,
             verbose: false,
             number: true,

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

@@ -18,11 +18,20 @@ class Main < CC::Engine::Analyzers::Base
 
           ]
           DEFAULT_MASS_THRESHOLD = 18
-          BASE_POINTS = 10_000
+          BASE_POINTS = 1_500_000
+          POINTS_PER_OVERAGE = 100_000
           TIMEOUT = 300
 
+          def calculate_points(issue)
+            BASE_POINTS + (overage(issue) * POINTS_PER_OVERAGE)
+          end
+
           private
 
+          def overage(issue)
+            issue.mass - mass_threshold
+          end
+
           def process_file(file)
             RubyParser.new.process(File.binread(file), file, TIMEOUT)
           end

lib/cc/engine/analyzers/violation.rb

@@ -8,8 +8,8 @@ module Analyzers
       class Violation
         attr_reader :issue
 
-        def initialize(base_points, issue, hashes)
-          @base_points = base_points
+        def initialize(language_strategy, issue, hashes)
+          @language_strategy = language_strategy
           @issue = issue
           @hashes = hashes
         end
@@ -34,7 +34,7 @@ def report_name
 
         private
 
-        attr_reader :base_points, :hashes
+        attr_reader :language_strategy, :hashes
 
         def current_sexp
           @location ||= sorted_hashes.first
@@ -57,7 +57,7 @@ def name
         end
 
         def calculate_points
-          base_points * issue.mass
+          language_strategy.calculate_points(issue)
         end
 
         def format_location

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

@@ -4,60 +4,105 @@
 require 'flay'
 require 'tmpdir'
 
-RSpec.describe CC::Engine::Analyzers::Ruby::Main, in_tmpdir: true do
-  include AnalyzerSpecHelpers
+module CC::Engine::Analyzers
+  RSpec.describe Ruby::Main, in_tmpdir: true do
+    include AnalyzerSpecHelpers
 
-  describe "#run" do
-    it "prints an issue" do
-      create_source_file("foo.rb", <<-EORUBY)
-          describe '#ruby?' do
-            before { subject.type = 'ruby' }
+    describe "#run" do
+      it "prints an issue" do
+        create_source_file("foo.rb", <<-EORUBY)
+            describe '#ruby?' do
+              before { subject.type = 'ruby' }
 
-            it 'returns true' do
-              expect(subject.ruby?).to be true
+              it 'returns true' do
+                expect(subject.ruby?).to be true
+              end
             end
-          end
 
-          describe '#js?' do
-            before { subject.type = 'js' }
+            describe '#js?' do
+              before { subject.type = 'js' }
 
-            it 'returns true' do
-              expect(subject.js?).to be true
+              it 'returns true' do
+                expect(subject.js?).to be true
+              end
             end
-          end
-      EORUBY
-
-      result = run_engine(engine_conf).strip
-      json = JSON.parse(result)
-
-      expect(json["type"]).to eq("issue")
-      expect(json["check_name"]).to eq("Similar code")
-      expect(json["description"]).to eq("Similar code found in 1 other location")
-      expect(json["categories"]).to eq(["Duplication"])
-      expect(json["location"]).to eq({
-        "path" => "foo.rb",
-        "lines" => { "begin" => 1, "end" => 5 },
-      })
-      expect(json["remediation_points"]).to eq(360000)
-      expect(json["other_locations"]).to eq([
-        {"path" => "foo.rb", "lines" => { "begin" => 9, "end" => 13} },
-      ])
-      expect(json["content"]["body"]).to match /This issue has a mass of `36`/
-      expect(json["fingerprint"]).to eq("f21b75bbd135ec3ae6638364d5c73762")
+        EORUBY
+
+        result = run_engine(engine_conf).strip
+        json = JSON.parse(result)
+
+        expect(json["type"]).to eq("issue")
+        expect(json["check_name"]).to eq("Similar code")
+        expect(json["description"]).to eq("Similar code found in 1 other location")
+        expect(json["categories"]).to eq(["Duplication"])
+        expect(json["location"]).to eq({
+          "path" => "foo.rb",
+          "lines" => { "begin" => 1, "end" => 5 },
+        })
+        expect(json["remediation_points"]).to eq(3300000)
+        expect(json["other_locations"]).to eq([
+          {"path" => "foo.rb", "lines" => { "begin" => 9, "end" => 13} },
+        ])
+        expect(json["content"]["body"]).to match /This issue has a mass of `36`/
+        expect(json["fingerprint"]).to eq("f21b75bbd135ec3ae6638364d5c73762")
+      end
+
+      it "skips unparsable files" do
+        create_source_file("foo.rb", <<-EORUBY)
+          ---
+        EORUBY
+
+        expect {
+          expect(run_engine(engine_conf)).to eq("")
+        }.to output(/Skipping file/).to_stderr
+      end
     end
 
-    it "skips unparsable files" do
-      create_source_file("foo.rb", <<-EORUBY)
-        ---
-      EORUBY
+    describe "#calculate_points(issue)" do
+      let(:analyzer) { Ruby::Main.new(engine_config: engine_conf) }
+      let(:base_points) { Ruby::Main::BASE_POINTS }
+      let(:points_per) { Ruby::Main::POINTS_PER_OVERAGE }
+      let(:threshold) { Ruby::Main::DEFAULT_MASS_THRESHOLD }
+
+      context "when issue mass exceeds threshold" do
+        it "calculates mass overage points" do
+          issue = double(:issue, mass: threshold + 10)
+          overage = issue.mass - threshold
+
+          expected_points = base_points + overage * points_per
+          points = analyzer.calculate_points(issue)
+
+          expect(points).to eq(expected_points)
+        end
+      end
 
-      expect {
-        expect(run_engine(engine_conf)).to eq("")
-      }.to output(/Skipping file/).to_stderr
+      context "when issue mass is less than threshold" do
+        it "calculates mass overage points" do
+          issue = double(:issue, mass: threshold - 5)
+          overage = issue.mass - threshold
+
+          expected_points = base_points + overage * points_per
+          points = analyzer.calculate_points(issue)
+
+          expect(points).to eq(expected_points)
+        end
+      end
+
+      context "when issue mass equals threshold" do
+        it "calculates mass overage points" do
+          issue = double(:issue, mass: threshold)
+          overage = issue.mass - threshold
+
+          expected_points = base_points + overage * points_per
+          points = analyzer.calculate_points(issue)
+
+          expect(points).to eq(expected_points)
+        end
+      end
     end
-  end
 
-  def engine_conf
-    CC::Engine::Analyzers::EngineConfig.new({})
+    def engine_conf
+      EngineConfig.new({})
+    end
   end
 end