From 72cbcfade1081fd832cd30f4785acfe7f1d07111 Mon Sep 17 00:00:00 2001 From: Dom Corvasce Date: Fri, 10 Mar 2017 13:24:53 +0100 Subject: [PATCH 1/5] Fix #458: Overcommit fails if ESLint fails for whatever reason --- lib/overcommit/hook/pre_commit/es_lint.rb | 14 +++++++++++++- spec/overcommit/hook/pre_commit/es_lint_spec.rb | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/overcommit/hook/pre_commit/es_lint.rb b/lib/overcommit/hook/pre_commit/es_lint.rb index 82891ece..b3bab7bf 100644 --- a/lib/overcommit/hook/pre_commit/es_lint.rb +++ b/lib/overcommit/hook/pre_commit/es_lint.rb @@ -12,6 +12,9 @@ module Overcommit::Hook::PreCommit # enabled: true # command: ['npm', 'run', 'lint', '--', '-f', 'compact'] # + # If ESLint returns some errors, you can bindly pass by + # setting `bindly_pass` option to true. + # # Note: This hook supports only compact format. # # @see http://eslint.org/ @@ -19,15 +22,24 @@ class EsLint < Base def run result = execute(command, args: applicable_files) output = result.stdout.chomp + messages = output.split("\n").grep(/Warning|Error/) + + # Fail for issues relative to the tool used. + if messages.empty? && !result.success? + $stderr.puts result.stderrs + return :fail + end + return :pass if result.success? && output.empty? # example message: # path/to/file.js: line 1, col 0, Error - Error message (ruleName) extract_messages( - output.split("\n").grep(/Warning|Error/), + messages, /^(?(?:\w:)?[^:]+):[^\d]+(?\d+).*?(?Error|Warning)/, lambda { |type| type.downcase.to_sym } ) + end end end diff --git a/spec/overcommit/hook/pre_commit/es_lint_spec.rb b/spec/overcommit/hook/pre_commit/es_lint_spec.rb index b8119298..f60a9c95 100644 --- a/spec/overcommit/hook/pre_commit/es_lint_spec.rb +++ b/spec/overcommit/hook/pre_commit/es_lint_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'stringio' describe Overcommit::Hook::PreCommit::EsLint do let(:config) { Overcommit::ConfigurationLoader.default_configuration } @@ -9,6 +10,20 @@ subject.stub(:applicable_files).and_return(%w[file1.js file2.js]) end + context 'when eslint is unable to run' do + let(:result) { double('result') } + + before do + result.stub(:stderrs).and_return('SyntaxError: Use of const in strict mode.') + result.stub(:stdout).and_return('') + + result.stub(:success?).and_return(false) + subject.stub(:execute).and_return(result) + end + + it { should fail_hook } + end + context 'when eslint exits successfully' do let(:result) { double('result') } From 68cb8199ea535aafd888b8900c0c430722cfda6a Mon Sep 17 00:00:00 2001 From: Dom Corvasce Date: Fri, 10 Mar 2017 14:15:09 +0100 Subject: [PATCH 2/5] Add check for bindly_pass option --- lib/overcommit/hook/pre_commit/es_lint.rb | 2 +- spec/overcommit/hook/pre_commit/es_lint_spec.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/es_lint.rb b/lib/overcommit/hook/pre_commit/es_lint.rb index b3bab7bf..c8a66ba8 100644 --- a/lib/overcommit/hook/pre_commit/es_lint.rb +++ b/lib/overcommit/hook/pre_commit/es_lint.rb @@ -25,7 +25,7 @@ def run messages = output.split("\n").grep(/Warning|Error/) # Fail for issues relative to the tool used. - if messages.empty? && !result.success? + if !config['bindly_pass'] && messages.empty? && !result.success? $stderr.puts result.stderrs return :fail end diff --git a/spec/overcommit/hook/pre_commit/es_lint_spec.rb b/spec/overcommit/hook/pre_commit/es_lint_spec.rb index f60a9c95..4d005c9c 100644 --- a/spec/overcommit/hook/pre_commit/es_lint_spec.rb +++ b/spec/overcommit/hook/pre_commit/es_lint_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'stringio' describe Overcommit::Hook::PreCommit::EsLint do let(:config) { Overcommit::ConfigurationLoader.default_configuration } From b21fcac71976cd5f20bd51eb3b93463e107f9cae Mon Sep 17 00:00:00 2001 From: Dom Corvasce Date: Fri, 10 Mar 2017 14:27:11 +0100 Subject: [PATCH 3/5] Delete extra empty line --- lib/overcommit/hook/pre_commit/es_lint.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/overcommit/hook/pre_commit/es_lint.rb b/lib/overcommit/hook/pre_commit/es_lint.rb index c8a66ba8..cf2db3f9 100644 --- a/lib/overcommit/hook/pre_commit/es_lint.rb +++ b/lib/overcommit/hook/pre_commit/es_lint.rb @@ -39,7 +39,6 @@ def run /^(?(?:\w:)?[^:]+):[^\d]+(?\d+).*?(?Error|Warning)/, lambda { |type| type.downcase.to_sym } ) - end end end From 59f21884ef7743e50314ca9ef85c887815f7f172 Mon Sep 17 00:00:00 2001 From: Dom Corvasce Date: Sat, 11 Mar 2017 14:05:15 +0100 Subject: [PATCH 4/5] Return the stderr output along with exit status --- lib/overcommit/hook/pre_commit/es_lint.rb | 3 +-- spec/overcommit/hook/pre_commit/es_lint_spec.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/es_lint.rb b/lib/overcommit/hook/pre_commit/es_lint.rb index cf2db3f9..e827bdcb 100644 --- a/lib/overcommit/hook/pre_commit/es_lint.rb +++ b/lib/overcommit/hook/pre_commit/es_lint.rb @@ -26,8 +26,7 @@ def run # Fail for issues relative to the tool used. if !config['bindly_pass'] && messages.empty? && !result.success? - $stderr.puts result.stderrs - return :fail + return [:fail, result.stderr] end return :pass if result.success? && output.empty? diff --git a/spec/overcommit/hook/pre_commit/es_lint_spec.rb b/spec/overcommit/hook/pre_commit/es_lint_spec.rb index 4d005c9c..e6cfd3fb 100644 --- a/spec/overcommit/hook/pre_commit/es_lint_spec.rb +++ b/spec/overcommit/hook/pre_commit/es_lint_spec.rb @@ -13,7 +13,7 @@ let(:result) { double('result') } before do - result.stub(:stderrs).and_return('SyntaxError: Use of const in strict mode.') + result.stub(:stderr).and_return('SyntaxError: Use of const in strict mode.') result.stub(:stdout).and_return('') result.stub(:success?).and_return(false) From 106333287210d4c625a53432410becfd158b27d6 Mon Sep 17 00:00:00 2001 From: Dom Corvasce Date: Sun, 12 Mar 2017 12:31:15 +0100 Subject: [PATCH 5/5] Remove bindly_pass option --- lib/overcommit/hook/pre_commit/es_lint.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/overcommit/hook/pre_commit/es_lint.rb b/lib/overcommit/hook/pre_commit/es_lint.rb index e827bdcb..71c71b9f 100644 --- a/lib/overcommit/hook/pre_commit/es_lint.rb +++ b/lib/overcommit/hook/pre_commit/es_lint.rb @@ -12,9 +12,6 @@ module Overcommit::Hook::PreCommit # enabled: true # command: ['npm', 'run', 'lint', '--', '-f', 'compact'] # - # If ESLint returns some errors, you can bindly pass by - # setting `bindly_pass` option to true. - # # Note: This hook supports only compact format. # # @see http://eslint.org/ @@ -24,11 +21,7 @@ def run output = result.stdout.chomp messages = output.split("\n").grep(/Warning|Error/) - # Fail for issues relative to the tool used. - if !config['bindly_pass'] && messages.empty? && !result.success? - return [:fail, result.stderr] - end - + return [:fail, result.stderr] if messages.empty? && !result.success? return :pass if result.success? && output.empty? # example message: