add support for parsed line errors in ci and grep related checks

lib/plugins/pre_commit/checks/before_all.rb

@@ -13,7 +13,7 @@ def extra_grep
       end
 
       def message
-        "before(:all) found:\n"
+        "before(:all) found:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/ci.rb

@@ -7,7 +7,9 @@ class Ci < Plugin
 
       def call(_)
         return if system("rake #{Ci::CI_TASK_NAME} --silent")
-        "your test suite has failed, for the full output run `#{CI_TASK_NAME}`"
+        PreCommit::ErrorList.new(
+          "your test suite has failed, for the full output run `#{CI_TASK_NAME}`"
+        )
       end
 
       def self.description

lib/plugins/pre_commit/checks/console_log.rb

@@ -13,7 +13,7 @@ def extra_grep
       end
 
       def message
-        "console.log found:\n"
+        "console.log found:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/debugger.rb

@@ -9,7 +9,7 @@ def files_filter(staged_files)
       end
 
       def message
-        "debugger statement(s) found:\n"
+        "debugger statement(s) found:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/gemfile_path.rb

@@ -9,7 +9,7 @@ def files_filter(staged_files)
       end
 
       def message
-        "local path found in Gemfile:\n"
+        "local path found in Gemfile:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/merge_conflict.rb

@@ -5,7 +5,7 @@ module Checks
     class MergeConflict < Grep
 
       def message
-        "detected a merge conflict\n"
+        "detected a merge conflict"
       end
 
       def pattern

lib/plugins/pre_commit/checks/pry.rb

@@ -5,7 +5,7 @@ module Checks
     class Pry < Grep
 
       def message
-        "binding.pry found:\n"
+        "binding.pry found:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/rspec_focus.rb

@@ -9,7 +9,7 @@ def files_filter(staged_files)
       end
 
       def message
-        ":focus found in specs:\n"
+        ":focus found in specs:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/ruby_symbol_hashrockets.rb

@@ -9,7 +9,7 @@ def files_filter(staged_files)
       end
 
       def message
-        "detected :symbol => value hashrocket:\n"
+        "detected :symbol => value hashrocket:"
       end
 
       def pattern

lib/plugins/pre_commit/checks/tabs.rb

@@ -5,7 +5,7 @@ module Checks
     class Tabs < Grep
 
       def message
-        "detected tab before initial space:\n"
+        "detected tab before initial space:"
       end
 
       def pattern

lib/pre-commit/checks/grep.rb

@@ -1,4 +1,6 @@
 require 'pre-commit/checks/plugin'
+require 'pre-commit/error_list'
+require 'pre-commit/line'
 require 'shellwords'
 
 module PreCommit
@@ -35,11 +37,25 @@ def call(staged_files)
         return if staged_files.empty?
         errors = `#{grep} #{pattern} #{staged_files.join(" ")}#{extra_grep}`
         return unless $?.success?
-        "#{message}#{errors}"
+        parse_errors(message, errors)
       end
 
     private
 
+      def parse_errors(message, list)
+        result = PreCommit::ErrorList.new(message)
+        result.errors +=
+        list.split(/\n/).map do |line|
+          PreCommit::Line.new(nil, *parse_error(line))
+        end
+        result
+      end
+
+      def parse_error(line)
+        matches = /^([^:]+):([[:digit:]]+):(.*)$/.match(line)
+        matches and matches.captures
+      end
+
       def grep(grep_version = nil)
         grep_version ||= detect_grep_version
         if grep_version =~ /FreeBSD/

lib/pre-commit/checks/plugin.rb

@@ -1,5 +1,6 @@
 require 'plugins/pluginator/extensions/conversions'
 require 'pre-commit/checks/plugin/config_file'
+require 'pre-commit/line'
 
 module PreCommit
   module Checks

lib/pre-commit/error_list.rb

@@ -0,0 +1,23 @@
+require 'pre-commit/line'
+
+module PreCommit
+  class ErrorList < Struct.new :errors
+
+    def initialize(errors = [])
+      case errors
+      when "",nil then errors = []
+      when String then errors = [PreCommit::Line.new(errors)]
+      end
+      super errors
+    end
+
+    def to_a
+      errors.map(&:to_s)
+    end
+
+    def to_s
+      to_a.join("\n")
+    end
+
+  end
+end

lib/pre-commit/line.rb

