Fix #124 Finish implementing command chaining, and make it work for DMD

6 files changed, 292 insertions, 73 deletions

diff --git a/ale_linters/d/dmd.vim b/ale_linters/d/dmd.vim
index 0a6e9bf3..0c665d71 100644
--- a/ale_linters/d/dmd.vim
+++ b/ale_linters/d/dmd.vim
@@ -1,24 +1,54 @@
 " Author: w0rp <devw0rp@gmail.com>
 " Description: "dmd for D files"
 
-" A function for finding the dmd-wrapper script in the Vim runtime paths
-function! s:FindWrapperScript() abort
-    for l:parent in split(&runtimepath, ',')
-        " Expand the path to deal with ~ issues.
-        let l:path = expand(l:parent . '/' . 'dmd-wrapper')
-
-        if filereadable(l:path)
-            return l:path
+function! s:FindDUBConfig(buffer) abort
+    " Find a DUB configuration file in ancestor paths.
+    " The most DUB-specific names will be tried first.
+    for l:possible_filename in ['dub.sdl', 'dub.json', 'package.json']
+        let l:dub_file = ale#util#FindNearestFile(a:buffer, l:possible_filename)
+
+        if !empty(l:dub_file)
+            return l:dub_file
         endif
     endfor
+
+    return ''
 endfunction
 
-function! ale_linters#d#dmd#GetCommand(buffer) abort
-    let l:wrapper_script = s:FindWrapperScript()
+function! ale_linters#d#dmd#DUBCommand(buffer) abort
+    " If we can't run dub, then skip this command.
+    if !executable('dub')
+        " Returning an empty string skips to the DMD command.
+        return ''
+    endif
+
+    let l:dub_file = s:FindDUBConfig(a:buffer)
 
-    let l:command = l:wrapper_script . ' -o- -vcolumns -c'
+    if empty(l:dub_file)
+        return ''
+    endif
+
+    " To support older dub versions, we just change the directory to
+    " the directory where we found the dub config, and then run `dub describe`
+    " from that directory.
+    return 'cd ' . fnameescape(fnamemodify(l:dub_file, ':h'))
+    \   . ' && dub describe --import-paths'
+endfunction
+
+function! ale_linters#d#dmd#DMDCommand(buffer, dub_output) abort
+    let l:import_list = []
+
+    " Build a list of import paths generated from DUB, if available.
+    for l:line in a:dub_output
+        if !empty(l:line)
+            " The arguments must be '-Ifilename', not '-I filename'
+            call add(l:import_list, '-I' . fnameescape(l:line))
+        endif
+    endfor
 
-    return l:command
+    return g:ale#util#stdin_wrapper . ' .d dmd '
+    \   . join(l:import_list)
+    \   . ' -o- -vcolumns -c'
 endfunction
 
 function! ale_linters#d#dmd#Handle(buffer, lines) abort
@@ -57,8 +87,10 @@ endfunction
 
 call ale#linter#Define('d', {
 \   'name': 'dmd',
-\   'output_stream': 'stderr',
 \   'executable': 'dmd',
-\   'command_callback': 'ale_linters#d#dmd#GetCommand',
+\   'command_chain': [
+\       {'callback': 'ale_linters#d#dmd#DUBCommand', 'output_stream': 'stdout'},
+\       {'callback': 'ale_linters#d#dmd#DMDCommand', 'output_stream': 'stderr'},
+\   ],
 \   'callback': 'ale_linters#d#dmd#Handle',
 \})
diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim
index 08852e00..25be0457 100644
--- a/autoload/ale/engine.vim
+++ b/autoload/ale/engine.vim
@@ -53,6 +53,11 @@ function! ale#engine#ClearJob(job) abort
 endfunction
 
 function! s:StopPreviousJobs(buffer, linter) abort
+    if !has_key(g:ale_buffer_info, a:buffer)
+        " Do nothing if we didn't run anything for the buffer.
+        return
+    endif
+
     let l:new_job_list = []
 
     for l:job in g:ale_buffer_info[a:buffer].job_list
