Merge pull request #84 from codeclimate/abh-mass-node-size

Use sexp node size for issue mass, not flay score

Author: Ashley Baldwin-Hunter

config/contents/duplicated_code.md.erb

@@ -9,7 +9,7 @@ When you violate DRY, bugs and maintenance problems are sure to follow. Duplicat
 ## Issue Mass
 
 Duplicated code has a calculated mass, which can be thought of as a measure of how much logic has been duplicated.
-This issue has a mass of `<%= issue.mass %>`: if you would like to change the minimum mass that will be reported as an issue, please see the details in [`codeclimate-duplication`'s documentation](https://github.com/codeclimate/codeclimate-duplication).
+This issue has a mass of `<%= mass %>`: if you would like to change the minimum mass that will be reported as an issue, please see the details in [`codeclimate-duplication`'s documentation](https://github.com/codeclimate/codeclimate-duplication).
 
 ## Refactorings
 

lib/cc/engine/analyzers/analyzer_base.rb

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

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

@@ -22,14 +22,14 @@ class Main < CC::Engine::Analyzers::Base
           POINTS_PER_OVERAGE = 100_000
           TIMEOUT = 300
 
-          def calculate_points(issue)
-            BASE_POINTS + (overage(issue) * POINTS_PER_OVERAGE)
+          def calculate_points(mass)
+            BASE_POINTS + (overage(mass) * POINTS_PER_OVERAGE)
           end
 
           private
 
-          def overage(issue)
-            issue.mass - mass_threshold
+          def overage(mass)
+            mass - mass_threshold
           end
 
           def process_file(file)

lib/cc/engine/analyzers/violation.rb

@@ -6,11 +6,9 @@ module CC
   module Engine
     module Analyzers
       class Violation
-        attr_reader :issue
-
-        def initialize(language_strategy:, issue:, current_sexp:, other_sexps:)
+        def initialize(language_strategy:, identical:, current_sexp:, other_sexps:)
           @language_strategy = language_strategy
-          @issue = issue
+          @identical = identical
           @current_sexp = current_sexp
           @other_sexps = other_sexps
         end
@@ -33,20 +31,28 @@ def report_name
           "#{current_sexp.file}-#{current_sexp.line}"
         end
 
+        def mass
+          current_sexp.mass
+        end
+
         private
 
         attr_reader :language_strategy, :other_sexps, :current_sexp
 
         def check_name
-          if issue.identical?
+          if identical?
             "Identical code"
           else
             "Similar code"
           end
         end
 
+        def identical?
+          @identical
+        end
+
         def calculate_points
-          language_strategy.calculate_points(issue)
+          language_strategy.calculate_points(mass)
         end
 
         def format_location
@@ -71,7 +77,7 @@ def format_sexp(sexp)
         end
 
         def content_body
-          @_content_body ||= { "body": ViolationReadUp.new(issue).contents }
+          @_content_body ||= { "body": ViolationReadUp.new(mass).contents }
         end
 
         def fingerprint

lib/cc/engine/analyzers/violation_read_up.rb

@@ -4,8 +4,8 @@ module CC
   module Engine
     module Analyzers
       class ViolationReadUp
-        def initialize(issue)
-          @issue = issue
+        def initialize(mass)
+          @mass = mass
         end
 
         def contents
@@ -14,7 +14,7 @@ def contents
 
         private
 
-        attr_reader :issue
+        attr_reader :mass
 
         TEMPLATE_REL_PATH = "../../../../config/contents/duplicated_code.md.erb"
 

lib/cc/engine/analyzers/violations.rb

@@ -15,7 +15,7 @@ def each
             yield Violation.new(
               current_sexp: sexp,
               other_sexps: other_sexps(hashes.dup, i),
-              issue: issue,
+              identical: identical?,
               language_strategy: language_strategy,
             )
           end
@@ -29,6 +29,10 @@ def other_sexps(members, i)
           members.delete_at(i)
           members.sort_by(&:file)
         end
+
+        def identical?
+          issue.identical?
+        end
       end
     end
   end

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

@@ -28,12 +28,12 @@
         "path" => "foo.js",
         "lines" => { "begin" => 1, "end" => 1 },
       })
-      expect(json["remediation_points"]).to eq(297000)
+      expect(json["remediation_points"]).to eq(33_000)
       expect(json["other_locations"]).to eq([
         {"path" => "foo.js", "lines" => { "begin" => 2, "end" => 2} },
         {"path" => "foo.js", "lines" => { "begin" => 3, "end" => 3} }
       ])
-      expect(json["content"]["body"]).to match /This issue has a mass of `99`/
+      expect(json["content"]["body"]).to match /This issue has a mass of `11`/
       expect(json["fingerprint"]).to eq("55ae5d0990647ef496e9e0d315f9727d")
     end
 
@@ -56,12 +56,12 @@
         "path" => "foo.js",
         "lines" => { "begin" => 1, "end" => 1 },
       })
