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

1 files changed, 49 insertions, 48 deletions

diff --git a/ale_linters/cs/mcsc.vim b/ale_linters/cs/mcsc.vim
index 70dc4ab6..38a08556 100644
--- a/ale_linters/cs/mcsc.vim
+++ b/ale_linters/cs/mcsc.vim
@@ -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 !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,',')
-        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
+
+    " 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)
+         let l:path = '-lib:"' . join(l:path, '","') .'"'
+    else
         let l:path =''
     endif
-    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,',')
-        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
+
+    " 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)
+        let l:assemblies = '-r:"' . join(l:assemblies, '","') . '"'
+    else
         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
-	let l:out = tempname()
-	call ale#engine#ManageFile(a:buffer,l:out)
-    let l:cmd = 'cd ' . l:base . ';'
-    \    . 'mcs -unsafe' 
+
+    " register temporary module target file with ale
+    let l:out = tempname()
+    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
+    \    . ' -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],
@@ -82,5 +83,5 @@ call ale#linter#Define('cs',{
 \   'executable': 'mcs',
 \   'command_callback': 'ale_linters#cs#mcsc#GetCommand',
 \   'callback': 'ale_linters#cs#mcsc#Handle',
-\	'lint_file': 1
+\   'lint_file': 1
 \})