#482 - Fix Flow handling with relative paths

4 files changed, 130 insertions, 45 deletions

diff --git a/ale_linters/javascript/flow.vim b/ale_linters/javascript/flow.vim
index 4e1494e2..1b13e5d1 100644
--- a/ale_linters/javascript/flow.vim
+++ b/ale_linters/javascript/flow.vim
@@ -42,7 +42,9 @@ function! ale_linters#javascript#flow#Handle(buffer, lines) abort
             " Comments have no line of column information, so we skip them.
             " In certain cases, `l:message.loc.source` points to a different path
             " than the buffer one, thus we skip this loc information too.
-            if has_key(l:message, 'loc') && l:line ==# 0 && l:message.loc.source ==# expand('#' . a:buffer . ':p')
+            if has_key(l:message, 'loc')
+            \&& l:line ==# 0
+            \&& ale#path#IsBufferPath(a:buffer, l:message.loc.source)
                 let l:line = l:message.loc.start.line + 0
                 let l:col = l:message.loc.start.column + 0
             endif
diff --git a/autoload/ale/path.vim b/autoload/ale/path.vim
index 88aa482d..0365ceed 100644
--- a/autoload/ale/path.vim
+++ b/autoload/ale/path.vim
@@ -62,35 +62,19 @@ function! ale#path#IsAbsolute(filename) abort
     return a:filename[:0] ==# '/' || a:filename[1:2] ==# ':\'
 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#GetAbsPath(directory, filename) abort
-    " If the path is already absolute, then just resolve it.
-    if ale#path#IsAbsolute(a:filename)
-        return resolve(a:filename)
-    endif
-
-    " Get an absolute path to our containing directory.
-    " If our directory is relative, then we'll use the CWD.
-    let l:absolute_directory = ale#path#IsAbsolute(a:directory)
-    \   ? a:directory
-    \   : getcwd() . '/' . a:directory
-
-    " Resolve the relative path to the file with the absolute path to our
-    " directory.
-    return resolve(l:absolute_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
+function! ale#path#IsBufferPath(buffer, complex_filename) abort
+    let l:test_filename = simplify(a:complex_filename)
+
+    if l:test_filename[:1] ==# './'
+        let l:test_filename = l:test_filename[2:]
+    endif
+
     let l:buffer_filename = expand('#' . a:buffer . ':p')
-    let l:resolved_filename = ale#path#GetAbsPath(
-    \   fnamemodify(l:buffer_filename, ':h'),
-    \   a:filename
-    \)
 
-    return resolve(l:buffer_filename) ==# l:resolved_filename
+    return l:buffer_filename ==# l:test_filename
+    \   || l:buffer_filename[-len(l:test_filename):] ==# l:test_filename
 endfunction
 
 " Given a path, return every component of the path, moving upwards.
diff --git a/test/handler/test_flow_handler.vader b/test/handler/test_flow_handler.vader
index 597366f3..46b52229 100644
--- a/test/handler/test_flow_handler.vader
+++ b/test/handler/test_flow_handler.vader
@@ -8,7 +8,7 @@ After:
   call ale#linter#Reset()
 
 Execute(The flow handler should process errors correctly.):
