From f4c5d29c64ccb1aa02a0d77ca84e52b0ef84eca0 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Wed, 30 Aug 2017 10:59:40 -0700 Subject: [PATCH] Add a linter for Apache Thrift IDL files This linter works by invoking the `thrift` compiler with the buffer contents and reporting any parser and code generation issues. The handler rolls its own output-matching loop because we have the (unfortunate) requirement of handling error output that spans multiple lines. Unit tests cover both the command callback and handler, and there is initial documentation for all of the option variables. --- README.md | 1 + ale_linters/thrift/thrift.vim | 91 +++++++++++++++++++ doc/ale-thrift.txt | 46 ++++++++++ doc/ale.txt | 3 + .../test_thrift_command_callback.vader | 61 +++++++++++++ test/handler/test_thrift_handler.vader | 63 +++++++++++++ 6 files changed, 265 insertions(+) create mode 100644 ale_linters/thrift/thrift.vim create mode 100644 doc/ale-thrift.txt create mode 100644 test/command_callback/test_thrift_command_callback.vader create mode 100644 test/handler/test_thrift_handler.vader diff --git a/README.md b/README.md index 7bc96ba..449297f 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,7 @@ formatting. | Tcl | [nagelfar](http://nagelfar.sourceforge.net) !! | | Texinfo | [proselint](http://proselint.com/)| | Text^ | [proselint](http://proselint.com/), [vale](https://github.com/ValeLint/vale) | +| Thrift | [thrift](http://thrift.apache.org/) | | TypeScript | [eslint](http://eslint.org/), [tslint](https://github.com/palantir/tslint), tsserver, typecheck | | Verilog | [iverilog](https://github.com/steveicarus/iverilog), [verilator](http://www.veripool.org/projects/verilator/wiki/Intro) | | Vim | [vint](https://github.com/Kuniwak/vint) | diff --git a/ale_linters/thrift/thrift.vim b/ale_linters/thrift/thrift.vim new file mode 100644 index 0000000..2f62570 --- /dev/null +++ b/ale_linters/thrift/thrift.vim @@ -0,0 +1,91 @@ +" Author: Jon Parise + +call ale#Set('thrift_thrift_executable', 'thrift') +call ale#Set('thrift_thrift_generators', ['cpp']) +call ale#Set('thrift_thrift_includes', []) +call ale#Set('thrift_thrift_options', '-strict') + +function! ale_linters#thrift#thrift#GetExecutable(buffer) abort + return ale#Var(a:buffer, 'thrift_thrift_executable') +endfunction + +function! ale_linters#thrift#thrift#GetCommand(buffer) abort + let l:generators = ale#Var(a:buffer, 'thrift_thrift_generators') + let l:includes = ale#Var(a:buffer, 'thrift_thrift_includes') + + " The thrift compiler requires at least one generator. If none are set, + " fall back to our default value to avoid silently failing. We could also + " `throw` here, but that seems even less helpful. + if empty(l:generators) + let l:generators = ['cpp'] + endif + + let l:output_dir = tempname() + call mkdir(l:output_dir) + call ale#engine#ManageDirectory(a:buffer, l:output_dir) + + return ale#Escape(ale_linters#thrift#thrift#GetExecutable(a:buffer)) + \ . ' ' . join(map(copy(l:generators), "'--gen ' . v:val")) + \ . ' ' . join(map(copy(l:includes), "'-I ' . v:val")) + \ . ' ' . ale#Var(a:buffer, 'thrift_thrift_options') + \ . ' -out ' . ale#Escape(l:output_dir) + \ . ' %t' +endfunction + +function! ale_linters#thrift#thrift#Handle(buffer, lines) abort + " Matches lines like the following: + " + " [SEVERITY:/path/filename.thrift:31] Message text + " [ERROR:/path/filename.thrift:31] (last token was ';') + let l:pattern = '\v^\[(\u+):(.*):(\d+)\] (.*)$' + + let l:index = 0 + let l:output = [] + + " Roll our own output-matching loop instead of using ale#util#GetMatches + " because we need to support error messages that span multiple lines. + while l:index < len(a:lines) + let l:line = a:lines[l:index] + + let l:match = matchlist(l:line, l:pattern) + if empty(l:match) + let l:index += 1 + continue + endif + + let l:severity = l:match[1] + if l:severity is# 'WARNING' + let l:type = 'W' + else + let l:type = 'E' + endif + + " If our text looks like "(last token was ';')", the *next* line + " should contain a more descriptive error message. + let l:text = l:match[4] + if l:text =~# '\(last token was .*\)' + let l:index += 1 + let l:text = get(a:lines, l:index, 'Unknown error ' . l:text) + endif + + call add(l:output, { + \ 'lnum': l:match[3] + 0, + \ 'col': 0, + \ 'type': l:type, + \ 'text': l:text, + \}) + + let l:index += 1 + endwhile + + return l:output +endfunction + +call ale#linter#Define('thrift', { +\ 'name': 'thrift', +\ 'executable': 'thrift', +\ 'output_stream': 'both', +\ 'executable_callback': 'ale_linters#thrift#thrift#GetExecutable', +\ 'command_callback': 'ale_linters#thrift#thrift#GetCommand', +\ 'callback': 'ale_linters#thrift#thrift#Handle', +\}) diff --git a/doc/ale-thrift.txt b/doc/ale-thrift.txt new file mode 100644 index 0000000..ed858db --- /dev/null +++ b/doc/ale-thrift.txt @@ -0,0 +1,46 @@ +=============================================================================== +ALE Thrift Integration *ale-thrift-options* + + +=============================================================================== +thrift *ale-thrift-thrift* + +The `thrift` linter works by compiling the buffer's contents and reporting any +errors reported by the parser and the configured code generator(s). + +g:ale_thrift_thrift_executable *g:ale_thrift_thrift_executable* + *b:ale_thrift_thrift_executable* + Type: |String| + Default: `'thrift'` + + See |ale-integrations-local-executables| + + +g:ale_thrift_thrift_generators *g:ale_thrift_thrift_generators* + *b:ale_thrift_thrift_generators* + Type: |List| of |String|s + Default: `['cpp']` + + This list must contain one or more named code generators. Generator options + can be included as part of each string, e.g. `['py:dynamic']`. + + +g:ale_thrift_thrift_includes *g:ale_thrift_thrift_includes* + *b:ale_thrift_thrift_includes* + Type: |List| of |String|s + Default: `[]` + + This list contains paths that will be searched for thrift `include` + directives. + + +g:ale_thrift_thrift_options *g:ale_thrift_thrift_options* + *b:ale_thrift_thrift_options* + Type: |String| + Default: `'-strict'` + + This variable can be changed to customize the additional command-line + arguments that are passed to the thrift compiler. + +=============================================================================== + vim:tw=78:ts=2:sts=2:sw=2:ft=help:norl: diff --git a/doc/ale.txt b/doc/ale.txt index e3a098e..bbe0990 100644 --- a/doc/ale.txt +++ b/doc/ale.txt @@ -127,6 +127,8 @@ CONTENTS *ale-contents* tex...................................|ale-tex-options| chktex..............................|ale-tex-chktex| lacheck.............................|ale-tex-lacheck| + thrift................................|ale-thrift-options| + thrift..............................|ale-thrift-thrift| typescript............................|ale-typescript-options| eslint..............................|ale-typescript-eslint| tslint..............................|ale-typescript-tslint| @@ -248,6 +250,7 @@ Notes: * Tcl: `nagelfar`!! * Texinfo: `proselint` * Text^: `proselint`, `vale` +* Thrift: `thrift` * TypeScript: `eslint`, `tslint`, `tsserver`, `typecheck` * Verilog: `iverilog`, `verilator` * Vim: `vint` diff --git a/test/command_callback/test_thrift_command_callback.vader b/test/command_callback/test_thrift_command_callback.vader new file mode 100644 index 0000000..43487f4 --- /dev/null +++ b/test/command_callback/test_thrift_command_callback.vader @@ -0,0 +1,61 @@ +Before: + Save g:ale_thrift_thrift_executable + Save g:ale_thrift_thrift_generators + Save g:ale_thrift_thrift_includes + Save g:ale_thrift_thrift_options + + unlet! b:ale_thrift_thrift_executable + unlet! b:ale_thrift_thrift_generators + unlet! b:ale_thrift_thrift_includes + unlet! b:ale_thrift_thrift_options + + function! GetCommand(buffer) abort + call ale#engine#InitBufferInfo(a:buffer) + let l:result = ale_linters#thrift#thrift#GetCommand(a:buffer) + call ale#engine#Cleanup(a:buffer) + return l:result + endfunction + + runtime ale_linters/thrift/thrift.vim + +After: + Restore + delfunction GetCommand + unlet! b:ale_thrift_thrift_executable + unlet! b:ale_thrift_thrift_generators + unlet! b:ale_thrift_thrift_includes + unlet! b:ale_thrift_thrift_options + call ale#linter#Reset() + +Execute(The executable should be configurable): + AssertEqual 'thrift', ale_linters#thrift#thrift#GetExecutable(bufnr('')) + + let b:ale_thrift_thrift_executable = 'foobar' + AssertEqual 'foobar', ale_linters#thrift#thrift#GetExecutable(bufnr('')) + +Execute(The executable should be used in the command): + Assert GetCommand(bufnr('%')) =~# "^'thrift'" + + let b:ale_thrift_thrift_executable = 'foobar' + Assert GetCommand(bufnr('%')) =~# "^'foobar'" + +Execute(The list of generators should be configurable): + Assert GetCommand(bufnr('%')) =~# '--gen cpp' + + let b:ale_thrift_thrift_generators = ['java', 'py:dynamic'] + Assert GetCommand(bufnr('%')) =~# '--gen java --gen py:dynamic' + + let b:ale_thrift_thrift_generators = [] + Assert GetCommand(bufnr('%')) =~# '--gen cpp' + +Execute(The list of include paths should be configurable): + Assert GetCommand(bufnr('%')) !~# '-I' + + let b:ale_thrift_thrift_includes = ['included/path'] + Assert GetCommand(bufnr('%')) =~# '-I included/path' + +Execute(The string of compiler options should be configurable): + Assert GetCommand(bufnr('%')) =~# '-strict' + + let b:ale_thrift_thrift_options = '-strict --allow-64bit-consts' + Assert GetCommand(bufnr('%')) =~# '-strict --allow-64bit-consts' diff --git a/test/handler/test_thrift_handler.vader b/test/handler/test_thrift_handler.vader new file mode 100644 index 0000000..9bdb937 --- /dev/null +++ b/test/handler/test_thrift_handler.vader @@ -0,0 +1,63 @@ +Before: + runtime ale_linters/thrift/thrift.vim + +After: + call ale#linter#Reset() + +Execute(The thrift handler should handle basic warnings and errors): + AssertEqual + \ [ + \ { + \ 'lnum': 17, + \ 'col': 0, + \ 'type': 'W', + \ 'text': 'The "byte" type is a compatibility alias for "i8". Use i8" to emphasize the signedness of this type.', + \ }, + \ { + \ 'lnum': 20, + \ 'col': 0, + \ 'type': 'W', + \ 'text': 'Could not find include file include.thrift', + \ }, + \ { + \ 'lnum': 83, + \ 'col': 0, + \ 'type': 'E', + \ 'text': 'Enum FOO is already defined!', + \ }, + \ ], + \ ale_linters#thrift#thrift#Handle(1, [ + \ '[WARNING:/path/filename.thrift:17] The "byte" type is a compatibility alias for "i8". Use i8" to emphasize the signedness of this type.', + \ '[WARNING:/path/filename.thrift:20] Could not find include file include.thrift', + \ '[FAILURE:/path/filename.thrift:83] Enum FOO is already defined!', + \ ]) + +Execute(The thrift handler should handle multiline errors): + AssertEqual + \ [ + \ { + \ 'lnum': 75, + \ 'col': 0, + \ 'type': 'E', + \ 'text': 'This integer is too big: "11111111114213213453243"', + \ }, + \ { + \ 'lnum': 76, + \ 'col': 0, + \ 'type': 'E', + \ 'text': 'Implicit field keys are deprecated and not allowed with -strict', + \ }, + \ { + \ 'lnum': 77, + \ 'col': 0, + \ 'type': 'E', + \ 'text': "Unknown error (last token was ';')", + \ }, + \ ], + \ ale_linters#thrift#thrift#Handle(1, [ + \ "[ERROR:/path/filename.thrift:75] (last token was '11111111114213213453243')", + \ 'This integer is too big: "11111111114213213453243"', + \ "[ERROR:/path/filename.thrift:76] (last token was ';')", + \ 'Implicit field keys are deprecated and not allowed with -strict', + \ "[ERROR:/path/filename.thrift:77] (last token was ';')", + \ ])