Fix #3318 - Escape macros when parsing C flags

2 files changed, 128 insertions, 23 deletions

diff --git a/autoload/ale/c.vim b/autoload/ale/c.vim
index 4c7ee5bc..64668dd5 100644
--- a/autoload/ale/c.vim
+++ b/autoload/ale/c.vim
@@ -120,10 +120,23 @@ function! ale#c#ExpandAtArgs(path_prefix, raw_split_lines) abort
     return l:out_lines
 endfunction
 
+" Quote C/C++ a compiler argument, if needed.
+"
+" Quoting arguments might cause issues with some systems/compilers, so we only
+" quote them if we need to.
+function! ale#c#QuoteArg(arg) abort
+    if a:arg !~# '\v[#$&*()\\|[\]{};''"<>/?! ^%]'
+        return a:arg
+    endif
+
+    return ale#Escape(a:arg)
+endfunction
+
 function! ale#c#ParseCFlags(path_prefix, should_quote, raw_arguments) abort
     " Expand @file arguments now before parsing
     let l:arguments = ale#c#ExpandAtArgs(a:path_prefix, a:raw_arguments)
-    let l:arguments_to_use = []
+    " A list of [already_quoted, argument]
+    let l:items = []
     let l:option_index = 0
 
     while l:option_index < len(l:arguments)
