From 5a3fad4575c4c865a02a34283bce9e3c9328d77a Mon Sep 17 00:00:00 2001 From: ABaldwinHunter Date: Tue, 29 Dec 2015 02:26:28 -0500 Subject: [PATCH 1/2] Tune remediation points to match Classic Changes include: - Update PHP, Python, and JavaScript thresholds to match Classic - Increase base_points for all four languages - Add `points_per` value to each language - change calculation formula from - `base_points * issue.mass_threshold` to - `base_points + overage * points_per` Reference to rubric in classic for PHP, Ruby and JavaScript: https://github.com/codeclimate/analyzer/blob/master/lib/quality/rubric.rb#L12c And python: https://github.com/codeclimate/analyzer/blob/master/lib/quality/profile.rb#L41 --- lib/cc/engine/analyzers/analyzer_base.rb | 8 +++ lib/cc/engine/analyzers/javascript/main.rb | 2 - lib/cc/engine/analyzers/php/main.rb | 2 - lib/cc/engine/analyzers/python/main.rb | 2 - lib/cc/engine/analyzers/reporter.rb | 8 +-- lib/cc/engine/analyzers/ruby/main.rb | 2 +- lib/cc/engine/analyzers/violation.rb | 22 +++++-- .../engine/analyzers/javascript/main_spec.rb | 2 +- spec/cc/engine/analyzers/php/main_spec.rb | 4 +- spec/cc/engine/analyzers/python/main_spec.rb | 2 +- spec/cc/engine/analyzers/ruby/main_spec.rb | 2 +- spec/cc/engine/analyzers/violation_spec.rb | 59 +++++++++++++++++++ 12 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 spec/cc/engine/analyzers/violation_spec.rb diff --git a/lib/cc/engine/analyzers/analyzer_base.rb b/lib/cc/engine/analyzers/analyzer_base.rb index 8ccba8eb..b3db9b25 100644 --- a/lib/cc/engine/analyzers/analyzer_base.rb +++ b/lib/cc/engine/analyzers/analyzer_base.rb @@ -11,6 +11,10 @@ class Base ::RubyParser::SyntaxError, ] + DEFAULT_MASS_THRESHOLD = 28 + BASE_POINTS = 1_500_000 + POINTS_PER = 50_000 + def initialize(engine_config:) @engine_config = engine_config end @@ -37,6 +41,10 @@ def base_points self.class::BASE_POINTS end + def points_per + self.class::POINTS_PER + end + private attr_reader :engine_config diff --git a/lib/cc/engine/analyzers/javascript/main.rb b/lib/cc/engine/analyzers/javascript/main.rb index 59c1a900..6104cd8f 100644 --- a/lib/cc/engine/analyzers/javascript/main.rb +++ b/lib/cc/engine/analyzers/javascript/main.rb @@ -15,8 +15,6 @@ class Main < CC::Engine::Analyzers::Base "**/*.jsx" ] LANGUAGE = "javascript" - DEFAULT_MASS_THRESHOLD = 40 - BASE_POINTS = 3000 private diff --git a/lib/cc/engine/analyzers/php/main.rb b/lib/cc/engine/analyzers/php/main.rb index ddd7d638..da56a40b 100644 --- a/lib/cc/engine/analyzers/php/main.rb +++ b/lib/cc/engine/analyzers/php/main.rb @@ -14,8 +14,6 @@ class Main < CC::Engine::Analyzers::Base "**/*.inc", "**/*.module" ] - DEFAULT_MASS_THRESHOLD = 10 - BASE_POINTS = 4_000 private diff --git a/lib/cc/engine/analyzers/python/main.rb b/lib/cc/engine/analyzers/python/main.rb index 6ec2ca50..fc95338a 100644 --- a/lib/cc/engine/analyzers/python/main.rb +++ b/lib/cc/engine/analyzers/python/main.rb @@ -12,8 +12,6 @@ module Python class Main < CC::Engine::Analyzers::Base LANGUAGE = "python" DEFAULT_PATHS = ["**/*.py"] - DEFAULT_MASS_THRESHOLD = 40 - BASE_POINTS = 1000 private diff --git a/lib/cc/engine/analyzers/reporter.rb b/lib/cc/engine/analyzers/reporter.rb index a21a4604..bbe9f83d 100644 --- a/lib/cc/engine/analyzers/reporter.rb +++ b/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, diff --git a/lib/cc/engine/analyzers/ruby/main.rb b/lib/cc/engine/analyzers/ruby/main.rb index 4e6e6da3..506d40d2 100644 --- a/lib/cc/engine/analyzers/ruby/main.rb +++ b/lib/cc/engine/analyzers/ruby/main.rb @@ -18,7 +18,7 @@ class Main < CC::Engine::Analyzers::Base ] DEFAULT_MASS_THRESHOLD = 18 - BASE_POINTS = 10_000 + POINTS_PER = 100_000 TIMEOUT = 300 private diff --git a/lib/cc/engine/analyzers/violation.rb b/lib/cc/engine/analyzers/violation.rb index e064e841..aae1752e 100644 --- a/lib/cc/engine/analyzers/violation.rb +++ b/lib/cc/engine/analyzers/violation.rb @@ -8,8 +8,12 @@ module Analyzers class Violation attr_reader :issue - def initialize(base_points, issue, hashes) - @base_points = base_points + DEFAULT_POINTS = 1_500_000 + + def initialize(language, issue, hashes) + @base_points = language.base_points + @points_per = language.points_per + @threshold = language.mass_threshold @issue = issue @hashes = hashes end @@ -32,9 +36,17 @@ def report_name "#{current_sexp.file}-#{current_sexp.line}" end + def calculate_points + if issue.mass >= threshold + base_points + (overage * points_per) + else + DEFAULT_POINTS + end + end + private - attr_reader :base_points, :hashes + attr_reader :base_points, :points_per, :threshold, :hashes def current_sexp @location ||= sorted_hashes.first @@ -56,8 +68,8 @@ def name end end - def calculate_points - base_points * issue.mass + def overage + issue.mass - threshold end def format_location diff --git a/spec/cc/engine/analyzers/javascript/main_spec.rb b/spec/cc/engine/analyzers/javascript/main_spec.rb index c7b7113f..de70a6cb 100644 --- a/spec/cc/engine/analyzers/javascript/main_spec.rb +++ b/spec/cc/engine/analyzers/javascript/main_spec.rb @@ -28,7 +28,7 @@ "path" => "foo.js", "lines" => { "begin" => 1, "end" => 1 }, }) - expect(json["remediation_points"]).to eq(297000) + expect(json["remediation_points"]).to eq(6400000) expect(json["other_locations"]).to eq([ {"path" => "foo.js", "lines" => { "begin" => 2, "end" => 2} }, {"path" => "foo.js", "lines" => { "begin" => 3, "end" => 3} } diff --git a/spec/cc/engine/analyzers/php/main_spec.rb b/spec/cc/engine/analyzers/php/main_spec.rb index b27134fa..05236c0f 100644 --- a/spec/cc/engine/analyzers/php/main_spec.rb +++ b/spec/cc/engine/analyzers/php/main_spec.rb @@ -41,7 +41,7 @@ "path" => "foo.php", "lines" => { "begin" => 2, "end" => 6 }, }) - expect(json["remediation_points"]).to eq(176000) + expect(json["remediation_points"]).to eq(3450000) expect(json["other_locations"]).to eq([ {"path" => "foo.php", "lines" => { "begin" => 10, "end" => 14} }, ]) @@ -61,7 +61,7 @@ end def printed_issue - issue = {"type":"issue","check_name":"Identical code","description":"Similar code found in 1 other location","categories":["Duplication"],"location":{"path":"foo.php","lines":{"begin":2,"end":6}},"remediation_points":176000,"other_locations":[{"path":"foo.php","lines":{"begin":10,"end":14}}],"content":{"body": read_up}} + issue = {"type":"issue","check_name":"Identical code","description":"Similar code found in 1 other location","categories":["Duplication"],"location":{"path":"foo.php","lines":{"begin":2,"end":6}},"remediation_points":3450000,"other_locations":[{"path":"foo.php","lines":{"begin":10,"end":14}}],"content":{"body": read_up}} issue.to_json + "\0\n" end diff --git a/spec/cc/engine/analyzers/python/main_spec.rb b/spec/cc/engine/analyzers/python/main_spec.rb index c824f8f8..275c7d28 100644 --- a/spec/cc/engine/analyzers/python/main_spec.rb +++ b/spec/cc/engine/analyzers/python/main_spec.rb @@ -28,7 +28,7 @@ "path" => "foo.py", "lines" => { "begin" => 1, "end" => 1 }, }) - expect(json["remediation_points"]).to eq(54000) + expect(json["remediation_points"]).to eq(4000000) expect(json["other_locations"]).to eq([ {"path" => "foo.py", "lines" => { "begin" => 2, "end" => 2} }, {"path" => "foo.py", "lines" => { "begin" => 3, "end" => 3} } diff --git a/spec/cc/engine/analyzers/ruby/main_spec.rb b/spec/cc/engine/analyzers/ruby/main_spec.rb index 4f6bfe60..876675d3 100644 --- a/spec/cc/engine/analyzers/ruby/main_spec.rb +++ b/spec/cc/engine/analyzers/ruby/main_spec.rb @@ -38,7 +38,7 @@ "path" => "foo.rb", "lines" => { "begin" => 1, "end" => 5 }, }) - expect(json["remediation_points"]).to eq(360000) + expect(json["remediation_points"]).to eq(3300000) expect(json["other_locations"]).to eq([ {"path" => "foo.rb", "lines" => { "begin" => 9, "end" => 13} }, ]) diff --git a/spec/cc/engine/analyzers/violation_spec.rb b/spec/cc/engine/analyzers/violation_spec.rb new file mode 100644 index 00000000..1c133a17 --- /dev/null +++ b/spec/cc/engine/analyzers/violation_spec.rb @@ -0,0 +1,59 @@ +RSpec.describe CC::Engine::Analyzers::Violation, in_tmpdir: true do + include AnalyzerSpecHelpers + + describe "#calculate_points" do + context "when issue mass exceeds threshold" do + it "calculates mass overage points" do + language = stub_language(base_points: 100, points_per: 5, mass_threshold: 10) + issue = double(:issue, mass: 15) + hashes = [] + + expected_points = 100 + 5 * (issue.mass - 10) + + violation = CC::Engine::Analyzers::Violation.new(language, issue, hashes) + points = violation.calculate_points + + expect(points).to eq(expected_points) + end + end + + context "when issue mass is less than threshold" do + it "uses default" do + language = stub_language(base_points: 100, points_per: 5, mass_threshold: 18) + issue = double(:issue, mass: 15) + hashes = [] + + expected_points = CC::Engine::Analyzers::Violation::DEFAULT_POINTS + + violation = CC::Engine::Analyzers::Violation.new(language, issue, hashes) + points = violation.calculate_points + + expect(points).to eq(CC::Engine::Analyzers::Violation::DEFAULT_POINTS) + end + end + + context "when issue mass equals threshold" do + it "calculates remediation points" do + language = stub_language(base_points: 100, points_per: 5, mass_threshold: 18) + issue = double(:issue, mass: language.mass_threshold) + hashes = [] + + expected_points = 100 + 5 * (issue.mass - language.mass_threshold) + + violation = CC::Engine::Analyzers::Violation.new(language, issue, hashes) + points = violation.calculate_points + + expect(points).to eq(expected_points) + end + end + + def stub_language(args) + double( + :language, + base_points: args[:base_points], + points_per: args[:points_per], + mass_threshold: args[:mass_threshold] + ) + end + end +end From 474cbc17737d2d4f155ce7edffa2110741f37628 Mon Sep 17 00:00:00 2001 From: ABaldwinHunter Date: Tue, 29 Dec 2015 19:34:38 -0500 Subject: [PATCH 2/2] Refactor specs and rename points_per to points_per_overage --- lib/cc/engine/analyzers/analyzer_base.rb | 6 +++--- lib/cc/engine/analyzers/ruby/main.rb | 2 +- lib/cc/engine/analyzers/violation.rb | 20 +++++++++++++++----- spec/cc/engine/analyzers/violation_spec.rb | 15 +++------------ 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/lib/cc/engine/analyzers/analyzer_base.rb b/lib/cc/engine/analyzers/analyzer_base.rb index b3db9b25..366132a0 100644 --- a/lib/cc/engine/analyzers/analyzer_base.rb +++ b/lib/cc/engine/analyzers/analyzer_base.rb @@ -13,7 +13,7 @@ class Base DEFAULT_MASS_THRESHOLD = 28 BASE_POINTS = 1_500_000 - POINTS_PER = 50_000 + POINTS_PER_OVERAGE = 50_000 def initialize(engine_config:) @engine_config = engine_config @@ -41,8 +41,8 @@ def base_points self.class::BASE_POINTS end - def points_per - self.class::POINTS_PER + def points_per_overage + self.class::POINTS_PER_OVERAGE end private diff --git a/lib/cc/engine/analyzers/ruby/main.rb b/lib/cc/engine/analyzers/ruby/main.rb index 506d40d2..36ec8aee 100644 --- a/lib/cc/engine/analyzers/ruby/main.rb +++ b/lib/cc/engine/analyzers/ruby/main.rb @@ -18,7 +18,7 @@ class Main < CC::Engine::Analyzers::Base ] DEFAULT_MASS_THRESHOLD = 18 - POINTS_PER = 100_000 + POINTS_PER_OVERAGE = 100_000 TIMEOUT = 300 private diff --git a/lib/cc/engine/analyzers/violation.rb b/lib/cc/engine/analyzers/violation.rb index aae1752e..2abc8c9b 100644 --- a/lib/cc/engine/analyzers/violation.rb +++ b/lib/cc/engine/analyzers/violation.rb @@ -11,9 +11,7 @@ class Violation DEFAULT_POINTS = 1_500_000 def initialize(language, issue, hashes) - @base_points = language.base_points - @points_per = language.points_per - @threshold = language.mass_threshold + @language = language @issue = issue @hashes = hashes end @@ -38,7 +36,7 @@ def report_name def calculate_points if issue.mass >= threshold - base_points + (overage * points_per) + base_points + (overage * points_per_overage) else DEFAULT_POINTS end @@ -46,7 +44,19 @@ def calculate_points private - attr_reader :base_points, :points_per, :threshold, :hashes + attr_reader :language, :hashes + + def base_points + language.base_points + end + + def points_per_overage + language.points_per_overage + end + + def threshold + language.mass_threshold + end def current_sexp @location ||= sorted_hashes.first diff --git a/spec/cc/engine/analyzers/violation_spec.rb b/spec/cc/engine/analyzers/violation_spec.rb index 1c133a17..4f9f74f1 100644 --- a/spec/cc/engine/analyzers/violation_spec.rb +++ b/spec/cc/engine/analyzers/violation_spec.rb @@ -4,7 +4,7 @@ describe "#calculate_points" do context "when issue mass exceeds threshold" do it "calculates mass overage points" do - language = stub_language(base_points: 100, points_per: 5, mass_threshold: 10) + language = double(:language, base_points: 100, points_per_overage: 5, mass_threshold: 10) issue = double(:issue, mass: 15) hashes = [] @@ -19,7 +19,7 @@ context "when issue mass is less than threshold" do it "uses default" do - language = stub_language(base_points: 100, points_per: 5, mass_threshold: 18) + language = double(:language, base_points: 100, points_per_overage: 5, mass_threshold: 18) issue = double(:issue, mass: 15) hashes = [] @@ -34,7 +34,7 @@ context "when issue mass equals threshold" do it "calculates remediation points" do - language = stub_language(base_points: 100, points_per: 5, mass_threshold: 18) + language = double(:language, base_points: 100, points_per_overage: 5, mass_threshold: 18) issue = double(:issue, mass: language.mass_threshold) hashes = [] @@ -46,14 +46,5 @@ expect(points).to eq(expected_points) end end - - def stub_language(args) - double( - :language, - base_points: args[:base_points], - points_per: args[:points_per], - mass_threshold: args[:mass_threshold] - ) - end end end