Skip to content

Use sexp node size for issue mass, not flay score #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/contents/duplicated_code.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions lib/cc/engine/analyzers/analyzer_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions lib/cc/engine/analyzers/ruby/main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 13 additions & 7 deletions lib/cc/engine/analyzers/violation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/cc/engine/analyzers/violation_read_up.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,7 +14,7 @@ def contents

private

attr_reader :issue
attr_reader :mass

TEMPLATE_REL_PATH = "../../../../config/contents/duplicated_code.md.erb"

Expand Down
6 changes: 5 additions & 1 deletion lib/cc/engine/analyzers/violations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions spec/cc/engine/analyzers/javascript/main_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/cc/engine/analyzers/php/main_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions spec/cc/engine/analyzers/python/main_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
30 changes: 15 additions & 15 deletions spec/cc/engine/analyzers/ruby/main_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down