From 34e409ea21baa017776b84ec0620eea9f6ec429c Mon Sep 17 00:00:00 2001 From: w0rp Date: Fri, 28 Aug 2020 14:02:05 +0100 Subject: [PATCH] Close #3285 - lint_file is now dynamic `lint_file` can now be computed dynamically with a callback function, which can return a deferred result, as per `ale#command#Run`. This allows linters to dynamically switch between checking files on disk, or checking code on the fly. Some tests have been fixed on Windows. --- autoload/ale/engine.vim | 111 ++++++++++++--- autoload/ale/linter.vim | 10 +- doc/ale.txt | 46 +++--- supported-tools.md | 2 +- test/smoke_test.vader | 2 + test/test_computed_lint_file_values.vader | 134 ++++++++++++++++++ test/test_deferred_command_string.vader | 6 +- test/test_linter_defintion_processing.vader | 15 +- .../test_swiftlint_executable_detection.vader | 12 +- 9 files changed, 276 insertions(+), 62 deletions(-) create mode 100644 test/test_computed_lint_file_values.vader diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index b00ec453..ae0354b8 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -417,7 +417,7 @@ function! s:RunJob(command, options) abort let l:buffer = a:options.buffer let l:linter = a:options.linter let l:output_stream = a:options.output_stream - let l:read_buffer = a:options.read_buffer + let l:read_buffer = a:options.read_buffer && !a:options.lint_file let l:info = g:ale_buffer_info[l:buffer] let l:Callback = function('s:HandleExit', [{ @@ -508,10 +508,15 @@ function! s:AddProblemsFromOtherBuffers(buffer, linters) abort endif endfunction -function! s:RunIfExecutable(buffer, linter, executable) abort +function! s:RunIfExecutable(buffer, linter, lint_file, executable) abort if ale#command#IsDeferred(a:executable) let a:executable.result_callback = { - \ executable -> s:RunIfExecutable(a:buffer, a:linter, executable) + \ executable -> s:RunIfExecutable( + \ a:buffer, + \ a:linter, + \ a:lint_file, + \ executable + \ ) \} return 1 @@ -519,7 +524,7 @@ function! s:RunIfExecutable(buffer, linter, executable) abort if ale#engine#IsExecutable(a:buffer, a:executable) " Use different job types for file or linter jobs. - let l:job_type = a:linter.lint_file ? 'file_linter' : 'linter' + let l:job_type = a:lint_file ? 'file_linter' : 'linter' call setbufvar(a:buffer, 'ale_job_type', l:job_type) let l:command = ale#linter#GetCommand(a:buffer, a:linter) @@ -529,6 +534,7 @@ function! s:RunIfExecutable(buffer, linter, executable) abort \ 'linter': a:linter, \ 'output_stream': get(a:linter, 'output_stream', 'stdout'), \ 'read_buffer': a:linter.read_buffer, + \ 'lint_file': a:lint_file, \} return s:RunJob(l:command, l:options) @@ -540,33 +546,62 @@ endfunction " Run a linter for a buffer. " " Returns 1 if the linter was successfully run. -function! s:RunLinter(buffer, linter) abort +function! s:RunLinter(buffer, linter, lint_file) abort if !empty(a:linter.lsp) return ale#lsp_linter#CheckWithLSP(a:buffer, a:linter) else let l:executable = ale#linter#GetExecutable(a:buffer, a:linter) - return s:RunIfExecutable(a:buffer, a:linter, l:executable) + return s:RunIfExecutable(a:buffer, a:linter, a:lint_file, l:executable) endif return 0 endfunction -function! ale#engine#RunLinters(buffer, linters, should_lint_file) abort - " Initialise the buffer information if needed. - let l:new_buffer = ale#engine#InitBufferInfo(a:buffer) - call s:StopCurrentJobs(a:buffer, a:should_lint_file) - call s:RemoveProblemsForDisabledLinters(a:buffer, a:linters) +function! s:GetLintFileValues(slots, Callback) abort + let l:deferred_list = [] + let l:new_slots = [] - " We can only clear the results if we aren't checking the buffer. - let l:can_clear_results = !ale#engine#IsCheckingBuffer(a:buffer) + for [l:lint_file, l:linter] in a:slots + while ale#command#IsDeferred(l:lint_file) && has_key(l:lint_file, 'value') + " If we've already computed the return value, use it. + let l:lint_file = l:lint_file.value + endwhile - silent doautocmd User ALELintPre + if ale#command#IsDeferred(l:lint_file) + " If we are going to return the result later, wait for it. + call add(l:deferred_list, l:lint_file) + else + " If we have the value now, coerce it to 0 or 1. + let l:lint_file = l:lint_file is 1 + endif - for l:linter in a:linters + call add(l:new_slots, [l:lint_file, l:linter]) + endfor + + if !empty(l:deferred_list) + for l:deferred in l:deferred_list + let l:deferred.result_callback = + \ {-> s:GetLintFileValues(l:new_slots, a:Callback)} + endfor + else + call a:Callback(l:new_slots) + endif +endfunction + +function! s:RunLinters( +\ buffer, +\ slots, +\ should_lint_file, +\ new_buffer, +\ can_clear_results +\) abort + let l:can_clear_results = a:can_clear_results + + for [l:lint_file, l:linter] in a:slots " Only run lint_file linters if we should. - if !l:linter.lint_file || a:should_lint_file - if s:RunLinter(a:buffer, l:linter) + if !l:lint_file || a:should_lint_file + if s:RunLinter(a:buffer, l:linter, l:lint_file) " If a single linter ran, we shouldn't clear everything. let l:can_clear_results = 0 endif @@ -581,11 +616,49 @@ function! ale#engine#RunLinters(buffer, linters, should_lint_file) abort " disabled, or ALE itself is disabled. if l:can_clear_results call ale#engine#SetResults(a:buffer, []) - elseif l:new_buffer - call s:AddProblemsFromOtherBuffers(a:buffer, a:linters) + elseif a:new_buffer + call s:AddProblemsFromOtherBuffers( + \ a:buffer, + \ map(copy(a:slots), 'v:val[1]') + \) endif endfunction +function! ale#engine#RunLinters(buffer, linters, should_lint_file) abort + " Initialise the buffer information if needed. + let l:new_buffer = ale#engine#InitBufferInfo(a:buffer) + call s:StopCurrentJobs(a:buffer, a:should_lint_file) + call s:RemoveProblemsForDisabledLinters(a:buffer, a:linters) + + " We can only clear the results if we aren't checking the buffer. + let l:can_clear_results = !ale#engine#IsCheckingBuffer(a:buffer) + + silent doautocmd User ALELintPre + + " Handle `lint_file` callbacks first. + let l:linter_slots = [] + + for l:linter in a:linters + let l:LintFile = l:linter.lint_file + + if type(l:LintFile) is v:t_func + let l:LintFile = l:LintFile(a:buffer) + endif + + call add(l:linter_slots, [l:LintFile, l:linter]) + endfor + + call s:GetLintFileValues(l:linter_slots, { + \ new_slots -> s:RunLinters( + \ a:buffer, + \ new_slots, + \ a:should_lint_file, + \ l:new_buffer, + \ l:can_clear_results, + \ ) + \}) +endfunction + " Clean up a buffer. " " This function will stop all current jobs for the buffer, diff --git a/autoload/ale/linter.vim b/autoload/ale/linter.vim index 8d89cd5c..b483fc19 100644 --- a/autoload/ale/linter.vim +++ b/autoload/ale/linter.vim @@ -211,21 +211,17 @@ function! ale#linter#PreProcess(filetype, linter) abort " file on disk. let l:obj.lint_file = get(a:linter, 'lint_file', 0) - if !s:IsBoolean(l:obj.lint_file) - throw '`lint_file` must be `0` or `1`' + if !s:IsBoolean(l:obj.lint_file) && type(l:obj.lint_file) isnot v:t_func + throw '`lint_file` must be `0`, `1`, or a Function' endif " An option indicating that the buffer should be read. - let l:obj.read_buffer = get(a:linter, 'read_buffer', !l:obj.lint_file) + let l:obj.read_buffer = get(a:linter, 'read_buffer', 1) if !s:IsBoolean(l:obj.read_buffer) throw '`read_buffer` must be `0` or `1`' endif - if l:obj.lint_file && l:obj.read_buffer - throw 'Only one of `lint_file` or `read_buffer` can be `1`' - endif - let l:obj.aliases = get(a:linter, 'aliases', []) if type(l:obj.aliases) isnot v:t_list diff --git a/doc/ale.txt b/doc/ale.txt index 14d2abbe..5ec542f9 100644 --- a/doc/ale.txt +++ b/doc/ale.txt @@ -3038,8 +3038,8 @@ ALELint *ALELint* Run ALE once for the current buffer. This command can be used to run ALE manually, instead of automatically, if desired. - This command will also run linters where `lint_file` is set to `1`, or in - other words linters which check the file instead of the Vim buffer. + This command will also run linters where `lint_file` is evaluates to `1`, + meaning linters which check the file instead of the Vim buffer. A plug mapping `(ale_lint)` is defined for this command. @@ -3252,9 +3252,9 @@ ale#Queue(delay, [linting_flag, buffer_number]) *ale#Queue()* The linters will always be run in the background. Calling this function again from the same buffer - An optional `linting_flag` argument can be given. If `linting_flag` - is `'lint_file'`, then linters where the `lint_file` option is set to `1` will be - run. Linters with `lint_file` set to `1` are not run by default. + An optional `linting_flag` argument can be given. If `linting_flag` is + `'lint_file'`, then linters where the `lint_file` option evaluates to `1` + will be run. Otherwise, those linters will not be run. An optional `buffer_number` argument can be given for specifying the buffer to check. The active buffer (`bufnr('')`) will be checked by default. @@ -3588,24 +3588,30 @@ ale#linter#Define(filetype, linter) *ale#linter#Define()* if a command manually reads from a temporary file instead, etc. + This option behaves as if it was set to `0` when the + `lint_file` option evaluates to `1`. + *ale-lint-file* - `lint_file` A |Number| (`0` or `1`) indicating whether a command - should read the file instead of the Vim buffer. This - option can be used for linters which must check the - file on disk, and which cannot check a Vim buffer - instead. + `lint_file` A |Number| (`0` or `1`), or a |Funcref| for a function + accepting a buffer number for computing either `0` or + `1`, indicating whether a command should read the file + instead of the Vim buffer. This option can be used + for linters which must check the file on disk, and + which cannot check a Vim buffer instead. - Linters set with this option will not be run as a - user types, per |g:ale_lint_on_text_changed|. Linters - will instead be run only when events occur against - the file on disk, including |g:ale_lint_on_enter| - and |g:ale_lint_on_save|. Linters with this option - set to `1` will also be run when linters are run - manually, per |ALELintPost-autocmd|. + The result can be computed with |ale#command#Run()|. - When this option is set to `1`, `read_buffer` will - be set automatically to `0`. The two options cannot - be used together. + Linters where the eventual value of this option + evaluates to `1` will not be run as a user types, per + |g:ale_lint_on_text_changed|. Linters will instead be + run only when events occur against the file on disk, + including |g:ale_lint_on_enter| and + |g:ale_lint_on_save|. Linters where this option + evaluates to `1` will also be run when the |ALELint| + command is run. + + When this option is evaluates to `1`, ALE will behave + as if `read_buffer` was set to `0`. *ale-lsp-linters* `lsp` A |String| for defining LSP (Language Server Protocol) diff --git a/supported-tools.md b/supported-tools.md index 70a992a1..66e46348 100644 --- a/supported-tools.md +++ b/supported-tools.md @@ -16,7 +16,7 @@ formatting. | Key | Definition | | ------------- | -------------------------------- | -| :floppy_disk: | Only checked when saved to disk | +| :floppy_disk: | May only run on files on disk | | :warning: | Disabled by default | --- diff --git a/test/smoke_test.vader b/test/smoke_test.vader index 53e08a8d..0b126cc6 100644 --- a/test/smoke_test.vader +++ b/test/smoke_test.vader @@ -1,8 +1,10 @@ Before: + Save g:ale_enabled Save g:ale_set_lists_synchronously Save g:ale_buffer_info Save &shell + let g:ale_enabled = 1 let g:ale_buffer_info = {} let g:ale_set_lists_synchronously = 1 diff --git a/test/test_computed_lint_file_values.vader b/test/test_computed_lint_file_values.vader new file mode 100644 index 00000000..399e96fe --- /dev/null +++ b/test/test_computed_lint_file_values.vader @@ -0,0 +1,134 @@ +Before: + Save g:ale_enabled + Save g:ale_run_synchronously + Save g:ale_set_lists_synchronously + Save g:ale_buffer_info + + let g:ale_enabled = 1 + let g:ale_buffer_info = {} + let g:ale_run_synchronously = 1 + let g:ale_set_lists_synchronously = 1 + + function! TestCallback(buffer, output) + " Windows adds extra spaces to the text from echo. + return [{ + \ 'lnum': 2, + \ 'col': 3, + \ 'text': 'testlinter1', + \}] + endfunction + function! TestCallback2(buffer, output) + " Windows adds extra spaces to the text from echo. + return [{ + \ 'lnum': 1, + \ 'col': 3, + \ 'text': 'testlinter2', + \}] + endfunction + function! TestCallback3(buffer, output) + " Windows adds extra spaces to the text from echo. + return [{ + \ 'lnum': 3, + \ 'col': 3, + \ 'text': 'testlinter3', + \}] + endfunction + + " These two linters computer their lint_file values after running commands. + call ale#linter#Define('foobar', { + \ 'name': 'testlinter1', + \ 'callback': 'TestCallback', + \ 'executable': has('win32') ? 'cmd' : 'echo', + \ 'command': has('win32') ? 'echo foo bar' : '/bin/sh -c ''echo foo bar''', + \ 'lint_file': {b -> ale#command#Run(b, 'echo', {-> 1})}, + \}) + call ale#linter#Define('foobar', { + \ 'name': 'testlinter2', + \ 'callback': 'TestCallback2', + \ 'executable': has('win32') ? 'cmd' : 'echo', + \ 'command': has('win32') ? 'echo foo bar' : '/bin/sh -c ''echo foo bar''', + \ 'lint_file': {b -> ale#command#Run(b, 'echo', {-> ale#command#Run(b, 'echo', {-> 1})})}, + \}) + " This one directly computes the result. + call ale#linter#Define('foobar', { + \ 'name': 'testlinter3', + \ 'callback': 'TestCallback3', + \ 'executable': has('win32') ? 'cmd' : 'echo', + \ 'command': has('win32') ? 'echo foo bar' : '/bin/sh -c ''echo foo bar''', + \ 'lint_file': {b -> 1}, + \}) + + let g:filename = tempname() + call writefile([], g:filename) + call ale#test#SetFilename(g:filename) + +After: + delfunction TestCallback + + call ale#engine#Cleanup(bufnr('')) + Restore + call ale#linter#Reset() + + " Items and markers, etc. + call setloclist(0, []) + call clearmatches() + call ale#sign#Clear() + + if filereadable(g:filename) + call delete(g:filename) + endif + + unlet g:filename + +Given foobar(A file with some lines): + foo + bar + baz + +Execute(lint_file results where the result is eventually computed should be run): + call ale#Queue(0, 'lint_file') + call ale#test#FlushJobs() + + AssertEqual + \ [ + \ { + \ 'bufnr': bufnr('%'), + \ 'lnum': 1, + \ 'vcol': 0, + \ 'col': 3, + \ 'text': 'testlinter2', + \ 'type': 'E', + \ 'nr': -1, + \ 'pattern': '', + \ 'valid': 1, + \ }, + \ { + \ 'bufnr': bufnr('%'), + \ 'lnum': 2, + \ 'vcol': 0, + \ 'col': 3, + \ 'text': 'testlinter1', + \ 'type': 'E', + \ 'nr': -1, + \ 'pattern': '', + \ 'valid': 1, + \ }, + \ { + \ 'bufnr': bufnr('%'), + \ 'lnum': 3, + \ 'vcol': 0, + \ 'col': 3, + \ 'text': 'testlinter3', + \ 'type': 'E', + \ 'nr': -1, + \ 'pattern': '', + \ 'valid': 1, + \ }, + \ ], + \ ale#test#GetLoclistWithoutModule() + +Execute(Linters where lint_file eventually evaluates to 1 shouldn't be run if we don't want to run them): + call ale#Queue(0, '') + call ale#test#FlushJobs() + + AssertEqual [], ale#test#GetLoclistWithoutModule() diff --git a/test/test_deferred_command_string.vader b/test/test_deferred_command_string.vader index 026be6fe..173b6bb2 100644 --- a/test/test_deferred_command_string.vader +++ b/test/test_deferred_command_string.vader @@ -12,7 +12,7 @@ Before: call ale#linter#Define('foobar', { \ 'name': 'lint_file_linter', \ 'callback': 'LintFileCallback', - \ 'executable': 'echo', + \ 'executable': has('win32') ? 'cmd' : 'echo', \ 'command': {b -> ale#command#Run(b, 'echo', {-> ale#command#Run(b, 'echo', {-> 'foo'})})}, \ 'read_buffer': 0, \}) @@ -28,7 +28,7 @@ After: Given foobar (Some imaginary filetype): Execute(It should be possible to compute an executable to check based on the result of commands): - AssertLinter 'echo', 'foo' + AssertLinter has('win32') ? 'cmd' : 'echo', 'foo' ALELint call ale#test#FlushJobs() @@ -40,7 +40,7 @@ Execute(It should be possible to compute an executable to check based on the res Execute(It handle the deferred command failing): let g:ale_emulate_job_failure = 1 - AssertLinter 'echo', 0 + AssertLinter has('win32') ? 'cmd' : 'echo', 0 ALELint call ale#test#FlushJobs() diff --git a/test/test_linter_defintion_processing.vader b/test/test_linter_defintion_processing.vader index 2c85299b..d000b158 100644 --- a/test/test_linter_defintion_processing.vader +++ b/test/test_linter_defintion_processing.vader @@ -174,7 +174,7 @@ Execute(PreProcess should process the lint_file option correctly): \} AssertThrows call ale#linter#PreProcess('testft', g:linter) - AssertEqual '`lint_file` must be `0` or `1`', g:vader_exception + AssertEqual '`lint_file` must be `0`, `1`, or a Function', g:vader_exception let g:linter.lint_file = 0 @@ -185,14 +185,17 @@ Execute(PreProcess should process the lint_file option correctly): let g:linter.lint_file = 1 AssertEqual 1, ale#linter#PreProcess('testft', g:linter).lint_file - " The default for read_buffer should change to 0 when lint_file is 1. - AssertEqual 0, ale#linter#PreProcess('testft', g:linter).read_buffer + " The default for read_buffer should still be 1 + AssertEqual 1, ale#linter#PreProcess('testft', g:linter).read_buffer let g:linter.read_buffer = 1 - " We shouldn't be able to set both options to 1 at the same time. - AssertThrows call ale#linter#PreProcess('testft', g:linter) - AssertEqual 'Only one of `lint_file` or `read_buffer` can be `1`', g:vader_exception + " We should be able to set `read_buffer` and `lint_file` at the same time. + AssertEqual 1, ale#linter#PreProcess('testft', g:linter).read_buffer + + let g:linter.lint_file = function('type') + + Assert type(ale#linter#PreProcess('testft', g:linter).lint_file) is v:t_func Execute(PreProcess should set a default value for lint_file): let g:linter = { diff --git a/test/test_swiftlint_executable_detection.vader b/test/test_swiftlint_executable_detection.vader index dfd4930b..ac83ff8f 100644 --- a/test/test_swiftlint_executable_detection.vader +++ b/test/test_swiftlint_executable_detection.vader @@ -23,22 +23,22 @@ Execute(React Native apps using CocoaPods should take precedence over the defaul call ale#test#SetFilename('swiftlint-test-files/react-native/testfile.swift') AssertEqual - \ ale#path#Simplify(g:dir . '/swiftlint-test-files/react-native/ios/Pods/SwiftLint/swiftlint'), - \ ale_linters#swift#swiftlint#GetExecutable(bufnr('')) + \ tolower(ale#path#Simplify(g:dir . '/swiftlint-test-files/react-native/ios/Pods/SwiftLint/swiftlint')), + \ tolower(ale_linters#swift#swiftlint#GetExecutable(bufnr(''))) Execute(CocoaPods installation should take precedence over the default executable): call ale#test#SetFilename('swiftlint-test-files/cocoapods/testfile.swift') AssertEqual - \ ale#path#Simplify(g:dir . '/swiftlint-test-files/cocoapods/Pods/SwiftLint/swiftlint'), - \ ale_linters#swift#swiftlint#GetExecutable(bufnr('')) + \ tolower(ale#path#Simplify(g:dir . '/swiftlint-test-files/cocoapods/Pods/SwiftLint/swiftlint')), + \ tolower(ale_linters#swift#swiftlint#GetExecutable(bufnr(''))) Execute(Top level CocoaPods installation should take precedence over React Native installation): call ale#test#SetFilename('swiftlint-test-files/cocoapods-and-react-native/testfile.swift') AssertEqual - \ ale#path#Simplify(g:dir . '/swiftlint-test-files/cocoapods-and-react-native/Pods/SwiftLint/swiftlint'), - \ ale_linters#swift#swiftlint#GetExecutable(bufnr('')) + \ tolower(ale#path#Simplify(g:dir . '/swiftlint-test-files/cocoapods-and-react-native/Pods/SwiftLint/swiftlint')), + \ tolower(ale_linters#swift#swiftlint#GetExecutable(bufnr(''))) Execute(use-global should override other versions): let g:ale_swift_swiftlint_use_global = 1