@@ -309,25 +314,48 @@ endfunction
 
 function! s:InvokeChain(buffer, linter, chain_index, input) abort
     let l:output_stream = get(a:linter, 'output_stream', 'stdout')
+    let l:chain_index = a:chain_index
+    let l:input = a:input
 
     if has_key(a:linter, 'command_chain')
-        " Run a chain of commands, one asychronous command after the other,
-        " so that many programs can be run in a sequence.
-        let l:chain_item = a:linter.command_chain[a:chain_index]
+        while l:chain_index < len(a:linter.command_chain)
+            " Run a chain of commands, one asychronous command after the other,
+            " so that many programs can be run in a sequence.
+            let l:chain_item = a:linter.command_chain[l:chain_index]
+
+            " The chain item can override the output_stream option.
+            if has_key(l:chain_item, 'output_stream')
+                let l:output_stream = l:chain_item.output_stream
+            endif
 
-        " The chain item can override the output_stream option.
-        if has_key(l:chain_item)
-            let l:output_stream = l:chain_item.output_stream
-        endif
+            if l:chain_index == 0
+                " The first callback in the chain takes only a buffer number.
+                let l:command = ale#util#GetFunction(l:chain_item.callback)(
+                \   a:buffer
+                \)
+            else
+                " The second callback in the chain takes some input too.
+                let l:command = ale#util#GetFunction(l:chain_item.callback)(
+                \   a:buffer,
+                \   l:input
+                \)
+            endif
 
-        let l:callback = ale#util#GetFunction(a:linter.callback)
+            if !empty(l:command)
+                " We hit a command to run, so we'll execute that
+                break
+            endif
 
-        if a:chain_index == 0
-            " The first callback in the chain takes only a buffer number.
-            let l:command = l:callback(a:buffer)
-        else
-            " The second callback in the chain takes some input too.
-            let l:command = l:callback(a:buffer, a:input)
+            " Command chain items can return an empty string to indicate that
+            " a command should be skipped, so we should try the next item
+            " with no input.
+            let l:input = []
+            let l:chain_index += 1
+        endwhile
+
+        if empty(l:command)
+            " Don't run any jobs if the last command is an empty string.
+            return
         endif
     elseif has_key(a:linter, 'command_callback')
         " If there is a callback for generating a command, call that instead.
@@ -340,7 +368,7 @@ function! s:InvokeChain(buffer, linter, chain_index, input) abort
     \   'buffer': a:buffer,
     \   'linter': a:linter,
     \   'output_stream': l:output_stream,
-    \   'next_chain_index': a:chain_index + 1,
+    \   'next_chain_index': l:chain_index + 1,
     \})
 endfunction
 
@@ -406,4 +434,27 @@ function! ale#engine#WaitForJobs(deadline) abort
     " prevents the occasional failure where this function exits after jobs
     " end, but before handlers are run.
     sleep 10ms
+
+    " We must check the buffer data again to see if new jobs started
+    " for command_chain linters.
+    let l:has_new_jobs = 0
+
+    for l:info in values(g:ale_buffer_info)
+        if !empty(l:info.job_list)
+            let l:has_new_jobs = 1
+        endif
+    endfor
+
+    if l:has_new_jobs
+        " We have to wait more. Offset the timeout by the time taken so far.
+        let l:now = system('date +%s%3N') + 0
+        let l:new_deadline = a:deadline - (l:now - l:start_time)
+
+        if l:new_deadline <= 0
+            " Enough time passed already, so stop immediately.
+            throw 'Jobs did not complete on time!'
+        endif
+
+        call ale#engine#WaitForJobs(l:new_deadline)
+    endif
 endfunction
diff --git a/autoload/ale/linter.vim b/autoload/ale/linter.vim
index 823359ce..0a5b9ae3 100644
--- a/autoload/ale/linter.vim
+++ b/autoload/ale/linter.vim
@@ -67,7 +67,37 @@ function! ale#linter#PreProcess(linter) abort
         throw 'Either `executable` or `executable_callback` must be defined'
     endif
 
