From b06b832447253404180ce6a97927755e4ae68c22 Mon Sep 17 00:00:00 2001 From: w0rp Date: Tue, 11 Apr 2017 20:32:57 +0100 Subject: [PATCH] #392 Report errors inside of headers, in a very basic way --- ale_linters/c/clang.vim | 2 +- ale_linters/c/gcc.vim | 2 +- ale_linters/coffee/coffee.vim | 2 +- ale_linters/cpp/clang.vim | 2 +- ale_linters/cpp/clangtidy.vim | 2 +- ale_linters/cpp/gcc.vim | 2 +- ale_linters/puppet/puppetlint.vim | 2 +- ale_linters/sh/shellcheck.vim | 2 +- ale_linters/swift/swiftlint.vim | 2 +- ale_linters/vim/vint.vim | 2 +- autoload/ale/handlers.vim | 30 --------- autoload/ale/handlers/gcc.vim | 78 +++++++++++++++++++++++ test/c_tests/broken.h | 1 + test/c_tests/test_gcc.vader | 53 +++++++++++---- test/handler/test_common_handlers.vader | 8 +-- test/handler/test_gcc_handler.vader | 68 ++++++++++++++++++++ test/handler/test_swiftlint_handler.vader | 8 +-- test/handler/test_vint_handler.vader | 36 +++++++++++ 18 files changed, 235 insertions(+), 67 deletions(-) create mode 100644 autoload/ale/handlers/gcc.vim create mode 100644 test/c_tests/broken.h create mode 100644 test/handler/test_gcc_handler.vader create mode 100644 test/handler/test_vint_handler.vader diff --git a/ale_linters/c/clang.vim b/ale_linters/c/clang.vim index 603e2b75..489245da 100644 --- a/ale_linters/c/clang.vim +++ b/ale_linters/c/clang.vim @@ -22,5 +22,5 @@ call ale#linter#Define('c', { \ 'output_stream': 'stderr', \ 'executable': 'clang', \ 'command_callback': 'ale_linters#c#clang#GetCommand', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/c/gcc.vim b/ale_linters/c/gcc.vim index a487909c..c89fae72 100644 --- a/ale_linters/c/gcc.vim +++ b/ale_linters/c/gcc.vim @@ -22,5 +22,5 @@ call ale#linter#Define('c', { \ 'output_stream': 'stderr', \ 'executable': 'gcc', \ 'command_callback': 'ale_linters#c#gcc#GetCommand', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/coffee/coffee.vim b/ale_linters/coffee/coffee.vim index ac9ef79e..f263a163 100644 --- a/ale_linters/coffee/coffee.vim +++ b/ale_linters/coffee/coffee.vim @@ -19,5 +19,5 @@ call ale#linter#Define('coffee', { \ 'executable_callback': 'ale_linters#coffee#coffee#GetExecutable', \ 'command_callback': 'ale_linters#coffee#coffee#GetCommand', \ 'output_stream': 'stderr', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/cpp/clang.vim b/ale_linters/cpp/clang.vim index 9915ac3a..e8af6dc2 100644 --- a/ale_linters/cpp/clang.vim +++ b/ale_linters/cpp/clang.vim @@ -19,5 +19,5 @@ call ale#linter#Define('cpp', { \ 'output_stream': 'stderr', \ 'executable': 'clang++', \ 'command_callback': 'ale_linters#cpp#clang#GetCommand', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/cpp/clangtidy.vim b/ale_linters/cpp/clangtidy.vim index 11088c43..6b72e1f5 100644 --- a/ale_linters/cpp/clangtidy.vim +++ b/ale_linters/cpp/clangtidy.vim @@ -14,5 +14,5 @@ call ale#linter#Define('cpp', { \ 'output_stream': 'stdout', \ 'executable': 'clang-tidy', \ 'command_callback': 'ale_linters#cpp#clangtidy#GetCommand', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/cpp/gcc.vim b/ale_linters/cpp/gcc.vim index ad1b93b7..f2261c4f 100644 --- a/ale_linters/cpp/gcc.vim +++ b/ale_linters/cpp/gcc.vim @@ -28,5 +28,5 @@ call ale#linter#Define('cpp', { \ 'output_stream': 'stderr', \ 'executable': 'g++', \ 'command_callback': 'ale_linters#cpp#gcc#GetCommand', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/puppet/puppetlint.vim b/ale_linters/puppet/puppetlint.vim index 05745cfe..f96f8f79 100644 --- a/ale_linters/puppet/puppetlint.vim +++ b/ale_linters/puppet/puppetlint.vim @@ -6,5 +6,5 @@ call ale#linter#Define('puppet', { \ 'command': 'puppet-lint --no-autoloader_layout-check' \ . ' --log-format "-:%{line}:%{column}: %{kind}: [%{check}] %{message}"' \ . ' %t', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/sh/shellcheck.vim b/ale_linters/sh/shellcheck.vim index bb556460..5f932b19 100644 --- a/ale_linters/sh/shellcheck.vim +++ b/ale_linters/sh/shellcheck.vim @@ -48,5 +48,5 @@ call ale#linter#Define('sh', { \ 'name': 'shellcheck', \ 'executable_callback': 'ale_linters#sh#shellcheck#GetExecutable', \ 'command_callback': 'ale_linters#sh#shellcheck#GetCommand', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/swift/swiftlint.vim b/ale_linters/swift/swiftlint.vim index aaa77a9f..b7dcf935 100644 --- a/ale_linters/swift/swiftlint.vim +++ b/ale_linters/swift/swiftlint.vim @@ -5,5 +5,5 @@ call ale#linter#Define('swift', { \ 'name': 'swiftlint', \ 'executable': 'swiftlint', \ 'command': 'swiftlint lint --use-stdin', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/ale_linters/vim/vint.vim b/ale_linters/vim/vint.vim index 4b2c5445..821a0bdc 100644 --- a/ale_linters/vim/vint.vim +++ b/ale_linters/vim/vint.vim @@ -20,5 +20,5 @@ call ale#linter#Define('vim', { \ . s:enable_neovim \ . s:format \ . ' %t', -\ 'callback': 'ale#handlers#HandleGCCFormat', +\ 'callback': 'ale#handlers#gcc#HandleGCCFormat', \}) diff --git a/autoload/ale/handlers.vim b/autoload/ale/handlers.vim index 96be427f..75d881fa 100644 --- a/autoload/ale/handlers.vim +++ b/autoload/ale/handlers.vim @@ -45,36 +45,6 @@ function! ale#handlers#HandleUnixFormatAsWarning(buffer, lines) abort return s:HandleUnixFormat(a:buffer, a:lines, 'W') endfunction -function! ale#handlers#HandleGCCFormat(buffer, lines) abort - " Look for lines like the following. - " - " :8:5: warning: conversion lacks type at end of format [-Wformat=] - " :10:27: error: invalid operands to binary - (have ‘int’ and ‘char *’) - " -:189:7: note: $/${} is unnecessary on arithmetic variables. [SC2004] - let l:pattern = '^.\+:\(\d\+\):\(\d\+\): \([^:]\+\): \(.\+\)$' - let l:output = [] - - for l:line in a:lines - let l:match = matchlist(l:line, l:pattern) - - if len(l:match) == 0 - continue - endif - - call add(l:output, { - \ 'bufnr': a:buffer, - \ 'lnum': l:match[1] + 0, - \ 'vcol': 0, - \ 'col': l:match[2] + 0, - \ 'text': l:match[4], - \ 'type': l:match[3] =~# 'error' ? 'E' : 'W', - \ 'nr': -1, - \}) - endfor - - return l:output -endfunction - function! ale#handlers#HandleCppCheckFormat(buffer, lines) abort " Look for lines like the following. " diff --git a/autoload/ale/handlers/gcc.vim b/autoload/ale/handlers/gcc.vim new file mode 100644 index 00000000..9084d0d4 --- /dev/null +++ b/autoload/ale/handlers/gcc.vim @@ -0,0 +1,78 @@ +scriptencoding utf-8 +" Author: w0rp +" Description: This file defines a handler function which ought to work for +" any program which outputs errors in the format that GCC uses. + +function! s:AddIncludedErrors(output, include_lnum, include_lines) abort + if a:include_lnum > 0 + call add(a:output, { + \ 'lnum': a:include_lnum, + \ 'type': 'E', + \ 'text': 'Problems were found in the header (See :ALEDetail)', + \ 'detail': join(a:include_lines, "\n"), + \}) + endif +endfunction + +function! ale#handlers#gcc#HandleGCCFormat(buffer, lines) abort + let l:include_pattern = '\v^(In file included | *)from [^:]*:(\d+)' + let l:include_lnum = 0 + let l:include_lines = [] + let l:included_filename = '' + " Look for lines like the following. + " + " :8:5: warning: conversion lacks type at end of format [-Wformat=] + " :10:27: error: invalid operands to binary - (have ‘int’ and ‘char *’) + " -:189:7: note: $/${} is unnecessary on arithmetic variables. [SC2004] + let l:pattern = '^\(.\+\):\(\d\+\):\(\d\+\): \([^:]\+\): \(.\+\)$' + let l:output = [] + + for l:line in a:lines + let l:match = matchlist(l:line, l:pattern) + + if empty(l:match) + " Check for matches in includes. + " We will keep matching lines until we hit the last file, which + " is our file. + let l:include_match = matchlist(l:line, l:include_pattern) + + if empty(l:include_match) + " If this isn't another include header line, then we + " need to collect it. + call add(l:include_lines, l:line) + else + " Get the line number out of the parsed include line, + " and reset the other variables. + let l:include_lnum = str2nr(l:include_match[2]) + let l:include_lines = [] + let l:included_filename = '' + endif + elseif l:include_lnum > 0 + \&& (empty(l:included_filename) || l:included_filename ==# l:match[1]) + " If we hit the first error after an include header, or the + " errors below have the same name as the first filename we see, + " then include these lines, and remember what that filename was. + let l:included_filename = l:match[1] + call add(l:include_lines, l:line) + else + " If we hit a regular error again, then add the previously + " collected lines as one error, and reset the include variables. + call s:AddIncludedErrors(l:output, l:include_lnum, l:include_lines) + let l:include_lnum = 0 + let l:include_lines = [] + let l:included_filename = '' + + call add(l:output, { + \ 'lnum': l:match[2] + 0, + \ 'col': l:match[3] + 0, + \ 'type': l:match[4] =~# 'error' ? 'E' : 'W', + \ 'text': l:match[5], + \}) + endif + endfor + + " Add remaining include errors after we go beyond the last line. + call s:AddIncludedErrors(l:output, l:include_lnum, l:include_lines) + + return l:output +endfunction diff --git a/test/c_tests/broken.h b/test/c_tests/broken.h new file mode 100644 index 00000000..3bd3571a --- /dev/null +++ b/test/c_tests/broken.h @@ -0,0 +1 @@ +{{{ diff --git a/test/c_tests/test_gcc.vader b/test/c_tests/test_gcc.vader index 67e4e426..0bf3eb1d 100644 --- a/test/c_tests/test_gcc.vader +++ b/test/c_tests/test_gcc.vader @@ -34,12 +34,9 @@ Given c (A test C file): Execute(Basic errors should be returned for GCC for C files): call ale#Lint() - AssertEqual [{ - \ 'lnum': 3, - \ 'col': 1, - \ }], - \ map(getloclist(0), '{''lnum'': v:val.lnum, ''col'': v:val.col}'), - \ 'No errors returned! Got: ' . GetCommandOutput() + AssertEqual + \ [{'lnum': 3, 'col': 1}], + \ map(getloclist(0), '{''lnum'': v:val.lnum, ''col'': v:val.col}') Assert match(getloclist(0)[0].text, '\v^expected .*;.* before .*\}.* token$') >= 0, \ 'Invalid error text: ' . getloclist(0)[0].text @@ -52,12 +49,42 @@ Given cpp (A test C++ file): Execute(Basic errors should be returned for GCC for C++ files): call ale#Lint() - AssertEqual [{ - \ 'lnum': 3, - \ 'col': 1, - \ }], - \ map(getloclist(0), '{''lnum'': v:val.lnum, ''col'': v:val.col}'), - \ 'No errors returned! Got: ' . GetCommandOutput() + AssertEqual + \ [{'lnum': 3, 'col': 1}], + \ map(getloclist(0), '{''lnum'': v:val.lnum, ''col'': v:val.col}') Assert match(getloclist(0)[0].text, '\v^expected .*;.* before .*\}.* token$') >= 0, - \ 'Invalid error text: ' . getloclist(0)[0].text + +Given c (A test C file with a header containing broken code): + // Some comment line + #include "broken.h" + + int main() { + return 0 + } + +Execute(Basic errors should be returned for GCC for C files with headers): + call ale#Lint() + + AssertEqual + \ [{'lnum': 2, 'col': 0}], + \ map(getloclist(0), '{''lnum'': v:val.lnum, ''col'': v:val.col}') + + AssertEqual 'Problems were found in the header (See :ALEDetail)', getloclist(0)[0].text + +Given cpp (A test C++ file with a header containing broken code): + // Some comment line + #include "broken.h" + + int main() { + return 0 + } + +Execute(Basic errors should be returned for GCC for C++ files with headers): + call ale#Lint() + + AssertEqual + \ [{'lnum': 2, 'col': 0}], + \ map(getloclist(0), '{''lnum'': v:val.lnum, ''col'': v:val.col}') + + AssertEqual 'Problems were found in the header (See :ALEDetail)', getloclist(0)[0].text diff --git a/test/handler/test_common_handlers.vader b/test/handler/test_common_handlers.vader index 0968a916..f087fb55 100644 --- a/test/handler/test_common_handlers.vader +++ b/test/handler/test_common_handlers.vader @@ -55,7 +55,7 @@ Then (The loclist should be correct): \], g:loclist Execute (Run HandleGCCFormat): - let g:loclist = ale#handlers#HandleGCCFormat(42, [ + let g:loclist = ale#handlers#gcc#HandleGCCFormat(42, [ \ ':8:5: warning: conversion lacks type at end of format [-Wformat=]', \ ':10:27: error: invalid operands to binary - (have ‘int’ and ‘char *’)', \]) @@ -63,18 +63,12 @@ Execute (Run HandleGCCFormat): Then (The loclist should be correct): AssertEqual [ \ { - \ 'bufnr': 42, - \ 'vcol': 0, - \ 'nr': -1, \ 'lnum': 8, \ 'col': 5, \ 'type': 'W', \ 'text': 'conversion lacks type at end of format [-Wformat=]', \ }, \ { - \ 'bufnr': 42, - \ 'vcol': 0, - \ 'nr': -1, \ 'lnum': 10, \ 'col': 27, \ 'type': 'E', diff --git a/test/handler/test_gcc_handler.vader b/test/handler/test_gcc_handler.vader new file mode 100644 index 00000000..3d5a24d4 --- /dev/null +++ b/test/handler/test_gcc_handler.vader @@ -0,0 +1,68 @@ +Execute(GCC errors from included files should be parsed correctly): + AssertEqual + \ [ + \ { + \ 'lnum': 3, + \ 'type': 'E', + \ 'text': 'Problems were found in the header (See :ALEDetail)', + \ 'detail': join([ + \ 'broken.h:1:1: error: expected identifier or ''('' before ''{'' token', + \ ' {{{', + \ ' ^', + \ ], "\n"), + \ }, + \ ], + \ ale#handlers#gcc#HandleGCCFormat(347, [ + \ 'In file included from :3:0:', + \ 'broken.h:1:1: error: expected identifier or ''('' before ''{'' token', + \ ' {{{', + \ ' ^', + \ ]) + + AssertEqual + \ [ + \ { + \ 'lnum': 3, + \ 'type': 'E', + \ 'text': 'Problems were found in the header (See :ALEDetail)', + \ 'detail': join([ + \ 'b.h:1:1: error: expected identifier or ''('' before ''{'' token', + \ ' {{{', + \ ' ^', + \ ], "\n"), + \ }, + \ ], + \ ale#handlers#gcc#HandleGCCFormat(347, [ + \ 'In file included from a.h:1:0,', + \ ' from test.c:3:', + \ 'b.h:1:1: error: expected identifier or ''('' before ''{'' token', + \ ' {{{', + \ ' ^', + \ ]) + + AssertEqual + \ [ + \ { + \ 'lnum': 3, + \ 'type': 'E', + \ 'text': 'Problems were found in the header (See :ALEDetail)', + \ 'detail': join([ + \ 'b.h:1:1: error: unknown type name ‘bad_type’', + \ ' bad_type x;', + \ ' ^', + \ 'b.h:2:1: error: unknown type name ‘other_bad_type’', + \ ' other_bad_type y;', + \ ' ^', + \ ], "\n"), + \ }, + \ ], + \ ale#handlers#gcc#HandleGCCFormat(347, [ + \ 'In file included from a.h:1:0,', + \ ' from test.c:3:', + \ 'b.h:1:1: error: unknown type name ‘bad_type’', + \ ' bad_type x;', + \ ' ^', + \ 'b.h:2:1: error: unknown type name ‘other_bad_type’', + \ ' other_bad_type y;', + \ ' ^', + \ ]) diff --git a/test/handler/test_swiftlint_handler.vader b/test/handler/test_swiftlint_handler.vader index 42fc8242..b77b4420 100644 --- a/test/handler/test_swiftlint_handler.vader +++ b/test/handler/test_swiftlint_handler.vader @@ -2,26 +2,20 @@ Execute(The swiftint handler should parse error messages correctly): AssertEqual \ [ \ { - \ 'bufnr': 347, \ 'lnum': 1, \ 'col': 7, \ 'text': 'Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)', \ 'type': 'W', - \ 'vcol': 0, - \ 'nr': -1, \ }, \ { - \ 'bufnr': 347, \ 'lnum': 1, \ 'col': 11, \ 'text': 'Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)', \ 'type': 'W', - \ 'vcol': 0, - \ 'nr': -1, \ }, \ \ ], - \ ale#handlers#HandleGCCFormat(347, [ + \ ale#handlers#gcc#HandleGCCFormat(347, [ \ 'This line should be ignored', \ ':1:7: warning: Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)', \ ':1:11: warning: Operator Usage Whitespace Violation: Operators should be surrounded by a single whitespace when they are being used. (operator_usage_whitespace)', diff --git a/test/handler/test_vint_handler.vader b/test/handler/test_vint_handler.vader new file mode 100644 index 00000000..efd33d10 --- /dev/null +++ b/test/handler/test_vint_handler.vader @@ -0,0 +1,36 @@ +Execute(The vint handler should parse error messages correctly): + :file! gxc.vim + + AssertEqual + \ [ + \ { + \ 'lnum': 1, + \ 'col': 1, + \ 'text': 'Use scriptencoding when multibyte char exists (see :help :script encoding)', + \ 'type': 'W', + \ }, + \ { + \ 'lnum': 3, + \ 'col': 17, + \ 'text': 'Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))', + \ 'type': 'W', + \ }, + \ { + \ 'lnum': 3, + \ 'col': 8, + \ 'text': 'Make the scope explicit like `l:filename` (see Anti-pattern of vimrc (Scope of identifier))', + \ 'type': 'W', + \ }, + \ { + \ 'lnum': 7, + \ 'col': 8, + \ 'text': 'Undefined variable: filename (see :help E738)', + \ 'type': 'W', + \ }, + \ ], + \ ale#handlers#gcc#HandleGCCFormat(347, [ + \ 'gcc.vim:1:1: warning: Use scriptencoding when multibyte char exists (see :help :script encoding)', + \ 'gcc.vim:3:17: warning: Use robust operators `==#` or `==?` instead of `==` (see Google VimScript Style Guide (Matching))', + \ 'gcc.vim:3:8: style_problem: Make the scope explicit like `l:filename` (see Anti-pattern of vimrc (Scope of identifier))', + \ 'gcc.vim:7:8: warning: Undefined variable: filename (see :help E738)', + \ ])