Merge pull request #1850 from rrosenblum/ruby_linters

Update all Ruby linters to work consistently with bundler

10 files changed, 97 insertions, 48 deletions

diff --git a/ale_linters/ruby/brakeman.vim b/ale_linters/ruby/brakeman.vim
index 85cfc184..122e0b5b 100644
--- a/ale_linters/ruby/brakeman.vim
+++ b/ale_linters/ruby/brakeman.vim
@@ -1,8 +1,9 @@
 " Author: Eddie Lebow https://github.com/elebow
 " Description: Brakeman, a static analyzer for Rails security
 
-let g:ale_ruby_brakeman_options =
-\   get(g:, 'ale_ruby_brakeman_options', '')
+call ale#Set('ruby_brakeman_options', '')
+call ale#Set('ruby_brakeman_executable', 'brakeman')
+call ale#Set('ruby_brakeman_options', '')
 
 function! ale_linters#ruby#brakeman#Handle(buffer, lines) abort
     let l:output = []
@@ -33,14 +34,17 @@ function! ale_linters#ruby#brakeman#GetCommand(buffer) abort
         return ''
     endif
 
-    return 'brakeman -f json -q '
+    let l:executable = ale#Var(a:buffer, 'ruby_brakeman_executable')
+
+    return ale#handlers#ruby#EscapeExecutable(l:executable, 'brakeman')
+    \    . ' -f json -q '
     \    . ale#Var(a:buffer, 'ruby_brakeman_options')
     \    . ' -p ' . ale#Escape(l:rails_root)
 endfunction
 
 call ale#linter#Define('ruby', {
 \    'name': 'brakeman',
-\    'executable': 'brakeman',
+\    'executable_callback': ale#VarFunc('ruby_brakeman_executable'),
 \    'command_callback': 'ale_linters#ruby#brakeman#GetCommand',
 \    'callback': 'ale_linters#ruby#brakeman#Handle',
 \    'lint_file': 1,
diff --git a/ale_linters/ruby/rails_best_practices.vim b/ale_linters/ruby/rails_best_practices.vim
index 4ba1f3fe..20cadca8 100644
--- a/ale_linters/ruby/rails_best_practices.vim
+++ b/ale_linters/ruby/rails_best_practices.vim
@@ -22,26 +22,18 @@ function! ale_linters#ruby#rails_best_practices#Handle(buffer, lines) abort
     return l:output
 endfunction
 
-function! ale_linters#ruby#rails_best_practices#GetExecutable(buffer) abort
-    return ale#Var(a:buffer, 'ruby_rails_best_practices_executable')
-endfunction
-
 function! ale_linters#ruby#rails_best_practices#GetCommand(buffer) abort
-    let l:executable = ale_linters#ruby#rails_best_practices#GetExecutable(a:buffer)
-    let l:exec_args = l:executable =~? 'bundle$'
-    \   ? ' exec rails_best_practices'
-    \   : ''
-
     let l:rails_root = ale#ruby#FindRailsRoot(a:buffer)
 
     if l:rails_root is? ''
         return ''
     endif
 
+    let l:executable = ale#Var(a:buffer, 'ruby_rails_best_practices_executable')
     let l:output_file = ale#Has('win32') ? '%t ' : '/dev/stdout '
     let l:cat_file = ale#Has('win32') ? '; type %t' : ''
 
-    return ale#Escape(l:executable) . l:exec_args
+    return ale#handlers#ruby#EscapeExecutable(l:executable, 'rails_best_practices')
     \    . ' --silent -f json --output-file ' . l:output_file
     \    . ale#Var(a:buffer, 'ruby_rails_best_practices_options')
     \    . ale#Escape(l:rails_root)
@@ -50,7 +42,7 @@ endfunction
 
 call ale#linter#Define('ruby', {
 \    'name': 'rails_best_practices',
-\    'executable_callback': 'ale_linters#ruby#rails_best_practices#GetExecutable',
+\    'executable_callback': ale#VarFunc('ruby_rails_best_practices_executable'),
 \    'command_callback': 'ale_linters#ruby#rails_best_practices#GetCommand',
 \    'callback': 'ale_linters#ruby#rails_best_practices#Handle',
 \    'lint_file': 1,
diff --git a/ale_linters/ruby/reek.vim b/ale_linters/ruby/reek.vim
index aa5d8d70..eefc4ecf 100644
--- a/ale_linters/ruby/reek.vim
+++ b/ale_linters/ruby/reek.vim
@@ -3,6 +3,8 @@
 
 call ale#Set('ruby_reek_show_context', 0)
 call ale#Set('ruby_reek_show_wiki_link', 0)
