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

@@ -1,4 +1,4 @@
-" Author: Bjorn Neergaard <bjorn@neersighted.com>
+" Authors: Bjorn Neergaard <bjorn@neersighted.com>, Vytautas Macionis <vytautas.macionis@manomail.de>
 " 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:
+    let l:version_group = ale#semver#GTE(a:version, [5, 0, 0]) ? '>=5.0.0' : '<5.0.0'
-    "
-    " test.yml:35: [EANSIBLE0002] Trailing whitespace
-    let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?: \[?([[:alnum:]]+)\]? (.*)$'
     let l:output = []
 
-    for l:match in ale#util#GetMatches(a:lines, l:pattern)
+    if '>=5.0.0' is# l:version_group
-        let l:code = l:match[4]
+        " 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'
+        for l:match in ale#util#GetMatches(a:lines, l:pattern)
-        \&& !ale#Var(a:buffer, 'warn_about_trailing_whitespace')
+            if ale#path#IsBufferPath(a:buffer, l:match[1])
-            " Skip warnings for trailing whitespace if the option is off.
+                call add(l:output, {
-            continue
+                \   'lnum': l:match[2] + 0,
-        endif
+                \   '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])
+    if '<5.0.0' is# l:version_group
-            call add(l:output, {
+        " Matches patterns line the following:
-            \   'lnum': l:match[2] + 0,
+        "      test.yml:35: [EANSIBLE0002] Trailing whitespace
-            \   'col': l:match[3] + 0,
+        let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?: \[?([[:alnum:]]+)\]? (.*)$'
-            \   'text': l:match[5],
+
-            \   'code': l:code,
+        for l:match in ale#util#GetMatches(a:lines, l:pattern)
-            \   'type': l:code[:0] is# 'E' ? 'E' : 'W',
+            let l:code = l:match[4]
-            \})
+
-        endif
+            if l:code is# 'EANSIBLE0002'
-    endfor
+            \&& !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',
+\   'command': {buffer -> ale#semver#RunWithVersionCheck(
-\   'callback': 'ale_linters#ansible#ansible_lint#Handle',
+\       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)},
+\   )},
 \})

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'

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(''), [
+  \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [4, 1, 2], [
-  \ fnamemodify(tempname(), ':h') . '/main.yml:35: [EANSIBLE0002] Trailing whitespace',
+  \ 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(''), [
+  \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [4, 1, 2], [
-  \ fnamemodify(tempname(), ':h') . '/main.yml:35: [EANSIBLE0002] Trailing whitespace',
+  \ 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(''), [
+  \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [4, 1, 2], [
-  \ fnamemodify(tempname(), ':h') . '/main.yml:6:6: E111 indentation is not a multiple of four',
+  \ 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(''), [
+  \ ale_linters#ansible#ansible_lint#Handle(bufnr(''), [5, 1, 2], [
-  \   '/foo/bar/roles/main.yml:6:6: E111 indentation is not a multiple of four',
+  \   '/foo/bar/roles/test_playbook.yml:6: [command-instead-of-module] [VERY_LOW] curl used in place of get_url or uri module',
   \ ])