From 4ddf74264397a0c739b1c6fd5f643505a31e1d11 Mon Sep 17 00:00:00 2001 From: w0rp Date: Wed, 9 Sep 2020 21:42:27 +0100 Subject: Close #2522 - Check pylint on the fly Newer versions of pylint will now check your code as you type. Older versions will still only check the file on disk. Co-authored-by: Oliver Wiegers --- ale_linters/python/pylint.vim | 38 +++++++--- autoload/ale/engine.vim | 83 +++++++++++++--------- doc/ale.txt | 5 +- .../test_pylint_command_callback.vader | 31 +++++--- test/test_computed_lint_file_values.vader | 16 +++++ 5 files changed, 119 insertions(+), 54 deletions(-) diff --git a/ale_linters/python/pylint.vim b/ale_linters/python/pylint.vim index b16d5355..44eea246 100644 --- a/ale_linters/python/pylint.vim +++ b/ale_linters/python/pylint.vim @@ -17,7 +17,7 @@ function! ale_linters#python#pylint#GetExecutable(buffer) abort return ale#python#FindExecutable(a:buffer, 'python_pylint', ['pylint']) endfunction -function! ale_linters#python#pylint#GetCommand(buffer) abort +function! ale_linters#python#pylint#GetCommand(buffer, version) abort let l:cd_string = '' if ale#Var(a:buffer, 'python_pylint_change_directory') @@ -38,17 +38,23 @@ function! ale_linters#python#pylint#GetCommand(buffer) abort return l:cd_string \ . ale#Escape(l:executable) . l:exec_args - \ . ' ' . ale#Var(a:buffer, 'python_pylint_options') + \ . ale#Pad(ale#Var(a:buffer, 'python_pylint_options')) \ . ' --output-format text --msg-template="{path}:{line}:{column}: {msg_id} ({symbol}) {msg}" --reports n' + \ . (ale#semver#GTE(a:version, [2, 4, 0]) ? ' --from-stdin' : '') \ . ' %s' endfunction function! ale_linters#python#pylint#Handle(buffer, lines) abort + let l:output = ale#python#HandleTraceback(a:lines, 10) + + if !empty(l:output) + return l:output + endif + " Matches patterns like the following: " " test.py:4:4: W0101 (unreachable) Unreachable code let l:pattern = '\v^[a-zA-Z]?:?[^:]+:(\d+):(\d+): ([[:alnum:]]+) \(([^(]*)\) (.*)$' - let l:output = [] for l:match in ale#util#GetMatches(a:lines, l:pattern) "let l:failed = append(0, l:match) @@ -71,13 +77,19 @@ function! ale_linters#python#pylint#Handle(buffer, lines) abort let l:code_out = l:match[4] endif - call add(l:output, { + let l:item = { \ 'lnum': l:match[1] + 0, \ 'col': l:match[2] + 1, \ 'text': l:match[5], \ 'code': l:code_out, - \ 'type': l:code[:0] is# 'E' ? 'E' : 'W', - \}) + \ 'type': 'W', + \} + + if l:code[:0] is# 'E' + let l:item.type = 'E' + endif + + call add(l:output, l:item) endfor return l:output @@ -86,7 +98,17 @@ endfunction call ale#linter#Define('python', { \ 'name': 'pylint', \ 'executable': function('ale_linters#python#pylint#GetExecutable'), -\ 'command': function('ale_linters#python#pylint#GetCommand'), +\ 'lint_file': {buffer -> ale#semver#RunWithVersionCheck( +\ buffer, +\ ale#Var(buffer, 'python_pylint_executable'), +\ '%e --version', +\ {buffer, version -> !ale#semver#GTE(version, [2, 4, 0])}, +\ )}, +\ 'command': {buffer -> ale#semver#RunWithVersionCheck( +\ buffer, +\ ale#Var(buffer, 'python_pylint_executable'), +\ '%e --version', +\ function('ale_linters#python#pylint#GetCommand'), +\ )}, \ 'callback': 'ale_linters#python#pylint#Handle', -\ 'lint_file': 1, \}) diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index 63195d0f..3cafa25c 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -444,7 +444,7 @@ function! s:RunJob(command, options) abort return 1 endfunction -function! s:StopCurrentJobs(buffer, clear_lint_file_jobs) abort +function! s:StopCurrentJobs(buffer, clear_lint_file_jobs, linter_slots) abort let l:info = get(g:ale_buffer_info, a:buffer, {}) call ale#command#StopJobs(a:buffer, 'linter') @@ -453,13 +453,23 @@ function! s:StopCurrentJobs(buffer, clear_lint_file_jobs) abort call ale#command#StopJobs(a:buffer, 'file_linter') let l:info.active_linter_list = [] else + let l:lint_file_map = {} + + " Use a previously computed map of `lint_file` values to find + " linters that are used for linting files. + for [l:lint_file, l:linter] in a:linter_slots + if l:lint_file is 1 + let l:lint_file_map[l:linter.name] = 1 + endif + endfor + " Keep jobs for linting files when we're only linting buffers. - call filter(l:info.active_linter_list, 'get(v:val, ''lint_file'')') + call filter(l:info.active_linter_list, 'get(l:lint_file_map, v:val.name)') endif endfunction function! ale#engine#Stop(buffer) abort - call s:StopCurrentJobs(a:buffer, 1) + call s:StopCurrentJobs(a:buffer, 1, []) endfunction function! s:RemoveProblemsForDisabledLinters(buffer, linters) abort @@ -562,6 +572,22 @@ function! s:RunLinter(buffer, linter, lint_file) abort return 0 endfunction +function! s:GetLintFileSlots(buffer, linters) abort + 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 + + return l:linter_slots +endfunction + function! s:GetLintFileValues(slots, Callback) abort let l:deferred_list = [] let l:new_slots = [] @@ -595,12 +621,18 @@ endfunction function! s:RunLinters( \ buffer, +\ linters, \ slots, \ should_lint_file, \ new_buffer, -\ can_clear_results \) abort - let l:can_clear_results = a:can_clear_results + call s:StopCurrentJobs(a:buffer, a:should_lint_file, a:slots) + 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 for [l:lint_file, l:linter] in a:slots " Only run lint_file linters if we should. @@ -631,36 +663,19 @@ 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, - \ ) - \}) + call s:GetLintFileValues( + \ s:GetLintFileSlots(a:buffer, a:linters), + \ { + \ slots -> s:RunLinters( + \ a:buffer, + \ a:linters, + \ slots, + \ a:should_lint_file, + \ l:new_buffer, + \ ) + \ } + \) endfunction " Clean up a buffer. diff --git a/doc/ale.txt b/doc/ale.txt index e6b91be8..6ef137c1 100644 --- a/doc/ale.txt +++ b/doc/ale.txt @@ -179,8 +179,11 @@ script like so. > #!/usr/bin/env bash - exec docker run --rm -v "$(pwd):/data" cytopia/pylint "$@" + exec docker run -i --rm -v "$(pwd):/data" cytopia/pylint "$@" < + +You will run to run Docker commands with `-i` in order to read from stdin. + With the above script in mind, you might configure ALE to lint your Python project with `pylint` by providing the path to the script to execute, and mappings which describe how to between the two file systems in your diff --git a/test/command_callback/test_pylint_command_callback.vader b/test/command_callback/test_pylint_command_callback.vader index 755dd292..15f004b6 100644 --- a/test/command_callback/test_pylint_command_callback.vader +++ b/test/command_callback/test_pylint_command_callback.vader @@ -8,6 +8,8 @@ Before: let b:bin_dir = has('win32') ? 'Scripts' : 'bin' let b:command_tail = ' --output-format text --msg-template="{path}:{line}:{column}: {msg_id} ({symbol}) {msg}" --reports n %s' + GivenCommandOutput ['pylint 2.3.0'] + After: unlet! b:bin_dir unlet! b:executable @@ -17,26 +19,33 @@ After: Execute(The pylint callbacks should return the correct default values): AssertLinter 'pylint', - \ ale#path#CdString(expand('#' . bufnr('') . ':p:h')) - \ . ale#Escape('pylint') . ' ' . b:command_tail + \ ale#path#CdString(expand('%:p:h')) + \ . ale#Escape('pylint') . b:command_tail + +Execute(Pylint should run with the --from-stdin in new enough versions): + GivenCommandOutput ['pylint 2.4.0'] + + AssertLinter 'pylint', + \ ale#path#CdString(expand('%:p:h')) + \ . ale#Escape('pylint') . b:command_tail[:-3] . '--from-stdin %s' Execute(The option for disabling changing directories should work): let g:ale_python_pylint_change_directory = 0 - AssertLinter 'pylint', ale#Escape('pylint') . ' ' . b:command_tail + AssertLinter 'pylint', ale#Escape('pylint') . b:command_tail Execute(The pylint executable should be configurable, and escaped properly): let g:ale_python_pylint_executable = 'executable with spaces' AssertLinter 'executable with spaces', - \ ale#path#CdString(expand('#' . bufnr('') . ':p:h')) - \ . ale#Escape('executable with spaces') . ' ' . b:command_tail + \ ale#path#CdString(expand('%:p:h')) + \ . ale#Escape('executable with spaces') . b:command_tail Execute(The pylint command callback should let you set options): let g:ale_python_pylint_options = '--some-option' AssertLinter 'pylint', - \ ale#path#CdString(expand('#' . bufnr('') . ':p:h')) + \ ale#path#CdString(expand('%:p:h')) \ . ale#Escape('pylint') . ' --some-option' . b:command_tail Execute(The pylint callbacks shouldn't detect virtualenv directories where they don't exist): @@ -44,7 +53,7 @@ Execute(The pylint callbacks shouldn't detect virtualenv directories where they AssertLinter 'pylint', \ ale#path#CdString(ale#path#Simplify(g:dir . '/python_paths/no_virtualenv/subdir')) - \ . ale#Escape('pylint') . ' ' . b:command_tail + \ . ale#Escape('pylint') . b:command_tail Execute(The pylint callbacks should detect virtualenv directories): silent execute 'file ' . fnameescape(g:dir . '/python_paths/with_virtualenv/subdir/foo/bar.py') @@ -55,7 +64,7 @@ Execute(The pylint callbacks should detect virtualenv directories): AssertLinter b:executable, \ ale#path#CdString(ale#path#Simplify(g:dir . '/python_paths/with_virtualenv/subdir')) - \ . ale#Escape(b:executable) . ' ' . b:command_tail + \ . ale#Escape(b:executable) . b:command_tail Execute(You should able able to use the global pylint instead): silent execute 'file ' . fnameescape(g:dir . '/python_paths/with_virtualenv/subdir/foo/bar.py') @@ -63,7 +72,7 @@ Execute(You should able able to use the global pylint instead): AssertLinter 'pylint', \ ale#path#CdString(ale#path#Simplify(g:dir . '/python_paths/with_virtualenv/subdir')) - \ . ale#Escape('pylint') . ' ' . b:command_tail + \ . ale#Escape('pylint') . b:command_tail Execute(Setting executable to 'pipenv' appends 'run pylint'): let g:ale_python_pylint_executable = 'path/to/pipenv' @@ -71,7 +80,7 @@ Execute(Setting executable to 'pipenv' appends 'run pylint'): AssertLinter 'path/to/pipenv', \ ale#path#CdString(expand('#' . bufnr('') . ':p:h')) \ . ale#Escape('path/to/pipenv') . ' run pylint' - \ . ' --output-format text --msg-template="{path}:{line}:{column}: {msg_id} ({symbol}) {msg}" --reports n %s' + \ . ' --output-format text --msg-template="{path}:{line}:{column}: {msg_id} ({symbol}) {msg}" --reports n %s' Execute(Pipenv is detected when python_pylint_auto_pipenv is set): let g:ale_python_pylint_auto_pipenv = 1 @@ -80,4 +89,4 @@ Execute(Pipenv is detected when python_pylint_auto_pipenv is set): AssertLinter 'pipenv', \ ale#path#CdString(expand('#' . bufnr('') . ':p:h')) \ . ale#Escape('pipenv') . ' run pylint' - \ . ' --output-format text --msg-template="{path}:{line}:{column}: {msg_id} ({symbol}) {msg}" --reports n %s' + \ . ' --output-format text --msg-template="{path}:{line}:{column}: {msg_id} ({symbol}) {msg}" --reports n %s' diff --git a/test/test_computed_lint_file_values.vader b/test/test_computed_lint_file_values.vader index 399e96fe..ed0d4c0c 100644 --- a/test/test_computed_lint_file_values.vader +++ b/test/test_computed_lint_file_values.vader @@ -132,3 +132,19 @@ Execute(Linters where lint_file eventually evaluates to 1 shouldn't be run if we call ale#test#FlushJobs() AssertEqual [], ale#test#GetLoclistWithoutModule() + +Execute(Keeping computed lint_file jobs running should work): + AssertEqual 'testlinter2', ale#linter#Get('foobar')[1].name + + call ale#engine#InitBufferInfo(bufnr('')) + + call ale#engine#MarkLinterActive( + \ g:ale_buffer_info[bufnr('')], + \ ale#linter#Get('foobar')[1] + \) + call ale#engine#RunLinters(bufnr(''), ale#linter#Get('foobar'), 0) + + Assert !empty(g:ale_buffer_info[bufnr('')].active_linter_list), + \ 'The active linter list was empty' + Assert ale#engine#IsCheckingBuffer(bufnr('')), + \ 'The IsCheckingBuffer function returned 0' -- cgit v1.2.3