From 16355e7ed7fe404d313a05ed726bf8f4ecb4ead0 Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Wed, 18 Nov 2015 12:20:34 -0500 Subject: [PATCH 1/3] readup: consistent headline capitalization --- config/contents/duplicated_code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/contents/duplicated_code.md b/config/contents/duplicated_code.md index ac3e329d..64571c75 100644 --- a/config/contents/duplicated_code.md +++ b/config/contents/duplicated_code.md @@ -1,4 +1,4 @@ -## Duplicated code +## Duplicated Code Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states: From 440810739345c40755d2cf3c4ff5db16aad54148 Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Wed, 18 Nov 2015 13:01:46 -0500 Subject: [PATCH 2/3] fix & dockerize bin/test this was old & not up to date or useful: I'm co-opting it to run the spec suite in docker to avoid local setup woes. --- bin/test | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bin/test b/bin/test index 3d5d2584..5da597c1 100755 --- a/bin/test +++ b/bin/test @@ -1,7 +1,5 @@ -#!/usr/bin/env ruby +#!/bin/sh +set -ex -module_path = File.expand_path('../../node_modules/esprima', __FILE__) - -print `npm install esprima` unless File.directory?(module_path) - -print `script -q /dev/null bundle exec rake ` +docker build -t codeclimate/codeclimate-duplication . +docker run codeclimate/codeclimate-duplication bundle exec rake From 4af82cc902c25d9b8276292878702cedab71a2bb Mon Sep 17 00:00:00 2001 From: Will Fleming Date: Wed, 18 Nov 2015 12:27:13 -0500 Subject: [PATCH 3/3] put reported issue's mass in read up contents One aspect of the duplication engine that is different from other engines and that can be frustrating is that the main dial you can turn to adjust the results, mass threshold, is a completely invisible value once the issue has been reported to CC (either .com or CLI). As a user, this makes the engine difficult to configure because even though the README documents how to set the mass threshold, the fact that you can't ever see what mass issues have makes the feedback loop between getting results and deciding what your settings should be basically impossible. This is an attempt to remediate this problem by describing generally what mass is, and what its specific value for an issue is, within the read up contents. This makes the value accessible without unduly cluttering the description with that value. --- ...licated_code.md => duplicated_code.md.erb} | 5 ++++ lib/cc/engine/analyzers/violation.rb | 11 ++----- lib/cc/engine/analyzers/violation_read_up.rb | 29 +++++++++++++++++++ lib/cc/engine/duplication.rb | 1 - .../engine/analyzers/javascript/main_spec.rb | 2 +- spec/cc/engine/analyzers/php/main_spec.rb | 2 +- spec/cc/engine/analyzers/python/main_spec.rb | 2 +- spec/cc/engine/analyzers/ruby/main_spec.rb | 2 +- spec/spec_helper.rb | 2 -- spec/support/helpers/read_up_helpers.rb | 12 -------- 10 files changed, 41 insertions(+), 27 deletions(-) rename config/contents/{duplicated_code.md => duplicated_code.md.erb} (78%) create mode 100644 lib/cc/engine/analyzers/violation_read_up.rb delete mode 100644 spec/support/helpers/read_up_helpers.rb diff --git a/config/contents/duplicated_code.md b/config/contents/duplicated_code.md.erb similarity index 78% rename from config/contents/duplicated_code.md rename to config/contents/duplicated_code.md.erb index 64571c75..1a6fe052 100644 --- a/config/contents/duplicated_code.md +++ b/config/contents/duplicated_code.md.erb @@ -6,6 +6,11 @@ Duplicated code can lead to software that is hard to understand and difficult to When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways). +## 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). + ## Refactorings * [Extract Method](http://sourcemaking.com/refactoring/extract-method) diff --git a/lib/cc/engine/analyzers/violation.rb b/lib/cc/engine/analyzers/violation.rb index b562550a..e064e841 100644 --- a/lib/cc/engine/analyzers/violation.rb +++ b/lib/cc/engine/analyzers/violation.rb @@ -1,3 +1,5 @@ +require "cc/engine/analyzers/sexp_lines" +require "cc/engine/analyzers/violation_read_up" require "digest" module CC @@ -80,14 +82,7 @@ def format_sexp(sexp) end def content_body - @_content_body ||= { "body": File.read(read_up_path) } - end - - def read_up_path - relative_path = "../../../../config/contents/duplicated_code.md" - File.expand_path( - File.join(File.dirname(__FILE__), relative_path) - ) + @_content_body ||= { "body": ViolationReadUp.new(issue).contents } end def fingerprint diff --git a/lib/cc/engine/analyzers/violation_read_up.rb b/lib/cc/engine/analyzers/violation_read_up.rb new file mode 100644 index 00000000..27d9367a --- /dev/null +++ b/lib/cc/engine/analyzers/violation_read_up.rb @@ -0,0 +1,29 @@ +require "erb" + +module CC + module Engine + module Analyzers + class ViolationReadUp + def initialize(issue) + @issue = issue + end + + def contents + ERB.new(File.read(template_path)).result(binding) + end + + private + + attr_reader :issue + + TEMPLATE_REL_PATH = "../../../../config/contents/duplicated_code.md.erb" + + def template_path + File.expand_path( + File.join(File.dirname(__FILE__), TEMPLATE_REL_PATH) + ) + end + end + end + end +end diff --git a/lib/cc/engine/duplication.rb b/lib/cc/engine/duplication.rb index 803b5e15..fcc339b5 100644 --- a/lib/cc/engine/duplication.rb +++ b/lib/cc/engine/duplication.rb @@ -6,7 +6,6 @@ require 'cc/engine/analyzers/reporter' require 'cc/engine/analyzers/engine_config' require 'cc/engine/analyzers/sexp' -require 'cc/engine/analyzers/sexp_lines' require 'flay' require 'json' diff --git a/spec/cc/engine/analyzers/javascript/main_spec.rb b/spec/cc/engine/analyzers/javascript/main_spec.rb index 5cb46b8a..ecd3bafc 100644 --- a/spec/cc/engine/analyzers/javascript/main_spec.rb +++ b/spec/cc/engine/analyzers/javascript/main_spec.rb @@ -41,7 +41,7 @@ {"path" => "foo.js", "lines" => { "begin" => 2, "end" => 2} }, {"path" => "foo.js", "lines" => { "begin" => 3, "end" => 3} } ]) - expect(json["content"]).to eq({ "body" => read_up }) + expect(json["content"]["body"]).to match /This issue has a mass of `99`/ expect(json["fingerprint"]).to eq("55ae5d0990647ef496e9e0d315f9727d") end end diff --git a/spec/cc/engine/analyzers/php/main_spec.rb b/spec/cc/engine/analyzers/php/main_spec.rb index cbc3efd6..91b0e015 100644 --- a/spec/cc/engine/analyzers/php/main_spec.rb +++ b/spec/cc/engine/analyzers/php/main_spec.rb @@ -53,7 +53,7 @@ expect(json["other_locations"]).to eq([ {"path" => "foo.php", "lines" => { "begin" => 10, "end" => 14} }, ]) - expect(json["content"]).to eq({ "body" => read_up }) + expect(json["content"]["body"]).to match /This issue has a mass of `44`/ expect(json["fingerprint"]).to eq("667da0e2bab866aa2fe9d014a65d57d9") end end diff --git a/spec/cc/engine/analyzers/python/main_spec.rb b/spec/cc/engine/analyzers/python/main_spec.rb index 4c570594..a0f4366e 100644 --- a/spec/cc/engine/analyzers/python/main_spec.rb +++ b/spec/cc/engine/analyzers/python/main_spec.rb @@ -41,7 +41,7 @@ {"path" => "foo.py", "lines" => { "begin" => 2, "end" => 2} }, {"path" => "foo.py", "lines" => { "begin" => 3, "end" => 3} } ]) - expect(json["content"]).to eq({ "body" => read_up }) + expect(json["content"]["body"]).to match /This issue has a mass of `54`/ expect(json["fingerprint"]).to eq("42b832387c997f54a2012efb2159aefc") end end diff --git a/spec/cc/engine/analyzers/ruby/main_spec.rb b/spec/cc/engine/analyzers/ruby/main_spec.rb index 863520cf..b1099f49 100644 --- a/spec/cc/engine/analyzers/ruby/main_spec.rb +++ b/spec/cc/engine/analyzers/ruby/main_spec.rb @@ -50,7 +50,7 @@ expect(json["other_locations"]).to eq([ {"path" => "foo.rb", "lines" => { "begin" => 9, "end" => 13} }, ]) - expect(json["content"]).to eq({ "body" => read_up }) + expect(json["content"]["body"]).to match /This issue has a mass of `36`/ expect(json["fingerprint"]).to eq("f21b75bbd135ec3ae6638364d5c73762") end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8704c4a2..a090d825 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,6 +14,4 @@ config.order = :random config.disable_monkey_patching! - - config.include ReadUpHelpers end diff --git a/spec/support/helpers/read_up_helpers.rb b/spec/support/helpers/read_up_helpers.rb deleted file mode 100644 index c6a90821..00000000 --- a/spec/support/helpers/read_up_helpers.rb +++ /dev/null @@ -1,12 +0,0 @@ -module ReadUpHelpers - def read_up - File.read(read_up_path) - end - - def read_up_path - relative_path = "../../../config/contents/duplicated_code.md" - File.expand_path( - File.join(File.dirname(__FILE__), relative_path) - ) - end -end