@@ -149,26 +162,25 @@ function! ale#c#ParseCFlags(path_prefix, should_quote, raw_arguments) abort
             if !ale#path#IsAbsolute(l:arg)
                 let l:rel_path = substitute(l:arg, '"', '', 'g')
                 let l:rel_path = substitute(l:rel_path, '''', '', 'g')
-                let l:arg = ale#Escape(
-                \   ale#path#GetAbsPath(a:path_prefix, l:rel_path)
-                \)
+                let l:arg = ale#path#GetAbsPath(a:path_prefix, l:rel_path)
             endif
 
-            call add(l:arguments_to_use, l:option)
-            call add(l:arguments_to_use, l:arg)
+            call add(l:items, [1, l:option])
+            call add(l:items, [1, ale#Escape(l:arg)])
         " Options with arg that can be grouped with the option or separate
         elseif stridx(l:option, '-D') == 0 || stridx(l:option, '-B') == 0
-            call add(l:arguments_to_use, l:option)
-
             if l:option is# '-D' || l:option is# '-B'
-                call add(l:arguments_to_use, l:arguments[l:option_index])
+                call add(l:items, [1, l:option])
+                call add(l:items, [0, l:arguments[l:option_index]])
                 let l:option_index = l:option_index + 1
+            else
+                call add(l:items, [0, l:option])
             endif
         " Options that have an argument (always separate)
         elseif l:option is# '-iprefix' || stridx(l:option, '-iwithprefix') == 0
         \ || l:option is# '-isysroot' || l:option is# '-imultilib'
-            call add(l:arguments_to_use, l:option)
-            call add(l:arguments_to_use, l:arguments[l:option_index])
+            call add(l:items, [0, l:option])
+            call add(l:items, [0, l:arguments[l:option_index]])
             let l:option_index = l:option_index + 1
         " Options without argument
         elseif (stridx(l:option, '-W') == 0 && stridx(l:option, '-Wa,') != 0 && stridx(l:option, '-Wl,') != 0 && stridx(l:option, '-Wp,') != 0)
@@ -180,11 +192,19 @@ function! ale#c#ParseCFlags(path_prefix, should_quote, raw_arguments) abort
         \ || stridx(l:option, '-nostdinc') == 0 || stridx(l:option, '-iplugindir=') == 0
         \ || stridx(l:option, '--sysroot=') == 0 || l:option is# '--no-sysroot-suffix'
         \ || stridx(l:option, '-m') == 0
-            call add(l:arguments_to_use, l:option)
+            call add(l:items, [0, l:option])
         endif
     endwhile
 
-    return join(l:arguments_to_use, ' ')
+    if a:should_quote
+        " Quote C arguments that haven't already been quoted above.
+        " If and only if we've been asked to quote them.
+        call map(l:items, 'v:val[0] ? v:val[1] : ale#c#QuoteArg(v:val[1])')
+    else
+        call map(l:items, 'v:val[1]')
+    endif
+
+    return join(l:items, ' ')
 endfunction
 
 function! ale#c#ParseCFlagsFromMakeOutput(buffer, make_output) abort
diff --git a/test/test_c_flag_parsing.vader b/test/test_c_flag_parsing.vader
index abd63527..076be6a1 100644
--- a/test/test_c_flag_parsing.vader
+++ b/test/test_c_flag_parsing.vader
@@ -26,7 +26,7 @@ Execute(The CFlags parser should be able to parse include directives):
   \ ale#c#ParseCFlagsFromMakeOutput(bufnr(''), ['gcc -Isubdir -c file.c'])
 
   AssertEqual
-  \ '-isystem ' . '/usr/include/dir',
+  \ '-isystem ' . ale#Escape('/usr/include/dir'),
   \ ale#c#ParseCFlagsFromMakeOutput(bufnr(''), ['gcc -isystem /usr/include/dir -c file.c'])
 
 Execute(ParseCFlags should ignore -c and -o):
@@ -161,7 +161,7 @@ Execute(ParseCompileCommandsFlags should parse some basic flags):
 
   " We should read the absolute path filename entry, not the other ones.
   AssertEqual
-  \ '-I ' . ale#path#Simplify('/usr/include/xmms2'),
+  \ '-I ' . ale#Escape(ale#path#Simplify('/usr/include/xmms2')),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {
@@ -211,7 +211,7 @@ Execute(ParseCompileCommandsFlags should fall back to files with the same name):
 
   " We should prefer the basename file flags, not the base dirname flags.
   AssertEqual
-  \ '-I ' . ale#path#Simplify('/usr/include/xmms2'),
+  \ '-I ' . ale#Escape(ale#path#Simplify('/usr/include/xmms2')),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {
@@ -243,7 +243,7 @@ Execute(ParseCompileCommandsFlags should parse flags for exact directory matches
 
   " We should ues the exact directory flags, not the file basename flags.
   AssertEqual
-  \ '-I ' . ale#path#Simplify('/usr/include/xmms2'),
+  \ '-I ' . ale#Escape(ale#path#Simplify('/usr/include/xmms2')),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {
@@ -283,7 +283,7 @@ Execute(ParseCompileCommandsFlags should fall back to files in the same director
   silent noautocmd execute 'file! ' . fnameescape(ale#path#Simplify('/foo/bar/xmms2-mpris/src/xmms2-mpris.c'))
 
   AssertEqual
-  \ '-I ' . ale#path#Simplify('/usr/include/xmms2'),
+  \ '-I ' . ale#Escape(ale#path#Simplify('/usr/include/xmms2')),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {},
@@ -322,7 +322,7 @@ Execute(ParseCompileCommandsFlags should take commands from matching .c files fo
   silent noautocmd execute 'file! ' . fnameescape(ale#path#Simplify('/foo/bar/xmms2-mpris/src/xmms2-mpris.h'))
 
   AssertEqual
-  \ '-I /usr/include/xmms2',
+  \ '-I ' . ale#Escape('/usr/include/xmms2'),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {
@@ -343,7 +343,7 @@ Execute(ParseCompileCommandsFlags should take commands from matching .cpp files
   silent noautocmd execute 'file! ' . fnameescape(ale#path#Simplify('/foo/bar/xmms2-mpris/src/xmms2-mpris.hpp'))
 
   AssertEqual
-  \ '-I /usr/include/xmms2',
+  \ '-I ' . ale#Escape('/usr/include/xmms2'),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {
@@ -365,7 +365,7 @@ Execute(ParseCompileCommandsFlags should take commands from matching .cpp files
   silent noautocmd execute 'file! ' . fnameescape(ale#path#Simplify('/foo/bar/xmms2-mpris/src/xmms2-mpris.h'))
 
   AssertEqual
-  \ '-I /usr/include/xmms2',
+  \ '-I ' . ale#Escape('/usr/include/xmms2'),
   \ ale#c#ParseCompileCommandsFlags(
   \   bufnr(''),
   \   {
@@ -462,9 +462,10 @@ Execute(We should include several important flags):
   \ . ' -idirafter ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/incafter'))
   \ . ' -iframework ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/incframework'))
   \ . ' -include ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/foo bar'))
-  \ . ' -Dmacro=value'
+  \ . ' -Dmacro="value"'
   \ . ' -DGoal=9'
   \ . ' -D macro2'
+  \ . ' -D macro3="value"'
   \ . ' -Bbdir'
   \ . ' -B bdir2'
   \ . ' -iprefix prefix -iwithprefix prefix2 -iwithprefixbefore prefix3'
@@ -490,10 +491,92 @@ Execute(We should include several important flags):
   \     'incframework',
   \     '-include',
   \     '''foo bar''',
-  \     '-Dmacro=value',
+  \     '-Dmacro="value"',
   \     '-DGoal=9',
   \     '-D',
   \     'macro2',
+  \     '-D',
+  \     'macro3="value"',
+  \     '-Bbdir',
+  \     '-B',
+  \     'bdir2',
+  \     '-iprefix',
+  \     'prefix',
+  \     '-iwithprefix',
+  \     'prefix2',
+  \     '-iwithprefixbefore',
+  \     'prefix3',
+  \     '-isysroot',
+  \     'sysroot',
+  \     '--sysroot=test',
+  \     '--no-sysroot-suffix',
+  \     '-imultilib',
+  \     'multidir',
+  \     '-Wsome-warning',
+  \     '-std=c89',
+  \     '-pedantic',
+  \     '-pedantic-errors',
+  \     '-ansi',
+  \     '-foption',
+  \     '-O2',
+  \     '-C',
+  \     '-CC',
+  \     '-trigraphs',
+  \     '-nostdinc',
+  \     '-nostdinc++',
+  \     '-iplugindir=dir',
+  \     '-march=native',
+  \     '-w',
+  \   ],
+  \ )
+
+Execute(We should quote the flags we need to quote):
+  AssertEqual
+  \ '-I ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/inc'))
+  \ . ' -I ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/include'))
+  \ . ' -iquote ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/incquote'))
+  \ . ' -isystem ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/incsystem'))
+  \ . ' -idirafter ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/incafter'))
+  \ . ' -iframework ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/incframework'))
+  \ . ' -include ' . ale#Escape(ale#path#Simplify(g:dir . '/test_c_projects/makefile_project/foo bar'))
+  \ . ' ' . ale#Escape('-Dmacro="value"')
+  \ . ' -DGoal=9'
+  \ . ' -D macro2'
+  \ . ' -D ' . ale#Escape('macro3="value"')
+  \ . ' -Bbdir'
+  \ . ' -B bdir2'
+  \ . ' -iprefix prefix -iwithprefix prefix2 -iwithprefixbefore prefix3'
+  \ . ' -isysroot sysroot --sysroot=test'
+  \ . ' ' . ale#Escape('--sysroot="quoted"')
+  \ . ' ' . ale#Escape('--sysroot=foo bar')
+  \ . ' --no-sysroot-suffix -imultilib multidir'
+  \ . ' -Wsome-warning -std=c89 -pedantic -pedantic-errors -ansi'
+  \ . ' -foption -O2 -C -CC -trigraphs -nostdinc -nostdinc++'
+  \ . ' -iplugindir=dir -march=native -w',
+  \ ale#c#ParseCFlags(
+  \   ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'),
+  \   1,
+  \   [
+  \     'gcc',
+  \     '-Iinc',
+  \     '-I',
+  \     'include',
+  \     '-iquote',
+  \     'incquote',
+  \     '-isystem',
+  \     'incsystem',
+  \     '-idirafter',
+  \     'incafter',
+  \     '-iframework',
+  \     'incframework',
+  \     '-include',
+  \     '''foo bar''',
+  \     '-Dmacro="value"',
+  \     '-DGoal=9',
+  \     '-D',
+  \     'macro2',
+  \     '-D',
+  \     'macro3="value"',
   \     '-Bbdir',
   \     '-B',
   \     'bdir2',
@@ -506,6 +589,8 @@ Execute(We should include several important flags):
   \     '-isysroot',
   \     'sysroot',
   \     '--sysroot=test',
+  \     '--sysroot="quoted"',
+  \     '--sysroot=foo bar',
   \     '--no-sysroot-suffix',
   \     '-imultilib',
   \     'multidir',