Skip to content

Commit 85f348f

Browse files
committedSep 9, 2015
Merge pull request #17 from codeclimate/ag-contents
Check contents
2 parents 3ddb298 + 1e45e0f commit 85f348f

File tree

7 files changed

+120
-4
lines changed

7 files changed

+120
-4
lines changed
 

‎Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
source 'https://rubygems.org'
22

3+
gem "activesupport", require: false
34
gem "rubocop", require: false
45
gem "rubocop-rspec", require: false
56
gem "pry", require: false

‎Gemfile.lock

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
GEM
22
remote: https://rubygems.org/
33
specs:
4+
activesupport (4.2.4)
5+
i18n (~> 0.7)
6+
json (~> 1.7, >= 1.7.7)
7+
minitest (~> 5.1)
8+
thread_safe (~> 0.3, >= 0.3.4)
9+
tzinfo (~> 1.1)
410
ansi (1.5.0)
511
ast (2.1.0)
612
astrolabe (1.3.1)
713
parser (~> 2.2)
814
builder (3.2.2)
915
coderay (1.1.0)
16+
i18n (0.7.0)
17+
json (1.8.3)
1018
metaclass (0.0.4)
1119
method_source (0.8.2)
1220
minitest (5.8.0)
@@ -35,11 +43,15 @@ GEM
3543
rubocop-rspec (1.3.0)
3644
ruby-progressbar (1.7.5)
3745
slop (3.6.0)
46+
thread_safe (0.3.5)
47+
tzinfo (1.2.2)
48+
thread_safe (~> 0.1)
3849

3950
PLATFORMS
4051
ruby
4152

4253
DEPENDENCIES
54+
activesupport
4355
minitest
4456
minitest-reporters
4557
mocha