+call ale#Set('ruby_reek_options', '')
+call ale#Set('ruby_reek_executable', 'reek')
 
 function! ale_linters#ruby#reek#VersionCheck(buffer) abort
     " If we have previously stored the version number in a cache, then
@@ -12,18 +14,23 @@ function! ale_linters#ruby#reek#VersionCheck(buffer) abort
         return ''
     endif
 
-    return 'reek --version'
+    let l:executable = ale#Var(a:buffer, 'ruby_reek_executable')
+
+    return ale#handlers#ruby#EscapeExecutable(l:executable, 'reek')
+    \   . ' --version'
 endfunction
 
 function! ale_linters#ruby#reek#GetCommand(buffer, version_output) abort
     let l:version = ale#semver#GetVersion('reek', a:version_output)
+    let l:executable = ale#Var(a:buffer, 'ruby_reek_executable')
 
     " Tell reek what the filename is if the version of reek is new enough.
     let l:display_name_args = ale#semver#GTE(l:version, [5, 0, 0])
     \   ? ' --stdin-filename %s'
     \   : ''
 
-    return 'reek -f json --no-progress --no-color'
+    return ale#handlers#ruby#EscapeExecutable(l:executable, 'reek')
+    \   . ' -f json --no-progress --no-color'
     \   . l:display_name_args
 endfunction
 
@@ -62,7 +69,7 @@ endfunction
 
 call ale#linter#Define('ruby', {
 \   'name': 'reek',
-\   'executable': 'reek',
+\   'executable_callback': ale#VarFunc('ruby_reek_executable'),
 \   'command_chain': [
 \       {'callback': 'ale_linters#ruby#reek#VersionCheck'},
 \       {'callback': 'ale_linters#ruby#reek#GetCommand'},
diff --git a/ale_linters/ruby/rubocop.vim b/ale_linters/ruby/rubocop.vim
index 777f457a..58d73407 100644
--- a/ale_linters/ruby/rubocop.vim
+++ b/ale_linters/ruby/rubocop.vim
@@ -2,12 +2,9 @@
 " Description: RuboCop, a code style analyzer for Ruby files
 
 function! ale_linters#ruby#rubocop#GetCommand(buffer) abort
-    let l:executable = ale#handlers#rubocop#GetExecutable(a:buffer)
-    let l:exec_args = l:executable =~? 'bundle$'
-    \   ? ' exec rubocop'
-    \   : ''
+    let l:executable = ale#Var(a:buffer, 'ruby_rubocop_executable')
 
-    return ale#Escape(l:executable) . l:exec_args
+    return ale#handlers#ruby#EscapeExecutable(l:executable, 'rubocop')
     \   . ' --format json --force-exclusion '
     \   . ale#Var(a:buffer, 'ruby_rubocop_options')
     \   . ' --stdin ' . ale#Escape(expand('#' . a:buffer . ':p'))
@@ -55,7 +52,7 @@ endfunction
 
 call ale#linter#Define('ruby', {
 \   'name': 'rubocop',
-\   'executable_callback': 'ale#handlers#rubocop#GetExecutable',
+\   'executable_callback': ale#VarFunc('ruby_rubocop_executable'),
 \   'command_callback': 'ale_linters#ruby#rubocop#GetCommand',
 \   'callback': 'ale_linters#ruby#rubocop#Handle',
 \})
diff --git a/autoload/ale/fixers/rubocop.vim b/autoload/ale/fixers/rubocop.vim
index 0a39ef62..a4613817 100644
--- a/autoload/ale/fixers/rubocop.vim
+++ b/autoload/ale/fixers/rubocop.vim
@@ -1,12 +1,12 @@
+call ale#Set('ruby_rubocop_options', '')
+call ale#Set('ruby_rubocop_executable', 'rubocop')
+
 function! ale#fixers#rubocop#GetCommand(buffer) abort
-    let l:executable = ale#handlers#rubocop#GetExecutable(a:buffer)
-    let l:exec_args = l:executable =~? 'bundle$'
-    \   ? ' exec rubocop'
-    \   : ''
+    let l:executable = ale#Var(a:buffer, 'ruby_rubocop_executable')
     let l:config = ale#path#FindNearestFile(a:buffer, '.rubocop.yml')
     let l:options = ale#Var(a:buffer, 'ruby_rubocop_options')
 
