Added additional unit tests + adapted review comments #1167

ale_linters/c/clang.vim

@@ -9,15 +9,7 @@ function! ale_linters#c#clang#GetExecutable(buffer) abort
 endfunction
 
 function! ale_linters#c#clang#GetCommand(buffer, output) abort
-    let l:cflags = []
+    let l:cflags = ale#c#GetCFlags(a:buffer, a:output)
-    if !empty(a:output)
-        let l:cflags = join(ale#c#ParseMakefile(a:buffer, join(a:output, '\n')), ' ')
-    endif
-    if empty(l:cflags)
-        let l:cflags = ale#c#IncludeOptions(ale#c#FindLocalHeaderPaths(a:buffer))
-    else
-        let l:cflags .= ' '
-    endif
 
     " -iquote with the directory the file is in makes #include work for
     "  headers in the same directory.
@@ -33,7 +25,7 @@ call ale#linter#Define('c', {
 \   'output_stream': 'stderr',
 \   'executable_callback': 'ale_linters#c#clang#GetExecutable',
 \   'command_chain': [
-\       {'callback': 'ale#c#ParseMakefile', 'output_stream': 'stdout'},
+\       {'callback': 'ale#c#GetMakeCommand', 'output_stream': 'stdout'},
 \       {'callback': 'ale_linters#c#clang#GetCommand'}
 \   ],
 \   'callback': 'ale#handlers#gcc#HandleGCCFormat',

ale_linters/c/gcc.vim

@@ -9,15 +9,7 @@ function! ale_linters#c#gcc#GetExecutable(buffer) abort
 endfunction
 
 function! ale_linters#c#gcc#GetCommand(buffer, output) abort
-    let l:cflags = []
+    let l:cflags = ale#c#GetCFlags(a:buffer, a:output)
-    if !empty(a:output)
-        let l:cflags = join(ale#c#ParseCFlags(a:buffer, join(a:output, '\n')), ' ')
-    endif
-    if empty(l:cflags)
-        let l:cflags = ale#c#IncludeOptions(ale#c#FindLocalHeaderPaths(a:buffer))
-    else
-        let l:cflags .= ' '
-    endif
 
     " -iquote with the directory the file is in makes #include work for
     "  headers in the same directory.
@@ -33,7 +25,7 @@ call ale#linter#Define('c', {
 \   'output_stream': 'stderr',
 \   'executable_callback': 'ale_linters#c#gcc#GetExecutable',
 \   'command_chain': [
-\       {'callback': 'ale#c#ParseMakefile', 'output_stream': 'stdout'},
+\       {'callback': 'ale#c#GetMakeCommand', 'output_stream': 'stdout'},
 \       {'callback': 'ale_linters#c#gcc#GetCommand'}
 \   ],
 \   'callback': 'ale#handlers#gcc#HandleGCCFormat',

autoload/ale/c.vim

@@ -23,14 +23,14 @@ function! ale#c#FindProjectRoot(buffer) abort
     return ''
 endfunction
 
-function! ale#c#ParseCFlagsToList(buffer, cflags) abort
+function! ale#c#ParseCFlagsToList(path_prefix, cflags) abort
-    let l:project_root = ale#c#FindProjectRoot(a:buffer)
     let l:previous_option = ''
     let l:shell_option = 0
     let l:macro_option = 0
     let l:cflags_list = []
 
     for l:option in a:cflags
+
         " Check if cflag contained spaces
         if l:shell_option || stridx(l:option, '=`') >= 0
             " Cflag contained shell command with spaces (ex. -D='date +%s')
@@ -51,16 +51,19 @@ function! ale#c#ParseCFlagsToList(buffer, cflags) abort
             endif
             let l:macro_option = 0
         endif
+
         if l:previous_option isnot? ''
             let l:option = l:previous_option
             let l:previous_option = ''
         endif
+
         " Fix relative paths if needed
         if stridx(l:option, '-I') >= 0
             if stridx(l:option, '-I' . s:sep) < 0
-                let l:option = '-I' . l:project_root . s:sep . l:option[2:]
+                let l:option = '-I' . a:path_prefix . s:sep . l:option[2:]
             endif
         endif
+
         " Parse the cflag
         if stridx(l:option, '-I') >= 0 ||
            \ stridx(l:option, '-D') >= 0
@@ -69,35 +72,50 @@ function! ale#c#ParseCFlagsToList(buffer, cflags) abort
             endif
         endif
     endfor
+
     return l:cflags_list
 endfunction
 
 function! ale#c#ParseCFlags(buffer, stdout_make) abort
-    if g:ale_c_parse_makefile
+    if !g:ale_c_parse_makefile
-        for l:cflags in split(a:stdout_make, '\n')
+        return []
-            if stridx(l:cflags, expand('#' . a:buffer . '...'))
-                let l:cflags = split(l:cflags)
-                break
-            endif
-        endfor
-        if !empty(l:cflags)
-            return ale#c#ParseCFlagsToList(a:buffer, l:cflags)
-       endif
     endif
-    retur []
+
+    let l:buffer_filename = expand('#' . a:buffer . '...')
+
+    for l:cflags in split(a:stdout_make, '\n')
+        if stridx(l:cflags, l:buffer_filename)
+            let l:cflags = split(l:cflags)
+            break
+        endif
+    endfor
+
+    let l:makefile_path = ale#path#FindNearestFile(a:buffer, 'Makefile')
+    return ale#c#ParseCFlagsToList(fnamemodify(l:makefile_path, ':p:h'), l:cflags)
 endfunction
 
-function! ale#c#ParseMakefile(buffer) abort
+function! ale#c#GetCFlags(buffer, output) abort
-    if g:ale_c_parse_makefile
+    let l:cflags = ' '
-        let l:project_root = ale#c#FindProjectRoot(a:buffer)
-        let l:project_cflags = []
 
-        if !empty(l:project_root)
+    if g:ale_c_parse_makefile && !empty(a:output)
-            if !empty(globpath(l:project_root, 'Makefile', 0))
+        let l:cflags = join(ale#c#ParseCFlags(a:buffer, join(a:output, '\n')), ' ') . ' '
-                return 'cd '. l:project_root . ' && make -n'
+    endif
-            endif
+
+    if l:cflags is# ' '
+        let l:cflags = ale#c#IncludeOptions(ale#c#FindLocalHeaderPaths(a:buffer))
+    endif
+
+    return l:cflags
+endfunction
+
+function! ale#c#GetMakeCommand(buffer) abort
+    if g:ale_c_parse_makefile
+        let l:makefile_path = ale#path#FindNearestFile(a:buffer, 'Makefile')
+        if !empty(l:makefile_path)
+            return 'cd '. fnamemodify(l:makefile_path, ':p:h') . ' && make -n'
         endif
     endif
+
     return ''
 endfunction
 

test/test_c_parse_makefile.vader

@@ -19,20 +19,6 @@ After:
   call ale#test#RestoreDirectory()
   call ale#linter#Reset()
 
-" Run this only once for this series of tests. The cleanup Execute step
-" will run at the bottom of this file.
-"
-" We need to move .git/HEAD away so we don't match it, as we need to test
-" functions which look for .git/HEAD.
-Execute(Move .git/HEAD to a temp dir):
-  let g:temp_head_filename = tempname()
-  let g:head_filename = findfile('.git/HEAD', ';')
-
-  if !empty(g:head_filename)
-    call writefile(readfile(g:head_filename, 'b'), g:temp_head_filename, 'b')
-    call delete(g:head_filename)
-  endif
-
 Execute(The CFlags parser should be able to parse include directives):
   runtime! ale_linters/c/gcc.vim
 
@@ -72,11 +58,52 @@ Execute(The CFlags parser should be able to parse shell directives with spaces):
   \    '-DTEST=`date +%s`']
   \ , ale#c#ParseCFlags(bufnr(''), 'gcc -Isubdir -DTEST=`date +%s` -c file.c')
 
-Execute(Move .git/HEAD back):
+Execute(The CFlagsToList parser should be able to parse multiple cflags):
-  if !empty(g:head_filename)
+  runtime! ale_linters/c/gcc.vim
-    call writefile(readfile(g:temp_head_filename, 'b'), g:head_filename, 'b')
-    call delete(g:temp_head_filename)
-  endif
 
-  unlet! g:temp_head_filename
+  call ale#test#SetFilename('test_c_projects/makefile_project/subdir/file.c')
-  unlet! g:head_filename
+
+  AssertEqual
+  \   ['-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir'),
+  \    '-DTEST=`date +%s`']
+  \ , ale#c#ParseCFlagsToList(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'),
+  \                           split('gcc -Isubdir -DTEST=`date +%s` -c file.c'))
+
+Execute(The CFlagsToList parser should be able to parse multiple cflags #2):
+  runtime! ale_linters/c/gcc.vim
+
+  call ale#test#SetFilename('test_c_projects/makefile_project/subdir/file.c')
+
+  AssertEqual
+  \   ['-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir'),
+  \    '-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include'),
+  \    '-DTEST=`date +%s`']
+  \ , ale#c#ParseCFlagsToList(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'),
+  \                           split('gcc -Isubdir -Ikernel/include -DTEST=`date +%s` -c file.c'))
+
+Execute(The CFlagsToList parser should be able to parse multiple cflags #3):
+  runtime! ale_linters/c/gcc.vim
+
+  call ale#test#SetFilename('test_c_projects/makefile_project/subdir/file.c')
+
+  AssertEqual
+  \   ['-Dgoal=9',
+  \    '-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir'),
+  \    '-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include'),
+  \    '-DTEST=`date +%s`']
+  \ , ale#c#ParseCFlagsToList(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'),
+  \                           split('gcc -Dgoal=9 -Isubdir -Ikernel/include -DTEST=`date +%s` -c file.c'))
+
+Execute(The CFlagsToList parser should be able to parse multiple cflags #4):
+  runtime! ale_linters/c/gcc.vim
+
+  call ale#test#SetFilename('test_c_projects/makefile_project/subdir/file.c')
+
+  AssertEqual
+  \   ['-Dgoal=9',
+  \    '-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir'),
+  \    '-I' . ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include'),
+  \    '-DTEST=`date +%s`']
+  \ , ale#c#ParseCFlagsToList(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'),
+  \                           split('gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' .
+  \                                 '-Ikernel/include -DTEST=`date +%s` -c file.c'))

test/test_c_projects/makefile_project/Makefile

@@ -1,3 +0,0 @@
-file.o : subdir/file.c
-	cc -c subdir/file.c -Isubdir
-