‎config/contents/metrics/abc_size.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# ABC Size
2+
3+
ABC Size is a measure of complexity that counts the number of Assignments, Branches (method calls), and Conditionals in your code. It is related to Cyclomatic complexity, which has its origins in the earliest research on static analysis and remains a useful metric of complexity to this day. Cyclomatic complexity correlates the number of potential pathways through a given unit of code with complexity - a unit of code with a lot of potential pathways will have a high cyclomatic complexity score.
4+
5+
Branches -- code that causes decisions to be made -- are the central source of complexity according to cyclomatic complexity. `if` statements, `for` loops, and the predicates in these statements all create execution paths and increase complexity.
6+
7+
Functions with high cyclomatic complexity are harder to test and understand, leading to bugs and higher maintenance costs. Refactor at will.
8+
9+
#### Solutions
10+
11+
* Simplify any branching logic that isn't necessary in your function - collapse redundant branches.
12+
* Extract blocks of logic to private methods where possible.
13+
* Document and follow the pathways through the function and see if you can group functionality to ease in refactoring.
14+
15+
#### Further Reading
16+
17+
* [C2 Wiki: AbcMetric](http://c2.com/cgi/wiki?AbcMetric)
18+
* [SOLID: Part 1 - The Single Responsibility Principle](http://code.tutsplus.com/tutorials/solid-part-1-the-single-responsibility-principle--net-36074)
19+
* ["Clean Code: A Handbook of Agile Software Craftsmanship"](http://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882) - Robert C. Martin
20+
* ["Refactoring: Improving the Design of Existing Code"](http://www.amazon.com/gp/product/0201485672/) - Martin Fowler et. al.
21+
* ["The Single Responsibility Principle"](http://programmer.97things.oreilly.com/wiki/index.php/The_Single_Responsibility_Principle) - Robert C. Martin
22+
* [Your code sucks, let’s fix it (Video)](https://www.youtube.com/watch?v=GtB5DAfOWMQ) — Rafael Dohms
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Class Length
2+
3+
The longer a class is, the harder it is to break down, test, and adequately express with a great name. Classes that are long will grow over time and become harder to manage, so it is usually a worthwhile investment to simplify classes by refactoring into smaller, more discrete units of functionality.
4+
5+
Classes that are too long tend to have too many responsibilities. Though there isn't always a direct correlation between *length* and *number of responsibilities*, it is still a good guideline to "keep it small". From "Clean Code" by Robert "Uncle Bob" Martin:
6+
7+
> "The Single Responsibility Principle (SRP) states that a class or module should have one,
8+
and only one, reason to change. This principle gives us both a definition of responsibility,
9+
and a guidelines for class size. Classes should have one responsibility—one reason to
10+
change." - Robert "Uncle Bob" Martin, "Clean Code"
11+
12+
Seek to create classes with as close to one responsibility as possible and plan to work on those that don’t.
13+
14+
#### Solutions
15+
16+
* Document - What does this class do? Count its responsibilities
17+
* Test - Make sure that your class is tested, and that all of the lines in the explanation above are accounted for.
18+
* Refactor - Take small pieces of the class and pull them out into other collaborator classes. Simplify and clarify the intent of the class by repeating this process until the class has as few responsibilities as possible.
19+
* As you go through this process, think deliberately about the names of your classes: do each of them indicate a clear, single responsibility?
20+
21+
#### Further Reading
22+
23+
* [SOLID: Part 1 - The Single Responsibility Principle](http://code.tutsplus.com/tutorials/solid-part-1-the-single-responsibility-principle--net-36074)
24+
* ["Clean Code: A Handbook of Agile Software Craftsmanship"](http://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882) - Robert C. Martin
25+
* ["Refactoring: Improving the Design of Existing Code"](http://www.amazon.com/gp/product/0201485672/) - Martin Fowler et. al.
26+
* ["The Rule of 30: When is a method, class or subsystem too big?"](http://swreflections.blogspot.com/2012/12/rule-of-30-when-is-method-class-or.html) - Jim Bird
27+
* ["The Single Responsibility Principle"](http://programmer.97things.oreilly.com/wiki/index.php/The_Single_Responsibility_Principle) - Robert C. Martin
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Module Length
2+
3+
The longer a module is, the harder it is to break down, test, and adequately express with a great name. MOdules that are long will grow over time and become harder to manage, so it is usually a worthwhile investment to simplify modules by refactoring into smaller, more discrete units of functionality.
4+
5+
MOdules that are too long tend to have too many responsibilities. Though there isn't always a direct correlation between *length* and *number of responsibilities*, it is still a good guideline to "keep it small". From "Clean Code" by Robert "Uncle Bob" Martin:
6+
7+
> "The Single Responsibility Principle (SRP) states that a class or module should have one,
8+
and only one, reason to change. This principle gives us both a definition of responsibility,
9+
and a guidelines for class size. Classes should have one responsibility—one reason to
10+
change." - Robert "Uncle Bob" Martin, "Clean Code"
11+
12+
Seek to create modules with as close to one responsibility as possible and plan to work on those that don’t.
13+
14+
#### Solutions
15+
16+
* Document - What does this module do? Count its responsibilities
17+
* Test - Make sure that your module is tested, and that all of the lines in the explanation above are accounted for.
18+
* Refactor - Take small pieces of the module and pull them out into other collaborator modules. Simplify and clarify the intent of the module by repeating this process until the module has as few responsibilities as possible.
19+
* As you go through this process, think deliberately about the names of your module: do each of them indicate a clear, single responsibility?
20+
21+
#### Further Reading
22+
23+
* [SOLID: Part 1 - The Single Responsibility Principle](http://code.tutsplus.com/tutorials/solid-part-1-the-single-responsibility-principle--net-36074)
24+
* ["Clean Code: A Handbook of Agile Software Craftsmanship"](http://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882) - Robert C. Martin
25+
* ["Refactoring: Improving the Design of Existing Code"](http://www.amazon.com/gp/product/0201485672/) - Martin Fowler et. al.
26+
* ["The Rule of 30: When is a method, class or subsystem too big?"](http://swreflections.blogspot.com/2012/12/rule-of-30-when-is-method-class-or.html) - Jim Bird
27+
* ["The Single Responsibility Principle"](http://programmer.97things.oreilly.com/wiki/index.php/The_Single_Responsibility_Principle) - Robert C. Martin

‎lib/cc/engine/rubocop.rb

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
require "rubocop"
44
require "rubocop/cop/method_complexity_patch"
55
require "cc/engine/category_parser"
6+
require "active_support"
7+
require "active_support/core_ext"
68

79
module CC
810
module Engine
@@ -83,7 +85,7 @@ def violation_positions(location)
8385
end
8486

8587
def violation_json(violation, local_path)
86-
{
88+
violation_hash = {
8789
type: "Issue",
8890
check_name: "Rubocop/#{violation.cop_name}",
8991
description: violation.message,
@@ -93,7 +95,15 @@ def violation_json(violation, local_path)
9395
path: local_path,
9496
positions: violation_positions(violation.location),
9597
},
96-
}.to_json
98+
}
99+
body = content_body(violation.cop_name)
100+
violation_hash.merge!(content: { body: body }) if body.present?
101+
violation_hash.to_json
102+
end
103+
104+
def content_body(cop_name)
105+
path = File.expand_path("../../../../config/contents/#{cop_name.underscore}.md", __FILE__)
106+
File.exist?(path) ? File.read(path) : nil
97107
end
98108
end
99109
end

‎spec/cc/engine/rubocop_spec.rb

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,27 @@ def method(a,b,c,d,e,f,g)
201201
assert_equal location, result["location"]
202202
end
203203

204+
it "includes issue content when available" do
205+
lines = " test\n" * 101
206+
create_source_file("klass.rb", "class Klass\n#{lines}end")
207+
208+
output = run_engine
209+
210+
assert includes_content_for?(output, "Metrics/ClassLength")
211+
end
212+
204213
def includes_check?(output, cop_name)
205-
issues = output.split("\0").map { |x| JSON.parse(x) }
214+
!!issues(output).detect { |i| i["check_name"] =~ /#{cop_name}$/ }
215+
end
216+
217+
def includes_content_for?(output, cop_name)
218+
issue = issues(output).detect { |i| i["check_name"] =~ /#{cop_name}$/ }
206219

207-
!!issues.detect { |i| i["check_name"] =~ /#{cop_name}$/ }
220+
issue["content"]["body"].present?
221+
end
222+
223+
def issues(output)
224+
issues = output.split("\0").map { |x| JSON.parse(x) }
208225
end
209226

210227
def create_source_file(path, content)

0 commit comments

Comments
 (0)
Please sign in to comment.