From 8a772905539ccfe306219addae5a506def88fe44 Mon Sep 17 00:00:00 2001 From: Olaf Alders Date: Fri, 2 Mar 2018 16:04:52 -0500 Subject: [PATCH] [WIP] Begin to distinguish between Perl warnings and errors (#933) * If a Perl script compiles, there are only warnings and no errors * Let the first Perl error or warning win. Take the following example: *** sub foo { my $thing; *** This might have the following messages when we compile it: Missing right curly or square bracket at warning.pl line 7, at end of line syntax error at warning.pl line 7, at EOF warning.pl had compilation errors. With the current behaviour, we just get a "syntax error" message, which isn't all that helpful. With this patch we get "Missing right curly or square bracket". * Fix variable scope and pattern matching syntax * Use named variable to enhance clarity when matching Perl output * Add more tests for Perl linter * Remove unnecessary parens * Simplify check for pattern match --- ale_linters/perl/perl.vim | 14 ++++++++-- test/handler/test_perl_handler.vader | 39 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/ale_linters/perl/perl.vim b/ale_linters/perl/perl.vim index fcc88f35..1b9aa95e 100644 --- a/ale_linters/perl/perl.vim +++ b/ale_linters/perl/perl.vim @@ -27,12 +27,20 @@ function! ale_linters#perl#perl#Handle(buffer, lines) abort let l:output = [] let l:basename = expand('#' . a:buffer . ':t') + let l:type = 'E' + if a:lines[-1] =~# 'syntax OK' + let l:type = 'W' + endif + + let l:seen = {} + for l:match in ale#util#GetMatches(a:lines, l:pattern) let l:line = l:match[3] + let l:file = l:match[2] let l:text = l:match[1] - let l:type = 'E' - if ale#path#IsBufferPath(a:buffer, l:match[2]) + if ale#path#IsBufferPath(a:buffer, l:file) + \ && !has_key(l:seen,l:line) \ && ( \ l:text isnot# 'BEGIN failed--compilation aborted' \ || empty(l:output) @@ -43,6 +51,8 @@ function! ale_linters#perl#perl#Handle(buffer, lines) abort \ 'text': l:text, \ 'type': l:type, \}) + + let l:seen[l:line] = 1 endif endfor diff --git a/test/handler/test_perl_handler.vader b/test/handler/test_perl_handler.vader index 75e8f226..c5791d76 100644 --- a/test/handler/test_perl_handler.vader +++ b/test/handler/test_perl_handler.vader @@ -47,3 +47,42 @@ Execute(The Perl linter should complain about failing to locate modules): \ 'Unable to build `ro` accessor for slot `path` in `App::CPANFileUpdate` because the slot cannot be found. at /extlib/Method/Traits.pm line 189.', \ 'BEGIN failed--compilation aborted at - line 10.', \ ]) + +Execute(The Perl linter should not report warnings as errors): + AssertEqual + \ [ + \ {'lnum': '5', 'type': 'W', 'text': '"my" variable $foo masks earlier declaration in same scope'}, + \ ], + \ ale_linters#perl#perl#Handle(bufnr(''), [ + \ '"my" variable $foo masks earlier declaration in same scope at - line 5.', + \ 't.pl syntax OK', + \ ]) + +Execute(The Perl linter does not default to reporting generic error): + AssertEqual + \ [ + \ {'lnum': '8', 'type': 'E', 'text': 'Missing right curly or square bracket'}, + \ ], + \ ale_linters#perl#perl#Handle(bufnr(''), [ + \ 'Missing right curly or square bracket at - line 8, at end of line', + \ 'syntax error at - line 8, at EOF', + \ 'Execution of t.pl aborted due to compilation errors.', + \ ]) + +" The first "error" is actually a warning, but the current implementation +" doesn't have a good way of teasing out the warnings from amongst the +" errors. If we're able to do this in future, then we'll want to switch +" the first "E" to a "W". + +Execute(The Perl linter reports errors even when mixed with warnings): + AssertEqual + \ [ + \ {'lnum': '5', 'type': 'E', 'text': '"my" variable $foo masks earlier declaration in same scope'}, + \ {'lnum': '8', 'type': 'E', 'text': 'Missing right curly or square bracket'}, + \ ], + \ ale_linters#perl#perl#Handle(bufnr(''), [ + \ '"my" variable $foo masks earlier declaration in same scope at - line 5.', + \ 'Missing right curly or square bracket at - line 8, at end of line', + \ 'syntax error at - line 8, at EOF', + \ 'Execution of t.pl aborted due to compilation errors.', + \ ])