From 829f87bc6a63cd442c7e2bf55870faa8804668a4 Mon Sep 17 00:00:00 2001 From: w0rp Date: Sat, 4 Feb 2017 18:30:30 +0000 Subject: [PATCH] Fix #124 Finish implementing command chaining, and make it work for DMD --- ale_linters/d/dmd.vim | 58 +++++++++++--- autoload/ale/engine.vim | 81 +++++++++++++++---- autoload/ale/linter.vim | 32 +++++++- dmd-wrapper | 43 ---------- test/test_command_chain.vader | 61 ++++++++++++++ test/test_linter_defintion_processing.vader | 88 +++++++++++++++++++++ 6 files changed, 291 insertions(+), 72 deletions(-) delete mode 100755 dmd-wrapper create mode 100644 test/test_command_chain.vader diff --git a/ale_linters/d/dmd.vim b/ale_linters/d/dmd.vim index 0a6e9bf..0c665d7 100644 --- a/ale_linters/d/dmd.vim +++ b/ale_linters/d/dmd.vim @@ -1,24 +1,54 @@ " Author: w0rp " 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') +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 filereadable(l:path) - return l:path + 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:command = l:wrapper_script . ' -o- -vcolumns -c' + let l:dub_file = s:FindDUBConfig(a:buffer) - return l:command + 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 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 08852e0..25be045 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) - 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, 'output_stream') + let l:output_stream = l:chain_item.output_stream + endif - let l:callback = ale#util#GetFunction(a:linter.callback) + 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 - 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) + if !empty(l:command) + " We hit a command to run, so we'll execute that + break + endif + + " 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 823359c..0a5b9ae 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 2db358e..0000000 --- a/dmd-wrapper +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env bash - -# Author: w0rp , hauleth -# 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 0000000..f471e15 --- /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 52d4053..4b5c6a7 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