-      expect(json["remediation_points"]).to eq(99000)
+      expect(json["remediation_points"]).to eq(33_000)
       expect(json["other_locations"]).to eq([
         {"path" => "foo.js", "lines" => { "begin" => 2, "end" => 2} },
         {"path" => "foo.js", "lines" => { "begin" => 3, "end" => 3} }
       ])
-      expect(json["content"]["body"]).to match /This issue has a mass of `33`/
+      expect(json["content"]["body"]).to match /This issue has a mass of `11`/
       expect(json["fingerprint"]).to eq("55ae5d0990647ef496e9e0d315f9727d")
     end
 

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

@@ -41,11 +41,11 @@
         "path" => "foo.php",
         "lines" => { "begin" => 2, "end" => 6 },
       })
-      expect(json["remediation_points"]).to eq(176000)
+      expect(json["remediation_points"]).to eq(44_000)
       expect(json["other_locations"]).to eq([
         {"path" => "foo.php", "lines" => { "begin" => 10, "end" => 14} },
       ])
-      expect(json["content"]["body"]).to match /This issue has a mass of `44`/
+      expect(json["content"]["body"]).to match /This issue has a mass of `11`/
       expect(json["fingerprint"]).to eq("667da0e2bab866aa2fe9d014a65d57d9")
     end
 

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

@@ -28,12 +28,12 @@
         "path" => "foo.py",
         "lines" => { "begin" => 1, "end" => 1 },
       })
-      expect(json["remediation_points"]).to eq(54000)
+      expect(json["remediation_points"]).to eq(6_000)
       expect(json["other_locations"]).to eq([
         {"path" => "foo.py", "lines" => { "begin" => 2, "end" => 2} },
         {"path" => "foo.py", "lines" => { "begin" => 3, "end" => 3} }
       ])
-      expect(json["content"]["body"]).to match /This issue has a mass of `54`/
+      expect(json["content"]["body"]).to match /This issue has a mass of `6`/
       expect(json["fingerprint"]).to eq("42b832387c997f54a2012efb2159aefc")
     end
 
@@ -56,12 +56,12 @@
         "path" => "foo.py",
         "lines" => { "begin" => 1, "end" => 1 },
       })
-      expect(json["remediation_points"]).to eq(18000)
+      expect(json["remediation_points"]).to eq(6_000)
       expect(json["other_locations"]).to eq([
         {"path" => "foo.py", "lines" => { "begin" => 2, "end" => 2} },
         {"path" => "foo.py", "lines" => { "begin" => 3, "end" => 3} }
       ])
-      expect(json["content"]["body"]).to match /This issue has a mass of `18`/
+      expect(json["content"]["body"]).to match /This issue has a mass of `6`/
       expect(json["fingerprint"]).to eq("42b832387c997f54a2012efb2159aefc")
     end
 

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

@@ -40,11 +40,11 @@ module CC::Engine::Analyzers
           "path" => "foo.rb",
           "lines" => { "begin" => 1, "end" => 5 },
         })
-        expect(json["remediation_points"]).to eq(3300000)
+        expect(json["remediation_points"]).to eq(1_500_000)
         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["content"]["body"]).to match /This issue has a mass of `18`/
         expect(json["fingerprint"]).to eq("f21b75bbd135ec3ae6638364d5c73762")
       end
 
@@ -91,43 +91,43 @@ module CC::Engine::Analyzers
       end
     end
 
-    describe "#calculate_points(issue)" do
+    describe "#calculate_points(mass)" 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
+      context "when mass exceeds threshold" do
         it "calculates mass overage points" do
-          issue = double(:issue, mass: threshold + 10)
-          overage = issue.mass - threshold
+          mass = threshold + 10
+          overage = mass - threshold
 
           expected_points = base_points + overage * points_per
-          points = analyzer.calculate_points(issue)
+          points = analyzer.calculate_points(mass)
 
           expect(points).to eq(expected_points)
         end
       end
 
-      context "when issue mass is less than threshold" do
+      context "when mass is less than threshold" do
         it "calculates mass overage points" do
-          issue = double(:issue, mass: threshold - 5)
-          overage = issue.mass - threshold
+          mass = threshold - 5
+          overage = mass - threshold
 
           expected_points = base_points + overage * points_per
-          points = analyzer.calculate_points(issue)
+          points = analyzer.calculate_points(mass)
 
           expect(points).to eq(expected_points)
         end
       end
 
-      context "when issue mass equals threshold" do
+      context "when mass equals threshold" do
         it "calculates mass overage points" do
-          issue = double(:issue, mass: threshold)
-          overage = issue.mass - threshold
+          mass = threshold
+          overage = mass - threshold
 
           expected_points = base_points + overage * points_per
-          points = analyzer.calculate_points(issue)
+          points = analyzer.calculate_points(mass)
 
           expect(points).to eq(expected_points)
         end