From af177d7825a3e78cc2022710d61eff8a685832ec Mon Sep 17 00:00:00 2001 From: w0rp Date: Thu, 27 Aug 2020 19:33:43 +0100 Subject: [PATCH] #3318 Refactor C flag parsing to set up for quoting arguments --- autoload/ale/c.vim | 64 ++++++----- test/test_c_flag_parsing.vader | 195 ++++++++++++++++++++++----------- 2 files changed, 163 insertions(+), 96 deletions(-) diff --git a/autoload/ale/c.vim b/autoload/ale/c.vim index 6f18ce4c..eac9879c 100644 --- a/autoload/ale/c.vim +++ b/autoload/ale/c.vim @@ -8,6 +8,11 @@ let s:sep = has('win32') ? '\' : '/' " Set just so tests can override it. let g:__ale_c_project_filenames = ['.git/HEAD', 'configure', 'Makefile', 'CMakeLists.txt'] +let g:ale_c_build_dir_names = get(g:, 'ale_c_build_dir_names', [ +\ 'build', +\ 'bin', +\]) + function! s:CanParseMakefile(buffer) abort " Something somewhere seems to delete this setting in tests, so ensure we " always have a default value. @@ -115,16 +120,14 @@ function! ale#c#ExpandAtArgs(path_prefix, raw_split_lines) abort return l:out_lines endfunction -function! ale#c#ParseCFlags(path_prefix, cflag_line) abort - let l:cflags_list = [] - - let l:raw_split_lines = ale#c#ShellSplit(a:cflag_line) +function! ale#c#ParseCFlags(path_prefix, should_quote, raw_arguments) abort " Expand @file arguments now before parsing - let l:split_lines = ale#c#ExpandAtArgs(a:path_prefix, l:raw_split_lines) + let l:arguments = ale#c#ExpandAtArgs(a:path_prefix, a:raw_arguments) + let l:arguments_to_use = [] let l:option_index = 0 - while l:option_index < len(l:split_lines) - let l:option = l:split_lines[l:option_index] + while l:option_index < len(l:arguments) + let l:option = l:arguments[l:option_index] let l:option_index = l:option_index + 1 " Include options, that may need relative path fix @@ -138,7 +141,7 @@ function! ale#c#ParseCFlags(path_prefix, cflag_line) abort let l:arg = join(split(l:option, '\zs')[2:], '') let l:option = '-I' else - let l:arg = l:split_lines[l:option_index] + let l:arg = l:arguments[l:option_index] let l:option_index = l:option_index + 1 endif @@ -151,21 +154,21 @@ function! ale#c#ParseCFlags(path_prefix, cflag_line) abort \) endif - call add(l:cflags_list, l:option) - call add(l:cflags_list, l:arg) + call add(l:arguments_to_use, l:option) + call add(l:arguments_to_use, 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:cflags_list, l:option) + call add(l:arguments_to_use, l:option) if l:option is# '-D' || l:option is# '-B' - call add(l:cflags_list, l:split_lines[l:option_index]) + call add(l:arguments_to_use, l:arguments[l:option_index]) let l:option_index = l:option_index + 1 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:cflags_list, l:option) - call add(l:cflags_list, l:split_lines[l:option_index]) + call add(l:arguments_to_use, l:option) + call add(l:arguments_to_use, 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) @@ -177,11 +180,11 @@ function! ale#c#ParseCFlags(path_prefix, cflag_line) 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:cflags_list, l:option) + call add(l:arguments_to_use, l:option) endif endwhile - return join(l:cflags_list, ' ') + return join(l:arguments_to_use, ' ') endfunction function! ale#c#ParseCFlagsFromMakeOutput(buffer, make_output) abort @@ -203,7 +206,7 @@ function! ale#c#ParseCFlagsFromMakeOutput(buffer, make_output) abort let l:makefile_path = ale#path#FindNearestFile(a:buffer, 'Makefile') let l:makefile_dir = fnamemodify(l:makefile_path, ':p:h') - return ale#c#ParseCFlags(l:makefile_dir, l:cflag_line) + return ale#c#ParseCFlags(l:makefile_dir, 0, ale#c#ShellSplit(l:cflag_line)) endfunction " Given a buffer number, find the project directory containing @@ -333,14 +336,16 @@ function! s:GetLookupFromCompileCommandsFile(compile_commands_file) abort return l:empty endfunction -function! ale#c#GetCompileCommand(json_item) abort - if has_key(a:json_item, 'command') - return a:json_item.command - elseif has_key(a:json_item, 'arguments') - return join(a:json_item.arguments, ' ') +" Get [should_quote, arguments] from either 'command' or 'arguments' +" 'arguments' should be quoted later, the split 'command' strings should not. +function! s:GetArguments(json_item) abort + if has_key(a:json_item, 'arguments') + return [1, a:json_item.arguments] + elseif has_key(a:json_item, 'command') + return [0, ale#c#ShellSplit(a:json_item.command)] endif - return '' + return [0, []] endfunction function! ale#c#ParseCompileCommandsFlags(buffer, file_lookup, dir_lookup) abort @@ -398,7 +403,9 @@ function! ale#c#ParseCompileCommandsFlags(buffer, file_lookup, dir_lookup) abort \ && l:filename[-len(l:source_file):] is? l:source_file \ ) \) - return ale#c#ParseCFlags(l:item.directory, ale#c#GetCompileCommand(l:item)) + let [l:should_quote, l:args] = s:GetArguments(l:item) + + return ale#c#ParseCFlags(l:item.directory, l:should_quote, l:args) endif endfor @@ -406,7 +413,9 @@ function! ale#c#ParseCompileCommandsFlags(buffer, file_lookup, dir_lookup) abort let l:filename = ale#path#GetAbsPath(l:item.directory, l:item.file) if ale#path#Simplify(fnamemodify(l:filename, ':h')) is? l:dir - return ale#c#ParseCFlags(l:item.directory, ale#c#GetCompileCommand(l:item)) + let [l:should_quote, l:args] = s:GetArguments(l:item) + + return ale#c#ParseCFlags(l:item.directory, l:should_quote, l:args) endif endfor @@ -517,8 +526,3 @@ function! ale#c#IncludeOptions(include_paths) abort return join(l:option_list) endfunction - -let g:ale_c_build_dir_names = get(g:, 'ale_c_build_dir_names', [ -\ 'build', -\ 'bin', -\]) diff --git a/test/test_c_flag_parsing.vader b/test/test_c_flag_parsing.vader index 6656d508..d9541037 100644 --- a/test/test_c_flag_parsing.vader +++ b/test/test_c_flag_parsing.vader @@ -5,7 +5,15 @@ Before: let g:ale_c_parse_makefile = 1 + function SplitAndParse(path_prefix, command) abort + let l:args = ale#c#ShellSplit(a:command) + + return ale#c#ParseCFlags(a:path_prefix, 0, l:args) + endfunction + After: + delfunction SplitAndParse + Restore call ale#test#RestoreDirectory() @@ -57,48 +65,21 @@ Execute(ParseCFlags should be able to parse flags with relative paths): \ '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( + \ SplitAndParse( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), \ 'gcc -Isubdir ' \ . '-I'. ale#path#Simplify('kernel/include') \ . ' -DTEST=`date +%s` -c file.c' \ ) -Execute(ParseCFlags should be able to parse -Dgoal): - AssertEqual - \ '-Dgoal=9' - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) - \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( - \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), - \ 'gcc -Dgoal=9 -Isubdir ' - \ . '-I'. ale#path#Simplify('kernel/include') - \ . ' -DTEST=`date +%s` -c file.c' - \ ) - -Execute(ParseCFlags should ignore -T and other arguments): - AssertEqual - \ '-Dgoal=9' - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) - \ . ' ' . '--sysroot=subdir' - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) - \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( - \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), - \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir --sysroot=subdir ' - \ . '-I'. ale#path#Simplify('kernel/include') - \ . ' -DTEST=`date +%s` -c file.c' - \ ) - -Execute(ParseCFlags should handle paths with spaces in double quotes): +Execute(We should handle paths with spaces in double quotes): AssertEqual \ '-Dgoal=9' \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir with spaces')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( + \ SplitAndParse( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' \ . '-I"dir with spaces"' . ' -I'. ale#path#Simplify('kernel/include') @@ -112,7 +93,7 @@ Execute(ParseCFlags should handle paths with spaces in single quotes): \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir with spaces')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( + \ SplitAndParse( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' \ . '-I''dir with spaces''' . ' -I'. ale#path#Simplify('kernel/include') @@ -127,7 +108,7 @@ Execute(ParseCFlags should handle paths with minuses): \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir-with-dash')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( + \ SplitAndParse( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' \ . '-I''dir with spaces''' . ' -Idir-with-dash' @@ -135,7 +116,7 @@ Execute(ParseCFlags should handle paths with minuses): \ . ' -DTEST=`date +%s` -c file.c' \ ) -Execute(ParseCFlags should handle -D with minuses): +Execute(We should handle -D with minuses): AssertEqual \ '-Dgoal=9' \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) @@ -144,7 +125,7 @@ Execute(ParseCFlags should handle -D with minuses): \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir-with-dash')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')) \ . ' -DTEST=`date +%s`', - \ ale#c#ParseCFlags( + \ SplitAndParse( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' \ . '-Dmacro-with-dash ' @@ -153,7 +134,7 @@ Execute(ParseCFlags should handle -D with minuses): \ . ' -DTEST=`date +%s` -c file.c' \ ) -Execute(ParseCFlags should handle flags at the end of the line): +Execute(We should handle flags at the end of the line): AssertEqual \ '-Dgoal=9' \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) @@ -161,7 +142,7 @@ Execute(ParseCFlags should handle flags at the end of the line): \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir with spaces')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir-with-dash')) \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')), - \ ale#c#ParseCFlags( + \ SplitAndParse( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' \ . '-Dmacro-with-dash ' @@ -424,32 +405,52 @@ Execute(ParseCompileCommandsFlags should not take commands from .c files for .h \ }, \ ) -Execute(ParseCFlags should not merge flags): +Execute(ShellSplit should not merge flags): AssertEqual - \ '-Dgoal=9' - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/subdir')) - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir with spaces')) - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/dir-with-dash')) - \ . ' ' . '-I' . ' ' . ale#Escape(ale#path#Simplify(g:dir. '/test_c_projects/makefile_project/kernel/include')), - \ ale#c#ParseCFlags( - \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), + \ [ + \ 'gcc', + \ '-Dgoal=9', + \ '-Tlinkerfile.ld', + \ 'blabla', + \ '-Isubdir', + \ 'subdir/somedep1.o', + \ 'subdir/somedep2.o', + \ '-I''dir with spaces''', + \ '-Idir-with-dash', + \ 'subdir/somedep3.o', + \ 'subdir/somedep4.o', + \ '-Ikernel/include', + \ 'subdir/somedep5.o', + \ 'subdir/somedep6.o', + \ ], + \ ale#c#ShellSplit( \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla -Isubdir ' \ . 'subdir/somedep1.o ' . 'subdir/somedep2.o ' \ . '-I''dir with spaces''' . ' -Idir-with-dash ' \ . 'subdir/somedep3.o ' . 'subdir/somedep4.o ' \ . ' -I'. ale#path#Simplify('kernel/include') . ' ' - \ . 'subdir/somedep5.o ' . 'subdir/somedep6.o ' + \ . 'subdir/somedep5.o ' . 'subdir/somedep6.o' \ ) -Execute(ParseCFlags should handle parenthesis and quotes): +Execute(ShellSplit should handle parenthesis and quotes): AssertEqual - \ '-Dgoal=9 -Dtest1="('' '')" -Dtest2=''(` `)'' -Dtest3=`(" ")`', - \ ale#c#ParseCFlags( - \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), + \ [ + \ 'gcc', + \ '-Dgoal=9', + \ '-Tlinkerfile.ld', + \ 'blabla', + \ '-Dtest1="('' '')"', + \ 'file1.o', + \ '-Dtest2=''(` `)''', + \ 'file2.o', + \ '-Dtest3=`(" ")`', + \ 'file3.o', + \ ] , + \ ale#c#ShellSplit( \ 'gcc -Dgoal=9 -Tlinkerfile.ld blabla ' \ . '-Dtest1="('' '')" file1.o ' \ . '-Dtest2=''(` `)'' file2.o ' - \ . '-Dtest3=`(" ")` file3.o ' + \ . '-Dtest3=`(" ")` file3.o' \ ) Execute(We should include several important flags): @@ -461,7 +462,11 @@ 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 -D macro2 -Bbdir -B bdir2' + \ . ' -Dmacro=value' + \ . ' -DGoal=9' + \ . ' -D macro2' + \ . ' -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' @@ -469,15 +474,57 @@ Execute(We should include several important flags): \ . ' -iplugindir=dir -march=native -w', \ ale#c#ParseCFlags( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), - \ 'gcc' - \ . ' -Iinc -I include -iquote incquote -isystem incsystem -idirafter incafter -iframework incframework' - \ . ' -include ''foo bar''' - \ . ' -Dmacro=value -D macro2 -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' + \ 0, + \ [ + \ 'gcc', + \ '-Iinc', + \ '-I', + \ 'include', + \ '-iquote', + \ 'incquote', + \ '-isystem', + \ 'incsystem', + \ '-idirafter', + \ 'incafter', + \ '-iframework', + \ 'incframework', + \ '-include', + \ '''foo bar''', + \ '-Dmacro=value', + \ '-DGoal=9', + \ '-D', + \ 'macro2', + \ '-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 exclude other flags that cause problems): @@ -485,8 +532,21 @@ Execute(We should exclude other flags that cause problems): \ '', \ ale#c#ParseCFlags( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), - \ 'gcc -Wl,option -Wa,option -Wp,option filename.c somelib.a ' - \ . '-fdump-file=name -fdiagnostics-arg -fno-show-column -fstack-usage' + \ 0, + \ [ + \ 'gcc', + \ '-Wl,option', + \ '-Wa,option', + \ '-Wp,option', + \ '-c', + \ 'filename.c', + \ 'somelib.a', + \ '-fdump-file=name', + \ '-fdiagnostics-arg', + \ '-fno-show-column', + \ '-fstack-usage', + \ '-Tlinkerfile.ld', + \ ], \ ) Execute(We should expand @file in CFlags): @@ -494,8 +554,11 @@ Execute(We should expand @file in CFlags): \ '-DARGS1 -DARGS2 -O2', \ ale#c#ParseCFlags( \ ale#path#Simplify(g:dir. '/test_c_projects/makefile_project'), - \ 'gcc' - \ . ' -g' - \ . ' @./args' - \ . ' -O2', + \ 0, + \ [ + \ 'gcc', + \ '-g', + \ '@./args', + \ '-O2', + \ ], \ )