From a8c5e0f4dce14b9dad8e4a238ebc93d8aa6ed28e Mon Sep 17 00:00:00 2001 From: w0rp Date: Thu, 9 Nov 2017 23:42:54 +0000 Subject: [PATCH] Simplfy semver handling and share the semver version cache across everything --- ale_linters/javascript/flow.vim | 13 ++-- ale_linters/python/flake8.vim | 43 ++----------- ale_linters/rust/cargo.vim | 62 ++++-------------- ale_linters/sh/shellcheck.vim | 31 ++------- ale_linters/vim/vint.vim | 20 ++---- autoload/ale/handlers/gcc.vim | 12 ---- autoload/ale/semver.vim | 52 +++++++++++---- .../test_cargo_command_callbacks.vader | 1 + .../test_flake8_command_callback.vader | 11 ++-- test/handler/test_gcc_handler.vader | 11 ---- test/test_flow_command.vader | 1 + test/test_semver_utils.vader | 64 ++++++++++++++----- 12 files changed, 133 insertions(+), 188 deletions(-) diff --git a/ale_linters/javascript/flow.vim b/ale_linters/javascript/flow.vim index 0dd64535..6d51628b 100644 --- a/ale_linters/javascript/flow.vim +++ b/ale_linters/javascript/flow.vim @@ -23,18 +23,15 @@ function! ale_linters#javascript#flow#GetCommand(buffer, version_lines) abort return '' endif - let l:use_respect_pragma = 1 + let l:executable = ale_linters#javascript#flow#GetExecutable(a:buffer) + let l:version = ale#semver#GetVersion(l:executable, a:version_lines) " If we can parse the version number, then only use --respect-pragma " if the version is >= 0.36.0, which added the argument. - for l:match in ale#util#GetMatches(a:version_lines, '\v\d+\.\d+\.\d+$') - let l:use_respect_pragma = ale#semver#GreaterOrEqual( - \ ale#semver#Parse(l:match[0]), - \ [0, 36, 0] - \) - endfor + let l:use_respect_pragma = empty(l:version) + \ || ale#semver#GTE(l:version, [0, 36]) - return ale#Escape(ale_linters#javascript#flow#GetExecutable(a:buffer)) + return ale#Escape(l:executable) \ . ' check-contents' \ . (l:use_respect_pragma ? ' --respect-pragma': '') \ . ' --json --from ale %s' diff --git a/ale_linters/python/flake8.vim b/ale_linters/python/flake8.vim index 8aa4c4df..501db0b1 100644 --- a/ale_linters/python/flake8.vim +++ b/ale_linters/python/flake8.vim @@ -10,10 +10,6 @@ let g:ale_python_flake8_options = \ get(g:, 'ale_python_flake8_options', s:default_options) let g:ale_python_flake8_use_global = get(g:, 'ale_python_flake8_use_global', 0) -" A map from Python executable paths to semver strings parsed for those -" executables, so we don't have to look up the version number constantly. -let s:version_cache = {} - function! s:UsingModule(buffer) abort return ale#Var(a:buffer, 'python_flake8_options') =~# ' *-m flake8' endfunction @@ -26,62 +22,35 @@ function! ale_linters#python#flake8#GetExecutable(buffer) abort return ale#Var(a:buffer, 'python_flake8_executable') endfunction -function! ale_linters#python#flake8#ClearVersionCache() abort - let s:version_cache = {} -endfunction - function! ale_linters#python#flake8#VersionCheck(buffer) abort let l:executable = ale_linters#python#flake8#GetExecutable(a:buffer) " If we have previously stored the version number in a cache, then " don't look it up again. - if has_key(s:version_cache, l:executable) + if ale#semver#HasVersion(l:executable) " Returning an empty string skips this command. return '' endif - let l:executable = ale#Escape(ale_linters#python#flake8#GetExecutable(a:buffer)) + let l:executable = ale#Escape(l:executable) let l:module_string = s:UsingModule(a:buffer) ? ' -m flake8' : '' return l:executable . l:module_string . ' --version' endfunction -" Get the flake8 version from the output, or the cache. -function! s:GetVersion(buffer, version_output) abort - let l:executable = ale_linters#python#flake8#GetExecutable(a:buffer) - let l:version = [] - - " Get the version from the cache. - if has_key(s:version_cache, l:executable) - return s:version_cache[l:executable] - endif - - if !empty(a:version_output) - " Parse the version string, and store it in the cache. - let l:version = ale#semver#Parse(a:version_output[0]) - let s:version_cache[l:executable] = l:version - endif - - return l:version -endfunction - -" flake8 versions 3 and up support the --stdin-display-name argument. -function! s:SupportsDisplayName(version) abort - return !empty(a:version) && ale#semver#GreaterOrEqual(a:version, [3, 0, 0]) -endfunction - function! ale_linters#python#flake8#GetCommand(buffer, version_output) abort - let l:version = s:GetVersion(a:buffer, a:version_output) + let l:executable = ale_linters#python#flake8#GetExecutable(a:buffer) + let l:version = ale#semver#GetVersion(l:executable, a:version_output) " Only include the --stdin-display-name argument if we can parse the " flake8 version, and it is recent enough to support it. - let l:display_name_args = s:SupportsDisplayName(l:version) + let l:display_name_args = ale#semver#GTE(l:version, [3, 0, 0]) \ ? ' --stdin-display-name %s' \ : '' let l:options = ale#Var(a:buffer, 'python_flake8_options') - return ale#Escape(ale_linters#python#flake8#GetExecutable(a:buffer)) + return ale#Escape(l:executable) \ . (!empty(l:options) ? ' ' . l:options : '') \ . ' --format=default' \ . l:display_name_args . ' -' diff --git a/ale_linters/rust/cargo.vim b/ale_linters/rust/cargo.vim index f41cb4b6..ae26fab4 100644 --- a/ale_linters/rust/cargo.vim +++ b/ale_linters/rust/cargo.vim @@ -4,8 +4,6 @@ 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# '' return 'cargo' @@ -17,59 +15,23 @@ function! ale_linters#rust#cargo#GetCargoExecutable(bufnr) abort endfunction 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]) + return !ale#semver#HasVersion('cargo') + \ ? 'cargo --version' + \ : '' 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' - \ : '' + let l:version = ale#semver#GetVersion('cargo', a:version_output) + + let l:use_check = ale#Var(a:buffer, 'rust_cargo_use_check') + \ && ale#semver#GTE(l:version, [0, 17, 0]) + let l:use_all_targets = l:use_check + \ && ale#Var(a:buffer, 'rust_cargo_check_all_targets') + \ && ale#semver#GTE(l:version, [0, 22, 0]) return 'cargo ' - \ . l:command - \ . l:all_targets + \ . (l:use_check ? 'check' : 'build') + \ . (l:use_all_targets ? ' --all-targets' : '') \ . ' --frozen --message-format=json -q' endfunction diff --git a/ale_linters/sh/shellcheck.vim b/ale_linters/sh/shellcheck.vim index 1f6bdf81..e79b3b88 100644 --- a/ale_linters/sh/shellcheck.vim +++ b/ale_linters/sh/shellcheck.vim @@ -15,8 +15,6 @@ let g:ale_sh_shellcheck_executable = let g:ale_sh_shellcheck_options = \ get(g:, 'ale_sh_shellcheck_options', '') -let s:version_cache = {} - function! ale_linters#sh#shellcheck#GetExecutable(buffer) abort return ale#Var(a:buffer, 'sh_shellcheck_executable') endfunction @@ -49,38 +47,19 @@ function! ale_linters#sh#shellcheck#VersionCheck(buffer) abort let l:executable = ale_linters#sh#shellcheck#GetExecutable(a:buffer) " Don't check the version again if we've already cached it. - if has_key(s:version_cache, l:executable) - return '' - endif - - return ale#Escape(l:executable) . ' --version' -endfunction - -" Get the shellcheck version from the cache, or parse it and cache it. -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:CanUseExternalOption(version) abort - return !empty(a:version) - \ && ale#semver#GreaterOrEqual(a:version, [0, 4, 0]) + return !ale#semver#HasVersion(l:executable) + \ ? ale#Escape(l:executable) . ' --version' + \ : '' endfunction function! ale_linters#sh#shellcheck#GetCommand(buffer, version_output) abort let l:executable = ale_linters#sh#shellcheck#GetExecutable(a:buffer) - let l:version = s:GetVersion(l:executable, a:version_output) + let l:version = ale#semver#GetVersion(l:executable, a:version_output) let l:options = ale#Var(a:buffer, 'sh_shellcheck_options') let l:exclude_option = ale#Var(a:buffer, 'sh_shellcheck_exclusions') let l:dialect = ale_linters#sh#shellcheck#GetDialectArgument(a:buffer) - let l:external_option = s:CanUseExternalOption(l:version) ? ' -x' : '' + let l:external_option = ale#semver#GTE(l:version, [0, 4, 0]) ? ' -x' : '' return ale#path#BufferCdString(a:buffer) \ . ale#Escape(l:executable) diff --git a/ale_linters/vim/vint.vim b/ale_linters/vim/vint.vim index adf2b4ab..dfa00dc0 100644 --- a/ale_linters/vim/vint.vim +++ b/ale_linters/vim/vint.vim @@ -6,25 +6,19 @@ let g:ale_vim_vint_show_style_issues = \ get(g:, 'ale_vim_vint_show_style_issues', 1) let s:enable_neovim = has('nvim') ? ' --enable-neovim ' : '' let s:format = '-f "{file_path}:{line_number}:{column_number}: {severity}: {description} (see {reference})"' -let s:vint_version = [] function! ale_linters#vim#vint#VersionCommand(buffer) abort - if empty(s:vint_version) - " Check the Vint version if we haven't checked it already. - return 'vint --version' - endif - - return '' + " Check the Vint version if we haven't checked it already. + return !ale#semver#HasVersion('vint') + \ ? 'vint --version' + \ : '' endfunction function! ale_linters#vim#vint#GetCommand(buffer, version_output) abort - if empty(s:vint_version) && !empty(a:version_output) - " Parse the version out of the --version output. - let s:vint_version = ale#semver#Parse(join(a:version_output, "\n")) - endif + let l:version = ale#semver#GetVersion('vint', a:version_output) - let l:can_use_no_color_flag = empty(s:vint_version) - \ || ale#semver#GreaterOrEqual(s:vint_version, [0, 3, 7]) + let l:can_use_no_color_flag = empty(l:version) + \ || ale#semver#GTE(l:version, [0, 3, 7]) let l:warning_flag = ale#Var(a:buffer, 'vim_vint_show_style_issues') ? '-s' : '-w' diff --git a/autoload/ale/handlers/gcc.vim b/autoload/ale/handlers/gcc.vim index 256cd01d..9ec7b110 100644 --- a/autoload/ale/handlers/gcc.vim +++ b/autoload/ale/handlers/gcc.vim @@ -18,18 +18,6 @@ function! s:RemoveUnicodeQuotes(text) abort return l:text endfunction -function! ale#handlers#gcc#ParseGCCVersion(lines) abort - for l:line in a:lines - let l:match = matchstr(l:line, '\d\.\d\.\d') - - if !empty(l:match) - return ale#semver#Parse(l:match) - endif - endfor - - return [] -endfunction - function! ale#handlers#gcc#HandleGCCFormat(buffer, lines) abort " Look for lines like the following. " diff --git a/autoload/ale/semver.vim b/autoload/ale/semver.vim index b153dd1d..6b0fd34a 100644 --- a/autoload/ale/semver.vim +++ b/autoload/ale/semver.vim @@ -1,27 +1,55 @@ -" Given some text, parse a semantic versioning string from the text -" into a triple of integeers [major, minor, patch]. +let s:version_cache = {} + +" Reset the version cache used for parsing the version. +function! ale#semver#ResetVersionCache() abort + let s:version_cache = {} +endfunction + +" Given an executable name and some lines of output, which can be empty, +" parse the version from the lines of output, or return the cached version +" triple [major, minor, patch] " -" If no match can be performed, then an empty List will be returned instead. -function! ale#semver#Parse(text) abort - let l:match = matchlist(a:text, '^ *\(\d\+\)\.\(\d\+\)\.\(\d\+\)') +" If the version cannot be found, an empty List will be returned instead. +function! ale#semver#GetVersion(executable, version_lines) abort + let l:version = get(s:version_cache, a:executable, []) - if empty(l:match) - return [] - endif + for l:line in a:version_lines + let l:match = matchlist(l:line, '\v(\d+)\.(\d+)\.(\d+)') - return [l:match[1] + 0, l:match[2] + 0, l:match[3] + 0] + if !empty(l:match) + let l:version = [l:match[1] + 0, l:match[2] + 0, l:match[3] + 0] + let s:version_cache[a:executable] = l:version + + break + endif + endfor + + return l:version +endfunction + +" Return 1 if the semver version has been cached for a given executable. +function! ale#semver#HasVersion(executable) abort + return has_key(s:version_cache, a:executable) endfunction " Given two triples of integers [major, minor, patch], compare the triples -" and return 1 if the lhs is greater than or equal to the rhs. -function! ale#semver#GreaterOrEqual(lhs, rhs) abort +" and return 1 if the LHS is greater than or equal to the RHS. +" +" Pairs of [major, minor] can also be used for either argument. +" +" 0 will be returned if the LHS is an empty List. +function! ale#semver#GTE(lhs, rhs) abort + if empty(a:lhs) + return 0 + endif + if a:lhs[0] > a:rhs[0] return 1 elseif a:lhs[0] == a:rhs[0] if a:lhs[1] > a:rhs[1] return 1 elseif a:lhs[1] == a:rhs[1] - return a:lhs[2] >= a:rhs[2] + return get(a:lhs, 2) >= get(a:rhs, 2) endif endif diff --git a/test/command_callback/test_cargo_command_callbacks.vader b/test/command_callback/test_cargo_command_callbacks.vader index d808e197..10535516 100644 --- a/test/command_callback/test_cargo_command_callbacks.vader +++ b/test/command_callback/test_cargo_command_callbacks.vader @@ -17,6 +17,7 @@ After: call ale#test#RestoreDirectory() call ale#linter#Reset() + call ale#semver#ResetVersionCache() Execute(An empty string should be returned for the cargo executable when there's no Cargo.toml file): AssertEqual diff --git a/test/command_callback/test_flake8_command_callback.vader b/test/command_callback/test_flake8_command_callback.vader index a510f4c1..47d5c0fd 100644 --- a/test/command_callback/test_flake8_command_callback.vader +++ b/test/command_callback/test_flake8_command_callback.vader @@ -23,7 +23,7 @@ After: call ale#test#RestoreDirectory() call ale#linter#Reset() - call ale_linters#python#flake8#ClearVersionCache() + call ale#semver#ResetVersionCache() Execute(The flake8 callbacks should return the correct default values): AssertEqual @@ -35,8 +35,9 @@ Execute(The flake8 callbacks should return the correct default values): AssertEqual \ ale#Escape('flake8') . ' --format=default --stdin-display-name %s -', \ ale_linters#python#flake8#GetCommand(bufnr(''), ['3.0.0']) + " Try with older versions. - call ale_linters#python#flake8#ClearVersionCache() + call ale#semver#ResetVersionCache() AssertEqual \ ale#Escape('flake8') . ' --format=default -', \ ale_linters#python#flake8#GetCommand(bufnr(''), ['2.9.9']) @@ -49,7 +50,9 @@ Execute(The flake8 command callback should let you set options): \ . ' --some-option --format=default' \ . ' --stdin-display-name %s -', \ ale_linters#python#flake8#GetCommand(bufnr(''), ['3.0.4']) - call ale_linters#python#flake8#ClearVersionCache() + + call ale#semver#ResetVersionCache() + AssertEqual \ ale#Escape('flake8') \ . ' --some-option --format=default -', @@ -140,7 +143,7 @@ Execute(Using `python -m flake8` should be supported for running flake8): \ ale#Escape('python') . ' -m flake8 --some-option --format=default -', \ ale_linters#python#flake8#GetCommand(bufnr(''), ['2.9.9']) - call ale_linters#python#flake8#ClearVersionCache() + call ale#semver#ResetVersionCache() " Leading spaces shouldn't matter let g:ale_python_flake8_options = ' -m flake8 --some-option' diff --git a/test/handler/test_gcc_handler.vader b/test/handler/test_gcc_handler.vader index 9324273e..79f17899 100644 --- a/test/handler/test_gcc_handler.vader +++ b/test/handler/test_gcc_handler.vader @@ -71,17 +71,6 @@ Execute(GCC errors from included files should be parsed correctly): \ ' ^', \ ]) -Execute(GCC versions should be parsed correctly): - AssertEqual [4, 9, 1], ale#handlers#gcc#ParseGCCVersion([ - \ 'g++ (GCC) 4.9.1 20140922 (Red Hat 4.9.1-10)', - \]) - AssertEqual [4, 8, 4], ale#handlers#gcc#ParseGCCVersion([ - \ 'gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4', - \ 'Copyright (C) 2013 Free Software Foundation, Inc.', - \ 'This is free software; see the source for copying conditions. There is NO', - \ 'warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.', - \]) - Execute(The GCC handler shouldn't complain about #pragma once for headers): silent file! test.h diff --git a/test/test_flow_command.vader b/test/test_flow_command.vader index 32ceb57c..49546e94 100644 --- a/test/test_flow_command.vader +++ b/test/test_flow_command.vader @@ -5,6 +5,7 @@ Before: After: call ale#test#RestoreDirectory() call ale#linter#Reset() + call ale#semver#ResetVersionCache() Execute(flow should return a command to run if a .flowconfig file exists): call ale#test#SetFilename('flow/a/sub/dummy') diff --git a/test/test_semver_utils.vader b/test/test_semver_utils.vader index 9730b74b..30e9e818 100644 --- a/test/test_semver_utils.vader +++ b/test/test_semver_utils.vader @@ -1,16 +1,50 @@ -Execute(ParseSemver should return the correct results): - " We should be able to parse the semver string from flake8 - AssertEqual [3, 0, 4], ale#semver#Parse('3.0.4 (mccabe: 0.5.2, pyflakes: 1.2.3, pycodestyle: 2.0.0) CPython 2.7.12 on Linux') +After: + call ale#semver#ResetVersionCache() -Execute(GreaterOrEqual should compare triples correctly): - Assert ale#semver#GreaterOrEqual([3, 0, 4], [3, 0, 0]) - Assert ale#semver#GreaterOrEqual([3, 0, 0], [3, 0, 0]) - Assert ale#semver#GreaterOrEqual([3, 0, 0], [2, 0, 0]) - Assert ale#semver#GreaterOrEqual([3, 1, 0], [3, 1, 0]) - Assert ale#semver#GreaterOrEqual([3, 2, 0], [3, 1, 0]) - Assert ale#semver#GreaterOrEqual([3, 2, 2], [3, 1, 6]) - Assert ale#semver#GreaterOrEqual([3, 2, 5], [3, 2, 5]) - Assert ale#semver#GreaterOrEqual([3, 2, 6], [3, 2, 5]) - Assert !ale#semver#GreaterOrEqual([2, 9, 1], [3, 0, 0]) - Assert !ale#semver#GreaterOrEqual([3, 2, 3], [3, 3, 3]) - Assert !ale#semver#GreaterOrEqual([3, 3, 2], [3, 3, 3]) +Execute(GetVersion should return the version from the lines of output): + " We should be able to parse the semver string from flake8 + AssertEqual [3, 0, 4], ale#semver#GetVersion('dummy', [ + \ '3.0.4 (mccabe: 0.5.2, pyflakes: 1.2.3, pycodestyle: 2.0.0) CPython 2.7.12 on Linux', + \ '1.2.3', + \]) + +Execute(GetVersion should return an empty list when no vesrion can be found): + AssertEqual [], ale#semver#GetVersion('dummy', ['x']) + AssertEqual [], ale#semver#GetVersion('dummy', []) + +Execute(GetVersion should cache the version): + AssertEqual [], ale#semver#GetVersion('dummy', []) + AssertEqual [3, 4, 7], ale#semver#GetVersion('dummy', ['Version 3.4.7']) + AssertEqual [3, 4, 7], ale#semver#GetVersion('dummy', []) + +Execute(HasVersion should return 1 when the version has been cached): + call ale#semver#GetVersion('dummy', []) + AssertEqual 0, ale#semver#HasVersion('dummy') + call ale#semver#GetVersion('dummy', ['3.4.7']) + AssertEqual 1, ale#semver#HasVersion('dummy') + +Execute(GTE should compare triples correctly): + Assert ale#semver#GTE([3, 0, 4], [3, 0, 0]) + Assert ale#semver#GTE([3, 0, 0], [3, 0, 0]) + Assert ale#semver#GTE([3, 0, 0], [2, 0, 0]) + Assert ale#semver#GTE([3, 1, 0], [3, 1, 0]) + Assert ale#semver#GTE([3, 2, 0], [3, 1, 0]) + Assert ale#semver#GTE([3, 2, 2], [3, 1, 6]) + Assert ale#semver#GTE([3, 2, 5], [3, 2, 5]) + Assert ale#semver#GTE([3, 2, 6], [3, 2, 5]) + Assert !ale#semver#GTE([2, 9, 1], [3, 0, 0]) + Assert !ale#semver#GTE([3, 2, 3], [3, 3, 3]) + Assert !ale#semver#GTE([3, 3, 2], [3, 3, 3]) + +Execute(GTE should compare pairs correctly): + Assert ale#semver#GTE([3, 0], [3, 0, 0]) + Assert ale#semver#GTE([3, 0], [3, 0]) + Assert ale#semver#GTE([3, 1], [3, 0]) + Assert ale#semver#GTE([3, 1], [3, 0, 0]) + Assert ale#semver#GTE([3, 0, 1], [3, 0]) + Assert !ale#semver#GTE([3, 0], [3, 0, 1]) + Assert !ale#semver#GTE([3, 0], [3, 1]) + Assert !ale#semver#GTE([2, 9, 11], [3, 0]) + +Execute(GTE should permit the LHS to be an empty List): + Assert !ale#semver#GTE([], [0, 0, 0])