From 038e4a8c31feb03beea02c09d953affc8c460515 Mon Sep 17 00:00:00 2001 From: serapas Date: Sun, 28 Feb 2021 13:43:23 +0100 Subject: [PATCH] Fix ansible-lint linter definition. (#3601) * Fix ansible-lint linter definition. Use ansible-lint's feature auto-detection instead of temporary file. For auto-detection to work, ansible project has to be also a git repository. Don't use yaml rules. These are checked by yamllint. Refactor pattern to work with ansible-lint >=5.0 version. Clean-up obsolete test cases. * Pull Request changes --- ale_linters/ansible/ansible_lint.vim | 97 ++++++++++++++----- .../test_ansible_lint_command_callback.vader | 9 +- test/handler/test_ansible_lint_handler.vader | 63 +++++++++--- 3 files changed, 129 insertions(+), 40 deletions(-) diff --git a/ale_linters/ansible/ansible_lint.vim b/ale_linters/ansible/ansible_lint.vim index c4affa31..f83de01a 100644 --- a/ale_linters/ansible/ansible_lint.vim +++ b/ale_linters/ansible/ansible_lint.vim @@ -1,4 +1,4 @@ -" Author: Bjorn Neergaard +" Authors: Bjorn Neergaard , Vytautas Macionis " Description: ansible-lint for ansible-yaml files call ale#Set('ansible_ansible_lint_executable', 'ansible-lint') @@ -7,7 +7,7 @@ function! ale_linters#ansible#ansible_lint#GetExecutable(buffer) abort return ale#Var(a:buffer, 'ansible_ansible_lint_executable') endfunction -function! ale_linters#ansible#ansible_lint#Handle(buffer, lines) abort +function! ale_linters#ansible#ansible_lint#Handle(buffer, version, lines) abort for l:line in a:lines[:10] if match(l:line, '^Traceback') >= 0 return [{ @@ -18,39 +18,86 @@ function! ale_linters#ansible#ansible_lint#Handle(buffer, lines) abort endif endfor - " Matches patterns line the following: - " - " test.yml:35: [EANSIBLE0002] Trailing whitespace - let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?: \[?([[:alnum:]]+)\]? (.*)$' + let l:version_group = ale#semver#GTE(a:version, [5, 0, 0]) ? '>=5.0.0' : '<5.0.0' let l:output = [] - for l:match in ale#util#GetMatches(a:lines, l:pattern) - let l:code = l:match[4] + if '>=5.0.0' is# l:version_group + " Matches patterns line the following: + " test.yml:3:148: syntax-check 'var' is not a valid attribute for a Play + " roles/test/tasks/test.yml:8: [package-latest] [VERY_LOW] Package installs should not use latest + " D:\test\tasks\test.yml:8: [package-latest] [VERY_LOW] package installs should not use latest + let l:pattern = '\v^(%([a-zA-Z]:)?[^:]+):(\d+):%((\d+):)? %(\[([-[:alnum:]]+)\]) %(\[([_[:alnum:]]+)\]) (.*)$' + let l:error_codes = { 'VERY_HIGH': 'E', 'HIGH': 'E', 'MEDIUM': 'W', 'LOW': 'W', 'VERY_LOW': 'W', 'INFO': 'I' } - if l:code is# 'EANSIBLE0002' - \&& !ale#Var(a:buffer, 'warn_about_trailing_whitespace') - " Skip warnings for trailing whitespace if the option is off. - continue - endif + for l:match in ale#util#GetMatches(a:lines, l:pattern) + if ale#path#IsBufferPath(a:buffer, l:match[1]) + call add(l:output, { + \ 'lnum': l:match[2] + 0, + \ 'col': l:match[3] + 0, + \ 'text': l:match[6], + \ 'code': l:match[4], + \ 'type': l:error_codes[l:match[5]], + \}) + endif + endfor + endif - if ale#path#IsBufferPath(a:buffer, l:match[1]) - call add(l:output, { - \ 'lnum': l:match[2] + 0, - \ 'col': l:match[3] + 0, - \ 'text': l:match[5], - \ 'code': l:code, - \ 'type': l:code[:0] is# 'E' ? 'E' : 'W', - \}) - endif - endfor + if '<5.0.0' is# l:version_group + " Matches patterns line the following: + " test.yml:35: [EANSIBLE0002] Trailing whitespace + let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?: \[?([[:alnum:]]+)\]? (.*)$' + + for l:match in ale#util#GetMatches(a:lines, l:pattern) + let l:code = l:match[4] + + if l:code is# 'EANSIBLE0002' + \&& !ale#Var(a:buffer, 'warn_about_trailing_whitespace') + " Skip warnings for trailing whitespace if the option is off. + continue + endif + + if ale#path#IsBufferPath(a:buffer, l:match[1]) + call add(l:output, { + \ 'lnum': l:match[2] + 0, + \ 'col': l:match[3] + 0, + \ 'text': l:match[5], + \ 'code': l:code, + \ 'type': l:code[:0] is# 'E' ? 'E' : 'W', + \}) + endif + endfor + endif return l:output endfunction +function! ale_linters#ansible#ansible_lint#GetCommand(buffer, version) abort + let l:commands = { + \ '>=5.0.0': '%e --parseable-severity -x yaml', + \ '<5.0.0': '%e -p %t' + \} + let l:command = ale#semver#GTE(a:version, [5, 0]) ? l:commands['>=5.0.0'] : l:commands['<5.0.0'] + + return l:command +endfunction + call ale#linter#Define('ansible', { \ 'name': 'ansible_lint', \ 'aliases': ['ansible', 'ansible-lint'], \ 'executable': function('ale_linters#ansible#ansible_lint#GetExecutable'), -\ 'command': '%e -p %t', -\ 'callback': 'ale_linters#ansible#ansible_lint#Handle', +\ 'command': {buffer -> ale#semver#RunWithVersionCheck( +\ buffer, +\ ale_linters#ansible#ansible_lint#GetExecutable(buffer), +\ '%e --version', +\ function('ale_linters#ansible#ansible_lint#GetCommand'), +\ )}, +\ 'callback': {buffer, lines -> ale#semver#RunWithVersionCheck( +\ buffer, +\ ale_linters#ansible#ansible_lint#GetExecutable(buffer), +\ '%e --version', +\ {buffer, version -> ale_linters#ansible#ansible_lint#Handle( +\ buffer, +\ l:version, +\ lines)}, +\ )}, \}) diff --git a/test/command_callback/test_ansible_lint_command_callback.vader b/test/command_callback/test_ansible_lint_command_callback.vader index ddc6c6c8..a3fbde23 100644 --- a/test/command_callback/test_ansible_lint_command_callback.vader +++ b/test/command_callback/test_ansible_lint_command_callback.vader @@ -7,11 +7,16 @@ After: unlet! b:executable call ale#assert#TearDownLinterTest() -Execute(The ansible_lint command callback should return default string): +Execute(The ansible_lint version <5.0.0 command callback should return default string): + GivenCommandOutput ['v4.1.2'] AssertLinter 'ansible-lint', ale#Escape('ansible-lint') . ' -p %t' +Execute(The ansible_lint version >=5.0.0 command callback should return default string): + GivenCommandOutput ['v5.1.2'] + AssertLinter 'ansible-lint', ale#Escape('ansible-lint') . ' --parseable-severity -x yaml' + Execute(The ansible_lint executable should be configurable): let g:ale_ansible_ansible_lint_executable = '~/.local/bin/ansible-lint' - + GivenCommandOutput ['v4.1.2'] AssertLinter '~/.local/bin/ansible-lint', \ ale#Escape('~/.local/bin/ansible-lint') . ' -p %t' diff --git a/test/handler/test_ansible_lint_handler.vader b/test/handler/test_ansible_lint_handler.vader index 796277e6..28dbba30 100644 --- a/test/handler/test_ansible_lint_handler.vader +++ b/test/handler/test_ansible_lint_handler.vader @@ -1,15 +1,14 @@ Before: runtime ale_linters/ansible/ansible_lint.vim - call ale#test#SetFilename('main.yml') + call ale#test#SetFilename('test_playbook.yml') let b:ale_warn_about_trailing_whitespace = 1 After: unlet! b:ale_warn_about_trailing_whitespace - call ale#linter#Reset() -Execute(The ansible-lint handler should handle basic errors): +Execute(The ansible-lint handler for version group <5 should handle basic errors): AssertEqual \ [ \ { @@ -20,21 +19,44 @@ Execute(The ansible-lint handler should handle basic errors): \ 'code': 'EANSIBLE0002', \ }, \ ], - \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [ - \ fnamemodify(tempname(), ':h') . '/main.yml:35: [EANSIBLE0002] Trailing whitespace', + \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [4, 1, 2], [ + \ fnamemodify(tempname(), ':h') . '/test_playbook.yml:35: [EANSIBLE0002] Trailing whitespace', \ ]) -Execute(The ansible-lint handler should supress trailing whitespace output when the option is used): +Execute(The ansible-lint handler for version group <5 should supress trailing whitespace output when the option is used): let b:ale_warn_about_trailing_whitespace = 0 AssertEqual \ [ \ ], - \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [ - \ fnamemodify(tempname(), ':h') . '/main.yml:35: [EANSIBLE0002] Trailing whitespace', + \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [4, 1, 2], [ + \ fnamemodify(tempname(), ':h') . '/test_playbook.yml:35: [EANSIBLE0002] Trailing whitespace', \ ]) -Execute (The ansible-lint handler should handle names with spaces): + +Execute(The ansible-lint handler for version group >=5 should handle basic errors): + AssertEqual + \ [ + \ { + \ 'lnum': 35, + \ 'col': 0, + \ 'type': 'E', + \ 'text': 'File permissions unset or incorrect', + \ 'code': 'risky-file-permissions', + \ }, + \ ], + \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [5, 1, 2], [ + \ fnamemodify(tempname(), ':h') . '/test_playbook.yml:35: [risky-file-permissions] [VERY_HIGH] File permissions unset or incorrect', + \ ]) + +Before: + runtime ale_linters/ansible/ansible_lint.vim + call ale#test#SetFilename('test playbook.yml') + +After: + call ale#linter#Reset() + +Execute (The ansible-lint handler for version group <5 should handle names with spaces): AssertEqual \ [ \ { @@ -45,14 +67,29 @@ Execute (The ansible-lint handler should handle names with spaces): \ 'code': 'E111', \ }, \ ], - \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [ - \ fnamemodify(tempname(), ':h') . '/main.yml:6:6: E111 indentation is not a multiple of four', + \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [4, 1, 2], [ + \ fnamemodify(tempname(), ':h') . '/test playbook.yml:6:6: E111 indentation is not a multiple of four', + \ ]) + +Execute (The ansible-lint handler for version group >=5 should handle names with spaces): + AssertEqual + \ [ + \ { + \ 'lnum': 3, + \ 'col': 148, + \ 'type': 'E', + \ 'text': "'var' is not a valid attribute for a Play", + \ 'code': 'syntax-check', + \ }, + \ ], + \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [5, 1, 2], [ + \ fnamemodify(tempname(), ':h') . "/test playbook.yml:3:148: [syntax-check] [VERY_HIGH] 'var' is not a valid attribute for a Play", \ ]) Execute (The ansible-lint handler should ignore errors from other files): AssertEqual \ [ \ ], - \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [ - \ '/foo/bar/roles/main.yml:6:6: E111 indentation is not a multiple of four', + \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [5, 1, 2], [ + \ '/foo/bar/roles/test_playbook.yml:6: [command-instead-of-module] [VERY_LOW] curl used in place of get_url or uri module', \ ])