Fix #518 Fix handling of spaces in filenames for various linters

10 files changed, 200 insertions, 48 deletions

diff --git a/ale_linters/go/gobuild.vim b/ale_linters/go/gobuild.vim
index 419e67a0..62687534 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:output = []
+    let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?:? (.+)$'
 
-    for l:line in a:lines
-        let l:match = matchlist(l:line, l:pattern)
+    return ale#util#GetMatches(a:lines, l:pattern)
+endfunction
+
+function! ale_linters#go#gobuild#Handler(buffer, lines) abort
+    let l:output = []
 
+    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 b71747c3..6ad78cab 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#GetMatches(lines) abort
+    let l:pattern = '\v^([a-zA-Z]?:?[^:]+):(\d+):?(\d+)?:?:?(warning|error):?\s\*?(.+)$'
+
+    return ale#util#GetMatches(a:lines, l:pattern)
+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"
-
-    " 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\*\?\(.\+\)$'
     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 8c432f86..fff306ae 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 4777dd18..02f9758a 100644
--- a/autoload/ale/handlers/python.vim
+++ b/autoload/ale/handlers/python.vim
@@ -1,8 +1,6 @@
 " Author: w0rp <devw0rp@gmail.com>
 " 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 be0f0823..f90fd591 100644
--- a/autoload/ale/handlers/unix.vim
+++ b/autoload/ale/handlers/unix.vim
@@ -1,10 +1,8 @@
 " Author: w0rp <devw0rp@gmail.com>
 " 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 cbd4d886..6fd5142b 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 21a6f6a0..a9fc9149 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 00000000..7748dcc7
--- /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 00000000..3b622130
--- /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 77e678e4..e1b1562d 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'",
+  \ ])