@@ -0,0 +1,22 @@
+module PreCommit
+  class Line < Struct.new :message, :file, :line, :code
+
+    def to_s
+      result = message.to_s
+      unless empty? file
+        result = "#{result}#{"\n" unless empty?(result)}#{file}"
+        result = "#{result}:#{line}" unless empty? line
+        result = "#{result}:#{code}" unless empty? code
+      end
+      result
+    end
+
+  protected
+
+    def empty?(string)
+      string == nil || string == ""
+    end
+
+  end
+end
+

lib/pre-commit/message.rb

@@ -0,0 +1,17 @@
+module PreCommit
+  class Message < Struct.new :message, :lines
+
+    def to_s
+      result = message
+      result = "#{message}\n#{lines.map(&:to_s).join("\n")}" unless lines.nil?
+      result
+    end
+
+  protected
+
+    def empty?(string)
+      string == nil || string == ""
+    end
+
+  end
+end

lib/pre-commit/runner.rb

@@ -4,6 +4,7 @@
 require 'pre-commit/utils/staged_files'
 require 'pre-commit/configuration'
 require 'pre-commit/list_evaluator'
+require 'pre-commit/error_list'
 
 module PreCommit
   class Runner
@@ -63,18 +64,22 @@ def list_evaluator
     def warnings(list)
       <<-WARNINGS
 pre-commit: Some warnings were raised. These will not stop commit:
-#{list.join("\n")}
+#{errors_to_string(list)}
 WARNINGS
     end
 
     def checks(list)
       <<-ERRORS
 pre-commit: Stopping commit because of errors.
-#{list.join("\n")}
+#{errors_to_string(list)}
 pre-commit: You can bypass this check using `git commit -n`
 
 ERRORS
     end
 
+    def errors_to_string(list)
+      list.map(&:to_s).join("\n")
+    end
+
   end
 end

test/unit/plugins/pre_commit/checks/before_all_test.rb

@@ -21,16 +21,16 @@
   end
 
   it "fails if file contains before(:all)" do
-    subject.call([fixture_file('before_all_spec.rb')]).must_equal(<<-EXPECTED)
-before(:all) found:
-test/files/before_all_spec.rb:2:  before(:all) do
-EXPECTED
+    subject.call([fixture_file('before_all_spec.rb')]).to_a.must_equal([
+      "before(:all) found:",
+      "test/files/before_all_spec.rb:2:  before(:all) do"
+    ])
   end
 
   it "fails if file contains before :all" do
-    subject.call([fixture_file('before_all_spec_2.rb')]).must_equal(<<-EXPECTED)
-before(:all) found:
-test/files/before_all_spec_2.rb:2:  before :all do
-EXPECTED
+    subject.call([fixture_file('before_all_spec_2.rb')]).to_a.must_equal([
+      "before(:all) found:",
+      "test/files/before_all_spec_2.rb:2:  before :all do"
+    ])
   end
 end

test/unit/plugins/pre_commit/checks/ci_test.rb

@@ -12,7 +12,7 @@
 
   it "fails if rake fails" do
     check.stub :system, false do
-      check.call([]).must_equal "your test suite has failed, for the full output run `pre_commit:ci`"
+      check.call([]).to_s.must_equal "your test suite has failed, for the full output run `pre_commit:ci`"
     end
   end
 end

test/unit/plugins/pre_commit/checks/console_log_test.rb

@@ -27,9 +27,9 @@
   end
 
   it "fails if a js file has a console.log" do