-    if has_key(a:linter, 'command_callback')
+    if has_key(a:linter, 'command_chain')
+        let l:obj.command_chain = a:linter.command_chain
+
+        if type(l:obj.command_chain) != type([])
+            throw '`command_chain` must be a List'
+        endif
+
+        if empty(l:obj.command_chain)
+            throw '`command_chain` must contain at least one item'
+        endif
+
+        let l:link_index = 0
+
+        for l:link in l:obj.command_chain
+            let l:err_prefix = 'The `command_chain` item ' . l:link_index . ' '
+
+            if !s:IsCallback(get(l:link, 'callback'))
+                throw l:err_prefix . 'must define a `callback` function'
+            endif
+
+            if has_key(l:link, 'output_stream')
+                if type(l:link.output_stream) != type('')
+                \|| index(['stdout', 'stderr', 'both'], l:link.output_stream) < 0
+                    throw l:err_prefix . '`output_stream` flag must be '
+                    \   . "'stdout', 'stderr', or 'both'"
+                endif
+            endif
+
+            let l:link_index += 1
+        endfor
+    elseif has_key(a:linter, 'command_callback')
         let l:obj.command_callback = a:linter.command_callback
 
         if !s:IsCallback(l:obj.command_callback)
diff --git a/dmd-wrapper b/dmd-wrapper
deleted file mode 100755
index 2db358e7..00000000
--- a/dmd-wrapper
+++ /dev/null
@@ -1,43 +0,0 @@
-#!/usr/bin/env bash
-
-# Author: w0rp <devw0rp@gmail.com>, hauleth <lukasz@niemier.pl>
-# Description: This script wraps DMD so we can get something which is capable of reading
-#   D code from stdin.
-
-set -eu
-
-check_dubfile() {
-  [[ -e "$1/dub.json" || -e "$1/dub.sdl" || -e "$1/package.json" ]]
-}
-
-traverse() {
-  path=$(pwd)
-  while [ "$path" != "/" ] \
-    && ! check_dubfile "$path"
-  do
-    path=$(dirname "$path")
-  done
-
-  echo "$path"
-}
-
-import_line_options() {
-  root="$(traverse)"
-
-  if check_dubfile "$root"
-  then
-    dub describe --root="$root" --import-paths | awk '{ print "-I" $0 }'
-  else
-    echo -n
-  fi
-}
-
-temp_dir=$(mktemp -d 2>/dev/null || mktemp -d -t 'ale_linters')
-temp_file="$temp_dir/file.d"
-trap 'rm -r "$temp_dir"' EXIT
-
-while read -r; do
-  echo "$REPLY" >> "$temp_file"
-done
-
-dmd $(import_line_options) "$@" "$temp_file"
diff --git a/test/test_command_chain.vader b/test/test_command_chain.vader
new file mode 100644
index 00000000..f471e159
--- /dev/null
+++ b/test/test_command_chain.vader
@@ -0,0 +1,61 @@
+Before:
+  let g:linter_output = []
+  let g:first_echo_called = 0
+  let g:second_echo_called = 0
+  let g:final_callback_called = 0
+
+  function! CollectResults(buffer, output)
+    let g:final_callback_called = 1
+    let g:linter_output = a:output
+    return []
+  endfunction
+  function! RunFirstEcho(buffer)
+    let g:first_echo_called = 1
+
+    return 'echo foo'
+  endfunction
+  function! RunSecondEcho(buffer, output)
+    let g:second_echo_called = 1
+
+    return 'echo bar'
+  endfunction
+
+  call ale#linter#Define('foobar', {
+  \ 'name': 'testlinter',
+  \ 'callback': 'CollectResults',
+  \ 'executable': 'echo',
+  \ 'command_chain': [
+  \   {
+  \     'callback': 'RunFirstEcho',
+  \     'output_stream': 'stdout',
+  \   },
+  \   {
+  \     'callback': 'RunSecondEcho',
+  \     'output_stream': 'stdout',
+  \   },
+  \ ],
+  \})
+
+After:
+  unlet! g:first_echo_called
+  unlet! g:second_echo_called
+  unlet! g:final_callback_called
+  unlet! g:linter_output
+  let g:ale_buffer_info = {}
+  call ale#linter#Reset()
+  delfunction CollectResults
+  delfunction RunFirstEcho
+  delfunction RunSecondEcho
+
+Given foobar (Some imaginary filetype):
+  anything
+
+Execute(Check the results of running the chain):
+  AssertEqual 'foobar', &filetype
+  call ale#Lint()
+  call ale#engine#WaitForJobs(2000)
+
+  Assert g:first_echo_called, 'The first chain item was not called'
+  Assert g:second_echo_called, 'The second chain item was not called'
+  Assert g:final_callback_called, 'The final callback was not called'
+  AssertEqual ['bar'], g:linter_output
diff --git a/test/test_linter_defintion_processing.vader b/test/test_linter_defintion_processing.vader
index 52d40530..4b5c6a76 100644
--- a/test/test_linter_defintion_processing.vader
+++ b/test/test_linter_defintion_processing.vader
@@ -1,3 +1,9 @@
+Before:
+  let g:linter = {}
+
+After:
+  unlet g:linter
+
 Execute (PreProcess should throw when the linter object is not a Dictionary):
   AssertThrows call ale#linter#PreProcess('')
   AssertEqual 'The linter object must be a Dictionary', g:vader_exception
