From bc32e24203159945dcf3906a5e08261ccb06e065 Mon Sep 17 00:00:00 2001 From: Eddie Lebow Date: Wed, 12 Jul 2017 05:43:47 -0400 Subject: [PATCH] Add rails_best_practices handler (resolves #655) (#751) * Move FindRailsRoot() to more general location * Add rails_best_practices handler (resolves #655) * Update documentation for rails_best_practices Also add brakeman to *ale* documentation. * rails_best_practices: allow overriding the executable * rails_best_practices: format help correctly * rails_best_practices: capture tool output on Windows --- README.md | 2 +- ale_linters/ruby/brakeman.vim | 24 +------- ale_linters/ruby/rails_best_practices.vim | 59 +++++++++++++++++++ .../ale/handlers/rails_best_practices.vim | 6 ++ autoload/ale/ruby.vim | 22 +++++++ doc/ale-ruby.txt | 21 +++++++ doc/ale.txt | 2 + ...ails_best_practices_command_callback.vader | 42 +++++++++++++ .../test_rails_best_practices_handler.vader | 50 ++++++++++++++++ 9 files changed, 205 insertions(+), 23 deletions(-) create mode 100644 ale_linters/ruby/rails_best_practices.vim create mode 100644 autoload/ale/handlers/rails_best_practices.vim create mode 100644 autoload/ale/ruby.vim create mode 100644 test/command_callback/test_rails_best_practices_command_callback.vader create mode 100644 test/handler/test_rails_best_practices_handler.vader diff --git a/README.md b/README.md index 4f2bbf5..ccabc3a 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ name. That seems to be the fairest way to arrange this table. | ReasonML | [merlin](https://github.com/the-lambda-church/merlin) see `:help ale-integration-reason-merlin` for configuration instructions | reStructuredText | [proselint](http://proselint.com/)| | RPM spec | [rpmlint](https://github.com/rpm-software-management/rpmlint) (disabled by default; see `:help ale-integration-spec`) | -| Ruby | [brakeman](http://brakemanscanner.org/), [reek](https://github.com/troessner/reek), [rubocop](https://github.com/bbatsov/rubocop), [ruby](https://www.ruby-lang.org) | +| Ruby | [brakeman](http://brakemanscanner.org/), [rails_best_practices](https://github.com/flyerhzm/rails_best_practices), [reek](https://github.com/troessner/reek), [rubocop](https://github.com/bbatsov/rubocop), [ruby](https://www.ruby-lang.org) | | Rust | [rustc](https://www.rust-lang.org/), cargo (see `:help ale-integration-rust` for configuration instructions) | | SASS | [sass-lint](https://www.npmjs.com/package/sass-lint), [stylelint](https://github.com/stylelint/stylelint) | | SCSS | [sass-lint](https://www.npmjs.com/package/sass-lint), [scss-lint](https://github.com/brigade/scss-lint), [stylelint](https://github.com/stylelint/stylelint) | diff --git a/ale_linters/ruby/brakeman.vim b/ale_linters/ruby/brakeman.vim index 5ea531f..269c18f 100644 --- a/ale_linters/ruby/brakeman.vim +++ b/ale_linters/ruby/brakeman.vim @@ -15,7 +15,7 @@ function! ale_linters#ruby#brakeman#Handle(buffer, lines) abort for l:warning in l:result.warnings " Brakeman always outputs paths relative to the Rails app root - let l:rails_root = s:FindRailsRoot(a:buffer) + let l:rails_root = ale#ruby#FindRailsRoot(a:buffer) let l:warning_file = l:rails_root . '/' . l:warning.file if !ale#path#IsBufferPath(a:buffer, l:warning_file) @@ -36,7 +36,7 @@ function! ale_linters#ruby#brakeman#Handle(buffer, lines) abort endfunction function! ale_linters#ruby#brakeman#GetCommand(buffer) abort - let l:rails_root = s:FindRailsRoot(a:buffer) + let l:rails_root = ale#ruby#FindRailsRoot(a:buffer) if l:rails_root ==? '' return '' @@ -47,26 +47,6 @@ function! ale_linters#ruby#brakeman#GetCommand(buffer) abort \ . ' -p ' . ale#Escape(l:rails_root) endfunction -function! s:FindRailsRoot(buffer) abort - " Find the nearest dir contining "app", "db", and "config", and assume it is - " the root of a Rails app. - for l:name in ['app', 'config', 'db'] - let l:dir = fnamemodify( - \ ale#path#FindNearestDirectory(a:buffer, l:name), - \ ':h:h' - \) - - if l:dir !=# '.' - \&& isdirectory(l:dir . '/app') - \&& isdirectory(l:dir . '/config') - \&& isdirectory(l:dir . '/db') - return l:dir - endif - endfor - - return '' -endfunction - call ale#linter#Define('ruby', { \ 'name': 'brakeman', \ 'executable': 'brakeman', diff --git a/ale_linters/ruby/rails_best_practices.vim b/ale_linters/ruby/rails_best_practices.vim new file mode 100644 index 0000000..a3d9f7a --- /dev/null +++ b/ale_linters/ruby/rails_best_practices.vim @@ -0,0 +1,59 @@ +" Author: Eddie Lebow https://github.com/elebow +" Description: rails_best_practices, a code metric tool for rails projects + +let g:ale_ruby_rails_best_practices_options = +\ get(g:, 'ale_ruby_rails_best_practices_options', '') + +function! ale_linters#ruby#rails_best_practices#Handle(buffer, lines) abort + if len(a:lines) == 0 + return [] + endif + + let l:result = json_decode(join(a:lines, '')) + + let l:output = [] + + for l:warning in l:result + if !ale#path#IsBufferPath(a:buffer, l:warning.filename) + continue + endif + + call add(l:output, { + \ 'lnum': l:warning.line_number + 0, + \ 'type': 'W', + \ 'text': l:warning.message, + \}) + endfor + + return l:output +endfunction + +function! ale_linters#ruby#rails_best_practices#GetCommand(buffer) abort + let l:executable = ale#handlers#rails_best_practices#GetExecutable(a:buffer) + let l:exec_args = l:executable =~? 'bundle$' + \ ? ' exec rails_best_practices' + \ : '' + + let l:rails_root = ale#ruby#FindRailsRoot(a:buffer) + + if l:rails_root ==? '' + return '' + endif + + let l:output_file = ale#Has('win32') ? '%t ' : '/dev/stdout ' + let l:cat_file = ale#Has('win32') ? '; type %t' : '' + + return ale#Escape(l:executable) . l:exec_args + \ . ' --silent -f json --output-file ' . l:output_file + \ . ale#Var(a:buffer, 'ruby_rails_best_practices_options') + \ . ale#Escape(l:rails_root) + \ . l:cat_file +endfunction + +call ale#linter#Define('ruby', { +\ 'name': 'rails_best_practices', +\ 'executable_callback': 'ale#handlers#rails_best_practices#GetExecutable', +\ 'command_callback': 'ale_linters#ruby#rails_best_practices#GetCommand', +\ 'callback': 'ale_linters#ruby#rails_best_practices#Handle', +\ 'lint_file': 1, +\}) diff --git a/autoload/ale/handlers/rails_best_practices.vim b/autoload/ale/handlers/rails_best_practices.vim new file mode 100644 index 0000000..51bafbb --- /dev/null +++ b/autoload/ale/handlers/rails_best_practices.vim @@ -0,0 +1,6 @@ +call ale#Set('ruby_rails_best_practices_options', '') +call ale#Set('ruby_rails_best_practices_executable', 'rails_best_practices') + +function! ale#handlers#rails_best_practices#GetExecutable(buffer) abort + return ale#Var(a:buffer, 'ruby_rails_best_practices_executable') +endfunction diff --git a/autoload/ale/ruby.vim b/autoload/ale/ruby.vim new file mode 100644 index 0000000..503ff1d --- /dev/null +++ b/autoload/ale/ruby.vim @@ -0,0 +1,22 @@ +" Author: Eddie Lebow https://github.com/elebow +" Description: Functions for integrating with Ruby tools + +" Find the nearest dir contining "app", "db", and "config", and assume it is +" the root of a Rails app. +function! ale#ruby#FindRailsRoot(buffer) abort + for l:name in ['app', 'config', 'db'] + let l:dir = fnamemodify( + \ ale#path#FindNearestDirectory(a:buffer, l:name), + \ ':h:h' + \) + + if l:dir !=# '.' + \&& isdirectory(l:dir . '/app') + \&& isdirectory(l:dir . '/config') + \&& isdirectory(l:dir . '/db') + return l:dir + endif + endfor + + return '' +endfunction diff --git a/doc/ale-ruby.txt b/doc/ale-ruby.txt index 54ebbf5..c710a26 100644 --- a/doc/ale-ruby.txt +++ b/doc/ale-ruby.txt @@ -13,6 +13,27 @@ g:ale_ruby_brakeman_options *g:ale_ruby_brakeman_options* The contents of this variable will be passed through to brakeman. +=============================================================================== +rails_best_practices *ale-ruby-rails_best_practices* + +g:ale_ruby_rails_best_practices_executable + *g:ale_ruby_rails_best_practices_executable* + *b:ale_ruby_rails_best_practices_executable* + Type: String + Default: 'rails_best_practices' + + Override the invoked rails_best_practices binary. Set this to `'bundle'` to + invoke `'bundle` `exec` `rails_best_practices'`. + +g:ale_ruby_rails_best_practices_options + *g:ale_ruby_rails_best_practices_options* + *b:ale_ruby_rails_best_practices_options* + Type: |String| + Default: `''` + + The contents of this variable will be passed through to rails_best_practices. + + =============================================================================== reek *ale-ruby-reek* diff --git a/doc/ale.txt b/doc/ale.txt index d8de056..eafbbcc 100644 --- a/doc/ale.txt +++ b/doc/ale.txt @@ -84,6 +84,8 @@ CONTENTS *ale-contents* pylint..............................|ale-python-pylint| yapf................................|ale-python-yapf| ruby..................................|ale-ruby-options| + brakeman............................|ale-ruby-brakeman| + rails_best_practices................|ale-ruby-rails_best_practices| reek................................|ale-ruby-reek| rubocop.............................|ale-ruby-rubocop| rust..................................|ale-rust-options| diff --git a/test/command_callback/test_rails_best_practices_command_callback.vader b/test/command_callback/test_rails_best_practices_command_callback.vader new file mode 100644 index 0000000..09436be --- /dev/null +++ b/test/command_callback/test_rails_best_practices_command_callback.vader @@ -0,0 +1,42 @@ +Before: + Save g:ale_ruby_rails_best_practices_executable + + let g:ale_ruby_rails_best_practices_executable = 'rails_best_practices' + + runtime ale_linters/ruby/rails_best_practices.vim + call ale#test#SetDirectory('/testplugin/test/command_callback') + call ale#test#SetFilename('../ruby_fixtures/valid_rails_app/db/test.rb') + +After: + Restore + + call ale#test#RestoreDirectory() + +Execute(Executable should default to rails_best_practices): + AssertEqual + \ '''rails_best_practices'' --silent -f json --output-file /dev/stdout ' + \ . ale#Escape(simplify(g:dir . '/../ruby_fixtures/valid_rails_app')), + \ ale_linters#ruby#rails_best_practices#GetCommand(bufnr('')) + +Execute(Should be able to set a custom executable): + let g:ale_ruby_rails_best_practices_executable = 'bin/rails_best_practices' + + AssertEqual + \ '''bin/rails_best_practices'' --silent -f json --output-file /dev/stdout ' + \ . ale#Escape(simplify(g:dir . '/../ruby_fixtures/valid_rails_app')), + \ ale_linters#ruby#rails_best_practices#GetCommand(bufnr('')) + +Execute(Setting bundle appends 'exec rails_best_practices'): + let g:ale_ruby_rails_best_practices_executable = 'path to/bundle' + + AssertEqual + \ '''path to/bundle'' exec rails_best_practices --silent -f json --output-file /dev/stdout ' + \ . ale#Escape(simplify(g:dir . '/../ruby_fixtures/valid_rails_app')), + \ ale_linters#ruby#rails_best_practices#GetCommand(bufnr('')) + +Execute(Command callback should be empty when not in a valid Rails app): + call ale#test#SetFilename('../ruby_fixtures/not_a_rails_app/test.rb') + + AssertEqual + \ '', + \ ale_linters#ruby#rails_best_practices#GetCommand(bufnr('')) diff --git a/test/handler/test_rails_best_practices_handler.vader b/test/handler/test_rails_best_practices_handler.vader new file mode 100644 index 0000000..037d252 --- /dev/null +++ b/test/handler/test_rails_best_practices_handler.vader @@ -0,0 +1,50 @@ +Before: + " Switch to the test rails directory. + let b:path = getcwd() + silent! cd /testplugin/test/handler + cd ../ruby_fixtures/valid_rails_app/app/models + + runtime ale_linters/ruby/rails_best_practices.vim + +After: + " Switch back to whatever directory it was that we started on. + silent! 'cd ' . fnameescape(b:path) + unlet! b:path + + call ale#linter#Reset() + +Execute(The rails_best_practices handler should parse JSON correctly): + silent file! thing.rb + + AssertEqual + \ [ + \ { + \ 'lnum': 5, + \ 'text': 'use local variable', + \ 'type': 'W', + \ }, + \ { + \ 'lnum': 10, + \ 'text': 'other advice', + \ 'type': 'W', + \ } + \ ], + \ ale_linters#ruby#rails_best_practices#Handle(bufnr(''), [ + \ '[', + \ '{', + \ '"message": "use local variable",', + \ '"line_number": "5",', + \ '"filename": "/testplugin/test/ruby_fixtures/valid_rails_app/app/models/thing.rb"', + \ '},{', + \ '"message": "other advice",', + \ '"line_number": "10",', + \ '"filename": "/testplugin/test/ruby_fixtures/valid_rails_app/app/models/thing.rb"', + \ '}', + \ ']' + \ ]) + +Execute(The rails_best_practices handler should parse JSON correctly when there is no output from the tool): + AssertEqual + \ [], + \ ale_linters#ruby#rails_best_practices#Handle(347, [ + \ ])