Implemented review recommendations

Implements suggestions and recommendations suggested by the first review
of the "Advance C# linter based on mcs -t:module (#952)" pull request.

- Clarifies and simplifies description of linters and options
- Added links to help file and marked the mcsc linter as to be run only
  when file in buffer is saved or loaded.
- Added comments to the mcsc.vim file to clarify code
- removed type checks considered not necessary be reviewer.
- addresses findings by vader
- removed call to getcwd and cd in vim script
- handler expands file names relative to route of source tree into
  absolute pathes. Fixes errors not being marked when vim is started
  from subdirectory of source tree.
- implements tests for mcs.vim and mcsc.vim linter
This commit is contained in:
Xristoph Hintermüller 2017-09-26 09:19:53 +02:00
parent 0be77c60c5
commit 8f6044b8b6
8 changed files with 351 additions and 88 deletions

View File

@ -76,7 +76,7 @@ formatting.
| C | [cppcheck](http://cppcheck.sourceforge.net), [cpplint](https://github.com/google/styleguide/tree/gh-pages/cpplint), [gcc](https://gcc.gnu.org/), [clang](http://clang.llvm.org/), [clangtidy](http://clang.llvm.org/extra/clang-tidy/) !!, [clang-format](https://clang.llvm.org/docs/ClangFormat.html)|
| C++ (filetype cpp) | [clang](http://clang.llvm.org/), [clangcheck](http://clang.llvm.org/docs/ClangCheck.html) !!, [clangtidy](http://clang.llvm.org/extra/clang-tidy/) !!, [cppcheck](http://cppcheck.sourceforge.net), [cpplint](https://github.com/google/styleguide/tree/gh-pages/cpplint) !!, [gcc](https://gcc.gnu.org/), [clang-format](https://clang.llvm.org/docs/ClangFormat.html)|
| CUDA | [nvcc](http://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html) |
| C# | [mcs](http://www.mono-project.com/docs/about-mono/languages/csharp/) see:`help ale-cs-mcs` for details,[mcsc](http://www.mono-project.com/docs/about-mono/languages/csharp/) see:`help ale-cs-mcsc` for details and configuration|
| C# | [mcs](http://www.mono-project.com/docs/about-mono/languages/csharp/) see:`help ale-cs-mcs` for details, [mcsc](http://www.mono-project.com/docs/about-mono/languages/csharp/) !! see:`help ale-cs-mcsc` for details and configuration|
| Chef | [foodcritic](http://www.foodcritic.io/) |
| CMake | [cmakelint](https://github.com/richq/cmake-lint) |
| CoffeeScript | [coffee](http://coffeescript.org/), [coffeelint](https://www.npmjs.com/package/coffeelint) |

View File

@ -1,71 +1,72 @@
" general mcs options which are likely to stay constant across
" source trees like -pkg:dotnet
let g:ale_cs_mcsc_options = get(g:, 'ale_cs_mcsc_options', '')
let g:ale_cs_mcsc_source = get(g:,'ale_cs_mcsc_source','')
let g:ale_cs_mcsc_assembly_path = get(g:,'ale_cs_mcsc_assembly_path',[])
let g:ale_cs_mcsc_assemblies = get(g:,'ale_cs_mcsc_assemblies',[])
" path string pointing the linter to the base path of the
" source tree to check
let g:ale_cs_mcsc_source = get(g:, 'ale_cs_mcsc_source','.')
" list of search paths for additional assemblies to consider
let g:ale_cs_mcsc_assembly_path = get(g:, 'ale_cs_mcsc_assembly_path',[])
" list of assemblies to consider
let g:ale_cs_mcsc_assemblies = get(g:, 'ale_cs_mcsc_assemblies',[])
function! ale_linters#cs#mcsc#GetCommand(buffer) abort
let l:path = ale#Var(a:buffer,'cs_mcsc_assembly_path')
" if list of assembly search paths is not empty convert it to
" appropriate -lib: parameter of mcs
let l:path = ale#Var(a:buffer, 'cs_mcsc_assembly_path')
if !empty(l:path)
if type(l:path) == type('')
let l:path = '-lib:' . l:path
elseif type(l:path) == type([])
let l:path = '-lib:' . join(l:path,',')
let l:path = '-lib:"' . join(l:path, '","') .'"'
else
throw 'assembly path list must be string or list of path strings'
endif
elseif type(l:path) != type('')
if type(l:path) != type([])
throw 'assembly path list must be string or list of path strings'
endif
let l:path =''
endif
let l:assemblies = ale#Var(a:buffer,'cs_mcsc_assemblies')
" if list of assemblies to link is not empty convert it to the
" appropriate -r: parameter of mcs
let l:assemblies = ale#Var(a:buffer, 'cs_mcsc_assemblies')
if !empty(l:assemblies)
if type(l:assemblies) == type('')
let l:assemblies = '-r' . l:assemblies
elseif type(l:assemblies) == type([])
let l:assemblies = '-r:' . join(l:assemblies,',')
let l:assemblies = '-r:"' . join(l:assemblies, '","') . '"'
else
throw 'assembly list must be string or list of strings'
endif
elseif type(l:assemblies) != type('')
if type(l:assemblies) != type([])
throw 'assembly list must be string or list of string'
endif
let l:assemblies =''
endif
let l:base = ale#Var(a:buffer,'cs_mcsc_source')
let l:cwd = getcwd()
if isdirectory(l:base)
exe 'cd ' . l:base
elseif empty(l:base) && ( type(l:base) == type('') )
let l:base = '.'
else
throw 'ale_cs_mcs_source must point to an existing directory or empty string for current'
endif
" register temporary module target file with ale
let l:out = tempname()
call ale#engine#ManageFile(a:buffer,l:out)
let l:cmd = 'cd ' . l:base . ';'
call ale#engine#ManageFile(a:buffer, l:out)
" assemble linter command string to be executed by ale
" implicitly set -unsafe mcs flag set compilation
" target to module (-t:module), direct mcs output to
" temporary file (-out)
"
return 'cd "' . ale#Var(a:buffer, 'cs_mcsc_source') . '";'
\ . 'mcs -unsafe'
\ . ' ' . ale#Var(a:buffer, 'cs_mcsc_options')
\ . ' ' . l:path
\ . ' ' . l:assemblies
\ . ' -out:' . l:out
\ . ' -t:module'
\ . ' "' . join(glob('**/*.cs',v:false,v:true),'" "') . '"'
exe 'cd ' . l:cwd
return l:cmd
\ . ' -recurse:"*.cs"'
endfunction
function! ale_linters#cs#mcsc#Handle(buffer, lines) abort
" Look for lines like the following.
"
" Tests.cs(12,29): error CSXXXX: ; expected
"
" NOTE: pattern also captures file name as linter compiles all
" files within the source tree rooted at the specified source
" path and not just the file loaded in the buffer
let l:pattern = '^\(.\+\.cs\)(\(\d\+\),\(\d\+\)): \(.\+\): \(.\+\)'
let l:output = []
let l:source = ale#Var(a:buffer, 'cs_mcsc_source')
for l:match in ale#util#GetMatches(a:lines, l:pattern)
call add(l:output, {
\ 'filename': l:match[1],
\ 'filename': fnamemodify(l:source . '/' . l:match[1], ':p'),
\ 'lnum': l:match[2] + 0,
\ 'col': l:match[3] + 0,
\ 'text': l:match[4] . ': ' . l:match[5],

View File

@ -5,8 +5,9 @@ ALE C# Integration *ale-cs-options*
===============================================================================
mcs *ale-cs-mcs*
The mcs linter calls the mono mcs compiler setting the --parse and -unsafe
flags.
The mcs linter checks the syntax of the '*.cs' file loaded in the current
buffer only. It uses the --parse option of the mcs compiler and implicitly
sets the -unsafe flag.
g:ale_cs_mcs_options *g:ale_cs_mcs_options*
*b:ale_cs_mcs_options*
@ -14,62 +15,88 @@ g:ale_cs_mcs_options *g:ale_cs_mcs_options*
Type: String
Default: `''`
This variable can be changed to modify flags given to mcs. The options
--parse and -unsafe are implicitly set.
This variable can be changed to pass additional flags given to mcs.
NOTE: The -unsafe flag is selected implicitly and thus does not need to be
explicitly included in the |g:ale_cs_mcs_options| or |b:ale_cs_mcs_options|
parameter.
===============================================================================
mcsc *ale-cs-mcsc*
The mcsc linter uses the mono mcs compiler to generate a temporary module
target file (-t:module) including all '*.cs' files contained in the
directory by specified by |g:ale_cs_mcsc_source| or |b:ale_cs_mcsc_source|
variable and all sub directories. Currently none can be excluded from
linting. It uses the assembly directories as specified by
|g:ale_cs_mcsc_assembly_path| or |b:ale_cs_mcsc_assembly_path| and selects
the assembly files specified by |g:ale_cs_mcsc_assemblies| or
|b:ale_cs_mcsc_assemblies|. The mcs -unsafe option is set implicitly and has
not to be added using |g:ale_cs_mcsc_options| or |b:ale_cs_mcsc_options|
variable.
target file (-t:module). The module includes including all '*.cs' files
contained in the directory tree rooted at the path defined by the
|g:ale_cs_mcsc_source| or |b:ale_cs_mcsc_source| variable.
variable and all sub directories.
The paths to search for additional assembly ('*.dll') files can be
specified using the |g:ale_cs_mcsc_assembly_path| or
|b:ale_cs_mcsc_assembly_path| variable. The additional assembly files ('*.dll')
can be included through the |g:ale_cs_mcsc_assemblies| or
|b:ale_cs_mcsc_assemblies| parameter.
NOTE: mcs compiles sources in multiple phases. It stops compilation after
finding errors during the current phase.
For example assume a file named 'FileWithTypeError.cs' is edited and saved
which contains a Type error. In the same directory tree a file named
'FileWithSyntaxError.cs' exists which contains a syntax error
(eg.: a missing '{').
In that case mcs and thus mcsc linter will stop after the syntax check phase is
finished and report the syntax error in the file 'FileWithSyntaxError.cs'. The
Type error in the file 'FileWithTypeError.cs is not seen jet.
The only possibility to find the error in in 'FileWithTypeError.cs' is to fix
the syntax error in 'FileWithSyntaxError.cs' first. After saving mcs will
successfully pass the syntax check phase and advance to the next compilation
phase at which the Type error hidden in 'FileWithTypeError.cs' is found and
now can be indicated by ale.
g:ale_cs_mcsc_options *g:ale_cs_mcsc_options*
*b:ale_cs_mcsc_options*
Type: |String|
Default: `''`
This variable can be set to set further options for example adding packages
(eg.: -pkg:dotnet) with are not added per default.
This parameter can be used to define additional flags and parameters independent
of the source tree to be linted. The specified string is directly passed to
mcs compiler without any further change.
For example, to add the dotnet package which is not added per default
let g:ale_cs_mcs_options = '-pkg:dotnet'
NOTE: The mcs -unsafe option is included implicitly per default. Therefore it
is not necessary to specify it explicitly through the |g:ale_cs_mcsc_options|
or |b:ale_cs_mcsc_options| parameter.
g:ale_cs_mcsc_source *g:ale_cs_mcsc_source*
*b:ale_cs_mcsc_source*
Type: |String|
Default: `''`
This variable defines the base path of the directory tree the '*.cs' files
should be included into the compilation of the temporary module. If empty
the current directory is used.
This variable defines the root path of the directory tree searched for the
'*.cs' files to be linted. If empty the current working directory is used.
NOTE: Currently it is not possible to specify sub directories and
directory sub trees which shall not be searched for *.cs files.
g:ale_cs_mcsc_assembly_path *g:ale_cs_mcsc_assembly_path*
*b:ale_cs_mcsc_assembly_path*
Type: |List|
Default: `[]`
This variable defines a list of absolute or relative path strings pointing
to the location of the assembly files (*.dll) to be considered by mcsc
linter. If the list is not empty the list will be added to the mcsc command
line using the -lib: flag of mcs.
This variable defines a list of path strings to be searched for external
assembly ('*.dll') files. The list is passed to the mcs compiler using the
'-lib:' flag.
g:ale_cs_mcsc_assemblies *g:ale_cs_mcsc_assemblies*
*b:ale_cs_mcsc_assemblies*
Type: |List|
Default: `[]`
This variable defines a list of assembly files (*.dll) to be considered by
the mono mcs compiler when generating the temporary module. If the list is
not empty the list of assemblies will be added to the mcsc command
line using the -r: flag of mcs. To change the search path mcs uses to
locate the specified assembly files use |g:ale_cs_mcsc_assembly_path| or
|b:ale_cs_mcsc_assembly_path| variables
This variable defines a list of external assembly (*.dll) files required
by the mono mcs compiler to generate a valid module target. The list is
passed the mcs compiler using the '-r:' flag.
===============================================================================
vim:tw=78:ts=2:sts=2:sw=2:ft=help:norl:

View File

@ -36,6 +36,9 @@ CONTENTS *ale-contents*
cppcheck............................|ale-cpp-cppcheck|
cpplint.............................|ale-cpp-cpplint|
gcc.................................|ale-cpp-gcc|
c#....................................|ale-cs-options|
mcs.................................|ale-cs-mcs|
mcsc................................|ale-cs-mcsc|
css...................................|ale-css-options|
prettier............................|ale-css-prettier|
stylelint...........................|ale-css-stylelint|
@ -222,7 +225,7 @@ Notes:
* C: `cppcheck`, `cpplint`!!, `gcc`, `clang`, `clangtidy`!!, `clang-format`
* C++ (filetype cpp): `clang`, `clangcheck`!!, `clangtidy`!!, `cppcheck`, `cpplint`!!, `gcc`, `clang-format`
* CUDA: `nvcc`!!
* C#: `mcs`
* C#: `mcs`, `mcsc`!!
* Chef: `foodcritic`
* CMake: `cmakelint`
* CoffeeScript: `coffee`, `coffeelint`

View File

@ -0,0 +1,34 @@
Before:
Save g:ale_cs_mcs_options
unlet! g:ale_cs_mcs_options
runtime ale_linters/cs/mcs.vim
let b:command_tail = ' -unsafe --parse'
After:
Restore
unlet! b:command_tail
unlet! b:ale_cs_mcs_options
call ale#linter#Reset()
Execute(Check for proper default command):
let b:command = ale_linters#cs#mcs#GetCommand(bufnr(''))
let b:command = substitute(b:command,'\s\+',' ','g')
AssertEqual
\ b:command,
\ 'mcs -unsafe --parse %t'
Execute(The options should be be used in the command):
let b:ale_cs_mcs_options = '-pkg:dotnet'
let b:command = ale_linters#cs#mcs#GetCommand(bufnr(''))
let b:command = substitute(b:command,'\s\+',' ','g')
AssertEqual
\ b:command,
\ 'mcs' . b:command_tail . ' ' . b:ale_cs_mcs_options . ' %t',

View File

@ -0,0 +1,120 @@
Before:
Save g:ale_cs_mcsc_options
Save g:ale_cs_mcsc_source
Save g:ale_cs_mcsc_assembly_path
Save g:ale_cs_mcsc_assemblies
unlet! g:ale_cs_mcsc_options
unlet! g:ale_cs_mcsc_source
unlet! g:ale_cs_mcsc_assembly_path
unlet! g:ale_cs_mcsc_assemblies
let g:temppath = fnamemodify(tempname(), ':p:h')
let g:temppathpattern = substitute(escape(g:temppath, '\\/.*$^~[]'), '[\\/]*$', '[\\\\/]\\+\\S\\+','')
let g:sometempfile = fnamemodify(g:temppath .'/some_temp_file.tmp', ':p')
runtime ale_linters/cs/mcsc.vim
After:
Restore
unlet! b:ale_cs_mcsc_options
unlet! g:ale_cs_mcsc_source
unlet! g:ale_cs_mcsc_assembly_path
unlet! g:ale_cs_mcsc_assemblies
unlet! g:temppath
unlet! g:temppathpattern
unlet! g:sometempfile
call ale#linter#Reset()
Execute(Check for proper default command):
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command, '\s\+', ' ', 'g')
AssertEqual
\ b:command,
\ 'cd ".";mcs -unsafe -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'
Execute(The options should be be used in the command):
let g:ale_cs_mcsc_options = '-pkg:dotnet'
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command, '\s\+', ' ', 'g')
AssertEqual
\ b:command,
\ 'cd ".";mcs -unsafe ' . g:ale_cs_mcsc_options . ' -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'
Execute(The souce path should be be used in the command):
call ale#engine#Cleanup(bufnr(''))
call ale#engine#InitBufferInfo(bufnr(''))
let g:ale_cs_mcsc_source='../foo/bar'
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command, '\s\+', ' ', 'g')
AssertEqual
\ b:command,
\ 'cd "' . g:ale_cs_mcsc_source . '";mcs -unsafe -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'
Execute(The list of search pathes for assemblies should be be used in the command if not empty):
call ale#engine#Cleanup(bufnr(''))
call ale#engine#InitBufferInfo(bufnr(''))
let g:ale_cs_mcsc_assembly_path = [
\ '/usr/lib/mono',
\ '../foo/bar'
\]
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command, '\s\+', ' ', 'g')
AssertEqual
\ b:command,
\ 'cd ".";mcs -unsafe -lib:"' . join(g:ale_cs_mcsc_assembly_path,'","') . '" -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'
let g:ale_cs_mcsc_assembly_path = [
\]
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command, '\s\+', ' ', 'g')
AssertEqual
\ b:command,
\ 'cd ".";mcs -unsafe -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'
Execute(The list of assemblies should be be used in the command if not empty):
call ale#engine#Cleanup(bufnr(''))
call ale#engine#InitBufferInfo(bufnr(''))
let g:ale_cs_mcsc_assemblies = [
\ 'foo.dll',
\ 'bar.dll'
\]
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command,'\s\+',' ','g')
AssertEqual
\ b:command,
\ 'cd ".";mcs -unsafe -r:"' . join(g:ale_cs_mcsc_assemblies,'","') . '" -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'
let g:ale_cs_mcsc_assemblies = [
\]
let b:command = ale_linters#cs#mcsc#GetCommand(bufnr(''))
let b:command = substitute(b:command, '-out:' . g:temppathpattern, '-out:' . g:sometempfile, '')
let b:command = substitute(b:command,'\s\+',' ','g')
AssertEqual
\ b:command,
\ 'cd ".";mcs -unsafe -out:' . g:sometempfile . ' -t:module -recurse:"*.cs"'

View File

@ -0,0 +1,34 @@
Before:
runtime ale_linters/cs/mcs.vim
After:
call ale#linter#Reset()
Execute(The mcs handler should handle cannot find symbol errors):
AssertEqual
\ [
\ {
\ 'lnum': 12,
\ 'col' : 29,
\ 'text': 'error CS1001: ; expected',
\ 'type': 'E',
\ },
\ {
\ 'lnum': 101,
\ 'col': 0,
\ 'text': 'error CS1028: Unexpected processor directive (no #if for this #endif)',
\ 'type': 'E',
\ },
\ {
\ 'lnum': 10,
\ 'col': 12,
\ 'text': 'warning CS0123: some warning',
\ 'type': 'W',
\ },
\ ],
\ ale_linters#cs#mcs#Handle(347, [
\ 'Tests.cs(12,29): error CS1001: ; expected',
\ 'Tests.cs(101,0): error CS1028: Unexpected processor directive (no #if for this #endif)',
\ 'Tests.cs(10,12): warning CS0123: some warning',
\ 'Compilation failed: 2 error(s), 1 warnings',
\ ])