-  e! /home/w0rp/Downloads/graphql-js/src/language/parser.js
+  silent! noautocmd file /home/w0rp/Downloads/graphql-js/src/language/parser.js
 
   let g:flow_output = {
   \ "flowVersion": "0.39.0",
@@ -130,7 +130,7 @@ Execute(The flow handler should process errors correctly.):
   AssertEqual g:expected, g:actual
 
 Execute(The flow handler should fetch the correct location for the currently opened file, even when it's not in the first message.):
-  e! /Users/rav/Projects/vim-ale-flow/index.js
+  silent! noautocmd file /Users/rav/Projects/vim-ale-flow/index.js
 
   let g:flow_output = {
   \ "flowVersion": "0.44.0",
@@ -232,3 +232,107 @@ Execute(The flow handler should fetch the correct location for the currently ope
   \]
 
   AssertEqual g:expected, g:actual
+
+Execute(The flow handler should handle relative paths):
+  silent! noautocmd file /Users/rav/Projects/vim-ale-flow/index.js
+
+  let g:flow_output = {
+  \ "flowVersion": "0.44.0",
+  \ "errors": [{
+  \   "operation": {
+  \     "context": "  <Foo foo=\"bar\"/>, document.getElementById('foo')",
+  \     "descr": "React element `Foo`",
+  \     "type": "Blame",
+  \     "loc": {
+  \       "source": "vim-ale-flow/index.js",
+  \       "type": "SourceFile",
+  \       "start": {
+  \         "line": 6,
+  \         "column": 3,
+  \         "offset": 92
+  \       },
+  \       "end": {
+  \         "line": 6,
+  \         "column": 18,
+  \         "offset": 108
+  \       }
+  \     },
+  \     "path": "vim-ale-flow/index.js",
+  \     "line": 6,
+  \     "endline": 6,
+  \     "start": 3,
+  \     "end": 18
+  \   },
+  \   "kind": "infer",
+  \   "level": "error",
+  \   "message": [{
+  \     "context": "module.exports = function(props: Props) {",
+  \     "descr": "property `bar`",
+  \     "type": "Blame",
+  \     "loc": {
+  \       "source": "vim-ale-flow/foo.js",
+  \       "type": "SourceFile",
+  \       "start": {
+  \         "line": 9,
+  \         "column": 34,
+  \         "offset": 121
+  \       },
+  \       "end": {
+  \         "line": 9,
+  \         "column": 38,
+  \         "offset": 126
+  \       }
+  \     },
+  \     "path": "vim-ale-flow/foo.js",
+  \     "line": 9,
+  \     "endline": 9,
+  \     "start": 34,
+  \     "end": 38
+  \   }, {
+  \     "context": v:null,
+  \     "descr": "Property not found in",
+  \     "type": "Comment",
+  \     "path": "",
+  \     "line": 0,
+  \     "endline": 0,
+  \     "start": 1,
+  \     "end": 0
+  \   }, {
+  \     "context": "  <Foo foo=\"bar\"/>, document.getElementById('foo')",
+  \     "descr": "props of React element `Foo`",
+  \     "type": "Blame",
+  \     "loc": {
+  \       "source": "vim-ale-flow/index.js",
+  \       "type": "SourceFile",
+  \       "start": {
+  \         "line": 6,
+  \         "column": 3,
+  \         "offset": 92
+  \       },
+  \       "end": {
+  \         "line": 6,
+  \         "column": 18,
+  \         "offset": 108
+  \       }
+  \     },
+  \     "path": "vim-ale-flow/index.js",
+  \     "line": 6,
+  \     "endline": 6,
+  \     "start": 3,
+  \     "end": 18
+  \   }]
+  \ }],
+  \ "passed": v:false
+  \}
+
+  let g:actual = ale_linters#javascript#flow#Handle(bufnr(''), [json_encode(g:flow_output)])
+  let g:expected = [
+  \ {
+  \   'lnum': 6,
+  \   'col': 3,
+  \   'type': 'E',
+  \   'text': 'property `bar`: Property not found in props of React element `Foo` See also: React element `Foo`'
+  \ }
+  \]
+
+  AssertEqual g:expected, g:actual
diff --git a/test/test_path_equality.vader b/test/test_path_equality.vader
index b1f06967..5d92794f 100644
--- a/test/test_path_equality.vader
+++ b/test/test_path_equality.vader
@@ -1,18 +1,3 @@
-Execute(ale#path#GetAbsPath should handle simple relative paths):
-  AssertEqual '/foo/bar', ale#path#GetAbsPath('/foo', 'bar')
-  AssertEqual 'C:\foo/bar', ale#path#GetAbsPath('C:\foo', 'bar')
-  AssertEqual getcwd() . '/foo/bar', ale#path#GetAbsPath('foo', 'bar')
-
-Execute(ale#path#GetAbsPath should handle relative paths with dots):
-  AssertEqual '/foo/baz', ale#path#GetAbsPath('/foo', 'bar/sub/../../baz')
-  AssertEqual '/foo/baz', ale#path#GetAbsPath('/foo/', 'bar/sub/../../baz')
-  AssertEqual '/foo/other', ale#path#GetAbsPath('/foo/bar', '../other')
-  AssertEqual '/foo/other', ale#path#GetAbsPath('/foo/bar/', '../other')
-
-Execute(ale#path#GetAbsPath should handle absolute paths):
-  AssertEqual '/foo/bar', ale#path#GetAbsPath('/something else', '/foo/bar')
-  AssertEqual 'C:\foo/bar', ale#path#GetAbsPath('D:\another thing', 'C:\foo/bar')
-
 Execute(ale#path#IsBufferPath should match simple relative paths):
   silent file! foo.txt
 
@@ -25,7 +10,17 @@ Execute(ale#path#IsBufferPath should match absolute paths):
   Assert ale#path#IsBufferPath(bufnr(''), getcwd() . '/foo.txt'), 'No match for foo.txt'
   Assert !ale#path#IsBufferPath(bufnr(''), getcwd() . '/bar.txt'), 'Bad match for bar.txt'
 
-Execute(ale#path#IsBufferPath should match paths with dots):
+Execute(ale#path#IsBufferPath should match paths beginning with ./):
+  silent file! foo.txt
+
+  Assert ale#path#IsBufferPath(bufnr(''), './foo.txt'), 'No match for ./foo.txt'
+
+Execute(ale#path#IsBufferPath should match if our path ends with the test path):
+  silent file! foo/bar/baz.txt
+
+  Assert ale#path#IsBufferPath(bufnr(''), 'bar/baz.txt'), 'No match for bar/baz.txt'
+
+Execute(ale#path#IsBufferPath should match paths with redundant slashes):
   silent file! foo.txt
 
-  Assert ale#path#IsBufferPath(bufnr(''), './test/../foo.txt'), 'No match for ./test/../foo.txt'
+  Assert ale#path#IsBufferPath(bufnr(''), getcwd() . '////foo.txt'), 'No match for foo.txt'