From 0b4acd645395d4196f2e346d65aeddf063927f4e Mon Sep 17 00:00:00 2001 From: w0rp Date: Sat, 29 Apr 2017 17:33:18 +0100 Subject: [PATCH] Fix #518 Fix handling of spaces in filenames for various linters --- ale_linters/go/gobuild.vim | 29 +++++------ ale_linters/go/gometalinter.vim | 30 +++++------ ale_linters/python/mypy.vim | 4 +- autoload/ale/handlers/python.vim | 8 ++- autoload/ale/handlers/unix.vim | 4 +- autoload/ale/path.vim | 27 ++++++++++ test/handler/test_common_handlers.vader | 28 +++++++++++ test/handler/test_gobuild_handler.vader | 44 ++++++++++++++++ test/handler/test_gometalinter_handler.vader | 53 ++++++++++++++++++++ test/handler/test_mypy_handler.vader | 21 ++++++-- 10 files changed, 200 insertions(+), 48 deletions(-) create mode 100644 test/handler/test_gobuild_handler.vader create mode 100644 test/handler/test_gometalinter_handler.vader diff --git a/ale_linters/go/gobuild.vim b/ale_linters/go/gobuild.vim index 419e67a..6268753 100644 --- a/ale_linters/go/gobuild.vim +++ b/ale_linters/go/gobuild.vim @@ -25,35 +25,32 @@ function! ale_linters#go#gobuild#GetCommand(buffer, goenv_output) abort \ . ' && go test -c -o /dev/null ./' endfunction -function! ale_linters#go#gobuild#Handler(buffer, lines) abort - return ale_linters#go#gobuild#HandleGoBuildErrors(a:buffer, bufname(a:buffer), a:lines) -endfunction - -function! ale_linters#go#gobuild#HandleGoBuildErrors(buffer, full_filename, lines) abort - " Matches patterns line the following: +function! ale_linters#go#gobuild#GetMatches(lines) abort + " Matches patterns like the following: " " file.go:27: missing argument for Printf("%s"): format reads arg 2, have only 1 args " file.go:53:10: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) " file.go:5:2: expected declaration, found 'STRING' "log" " go test returns relative paths so use tail of filename as part of pattern matcher - let l:filename = fnamemodify(a:full_filename, ':t') - let l:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+' - let l:pattern = '^' . l:path_pattern . ':\(\d\+\):\?\(\d\+\)\?:\? \(.\+\)$' + let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?:? (.+)$' + + return ale#util#GetMatches(a:lines, l:pattern) +endfunction + +function! ale_linters#go#gobuild#Handler(buffer, lines) abort let l:output = [] - for l:line in a:lines - let l:match = matchlist(l:line, l:pattern) - + for l:match in ale_linters#go#gobuild#GetMatches(a:lines) " Omit errors from imported go packages - if len(l:match) == 0 || l:line !~ l:filename + if ale#path#IsBufferPath(a:buffer, l:match[0]) continue endif call add(l:output, { - \ 'lnum': l:match[1] + 0, - \ 'col': l:match[2] + 0, - \ 'text': l:match[3], + \ 'lnum': l:match[2] + 0, + \ 'col': l:match[3] + 0, + \ 'text': l:match[4], \ 'type': 'E', \}) endfor diff --git a/ale_linters/go/gometalinter.vim b/ale_linters/go/gometalinter.vim index b71747c..6ad78ca 100644 --- a/ale_linters/go/gometalinter.vim +++ b/ale_linters/go/gometalinter.vim @@ -11,32 +11,26 @@ function! ale_linters#go#gometalinter#GetCommand(buffer) abort \ . ' ' . fnameescape(fnamemodify(bufname(a:buffer), ':p:h')) endfunction -function! ale_linters#go#gometalinter#Handler(buffer, lines) abort - " Matches patterns line the following: - " - " file.go:27: missing argument for Printf("%s"): format reads arg 2, have only 1 args - " file.go:53:10: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) - " file.go:5:2: expected declaration, found 'STRING' "log" +function! ale_linters#go#gometalinter#GetMatches(lines) abort + let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?:?:?(warning|error):?\s\*?(.+)$' - " gometalinter returns relative paths so use tail of filename as part of pattern matcher - let l:filename = fnamemodify(bufname(a:buffer), ':t') - let l:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+' - let l:pattern = '^' . l:path_pattern . ':\(\d\+\):\?\(\d\+\)\?:\?:\?\(warning\|error\):\?\s\*\?\(.\+\)$' + return ale#util#GetMatches(a:lines, l:pattern) +endfunction + +function! ale_linters#go#gometalinter#Handler(buffer, lines) abort let l:output = [] - for l:line in a:lines - let l:match = matchlist(l:line, l:pattern) - + for l:match in ale_linters#go#gometalinter#GetMatches(a:lines) " Omit errors from files other than the one currently open - if len(l:match) == 0 || l:line !~ l:filename + if ale#path#IsBufferPath(a:buffer, l:match[0]) continue endif call add(l:output, { - \ 'lnum': l:match[1] + 0, - \ 'col': l:match[2] + 0, - \ 'text': l:match[4], - \ 'type': tolower(l:match[3]) ==# 'warning' ? 'W' : 'E', + \ 'lnum': l:match[2] + 0, + \ 'col': l:match[3] + 0, + \ 'type': tolower(l:match[4]) ==# 'warning' ? 'W' : 'E', + \ 'text': l:match[5], \}) endfor diff --git a/ale_linters/python/mypy.vim b/ale_linters/python/mypy.vim index 8c432f8..fff306a 100644 --- a/ale_linters/python/mypy.vim +++ b/ale_linters/python/mypy.vim @@ -15,8 +15,6 @@ function! ale_linters#python#mypy#GetCommand(buffer) abort \ . ' %t' endfunction -let s:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+' - function! ale_linters#python#mypy#Handle(buffer, lines) abort " Look for lines like the following: " @@ -25,7 +23,7 @@ function! ale_linters#python#mypy#Handle(buffer, lines) abort " Lines like these should be ignored below: " " file.py:4: note: (Stub files are from https://github.com/python/typeshed) - let l:pattern = '^' . s:path_pattern . ':\(\d\+\):\?\(\d\+\)\?: \([^:]\+\): \(.\+\)$' + let l:pattern = '\v^[a-zA-Z]?:?[^:]+:(\d+):?(\d+)?: ([^:]+): (.+)$' let l:output = [] for l:match in ale#util#GetMatches(a:lines, l:pattern) diff --git a/autoload/ale/handlers/python.vim b/autoload/ale/handlers/python.vim index 4777dd1..02f9758 100644 --- a/autoload/ale/handlers/python.vim +++ b/autoload/ale/handlers/python.vim @@ -1,8 +1,6 @@ " Author: w0rp " Description: Error handling for flake8, etc. -let s:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+' - function! ale#handlers#python#HandlePEP8Format(buffer, lines) abort " Matches patterns line the following: " @@ -10,7 +8,7 @@ function! ale#handlers#python#HandlePEP8Format(buffer, lines) abort " " stdin:6:6: E111 indentation is not a multiple of four " test.yml:35: [EANSIBLE0002] Trailing whitespace - let l:pattern = '^' . s:path_pattern . ':\(\d\+\):\?\(\d\+\)\?: \[\?\(\([[:alpha:]]\)[[:alnum:]]\+\)\]\? \(.*\)$' + let l:pattern = '\v^[a-zA-Z]?:?[^:]+:(\d+):?(\d+)?: \[?([[:alnum:]]+)\]? (.*)$' let l:output = [] for l:match in ale#util#GetMatches(a:lines, l:pattern) @@ -30,8 +28,8 @@ function! ale#handlers#python#HandlePEP8Format(buffer, lines) abort call add(l:output, { \ 'lnum': l:match[1] + 0, \ 'col': l:match[2] + 0, - \ 'text': l:code . ': ' . l:match[5], - \ 'type': l:match[4] ==# 'E' ? 'E' : 'W', + \ 'text': l:code . ': ' . l:match[4], + \ 'type': l:code[:0] ==# 'E' ? 'E' : 'W', \}) endfor diff --git a/autoload/ale/handlers/unix.vim b/autoload/ale/handlers/unix.vim index be0f082..f90fd59 100644 --- a/autoload/ale/handlers/unix.vim +++ b/autoload/ale/handlers/unix.vim @@ -1,10 +1,8 @@ " Author: w0rp " Description: Error handling for errors in a Unix format. -let s:path_pattern = '[a-zA-Z]\?\\\?:\?[[:alnum:]/\.\-_]\+' - function! s:HandleUnixFormat(buffer, lines, type) abort - let l:pattern = '^' . s:path_pattern . ':\(\d\+\):\?\(\d\+\)\?:\? \?\(.\+\)$' + let l:pattern = '\v^[a-zA-Z]?:?[^:]+:(\d+):?(\d+)?:? ?(.+)$' let l:output = [] for l:match in ale#util#GetMatches(a:lines, l:pattern) diff --git a/autoload/ale/path.vim b/autoload/ale/path.vim index cbd4d88..6fd5142 100644 --- a/autoload/ale/path.vim +++ b/autoload/ale/path.vim @@ -55,3 +55,30 @@ endfunction function! ale#path#BufferCdString(buffer) abort return ale#path#CdString(fnamemodify(bufname(a:buffer), ':p:h')) endfunction + +" Return 1 if a path is an absolute path. +function! ale#path#IsAbsolute(filename) abort + return match(a:filename, '^\v/|^[a-zA-Z]:\\') == 0 +endfunction + +" Given a directory and a filename, resolve the path, which may be relative +" or absolute, and get an absolute path to the file, following symlinks. +function! ale#path#Resolve(directory, filename) abort + return resolve( + \ ale#path#IsAbsolute(a:filename) + \ ? a:filename + \ : a:directory . '/' . a:filename + \) +endfunction + +" Given a buffer number and a relative or absolute path, return 1 if the +" two paths represent the same file on disk. +function! ale#path#IsBufferPath(buffer, filename) abort + let l:buffer_filename = expand('#' . a:buffer . ':p') + let l:resolved_filename = ale#path#Resolve( + \ fnamemodify(l:buffer_filename, ':h'), + \ a:filename + \) + + return resolve(l:buffer_filename) == l:resolved_filename +endfunction diff --git a/test/handler/test_common_handlers.vader b/test/handler/test_common_handlers.vader index 21a6f6a..a9fc914 100644 --- a/test/handler/test_common_handlers.vader +++ b/test/handler/test_common_handlers.vader @@ -40,6 +40,20 @@ Execute (HandlePEP8Format should handle the correct lines of output): \ "test.yml:35: [EANSIBLE0002] Trailing whitespace", \ ]) +Execute (HandlePEP8Format should handle names with spaces): + AssertEqual + \ [ + \ { + \ 'lnum': 6, + \ 'col': 6, + \ 'type': 'E', + \ 'text': 'E111: indentation is not a multiple of four', + \ }, + \ ], + \ ale#handlers#python#HandlePEP8Format(42, [ + \ 'C:\something\with spaces.py:6:6: E111 indentation is not a multiple of four', + \ ]) + Execute (HandleGCCFormat should handle the correct lines of output): AssertEqual \ [ @@ -122,6 +136,20 @@ Execute (HandleUnixFormatAsError should handle lines with no space after the col \ 'some_file.xyz:53:10:bar', \ ]) +Execute (HandleUnixFormatAsError should handle names with spaces): + AssertEqual + \ [ + \ { + \ 'lnum': 13, + \ 'col': 90, + \ 'type': 'E', + \ 'text': 'leonard.exclamation.30ppm More than 30 ppm of exclamations. Keep them under control.', + \ }, + \ ], + \ ale#handlers#unix#HandleAsError(42, [ + \ '/Users/rrj/Notes/Astro/Taurus December SM.txt:13:90: leonard.exclamation.30ppm More than 30 ppm of exclamations. Keep them under control.', + \ ]) + Execute (HandleUnixFormatAsWarning should handle some example lines of output): AssertEqual \ [ diff --git a/test/handler/test_gobuild_handler.vader b/test/handler/test_gobuild_handler.vader new file mode 100644 index 0000000..7748dcc --- /dev/null +++ b/test/handler/test_gobuild_handler.vader @@ -0,0 +1,44 @@ +Before: + runtime ale_linters/go/gobuild.vim + +After: + call ale#linter#Reset() + +Execute (The gobuild handler should handle names with spaces): + " We can't test Windows paths with the path resovling on Linux, but we can + " test the regex. + AssertEqual + \ [ + \ [ + \ 'C:\something\file with spaces.go', + \ '27', + \ '', + \ 'missing argument for Printf("%s"): format reads arg 2, have only 1 args', + \ ], + \ [ + \ 'C:\something\file with spaces.go', + \ '5', + \ '2', + \ 'expected declaration, found ''STRING'' "log"', + \ ], + \ ], + \ map(ale_linters#go#gobuild#GetMatches([ + \ 'C:\something\file with spaces.go:27: missing argument for Printf("%s"): format reads arg 2, have only 1 args', + \ 'C:\something\file with spaces.go:5:2: expected declaration, found ''STRING'' "log"', + \ ]), 'v:val[1:4]') + +Execute (The gobuild handler should handle relative paths correctly): + :file! /foo/bar/baz.go + + AssertEqual + \ [ + \ { + \ 'lnum': 27, + \ 'col': 0, + \ 'text': 'missing argument for Printf("%s"): format reads arg 2, have only 1 args', + \ 'type': 'E', + \ }, + \ ], + \ ale_linters#go#gobuild#Handler(42, [ + \ 'baz.go:27: missing argument for Printf("%s"): format reads arg 2, have only 1 args', + \ ]) diff --git a/test/handler/test_gometalinter_handler.vader b/test/handler/test_gometalinter_handler.vader new file mode 100644 index 0000000..3b62213 --- /dev/null +++ b/test/handler/test_gometalinter_handler.vader @@ -0,0 +1,53 @@ +Before: + runtime ale_linters/go/gometalinter.vim + +After: + call ale#linter#Reset() + +Execute (The gometalinter handler should handle names with spaces): + " We can't test Windows paths with the path resovling on Linux, but we can + " test the regex. + AssertEqual + \ [ + \ [ + \ 'C:\something\file with spaces.go', + \ '12', + \ '3', + \ 'warning', + \ 'expected ''package'', found ''IDENT'' gibberish (staticcheck)', + \ ], + \ [ + \ 'C:\something\file with spaces.go', + \ '37', + \ '5', + \ 'error', + \ 'expected ''package'', found ''IDENT'' gibberish (golint)', + \ ], + \ ], + \ map(ale_linters#go#gometalinter#GetMatches([ + \ 'C:\something\file with spaces.go:12:3:warning: expected ''package'', found ''IDENT'' gibberish (staticcheck)', + \ 'C:\something\file with spaces.go:37:5:error: expected ''package'', found ''IDENT'' gibberish (golint)', + \ ]), 'v:val[1:5]') + +Execute (The gometalinter handler should handle relative paths correctly): + :file! /foo/bar/baz.go + + AssertEqual + \ [ + \ { + \ 'lnum': 12, + \ 'col': 3, + \ 'text': 'expected ''package'', found ''IDENT'' gibberish (staticcheck)', + \ 'type': 'W', + \ }, + \ { + \ 'lnum': 37, + \ 'col': 5, + \ 'text': 'expected ''package'', found ''IDENT'' gibberish (golint)', + \ 'type': 'E', + \ }, + \ ], + \ ale_linters#go#gometalinter#Handler(42, [ + \ 'baz.go:12:3:warning: expected ''package'', found ''IDENT'' gibberish (staticcheck)', + \ 'baz.go:37:5:error: expected ''package'', found ''IDENT'' gibberish (golint)', + \ ]) diff --git a/test/handler/test_mypy_handler.vader b/test/handler/test_mypy_handler.vader index 77e678e..e1b1562 100644 --- a/test/handler/test_mypy_handler.vader +++ b/test/handler/test_mypy_handler.vader @@ -1,6 +1,10 @@ -Execute(The mypy handler should parse lines correctly): +Before: runtime ale_linters/python/mypy.vim +After: + call ale#linter#Reset() + +Execute(The mypy handler should parse lines correctly): AssertEqual \ [ \ { @@ -22,5 +26,16 @@ Execute(The mypy handler should parse lines correctly): \ "file.py:40:5: error: Some other problem", \ ]) -After: - call ale#linter#Reset() +Execute(The mypy handler should handle Windows names with spaces): + AssertEqual + \ [ + \ { + \ 'lnum': 4, + \ 'col': 0, + \ 'text': "No library stub file for module 'django.db'", + \ 'type': 'E', + \ }, + \ ], + \ ale_linters#python#mypy#Handle(347, [ + \ "C:\something\with spaces.py:4: error: No library stub file for module 'django.db'", + \ ])