View File

@ -0,0 +1,44 @@
Before:
Save g:ale_cs_mcsc_source
unlet! g:ale_cs_mcsc_source
runtime ale_linters/cs/mcsc.vim
After:
unlet! g:ale_cs_mcsc_source
call ale#linter#Reset()
Execute(The mcs handler should handle cannot find symbol errors):
let g:ale_cs_mcsc_source='/home/foo/project/bar'
AssertEqual
\ [
\ {
\ 'lnum': 12,
\ 'col' : 29,
\ 'text': 'error CS1001: ; expected',
\ 'type': 'E',
\ 'filename': '/home/foo/project/bar/Test.cs'
\ },
\ {
\ 'lnum': 101,
\ 'col': 0,
\ 'text': 'error CS1028: Unexpected processor directive (no #if for this #endif)',
\ 'type': 'E',
\ 'filename': '/home/foo/project/bar/Test.cs'
\ },
\ {
\ 'lnum': 10,
\ 'col': 12,
\ 'text': 'warning CS0123: some warning',
\ 'type': 'W',
\ 'filename': '/home/foo/project/bar/Test.cs'
\ },
\ ],
\ ale_linters#cs#mcsc#Handle(347, [
\ 'Test.cs(12,29): error CS1001: ; expected',
\ 'Test.cs(101,0): error CS1028: Unexpected processor directive (no #if for this #endif)',
\ 'Test.cs(10,12): warning CS0123: some warning',
\ 'Compilation failed: 2 error(s), 1 warnings',
\ ])