-    subject.call([fixture_file('console_log.js')]).must_equal(<<-EXPECTED)
-console.log found:
-test/files/console_log.js:6:    console.log(\"I'm in bar\");
-EXPECTED
+    subject.call([fixture_file('console_log.js')]).to_a.must_equal([
+      "console.log found:",
+      "test/files/console_log.js:6:    console.log(\"I'm in bar\");"
+    ])
   end
 end

test/unit/plugins/pre_commit/checks/debugger_test.rb

@@ -21,10 +21,10 @@
   end
 
   it "fails if file contains debugger" do
-    subject.call([fixture_file('debugger_file.rb')]).must_equal(<<-EXPECTED)
-debugger statement(s) found:
-test/files/debugger_file.rb:3:  	debugger
-EXPECTED
+    subject.call([fixture_file('debugger_file.rb')]).to_a.must_equal([
+      "debugger statement(s) found:",
+      "test/files/debugger_file.rb:3:  \tdebugger"
+    ])
   end
 
   it "Skips checking the Gemfile" do

test/unit/plugins/pre_commit/checks/gemfile_path_test.rb

@@ -39,20 +39,20 @@
     write "Gemfile", <<-RUBY
       gem "foo", :path => "xxxx"
     RUBY
-    check.call(["Gemfile"]).must_equal(<<-EXPECTED)
-local path found in Gemfile:
-Gemfile:1:      gem "foo", :path => "xxxx"
-EXPECTED
+    check.call(["Gemfile"]).to_a.must_equal([
+      "local path found in Gemfile:",
+      'Gemfile:1:      gem "foo", :path => "xxxx"'
+    ])
   end
 
   it "fails if Gemfile contains path:" do
     write "Gemfile", <<-RUBY
       gem "foo", path: "xxxx"
     RUBY
-    check.call(["Gemfile"]).must_equal(<<-EXPECTED)
-local path found in Gemfile:
-Gemfile:1:      gem "foo", path: "xxxx"
-EXPECTED
+    check.call(["Gemfile"]).to_a.must_equal([
+      "local path found in Gemfile:",
+      'Gemfile:1:      gem "foo", path: "xxxx"'
+    ])
   end
 
   it "allows a Gemfile path that is commented out" do

test/unit/plugins/pre_commit/checks/merge_conflict_test.rb

@@ -13,9 +13,9 @@
   end
 
   it "fails if file contains merge conflict" do
-    check.call([fixture_file('merge_conflict.rb')]).must_equal(<<-EXPECTED)
-detected a merge conflict
-test/files/merge_conflict.rb:3:<<<<<<< HEAD
-EXPECTED
+    check.call([fixture_file('merge_conflict.rb')]).to_a.must_equal([
+      "detected a merge conflict",
+      "test/files/merge_conflict.rb:3:<<<<<<< HEAD"
+    ])
   end
 end

test/unit/plugins/pre_commit/checks/pry_test.rb

@@ -13,9 +13,9 @@
   end
 
   it "fails if file contains pry" do
-    check.call([fixture_file('pry_file.rb')]).must_equal(<<-EXPECTED)
-binding.pry found:
-test/files/pry_file.rb:3:    binding.pry
-EXPECTED
+    check.call([fixture_file('pry_file.rb')]).to_a.must_equal([
+      "binding.pry found:",
+      "test/files/pry_file.rb:3:    binding.pry"
+    ])
   end
 end

test/unit/plugins/pre_commit/checks/rspec_focus_test.rb

@@ -17,7 +17,7 @@
   end
 
   it 'fails if focus specified on describe, context or example block using any valid syntax' do
-    check.call([fixture_file('rspec_focus_bad_spec.rb')]).must_equal(<<-EXPECTED)
+    check.call([fixture_file('rspec_focus_bad_spec.rb')]).to_s.must_equal("\
 :focus found in specs:
 test/files/rspec_focus_bad_spec.rb:2:  context 'with old hash syntax', :focus => true do
 test/files/rspec_focus_bad_spec.rb:3:    describe 'focus on describe', :focus => true do
@@ -27,8 +27,7 @@
 test/files/rspec_focus_bad_spec.rb:11:      it 'alerts with focus on example too', focus: true do
 test/files/rspec_focus_bad_spec.rb:16:  context 'with symbols as keys', :focus do
 test/files/rspec_focus_bad_spec.rb:17:    describe 'focus on describe', :focus do
-test/files/rspec_focus_bad_spec.rb:18:      it 'alerts with focus on example too', :focus do
-    EXPECTED
+test/files/rspec_focus_bad_spec.rb:18:      it 'alerts with focus on example too', :focus do")
   end
 
 end

test/unit/plugins/pre_commit/checks/ruby_symbol_hashrockets_test.rb

@@ -13,7 +13,7 @@
   end
 
   it "fails with invalid" do
-    result = check.call([fixture_file('wrong_hashrockets.rb')]).must_equal(<<-EXPECTED)
+    result = check.call([fixture_file('wrong_hashrockets.rb')]).to_s.must_equal("\
 detected :symbol => value hashrocket:
 test/files/wrong_hashrockets.rb:3:gem 'foo', :ref => 'v2.6.0'
 test/files/wrong_hashrockets.rb:5:{ :@test => \"foo_bar\" }
@@ -22,7 +22,6 @@
 test/files/wrong_hashrockets.rb:8:{ :test! => \"foo_bar\" }
 test/files/wrong_hashrockets.rb:9:{ :test? => \"foo_bar\" }
 test/files/wrong_hashrockets.rb:10:{ :test= => \"foo_bar\" }
-test/files/wrong_hashrockets.rb:11:{ :@@test => \"foo_bar\" }
-EXPECTED
+test/files/wrong_hashrockets.rb:11:{ :@@test => \"foo_bar\" }")
   end
 end