-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Ruby linter with Steep #4671
Open
lloeki
wants to merge
16
commits into
dense-analysis:master
Choose a base branch
from
lloeki:feature/ruby-steep-linter
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
36a27fb
Add Ruby linter with Steep
lloeki 0de6c1a
Run steep instead of using language server
lloeki a051863
Work around Steep path issue
lloeki 00dbeb9
Add simple tests for steep
lloeki b220b41
Add steep to supported tools
lloeki 5fff127
Pass linter
lloeki b3e645b
Add a comment regarding Steep's column counting
lloeki 27d614f
Make lnum an integer
lloeki 8856648
Add Steep handler test
lloeki fcb2bdd
Fix separator for Windows
lloeki 28120a3
Escape Windows path separators for substitute()
lloeki 18267d0
Use ALEInfo (I) group
lloeki 8579545
Use fnameescape instead of quotes
lloeki c9f59e9
Skip linting for files not under steep root
lloeki ebd9023
Add and pass tests covering proper steep root lookup
lloeki fd80bcf
Fix separator discrepancy
lloeki File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,172 @@ | ||
call ale#Set('ruby_steep_executable', 'steep') | ||
call ale#Set('ruby_steep_options', '') | ||
|
||
" Find the nearest dir containing a Steepfile | ||
function! ale_linters#ruby#steep#FindRoot(buffer) abort | ||
for l:name in ['Steepfile'] | ||
let l:dir = fnamemodify( | ||
\ ale#path#FindNearestFile(a:buffer, l:name), | ||
\ ':h' | ||
\) | ||
|
||
if l:dir isnot# '.' && isdirectory(l:dir) | ||
return l:dir | ||
endif | ||
endfor | ||
|
||
return '' | ||
endfunction | ||
|
||
" Rename path relative to root | ||
function! ale_linters#ruby#steep#RelativeToRoot(buffer, path) abort | ||
let l:separator = has('win32') ? '\' : '/' | ||
let l:steep_root = ale_linters#ruby#steep#FindRoot(a:buffer) | ||
|
||
" path isn't under root | ||
if l:steep_root == '' | ||
return '' | ||
endif | ||
|
||
let l:steep_root_prefix = l:steep_root . l:separator | ||
|
||
" win32 path separators get interpreted by substitute, escape them | ||
if has('win32') | ||
let l:steep_root_pat = substitute(l:steep_root_prefix, '\\', '\\\\', 'g') | ||
else | ||
let l:steep_root_pat = l:steep_root_prefix | ||
endif | ||
|
||
return substitute(a:path, l:steep_root_pat, '', '') | ||
endfunction | ||
|
||
function! ale_linters#ruby#steep#GetCommand(buffer) abort | ||
let l:executable = ale#Var(a:buffer, 'ruby_steep_executable') | ||
|
||
" steep check needs to apply some config from the file path so: | ||
" - steep check can't use stdin (no path) | ||
" - steep check can't use %t (path outside of project) | ||
" => we can only use %s | ||
|
||
" somehow :ALEInfo shows that ALE still appends '< %t' to the command | ||
" => luckily steep check ignores stdin | ||
|
||
" somehow steep has a problem with absolute path to file but a path | ||
" relative to Steepfile directory works: | ||
" see https://github.com/soutaro/steep/pull/975 | ||
" => change to Steepfile directory and remove leading path | ||
|
||
let l:buffer_filename = fnamemodify(bufname(a:buffer), ':p') | ||
let l:buffer_filename = fnameescape(l:buffer_filename) | ||
|
||
let l:relative = ale_linters#ruby#steep#RelativeToRoot(a:buffer, l:buffer_filename) | ||
|
||
" if file is not under steep root, steep can't type check | ||
if l:relative == '' | ||
" don't execute | ||
return '' | ||
endif | ||
|
||
return ale#ruby#EscapeExecutable(l:executable, 'steep') | ||
\ . ' check ' | ||
\ . ale#Var(a:buffer, 'ruby_steep_options') | ||
\ . ' ' . fnameescape(l:relative) | ||
endfunction | ||
|
||
function! ale_linters#ruby#steep#GetType(severity) abort | ||
if a:severity is? 'information' | ||
\|| a:severity is? 'hint' | ||
return 'I' | ||
endif | ||
|
||
if a:severity is? 'warning' | ||
return 'W' | ||
endif | ||
|
||
return 'E' | ||
endfunction | ||
|
||
" Handle output from steep | ||
function! ale_linters#ruby#steep#HandleOutput(buffer, lines) abort | ||
let l:output = [] | ||
|
||
let l:in = 0 | ||
let l:item = {} | ||
|
||
for l:line in a:lines | ||
" Look for first line of a message block | ||
" If not in-message (l:in == 0) that's expected | ||
" If in-message (l:in > 0) that's less expected but let's recover | ||
let l:match = matchlist(l:line, '^\([^:]*\):\([0-9]*\):\([0-9]*\): \[\([^]]*\)\] \(.*\)') | ||
|
||
if len(l:match) > 0 | ||
" Something is lingering: recover by pushing what is there | ||
if len(l:item) > 0 | ||
call add(l:output, l:item) | ||
let l:item = {} | ||
endif | ||
|
||
let l:filename = l:match[1] | ||
|
||
" Steep's reported column is offset by 1 (zero-indexed?) | ||
let l:item = { | ||
\ 'lnum': l:match[2] + 0, | ||
\ 'col': l:match[3] + 1, | ||
\ 'type': ale_linters#ruby#steep#GetType(l:match[4]), | ||
\ 'text': l:match[5], | ||
\} | ||
|
||
" Done with this line, mark being in-message and go on with next line | ||
let l:in = 1 | ||
continue | ||
endif | ||
|
||
" We're past the first line of a message block | ||
if l:in > 0 | ||
" Look for code in subsequent lines of the message block | ||
if l:line =~# '^│ Diagnostic ID:' | ||
let l:match = matchlist(l:line, '^│ Diagnostic ID: \(.*\)') | ||
|
||
if len(l:match) > 0 | ||
let l:item.code = l:match[1] | ||
endif | ||
|
||
" Done with the line | ||
continue | ||
endif | ||
|
||
" Look for last line of the message block | ||
if l:line =~# '^└' | ||
" Done with the line, mark looking for underline and go on with the next line | ||
let l:in = 2 | ||
continue | ||
endif | ||
|
||
" Look for underline right after last line | ||
if l:in == 2 | ||
let l:match = matchlist(l:line, '\([~][~]*\)') | ||
|
||
if len(l:match) > 0 | ||
let l:item.end_col = l:item['col'] + len(l:match[1]) - 1 | ||
endif | ||
|
||
call add(l:output, l:item) | ||
|
||
" Done with the line, mark looking for first line and go on with the next line | ||
let l:in = 0 | ||
let l:item = {} | ||
continue | ||
endif | ||
endif | ||
endfor | ||
|
||
return l:output | ||
endfunction | ||
|
||
call ale#linter#Define('ruby', { | ||
\ 'name': 'steep', | ||
\ 'executable': {b -> ale#Var(b, 'ruby_steep_executable')}, | ||
\ 'language': 'ruby', | ||
\ 'command': function('ale_linters#ruby#steep#GetCommand'), | ||
\ 'project_root': function('ale_linters#ruby#steep#FindRoot'), | ||
\ 'callback': 'ale_linters#ruby#steep#HandleOutput', | ||
\}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -572,6 +572,7 @@ Notes: | |
* `solargraph` | ||
* `sorbet` | ||
* `standardrb` | ||
* `steep` | ||
* `syntax_tree` | ||
* Rust | ||
* `cargo`!! | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
Before: | ||
runtime ale_linters/ruby/steep.vim | ||
|
||
After: | ||
call ale#linter#Reset() | ||
|
||
Execute(The steep handler should parse lines correctly): | ||
AssertEqual | ||
\ [ | ||
\ { | ||
\ 'lnum': 400, | ||
\ 'col': 18, | ||
\ 'end_col': 45, | ||
\ 'text': 'Method parameters are incompatible with declaration `(untyped, untyped, *untyped, **untyped) { () -> untyped } -> untyped`', | ||
\ 'code': 'Ruby::MethodArityMismatch', | ||
\ 'type': 'E', | ||
\ }, | ||
\ { | ||
\ 'lnum': 20, | ||
\ 'col': 9, | ||
\ 'end_col': 17, | ||
\ 'text': 'Cannot find implementation of method `::Frobz::FooBarBaz#method_name`', | ||
\ 'code': 'Ruby::MethodDefinitionMissing', | ||
\ 'type': 'W', | ||
\ }, | ||
\ { | ||
\ 'lnum': 30, | ||
\ 'col': 9, | ||
\ 'end_col': 17, | ||
\ 'text': 'Cannot find implementation of method `::Frobz::FooBarBaz#method_name`', | ||
\ 'code': 'Ruby::MethodDefinitionMissing', | ||
\ 'type': 'I', | ||
\ }, | ||
\ { | ||
\ 'lnum': 40, | ||
\ 'col': 9, | ||
\ 'end_col': 17, | ||
\ 'text': 'Cannot find implementation of method `::Frobz::FooBarBaz#method_name`', | ||
\ 'code': 'Ruby::MethodDefinitionMissing', | ||
\ 'type': 'I', | ||
\ }, | ||
\ ], | ||
\ ale_linters#ruby#steep#HandleOutput(347, [ | ||
\ '# Type checking files:', | ||
\ '', | ||
\ '...............................................................................................................................F..........F.F...F.', | ||
\ '', | ||
\ 'lib/frobz/foobar_baz.rb:400:17: [error] Method parameters are incompatible with declaration `(untyped, untyped, *untyped, **untyped) { () -> untyped } -> untyped`', | ||
\ '│ Diagnostic ID: Ruby::MethodArityMismatch', | ||
\ '│', | ||
\ '└ def frobz(obj, suffix, *args, &block)', | ||
\ ' ~~~~~~~~~~~~~~~~~~~~~~~~~~~~', | ||
\ '', | ||
\ 'lib/frobz/foobar_baz.rb:20:8: [warning] Cannot find implementation of method `::Frobz::FooBarBaz#method_name`', | ||
\ '│ Diagnostic ID: Ruby::MethodDefinitionMissing', | ||
\ '│', | ||
\ '└ class FooBarBaz', | ||
\ ' ~~~~~~~~~', | ||
\ '', | ||
\ 'lib/frobz/foobar_baz.rb:30:8: [information] Cannot find implementation of method `::Frobz::FooBarBaz#method_name`', | ||
\ '│ Diagnostic ID: Ruby::MethodDefinitionMissing', | ||
\ '│', | ||
\ '└ class FooBarBaz', | ||
\ ' ~~~~~~~~~', | ||
\ '', | ||
\ 'lib/frobz/foobar_baz.rb:40:8: [hint] Cannot find implementation of method `::Frobz::FooBarBaz#method_name`', | ||
\ '│ Diagnostic ID: Ruby::MethodDefinitionMissing', | ||
\ '│', | ||
\ '└ class FooBarBaz', | ||
\ ' ~~~~~~~~~', | ||
\ '', | ||
\ 'Detected 4 problems from 1 file', | ||
\ ]) | ||
|
||
Execute(The steep handler should handle when files are checked and no offenses are found): | ||
AssertEqual | ||
\ [], | ||
\ ale_linters#ruby#steep#HandleOutput(347, [ | ||
\ '# Type checking files:', | ||
\ '', | ||
\ '.............................................................................................................................................', | ||
\ '', | ||
\ 'No type error detected. 🧉', | ||
\ ]) | ||
|
||
Execute(The steep handler should handle when no files are checked): | ||
AssertEqual | ||
\ [], | ||
\ ale_linters#ruby#steep#HandleOutput(347, [ | ||
\ '# Type checking files:', | ||
\ '', | ||
\ '', | ||
\ '', | ||
\ 'No type error detected. 🧉', | ||
\ ]) | ||
|
||
Execute(The steep handler should handle empty output): | ||
AssertEqual [], ale_linters#ruby#steep#HandleOutput(347, ['']) | ||
AssertEqual [], ale_linters#ruby#steep#HandleOutput(347, []) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
" Author: Loic Nageleisen <https://github.com/lloeki> | ||
" Description: Tests for steep linter. | ||
Before: | ||
call ale#assert#SetUpLinterTest('ruby', 'steep') | ||
|
||
let g:ale_ruby_steep_executable = 'steep' | ||
|
||
After: | ||
call ale#assert#TearDownLinterTest() | ||
|
||
Execute(Executable should default to steep): | ||
call ale#test#SetFilename('../test-files/ruby/nested/foo/dummy.rb') | ||
AssertLinter 'steep', ale#Escape('steep') | ||
\ . ' check ' | ||
\ . ' dummy.rb' | ||
|
||
Execute(Should be able to set a custom executable): | ||
let g:ale_ruby_steep_executable = 'bin/steep' | ||
|
||
call ale#test#SetFilename('../test-files/ruby/nested/foo/dummy.rb') | ||
AssertLinter 'bin/steep' , ale#Escape('bin/steep') | ||
\ . ' check ' | ||
\ . ' dummy.rb' | ||
|
||
Execute(Setting bundle appends 'exec steep'): | ||
let g:ale_ruby_steep_executable = 'path to/bundle' | ||
|
||
call ale#test#SetFilename('../test-files/ruby/nested/foo/dummy.rb') | ||
AssertLinter 'path to/bundle', ale#Escape('path to/bundle') | ||
\ . ' exec steep' | ||
\ . ' check ' | ||
\ . ' dummy.rb' | ||
|
||
Execute(should accept options): | ||
let g:ale_ruby_steep_options = '--severity-level=hint' | ||
|
||
call ale#test#SetFilename('../test-files/ruby/nested/foo/dummy.rb') | ||
AssertLinter 'steep', ale#Escape('steep') | ||
\ . ' check' | ||
\ . ' --severity-level=hint' | ||
\ . ' dummy.rb' | ||
|
||
Execute(Should not lint files out of steep root): | ||
call ale#test#SetFilename('../test-files/ruby/nested/dummy.rb') | ||
AssertLinter 'steep', '' | ||
|
||
Execute(Should lint files at top steep root): | ||
call ale#test#SetFilename('../test-files/ruby/nested/foo/dummy.rb') | ||
AssertLinter 'steep', ale#Escape('steep') | ||
\ . ' check ' | ||
\ . ' dummy.rb' | ||
|
||
Execute(Should lint files below top steep root): | ||
call ale#test#SetFilename('../test-files/ruby/nested/foo/one/dummy.rb') | ||
AssertLinter 'steep', ale#Escape('steep') | ||
\ . ' check ' | ||
\ . ' one' . (has('win32') ? '\' : '/') . 'dummy.rb' | ||
|
||
Execute(Should lint files at nested steep root): | ||
call ale#test#SetFilename('../test-files/ruby/nested/foo/two/dummy.rb') | ||
AssertLinter 'steep', ale#Escape('steep') | ||
\ . ' check ' | ||
\ . ' dummy.rb' | ||
|
||
Execute(Should lint files below nested steep root): | ||
call ale#test#SetFilename('../test-files/ruby/nested/foo/two/three/dummy.rb') | ||
AssertLinter 'steep', ale#Escape('steep') | ||
\ . ' check ' | ||
\ . ' three' . (has('win32') ? '\' : '/') . 'dummy.rb' |
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a complex handler function. Add a test for this too. There's a directory for testing functions for handling results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should finally be able to get back to complete this within a few weeks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"a few weeks" heh. On it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@w0rp I have rebased and added the handler test.