-    return ale#Escape(l:executable) . l:exec_args
+    return ale#handlers#ruby#EscapeExecutable(l:executable, 'rubocop')
     \   . (!empty(l:config) ? ' --config ' . ale#Escape(l:config) : '')
     \   . (!empty(l:options) ? ' ' . l:options : '')
     \   . ' --auto-correct %t'
diff --git a/autoload/ale/handlers/rubocop.vim b/autoload/ale/handlers/rubocop.vim
deleted file mode 100644
index f6367cf5..00000000
--- a/autoload/ale/handlers/rubocop.vim
+++ /dev/null
@@ -1,6 +0,0 @@
-call ale#Set('ruby_rubocop_options', '')
-call ale#Set('ruby_rubocop_executable', 'rubocop')
-
-function! ale#handlers#rubocop#GetExecutable(buffer) abort
-    return ale#Var(a:buffer, 'ruby_rubocop_executable')
-endfunction
diff --git a/autoload/ale/handlers/ruby.vim b/autoload/ale/handlers/ruby.vim
index 110fe156..c28b8b75 100644
--- a/autoload/ale/handlers/ruby.vim
+++ b/autoload/ale/handlers/ruby.vim
@@ -37,3 +37,10 @@ function! ale#handlers#ruby#HandleSyntaxErrors(buffer, lines) abort
     return s:HandleSyntaxError(a:buffer, a:lines)
 endfunction
 
+function! ale#handlers#ruby#EscapeExecutable(executable, bundle_exec) abort
+    let l:exec_args = a:executable =~? 'bundle'
+    \   ? ' exec ' . a:bundle_exec
+    \   : ''
+
+    return ale#Escape(a:executable) . l:exec_args
+endfunction
diff --git a/doc/ale-ruby.txt b/doc/ale-ruby.txt
index ec7b07cf..810a1d45 100644
--- a/doc/ale-ruby.txt
+++ b/doc/ale-ruby.txt
@@ -5,6 +5,15 @@ ALE Ruby Integration                                         *ale-ruby-options*
 ===============================================================================
 brakeman                                                    *ale-ruby-brakeman*
 
+g:ale_ruby_brakeman_executable                 *g:ale_ruby_brakeman_executable*
+                                               *b:ale_ruby_brakeman_executable*
+  Type: String
+  Default: `'brakeman'`
+
+  Override the invoked brakeman binary. Set this to `'bundle'` to invoke
+  `'bundle` `exec` brakeman'.
+
+
 g:ale_ruby_brakeman_options                       *g:ale_ruby_brakeman_options*
                                                   *b:ale_ruby_brakeman_options*
   Type: |String|
@@ -20,10 +29,11 @@ g:ale_ruby_rails_best_practices_executable
                                    *g:ale_ruby_rails_best_practices_executable*
                                    *b:ale_ruby_rails_best_practices_executable*
   Type: String
-  Default: 'rails_best_practices'
+  Default: `'rails_best_practices'`
 
   Override the invoked rails_best_practices binary. Set this to `'bundle'` to
-  invoke `'bundle` `exec` `rails_best_practices'`.
+  invoke `'bundle` `exec` rails_best_practices'.
+
 
 g:ale_ruby_rails_best_practices_options
                                       *g:ale_ruby_rails_best_practices_options*
@@ -37,6 +47,15 @@ g:ale_ruby_rails_best_practices_options
 ===============================================================================
 reek                                                            *ale-ruby-reek*
 
+g:ale_ruby_reek_executable                         *g:ale_ruby_reek_executable*
+                                                   *b:ale_ruby_reek_executable*
+  Type: String
+  Default: `'reek'`
+
+  Override the invoked reek binary. Set this to `'bundle'` to invoke
+  `'bundle` `exec` reek'.
+
+
 g:ale_ruby_reek_show_context                     *g:ale_ruby_reek_show_context*
                                                  *b:ale_ruby_reek_show_context*
   Type: |Number|
@@ -63,8 +82,8 @@ g:ale_ruby_rubocop_executable                   *g:ale_ruby_rubocop_executable*
   Type: String
   Default: `'rubocop'`
 
-  Override the invoked rubocop binary. This is useful for running rubocop
-  from binstubs or a bundle.
+  Override the invoked rubocop binary. Set this to `'bundle'` to invoke
+  `'bundle` `exec` rubocop'.
 
 
 g:ale_ruby_rubocop_options                         *g:ale_ruby_rubocop_options*
diff --git a/test/command_callback/test_brakeman_command_callback.vader b/test/command_callback/test_brakeman_command_callback.vader
index 61be4caf..15dbbe1c 100644
--- a/test/command_callback/test_brakeman_command_callback.vader
+++ b/test/command_callback/test_brakeman_command_callback.vader
@@ -12,7 +12,8 @@ Execute(The brakeman command callback should detect absence of a valid Rails app
 Execute(The brakeman command callback should find a valid Rails app root):
   call ale#test#SetFilename('../ruby_fixtures/valid_rails_app/db/test.rb')
 
-  AssertLinter 'brakeman', 'brakeman -f json -q  -p '
+  AssertLinter 'brakeman', ale#Escape('brakeman')
+  \ . ' -f json -q  -p '
   \ . ale#Escape(ale#path#Simplify(g:dir . '/../ruby_fixtures/valid_rails_app'))
 
 Execute(The brakeman command callback should include configured options):
@@ -20,5 +21,17 @@ Execute(The brakeman command callback should include configured options):
 
   let g:ale_ruby_brakeman_options = '--combobulate'
 
-  AssertLinter 'brakeman', 'brakeman -f json -q --combobulate -p '
+  AssertLinter 'brakeman', ale#Escape('brakeman')
+  \ . ' -f json -q --combobulate -p '
+  \ . ale#Escape(ale#path#Simplify(g:dir . '/../ruby_fixtures/valid_rails_app'))
+
+Execute(Setting bundle appends 'exec brakeman'):
+  call ale#test#SetFilename('../ruby_fixtures/valid_rails_app/db/test.rb')
+
+  let g:ale_ruby_brakeman_executable = 'bundle'
+  let g:ale_ruby_brakeman_options = '--combobulate'
+
+  AssertLinter 'bundle', ale#Escape('bundle')
+  \ . ' exec brakeman'
+  \ . ' -f json -q --combobulate -p '
   \ . ale#Escape(ale#path#Simplify(g:dir . '/../ruby_fixtures/valid_rails_app'))
diff --git a/test/command_callback/test_reek_command_callback.vader b/test/command_callback/test_reek_command_callback.vader
index 059e5e36..963247aa 100644
--- a/test/command_callback/test_reek_command_callback.vader
+++ b/test/command_callback/test_reek_command_callback.vader
@@ -7,8 +7,8 @@ After:
 Execute(The reek callbacks should return the correct default values):
   WithChainResults ['reek 5.0.0']
   AssertLinter 'reek', [
-  \ 'reek --version',
-  \ 'reek -f json --no-progress --no-color --stdin-filename %s',
+  \ ale#Escape('reek') . ' --version',
+  \ ale#Escape('reek') . ' -f json --no-progress --no-color --stdin-filename %s',
   \]
 
   " Try with older versions.
@@ -16,19 +16,35 @@ Execute(The reek callbacks should return the correct default values):
 
   WithChainResults ['reek 4.8.2']
   AssertLinter 'reek', [
-  \ 'reek --version',
-  \ 'reek -f json --no-progress --no-color',
+  \ ale#Escape('reek') . ' --version',
+  \ ale#Escape('reek') . ' -f json --no-progress --no-color',
   \]
 
+Execute(Setting bundle appends 'exec reek'):
+  let g:ale_ruby_reek_executable = 'bundle'
+
+  WithChainResults ['reek 5.0.0']
+  AssertLinter 'bundle', ale#Escape('bundle')
+  \ . ' exec reek'
+  \ . ' -f json --no-progress --no-color --stdin-filename %s',
+
+  " Try with older versions.
+  call ale#semver#ResetVersionCache()
+
+  WithChainResults ['reek 4.8.2']
+  AssertLinter 'bundle', ale#Escape('bundle')
+  \ . ' exec reek'
+  \ . ' -f json --no-progress --no-color'
+
 Execute(The reek version check should be cached):
   WithChainResults ['reek 5.0.0']
   AssertLinter 'reek', [
-  \ 'reek --version',
-  \ 'reek -f json --no-progress --no-color --stdin-filename %s',
+  \ ale#Escape('reek') . ' --version',
+  \ ale#Escape('reek') . ' -f json --no-progress --no-color --stdin-filename %s',
   \]
 
   WithChainResults []
   AssertLinter 'reek', [
   \ '',
-  \ 'reek -f json --no-progress --no-color --stdin-filename %s',
+  \ ale#Escape('reek') . ' -f json --no-progress --no-color --stdin-filename %s',
   \]