Fix #626 - Automatically use `cargo check` and `cargo check --all-targets` for cargo versions that are new enough

4 files changed, 190 insertions, 8 deletions

diff --git a/ale_linters/rust/cargo.vim b/ale_linters/rust/cargo.vim
index c887ea79..f41cb4b6 100644
--- a/ale_linters/rust/cargo.vim
+++ b/ale_linters/rust/cargo.vim
@@ -1,7 +1,10 @@
 " Author: Daniel Schemala <istjanichtzufassen@gmail.com>
 " Description: rustc invoked by cargo for rust files
 
-let g:ale_rust_cargo_use_check = get(g:, 'ale_rust_cargo_use_check', 0)
+call ale#Set('rust_cargo_use_check', 1)
+call ale#Set('rust_cargo_check_all_targets', 1)
+
+let s:version_cache = {}
 
 function! ale_linters#rust#cargo#GetCargoExecutable(bufnr) abort
     if ale#path#FindNearestFile(a:bufnr, 'Cargo.toml') isnot# ''
@@ -13,18 +16,70 @@ function! ale_linters#rust#cargo#GetCargoExecutable(bufnr) abort
     endif
 endfunction
 
-function! ale_linters#rust#cargo#GetCommand(buffer) abort
-    let l:command = ale#Var(a:buffer, 'rust_cargo_use_check')
+function! ale_linters#rust#cargo#VersionCheck(buffer) abort
+    if has_key(s:version_cache, 'cargo')
+        return ''
+    endif
+
+    return 'cargo --version'
+endfunction
+
+function! s:GetVersion(executable, output) abort
+    let l:version = get(s:version_cache, a:executable, [])
+
+    for l:match in ale#util#GetMatches(a:output, '\v\d+\.\d+\.\d+')
+        let l:version = ale#semver#Parse(l:match[0])
+        let s:version_cache[a:executable] = l:version
+    endfor
+
+    return l:version
+endfunction
+
+function! s:CanUseCargoCheck(buffer, version) abort
+    " Allow `cargo check` to be disabled.
+    if !ale#Var(a:buffer, 'rust_cargo_use_check')
+        return 0
+    endif
+
+    return !empty(a:version)
+    \   && ale#semver#GreaterOrEqual(a:version, [0, 17, 0])
+endfunction
+
+function! s:CanUseAllTargets(buffer, version) abort
+    if !ale#Var(a:buffer, 'rust_cargo_use_check')
+        return 0
+    endif
+
+    if !ale#Var(a:buffer, 'rust_cargo_check_all_targets')
+        return 0
+    endif
+
+    return !empty(a:version)
+    \   && ale#semver#GreaterOrEqual(a:version, [0, 22, 0])
+endfunction
+
+function! ale_linters#rust#cargo#GetCommand(buffer, version_output) abort
+    let l:version = s:GetVersion('cargo', a:version_output)
+    let l:command = s:CanUseCargoCheck(a:buffer, l:version)
     \   ? 'check'
     \   : 'build'
+    let l:all_targets = s:CanUseAllTargets(a:buffer, l:version)
+    \   ? ' --all-targets'
+    \   : ''
 
-    return 'cargo ' . l:command . ' --frozen --message-format=json -q'
+    return 'cargo '
+    \   . l:command
+    \   . l:all_targets
+    \   . ' --frozen --message-format=json -q'
 endfunction
 
 call ale#linter#Define('rust', {
 \   'name': 'cargo',
 \   'executable_callback': 'ale_linters#rust#cargo#GetCargoExecutable',
-\   'command_callback': 'ale_linters#rust#cargo#GetCommand',
+\   'command_chain': [
+\       {'callback': 'ale_linters#rust#cargo#VersionCheck'},
+\       {'callback': 'ale_linters#rust#cargo#GetCommand'},
+\   ],
 \   'callback': 'ale#handlers#rust#HandleRustErrors',
 \   'output_stream': 'both',
 \   'lint_file': 1,
diff --git a/doc/ale-rust.txt b/doc/ale-rust.txt
index e20aea2c..15ffef01 100644
--- a/doc/ale-rust.txt
+++ b/doc/ale-rust.txt
@@ -41,10 +41,22 @@ cargo                                                          *ale-rust-cargo*
 g:ale_rust_cargo_use_check                         *g:ale_rust_cargo_use_check*
                                                    *b:ale_rust_cargo_use_check*
   Type: |Number|
-  Default: `0`
+  Default: `1`
 
