Convert built-in hooks to use required_executable option

The new `required_executable` option (and its accompanying
`install_command` option) allows us to remove a ton of boilerplate code
and specs, since it takes care of checking for the existence of the
executable.

This does introduce a behavior change in that all these hooks will now
fail (instead of warn or whatever their previous behavior was) in the
event the executable doesn't exist. This is acceptable since if you have
a hook enabled you shouldn't be ignoring it. If it isn't useful you
should disable it.

Change-Id: Icfe06e844e9ffe5f5234fd7c4629a1d54e02fe93
Reviewed-on: http://gerrit.causes.com/42289
Tested-by: jenkins <jenkins@causes.com>
Reviewed-by: Shane da Silva <shane.dasilva@brigade.com>

Author: sds

CHANGELOG.md

@@ -9,6 +9,7 @@
 * Add `required_executable` and `install_command` options that allow a hook
   to define an executable that must be in the `PATH` in order for it to work,
   and a command the user can use to install the executable if it doesn't exist
+* All built-in hooks will now fail if the required executable is not present
 
 ## 0.17.0
 

config/default.yml

@@ -22,8 +22,8 @@ PostCheckout:
     required: false
     quiet: false
   IndexTags:
-    description: 'Generating tags file from source'
     enabled: false
+    description: 'Generating tags file from source'
 
 # Hooks that are run after `git commit` is executed, before the commit message
 # editor is displayed. These hooks are ideal for syntax checkers, linters, and
@@ -50,18 +50,24 @@ PreCommit:
 
   BerksfileCheck:
     description: 'Checking Berksfile lock'
+    required_executable: 'berks'
+    install_command: 'gem install berks'
     include:
       - 'Berksfile'
       - 'Berksfile.lock'
 
   Brakeman:
     enabled: false
     description: 'Checking for security vulnerabilities'
+    required_executable: 'brakeman'
+    install_command: 'gem install brakeman'
     include:
       - '**/*.rb'
 
   BundleCheck:
     description: 'Checking Gemfile dependencies'
+    required_executable: 'bundle'
+    install_command: 'gem install bundler'
     include:
       - 'Gemfile'
       - 'Gemfile.lock'
@@ -70,22 +76,32 @@ PreCommit:
   ChamberSecurity:
     enabled: false
     description: 'Checking that settings have been secured with Chamber'
+    required_executable: 'chamber'
+    install_command: 'gem install chamber'
     include: 'config/settings.yml'
 
   CoffeeLint:
     description: 'Analyzing with coffeelint'
+    required_executable: 'coffeelint'
+    install_command: 'npm install -g coffeelint'
     include: '**/*.coffee'
 
   CssLint:
     description: 'Analyzing with csslint'
+    required_executable: 'csslint'
+    install_command: 'npm install -g csslint'
     include: '**/*.css'
 
   GoLint:
     description: 'Analyzing with golint'
+    required_executable: 'golint'
+    install_command: 'go get github.com/golang/lint/golint'
     include: '**/*.go'
 
   HamlLint:
     description: 'Analyzing with haml-lint'
+    required_executable: 'haml-lint'
+    install_command: 'gem install haml-lint'
     include: '**/*.haml'
 
   HardTabs:
@@ -105,10 +121,14 @@ PreCommit:
 
   Jscs:
     description: 'Analyzing with JSCS'
+    required_executable: 'jscs'
+    install_command: 'npm install -g jscs'
     include: '**/*.js'
 
   JsHint:
     description: 'Analyzing with JSHint'
+    required_executable: 'jshint'
+    install_command: 'npm install -g jshint'
     include: '**/*.js'
 
   JsonSyntax:
@@ -117,6 +137,8 @@ PreCommit:
 
   JsxHint:
     description: 'Analyzing with JSXHint'
+    required_executable: 'jsxhint'
+    install_command: 'npm install -g jsxhint'
     include: '**/*.jsx'
 
   LocalPathsInGemfile:
@@ -134,6 +156,8 @@ PreCommit:
 
   PythonFlake8:
     description: 'Analyzing with flake8'
+    required_executable: 'flake8'
+    install_command: 'pip install flake8'
     include: '**/*.py'
 
   RailsSchemaUpToDate:
@@ -145,6 +169,8 @@ PreCommit:
 
   Rubocop:
     description: 'Analyzing with Rubocop'
