Use hashes for violations, simplify everything.

lib/cane/abc_check.rb

@@ -1,7 +1,4 @@
 require 'ripper'
-
-require 'cane/abc_max_violation'
-require 'cane/syntax_violation'
 require 'set'
 
 module Cane
@@ -31,7 +28,7 @@ def find_violations(file_name)
     # Null object for when the file cannot be parsed.
     class InvalidAst < Struct.new(:file_name)
       def violations
-        [SyntaxViolation.new(file_name)]
+        [{file: file_name, description: "Files contained invalid syntax"}]
       end
     end
 
@@ -41,7 +38,12 @@ class RubyAst < Struct.new(:file_name, :max_allowed_complexity,
       def violations
         process_ast(sexps).
           select { |nesting, complexity| complexity > max_allowed_complexity }.
-          map { |x| AbcMaxViolation.new(file_name, x.first, x.last) }
+          map { |x| {
+            file:        file_name,
+            label:       x.first,
+            value:       x.last,
+            description: "Methods exceeded maximum allowed ABC complexity"
+          }}
       end
 
       protected
@@ -119,7 +121,7 @@ def file_names
     end
 
     def order(result)
-      result.sort_by(&:sort_index).reverse
+      result.sort_by {|x| x[:value].to_i }.reverse
     end
 
     def max_allowed_complexity

lib/cane/doc_check.rb

@@ -13,7 +13,12 @@ def find_violations(file_name)
       last_line = ""
       File.open(file_name, 'r:utf-8').lines.map.with_index do |line, number|
         result = if class_definition?(line) && !comment?(last_line)
-          UndocumentedClassViolation.new(file_name, number + 1, line)
+          {
+            file:        file_name,
+            line:        number + 1,
+            label:       extract_class_name(line),
+            description: "Classes are not documented"
+          }
         end
         last_line = line
         result
@@ -31,17 +36,6 @@ def class_definition?(line)
     def comment?(line)
       line =~ /^\s*#/
     end
-  end
-
-  # Value object used by DocCheck.
-  class UndocumentedClassViolation < Struct.new(:file_name, :number, :line)
-    def description
-      "Classes are not documented"
-    end
-
-    def columns
-      ["%s:%i" % [file_name, number], extract_class_name(line)]
-    end
 
     def extract_class_name(line)
       line.match(/class ([^\s;]+)/)[1]

lib/cane/style_check.rb

@@ -1,4 +1,3 @@
-require 'cane/style_violation'
 require 'set'
 
 module Cane
@@ -11,9 +10,12 @@ class StyleCheck < Struct.new(:opts)
     def violations
       file_list.map do |file_path|
         map_lines(file_path) do |line, line_number|
-          violations_for_line(line.chomp).map do |message|
-            StyleViolation.new(file_path, line_number + 1, message)
-          end
+          violations_for_line(line.chomp).map {|message| {
+            file:        file_path,
+            line:        line_number + 1,
+            label:       message,
+            description: "Lines violated style requirements"
+          }}
         end
       end.flatten
     end

lib/cane/threshold_check.rb

@@ -1,5 +1,3 @@
-require 'cane/threshold_violation'
-
 # Configurable check that allows the contents of a file to be compared against
 # a given value.
 class ThresholdCheck < Struct.new(:checks)
@@ -8,7 +6,12 @@ def violations
       value = value_from_file(file)
 
       unless value.send(operator, limit.to_f)
-        ThresholdViolation.new(file, operator, value, limit)
+        {
+          description: 'Quality threshold crossed',
+          label:       "%s is %s, should be %s %s" % [
+            file, operator, value, limit
+          ]
+        }
       end
     end.compact
   end

lib/cane/violation_formatter.rb

@@ -1,53 +1,59 @@
 require 'stringio'
+require 'ostruct'
 
 module Cane
 
   # Computes a string to be displayed as output from an array of violations
   # computed by the checks.
-  class ViolationFormatter < Struct.new(:violations)
-    def to_s
-      return '' if violations.empty?
-
-      grouped_violations.map do |description, violations|
-        format_group_header(description, violations) +
-          format_violations(violations)
-      end.flatten.join("\n") + "\n\n" + totals + "\n\n"
+  class ViolationFormatter
+    attr_reader :violations
+
+    def initialize(violations)
+      @violations = violations.map do |v|
+        v.merge(file_and_line: v[:line] ?
+          "%s:%i" % v.values_at(:file, :line) :
+          v[:file]
+        )
+      end
     end
 
-    protected
+    def to_s
+      return "" if violations.empty?
 
-    def totals
-      "Total Violations: #{violations.count}"
+      violations.group_by {|x| x[:description] }.map do |d, vs|
+        format_group_header(d, vs) +
+          format_violations(vs)
+      end.join("\n") + "\n\n" + totals + "\n\n"
     end
 
     def format_group_header(description, violations)
       ["", "%s (%i):" % [description, violations.length], ""]
     end
 
     def format_violations(violations)
-      column_widths = calculate_columm_widths(violations)
+      columns = [:file_and_line, :label, :value]
+
+      widths = column_widths(violations, columns)
 
-      violations.map do |violation|
-        format_violation(violation, column_widths)
+      violations.map do |v|
+        format_violation(v, widths)
       end
     end
 
-    def format_violation(violation, column_widths)
-      [
-        '  ' + violation.columns.map.with_index { |column, index|
-          "%-#{column_widths[index]}s" % column
-        }.join('  ')
-      ]
+    def column_widths(violations, columns)
+      columns.each_with_object({}) do |column, h|
+        h[column] = violations.map {|v| v[column].to_s.length }.max
+      end
     end
 
-    def calculate_columm_widths(violations)
-      violations.map { |violation|
-        violation.columns.map { |x| x.to_s.length }
-      }.transpose.map(&:max)
+    def format_violation(v, column_widths)
+      '  ' + column_widths.keys.map {|column|
+        v[column].to_s.ljust(column_widths[column])
+      }.join('  ').strip
     end
 
-    def grouped_violations
-      violations.group_by(&:description)
+    def totals
+      "Total Violations: #{violations.length}"
     end
   end
 end

spec/abc_check_spec.rb

@@ -19,8 +19,8 @@ def complex_method(a)
 
     violations = described_class.new(files: file_name, max: 1).violations
     violations.length.should == 1
-    violations[0].should be_instance_of(Cane::AbcMaxViolation)
-    violations[0].columns.should == [file_name, "Harness > complex_method", 2]
+    violations[0].values_at(:file, :label, :value).should ==
+      [file_name, "Harness > complex_method", 2]
   end
 
   it 'sorts violations by complexity' do
@@ -39,20 +39,19 @@ def complex_method(a)
 
     violations = described_class.new(files: file_name, max: 0).violations
     violations.length.should == 2
-    complexities = violations.map(&:complexity)
+    complexities = violations.map {|x| x[:value] }
     complexities.should == complexities.sort.reverse
   end
 
-  it 'creates a SyntaxViolation when code cannot be parsed' do
+  it 'creates a violation when code cannot be parsed' do
     file_name = make_file(<<-RUBY)
       class Harness
     RUBY
 
     violations = described_class.new(files: file_name).violations
     violations.length.should == 1
-    violations[0].should be_instance_of(Cane::SyntaxViolation)
-    violations[0].columns.should == [file_name]
-    violations[0].description.should be_instance_of(String)
+    violations[0][:file].should == file_name
+    violations[0][:description].should be_instance_of(String)
   end
 
   it 'skips declared exclusions' do
@@ -87,9 +86,8 @@ def other_meth
     violations = described_class.new(files: file_name, max: 0,
                                      exclusions: exclusions).violations
     violations.length.should == 1
-    violations[0].should be_instance_of(Cane::AbcMaxViolation)
-    columns = violations[0].columns
-    columns.should == [file_name, "Harness > Nested > other_meth", 1]
+    violations[0].values_at(:file, :label, :value).should ==
+      [file_name, "Harness > Nested > other_meth", 1]
   end
 
   def self.it_should_extract_method_name(method_name, label=method_name)
@@ -104,7 +102,7 @@ def #{method_name}(a)
       RUBY
 
       violations = described_class.new(files: file_name, max: 1).violations
-      violations[0].detail.should == "Harness > #{label}"
+      violations[0][:label].should == "Harness > #{label}"
     end
   end
 

spec/cane_spec.rb

@@ -59,7 +59,7 @@ def complex_method(a)
 
     output, exitstatus = run("--style-glob #{file_name} --style-measure 80")
     exitstatus.should == 0
-    output.should be_empty
+    output.should == ""
   end
 
   it 'allows upper bound of failed checks' do

spec/doc_check_spec.rb

@@ -19,14 +19,12 @@ class << self; end
     violations = described_class.new(files: file_name).violations
     violations.length.should == 2
 
-    violations[0].should be_instance_of(Cane::UndocumentedClassViolation)
-    violations[0].file_name.should == file_name
-    violations[0].number.should == 3
-    violations[0].columns.last.should eq("NoDoc")
+    violations[0].values_at(:file, :line, :label).should == [
+      file_name, 3, "NoDoc"
+    ]
 
-    violations[1].should be_instance_of(Cane::UndocumentedClassViolation)
-    violations[1].file_name.should == file_name
-    violations[1].number.should == 4
-    violations[1].columns.last.should eq("AlsoNoDoc")
+    violations[1].values_at(:file, :line, :label).should == [
+      file_name, 4, "AlsoNoDoc"
+    ]
   end
 end

spec/style_check_spec.rb

@@ -16,14 +16,17 @@
 
     violations = Cane::StyleCheck.new(files: file_name, measure: 80).violations
     violations.length.should == 2
-    violations[0].should be_instance_of(StyleViolation)
   end
 
   it 'skips declared exclusions' do
     file_name = make_file(ruby_with_style_issue)
 
-    violations = Cane::StyleCheck.new(files: file_name, measure: 80,
-                                      exclusions: [file_name]).violations
+    violations = Cane::StyleCheck.new(
+      files:      file_name,
+      measure:    80,
+      exclusions: [file_name]
+    ).violations
+
     violations.length.should == 0
   end
 end

spec/violation_formatter_spec.rb

@@ -4,18 +4,17 @@
 
 describe Cane::ViolationFormatter do
   def violation(description)
-    stub("violation",
-      description: description,
-      columns:     []
-    )
+    {
+      description: description
+    }
   end
 
   it 'includes number of violations in the group header' do
     described_class.new([violation("FAIL")]).to_s.should include("(1)")
   end
 
   it 'includes total number of violations' do
-    violations = [violation("FAIL1"),violation("FAIL2")]
+    violations = [violation("FAIL1"), violation("FAIL2")]
     result = described_class.new(violations).to_s
     result.should include("Total Violations: 2")
   end