@@ -123,3 +129,85 @@ Execute (PreProcess should accept a 'both' output_stream):
   \ 'command': 'echo',
   \ 'output_stream': 'both',
   \})
+
+Execute(PreProcess should complain if the command_chain is not a List):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': 'x',
+  \}
+  AssertThrows call ale#linter#PreProcess(g:linter)
+  AssertEqual '`command_chain` must be a List', g:vader_exception
+
+Execute(PreProcess should complain if the command_chain is empty):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [],
+  \}
+  AssertThrows call ale#linter#PreProcess(g:linter)
+  AssertEqual '`command_chain` must contain at least one item', g:vader_exception
+
+Execute(PreProcess should complain if the command_chain has no callback):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [{}],
+  \}
+  AssertThrows call ale#linter#PreProcess(g:linter)
+  AssertEqual 'The `command_chain` item 0 must define a `callback` function', g:vader_exception
+
+Execute(PreProcess should complain if the command_chain callback is not a function):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [{'callback': 2}],
+  \}
+  AssertThrows call ale#linter#PreProcess(g:linter)
+  AssertEqual 'The `command_chain` item 0 must define a `callback` function', g:vader_exception
+
+Execute(PreProcess should accept a chain with one callback):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [{'callback': 'foo'}],
+  \}
+  call ale#linter#PreProcess(g:linter)
+
+Execute(PreProcess should complain about invalid output_stream values in the chain):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [{'callback': 'foo', 'output_stream': ''}],
+  \}
+  AssertThrows call ale#linter#PreProcess(g:linter)
+  AssertEqual "The `command_chain` item 0 `output_stream` flag must be 'stdout', 'stderr', or 'both'", g:vader_exception
+
+Execute(PreProcess should complain about valid output_stream values in the chain):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [{'callback': 'foo', 'output_stream': 'stdout'}],
+  \}
+  call ale#linter#PreProcess(g:linter)
+  let g:linter.command_chain[0].output_stream = 'stderr'
+  call ale#linter#PreProcess(g:linter)
+  let g:linter.command_chain[0].output_stream = 'both'
+  call ale#linter#PreProcess(g:linter)
+
+Execute(PreProcess should complain about invalid chain items at higher indices):
+  let g:linter = {
+  \ 'name': 'x',
+  \ 'callback': 'x',
+  \ 'executable': 'x',
+  \ 'command_chain': [{'callback': 'foo'}, {'callback': 123}],
+  \}
+  AssertThrows call ale#linter#PreProcess(g:linter)
+  AssertEqual 'The `command_chain` item 1 must define a `callback` function', g:vader_exception