+    required_executable: 'rubocop'
+    install_command: 'gem install rubocop'
     include:
       - '**/*.gemspec'
       - '**/*.rake'
@@ -154,6 +180,8 @@ PreCommit:
 
   ScssLint:
     description: 'Analyzing with scss-lint'
+    required_executable: 'scss-lint'
+    install_command: 'gem install scss-lint'
     include: '**/*.scss'
 
   TrailingWhitespace:
@@ -163,6 +191,8 @@ PreCommit:
 
   TravisLint:
     description: 'Checking Travis CI configuration'
+    required_executable: 'travis-lint'
+    install_command: 'gem install travis-lint'
     include: '.travis.yml'
 
   YamlSyntax:

lib/overcommit/hook/pre_commit/berksfile_check.rb

@@ -5,15 +5,11 @@ class BerksfileCheck < Base
     LOCK_FILE = 'Berksfile.lock'
 
     def run
-      unless in_path?('berks')
-        return :warn, 'Berkshelf not installed -- run `gem install berkshelf`'
-      end
-
       # Ignore if Berksfile.lock is not tracked by git
       ignored_files = execute(%w[git ls-files -o -i --exclude-standard]).stdout.split("\n")
       return :pass if ignored_files.include?(LOCK_FILE)
 
-      result = execute(%w[berks list --quiet])
+      result = execute(%W[#{executable} list --quiet])
       unless result.success?
         return :fail, result.stderr
       end

lib/overcommit/hook/pre_commit/brakeman.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `brakeman` against any modified Ruby/Rails files.
   class Brakeman < Base
     def run
-      unless in_path?('brakeman')
-        return :warn, 'Run `gem install brakeman`'
-      end
-
-      result = execute(%w[brakeman --exit-on-warn --quiet --summary --only-files] +
+      result = execute(%W[#{executable} --exit-on-warn --quiet --summary --only-files] +
                        applicable_files)
       return :pass if result.success?
 

lib/overcommit/hook/pre_commit/bundle_check.rb

@@ -5,22 +5,18 @@ class BundleCheck < Base
     LOCK_FILE = 'Gemfile.lock'
 
     def run
-      unless in_path?('bundle')
-        return :warn, 'bundler not installed -- run `gem install bundler`'
-      end
-
       # Ignore if Gemfile.lock is not tracked by git
       ignored_files = execute(%w[git ls-files -o -i --exclude-standard]).stdout.split("\n")
       return :pass if ignored_files.include?(LOCK_FILE)
 
-      result = execute(%w[bundle check])
+      result = execute(%W[#{executable} check])
       unless result.success?
         return :fail, result.stdout
       end
 
       result = execute(%w[git diff --quiet --] + [LOCK_FILE])
       unless result.success?
-        return :fail, "#{LOCK_FILE} is not up-to-date -- run `bundle check`"
+        return :fail, "#{LOCK_FILE} is not up-to-date -- run `#{executable} check`"
       end
 
       :pass

lib/overcommit/hook/pre_commit/chamber_security.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `chamber secure` against any modified Chamber settings files
   class ChamberSecurity < Base
     def run
-      unless in_path?('chamber')
-        return :warn, 'Run `gem install chamber`'
-      end
-
-      result = execute(%w[chamber secure --echo --files] + applicable_files)
+      result = execute(%W[#{executable} secure --echo --files] + applicable_files)
 
       return :pass if result.stdout.empty?
       [:fail, "These settings appear to need to be secured but were not: #{result.stdout}"]

lib/overcommit/hook/pre_commit/coffee_lint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `coffeelint` against any modified CoffeeScript files.
   class CoffeeLint < Base
     def run
-      unless in_path?('coffeelint')
-        return :warn, 'Run `npm install -g coffeelint`'
-      end
-
-      result = execute(%w[coffeelint --quiet] + applicable_files)
+      result = execute(%W[#{executable} --quiet] + applicable_files)
       return :pass if result.success?
 
       [:fail, result.stdout]

lib/overcommit/hook/pre_commit/css_lint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `csslint` against any modified CSS files.
   class CssLint < Base
     def run
-      unless in_path?('csslint')
-        return :warn, 'csslint not installed -- run `npm install -g csslint`'
-      end
-
-      result = execute(%w[csslint --quiet --format=compact] + applicable_files)
+      result = execute(%W[#{executable} --quiet --format=compact] + applicable_files)
       return :pass if result.stdout !~ /Error - (?!Unknown @ rule)/
 
       [:fail, result.stdout]

lib/overcommit/hook/pre_commit/go_lint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `golint` against any modified Golang files.
   class GoLint < Base
     def run
-      unless in_path?('golint')
-        return :warn, 'Run `go get `github.com/golang/lint/golint`'
-      end
-
-      result = execute(%w[golint] + applicable_files)
+      result = execute([executable] + applicable_files)
       # Unfortunately the exit code is always 0
       return :pass if result.stdout.empty?
 

lib/overcommit/hook/pre_commit/haml_lint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `haml-lint` against any modified HAML files.
   class HamlLint < Base
     def run
-      unless in_path?('haml-lint')
-        return :warn, 'haml-lint not installed -- run `gem install haml-lint`'
-      end
-
-      result = execute(%w[haml-lint] + applicable_files)
+      result = execute([executable] + applicable_files)
       return :pass if result.success?
 
       # Keep lines from the output for files that we actually modified

lib/overcommit/hook/pre_commit/js_hint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `jshint` against any modified JavaScript files.
   class JsHint < Base
     def run
-      unless in_path?('jshint')
-        return :warn, 'jshint not installed -- run `npm install -g jshint`'
-      end
-
-      result = execute(%w[jshint] + applicable_files)
+      result = execute([executable] + applicable_files)
       output = result.stdout
 
       return :pass if output.empty?

lib/overcommit/hook/pre_commit/jscs.rb

@@ -3,11 +3,7 @@ module Overcommit::Hook::PreCommit
   # files.
   class Jscs < Base
     def run
-      unless in_path?('jscs')
-        return :warn, 'jscs not installed -- run `npm install -g jscs`'
-      end
-
-      result = execute(%w[jscs --reporter=inline] + applicable_files)
+      result = execute(%W[#{executable} --reporter=inline] + applicable_files)
       return :pass if result.success?
 
       if result.status == 1

lib/overcommit/hook/pre_commit/jsx_hint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `jsxhint` against any modified JSX files.
   class JsxHint < Base
     def run
-      unless in_path?('jsxhint')
-        return :warn, 'jsxhint not installed -- run `npm install -g jsxhint`'
-      end
-
-      result = execute(%w[jsxhint] + applicable_files)
+      result = execute([executable] + applicable_files)
       output = result.stdout
 
       return :pass if output.empty?

lib/overcommit/hook/pre_commit/python_flake8.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `flake8` against any modified Python files.
   class PythonFlake8 < Base
     def run
-      unless in_path?('flake8')
-        return :warn, 'flake8 not installed -- run `pip install flake8`'
-      end
-
-      result = execute(%w[flake8] + applicable_files)
+      result = execute([executable] + applicable_files)
       return :pass if result.success?
 
       [:fail, result.stdout]

lib/overcommit/hook/pre_commit/rubocop.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `rubocop` against any modified Ruby files.
   class Rubocop < Base
     def run
-      unless in_path?('rubocop')
-        return :warn, 'Rubocop not installed -- run `gem install rubocop`'
-      end
-
-      result = execute(%w[rubocop --format=emacs --force-exclusion] + applicable_files)
+      result = execute(%W[#{executable} --format=emacs --force-exclusion] + applicable_files)
       return :pass if result.success?
 
       output = result.stdout + result.stderr

lib/overcommit/hook/pre_commit/scss_lint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `scss-lint` against any modified SCSS files.
   class ScssLint < Base
     def run
-      unless in_path?('scss-lint')
-        return :warn, 'scss-lint not installed -- run `gem install scss-lint`'
-      end
-
-      result = execute(%w[scss-lint] + applicable_files)
+      result = execute([executable] + applicable_files)
       return :pass if result.success?
 
       # Keep lines from the output for files that we actually modified

lib/overcommit/hook/pre_commit/travis_lint.rb

@@ -2,11 +2,7 @@ module Overcommit::Hook::PreCommit
   # Runs `travis-lint` against any modified Travis CI files.
   class TravisLint < Base
     def run
-      unless in_path?('travis-lint')
-        return :warn, 'Run `gem install travis-lint`'
-      end
-
-      result = execute(%w[travis-lint] + applicable_files)
+      result = execute([executable] + applicable_files)
       return :pass if result.success?
 
       [:fail, result.stdout.strip]