-  When set to `1`, this option will cause ALE to use "cargo check" instead of
-  "cargo build". "cargo check" is supported since version 1.16.0 of Rust.
+  When set to `1`, this option will cause ALE to use `cargo check` instead of
+  `cargo build` . `cargo check` is supported since version 1.16.0 of Rust.
+
+  ALE will never use `cargo check` when the version of `cargo` is less than
+  0.17.0.
+
+
+g:ale_rust_cargo_check_all_targets         *g:ale_rust_cargo_check_all_targets*
+                                           *b:ale_rust_cargo_check_all_targets*
+  Type: |Number|
+  Default: `1`
+
+  When set to `1`, ALE will set the `--all-targets` option when `cargo check`
+  is used. See |g:ale_rust_cargo_use_check|,
 
 
 ===============================================================================
diff --git a/test/command_callback/cargo_paths/Cargo.toml b/test/command_callback/cargo_paths/Cargo.toml
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/test/command_callback/cargo_paths/Cargo.toml
diff --git a/test/command_callback/test_cargo_command_callbacks.vader b/test/command_callback/test_cargo_command_callbacks.vader
new file mode 100644
index 00000000..d808e197
--- /dev/null
+++ b/test/command_callback/test_cargo_command_callbacks.vader
@@ -0,0 +1,115 @@
+Before:
+  Save g:ale_rust_cargo_use_check
+  Save g:ale_rust_cargo_check_all_targets
+
+  unlet! g:ale_rust_cargo_use_check
+  unlet! g:ale_cargo_check_all_targets
+
+  runtime ale_linters/rust/cargo.vim
+  call ale#test#SetDirectory('/testplugin/test/command_callback')
+
+  let g:suffix = ' --frozen --message-format=json -q'
+
+After:
+  Restore
+
+  unlet! g:suffix
+
+  call ale#test#RestoreDirectory()
+  call ale#linter#Reset()
+
+Execute(An empty string should be returned for the cargo executable when there's no Cargo.toml file):
+  AssertEqual
+  \ '',
+  \ ale_linters#rust#cargo#GetCargoExecutable(bufnr(''))
+
+Execute(The executable should be returned when there is a Cargo.toml file):
+  call ale#test#SetFilename('cargo_paths/test.rs')
+
+  AssertEqual
+  \ 'cargo',
+  \ ale_linters#rust#cargo#GetCargoExecutable(bufnr(''))
+
+Execute(The VersionCheck function should return the --version command):
+  AssertEqual
+  \ 'cargo --version',
+  \ ale_linters#rust#cargo#VersionCheck(bufnr(''))
+
+Execute(The default command should be correct):
+  AssertEqual
+  \ 'cargo build' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [])
+
+Execute(`cargo check` should be used when the version is new enough):
+  AssertEqual
+  \ 'cargo check' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [
+  \   'cargo 0.17.0 (3423351a5 2017-10-06)',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ 'cargo check' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [])
+
+  AssertEqual '', ale_linters#rust#cargo#VersionCheck(bufnr(''))
+
+Execute(`cargo build` should be used when cargo is too old):
+  AssertEqual
+  \ 'cargo build' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [
+  \   'cargo 0.16.0 (3423351a5 2017-10-06)',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ 'cargo build' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [])
+
+  AssertEqual '', ale_linters#rust#cargo#VersionCheck(bufnr(''))
+
+Execute(`cargo build` should be used when g:ale_rust_cargo_use_check is set to 0):
+  let g:ale_rust_cargo_use_check = 0
+
+  AssertEqual
+  \ 'cargo build' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [
+  \   'cargo 0.24.0 (3423351a5 2017-10-06)',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ 'cargo build' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [])
+
+  AssertEqual '', ale_linters#rust#cargo#VersionCheck(bufnr(''))
+
+Execute(`cargo check --all-targets` should be used when the version is new enough):
+  AssertEqual
+  \ 'cargo check --all-targets' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [
+  \   'cargo 0.22.0 (3423351a5 2017-10-06)',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ 'cargo check --all-targets' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [])
+
+  AssertEqual '', ale_linters#rust#cargo#VersionCheck(bufnr(''))
+
+Execute(--all-targets should not be used when g:ale_rust_cargo_check_all_targets is set to 0):
+  let g:ale_rust_cargo_check_all_targets = 0
+
+  AssertEqual
+  \ 'cargo check' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [
+  \   'cargo 0.22.0 (3423351a5 2017-10-06)',
+  \ ])
+
+  " We should cache the version check
+  AssertEqual
+  \ 'cargo check' . g:suffix,
+  \ ale_linters#rust#cargo#GetCommand(bufnr(''), [])
+
+  AssertEqual '', ale_linters#rust#cargo#VersionCheck(bufnr(''))