Ale: Rubocop fixer ignores filename for Exclude statements

[CGamesPlay]

Rubocop isn't being provided with the correct filename when running with --auto-correct, and so it makes changes in violation of the Exclude and Include directives in the config file.

Information

VIM version

VIM - Vi IMproved 8.0 (2016 Sep 12, compiled Mar 22 2018 12:40:49)
macOS version

Operating System: macOS 10.12.6 (16G1314)

:ALEInfo

 Current Filetype: ruby
Available Linters: ['brakeman', 'rails_best_practices', 'reek', 'rubocop', 'ruby']
  Enabled Linters: ['brakeman', 'rails_best_practices', 'reek', 'rubocop', 'ruby']
 Linter Variables:

let g:ale_ruby_brakeman_options = ''
let g:ale_ruby_reek_show_context = 0
let g:ale_ruby_reek_show_wiki_link = 0
let g:ale_ruby_rubocop_executable = 'rubocop'
let g:ale_ruby_rubocop_options = ''
let g:ale_ruby_ruby_executable = 'ruby'
 Global Variables:

let g:ale_cache_executable_check_failures = 0
let g:ale_change_sign_column_color = 0
let g:ale_command_wrapper = ''
let g:ale_completion_delay = 100
let g:ale_completion_enabled = 0
let g:ale_completion_max_suggestions = 50
let g:ale_echo_cursor = 1
let g:ale_echo_msg_error_str = 'Error'
let g:ale_echo_msg_format = '%code: %%s'
let g:ale_echo_msg_info_str = 'Info'
let g:ale_echo_msg_warning_str = 'Warning'
let g:ale_enabled = 1
let g:ale_fix_on_save = 1
let g:ale_fixers = {'ruby': ['rubocop']}
let g:ale_history_enabled = 1
let g:ale_history_log_output = 1
let g:ale_keep_list_window_open = 0
let g:ale_lint_delay = 200
let g:ale_lint_on_enter = 1
let g:ale_lint_on_filetype_changed = 1
let g:ale_lint_on_save = 1
let g:ale_lint_on_text_changed = 'never'
let g:ale_lint_on_insert_leave = 0
let g:ale_linter_aliases = {}
let g:ale_linters = {}
let g:ale_linters_explicit = 0
let g:ale_list_window_size = 10
let g:ale_list_vertical = 0
let g:ale_loclist_msg_format = '%code: %%s'
let g:ale_max_buffer_history_size = 20
let g:ale_max_signs = -1
let g:ale_maximum_file_size = 0
let g:ale_open_list = 1
let g:ale_pattern_options = {}
let g:ale_pattern_options_enabled = 0
let g:ale_set_balloons = 1
let g:ale_set_highlights = 1
let g:ale_set_loclist = 1
let g:ale_set_quickfix = 0
let g:ale_set_signs = 1
let g:ale_sign_column_always = 1
let g:ale_sign_error = '>>'
let g:ale_sign_info = '--'
let g:ale_sign_offset = 1000000
let g:ale_sign_style_error = '>>'
let g:ale_sign_style_warning = '--'
let g:ale_sign_warning = '--'
let g:ale_statusline_format = ['%d error(s)', '%d warning(s)', 'OK']
let g:ale_type_map = {}
let g:ale_warn_about_trailing_blank_lines = 1
let g:ale_warn_about_trailing_whitespace = 1
  Command History:

(finished - exit code 2) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/Users/rpatterson/Projects/SimplyInsured/lib/plan_batch_importer.rb'' < ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1992/plan_batch_importer.rb''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1993/plan_batch_importer.rb''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 1) ['/bin/sh', '-c', '''rubocop'' --config ''/Users/rpatterson/Projects/SimplyInsured/.rubocop.yml'' --auto-correct ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1994/plan_batch_importer.rb''']
(executable check - failure) brakeman
(executable check - failure) rails_best_practices
(executable check - failure) reek
(finished - exit code 0) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/Users/rpatterson/Projects/SimplyInsured/lib/plan_batch_importer.rb'' < ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1995/plan_batch_importer.rb''']

<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.55.0","ruby_engine":"ruby","ruby_version":"2.1.5","ruby_patchlevel":"273","ruby_platform":"x86_64-darwin14.0"},"files":[{"path":"lib/plan_batch_importer.rb","offenses":[]}],"summary":{"offense_count":0,"target_file_count":1,"inspected_file_count":1}}
<<<OUTPUT ENDS>>>

(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1996/plan_batch_importer.rb''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 1) ['/bin/sh', '-c', '''rubocop'' --config ''/Users/rpatterson/Projects/SimplyInsured/.rubocop.yml'' --auto-correct ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1997/plan_batch_importer.rb''']
(executable check - failure) brakeman
(executable check - failure) rails_best_practices
(executable check - failure) reek
(finished - exit code 0) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/Users/rpatterson/Projects/SimplyInsured/lib/plan_batch_importer.rb'' < ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1998/plan_batch_importer.rb''']

<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.55.0","ruby_engine":"ruby","ruby_version":"2.1.5","ruby_patchlevel":"273","ruby_platform":"x86_64-darwin14.0"},"files":[{"path":"lib/plan_batch_importer.rb","offenses":[]}],"summary":{"offense_count":0,"target_file_count":1,"inspected_file_count":1}}
<<<OUTPUT ENDS>>>

(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/1999/plan_batch_importer.rb''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 1) ['/bin/sh', '-c', '''rubocop'' --config ''/Users/rpatterson/Projects/SimplyInsured/.rubocop.yml'' --auto-correct ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/2000/plan_batch_importer.rb''']
(executable check - failure) brakeman
(executable check - failure) rails_best_practices
(executable check - failure) reek
(finished - exit code 0) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/Users/rpatterson/Projects/SimplyInsured/lib/plan_batch_importer.rb'' < ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/2001/plan_batch_importer.rb''']

<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.55.0","ruby_engine":"ruby","ruby_version":"2.1.5","ruby_patchlevel":"273","ruby_platform":"x86_64-darwin14.0"},"files":[{"path":"lib/plan_batch_importer.rb","offenses":[]}],"summary":{"offense_count":0,"target_file_count":1,"inspected_file_count":1}}
<<<OUTPUT ENDS>>>

(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/var/folders/2r/0y4kz7r53y1bflwy7l87z4n40000gn/T/vbur5tQ/2002/plan_batch_importer.rb''']

<<<NO OUTPUT RETURNED>>>

What went wrong

Rubocop was called as rubocop --config [project]/.rubocop.yml --auto-correct [tmp]/plan_batch_importer.rb, which circumvented all of the Include and Exclude definitions.

Reproducing the bug

Steps for repeating the bug:

echo "''" > test.rb
cat <<EOF > .rubocop.yml
Style/StringLiterals:
  EnforcedStyle: double_quotes
  Exclude:
    - test.rb
EOF

Open test.rb in vim and run the ALE fixers. It should leave the file as is, but instead changed the contents of the file to "".

Fixing the bug

After playing with rubocop a bit, you could get away with invoking rubocop as rubocop --config [project]/.rubocop.yml --auto-correct --stdin [project]/test.rb < [tmp]/test.rb and rubocop would work correctly, however it would print output in the format of:

Inspecting 1 file
.

1 file inspected, no offenses detected
====================
''

So you'd have to slice out the part above the line of ====.

[w0rp]

Someone can give fixing this a go some time. Just try to keep linting on the fly working with it, think about workarounds, etc.

[Seybo]

@CGamesPlay did you manage to fix this bug in the plugin itself? If so can you share the solution here please?

I've tried to apply it to autoload/ale/fixers/rubocop.vim but can't quite figure out how to do that.

[CGamesPlay]

Unfortunately I haven't looked into the actual plugin. I just was able to figure out the way to invoke rubocop in a way that doesn't cause the problem due to my work with another rubocop integration.

[jpowell]

I was going to create an issue for this, and after digging found this one. The issue appears to be that the --force-exclusions option needs to be passed to the fixer, just as it was when introduced to the linter in https://github.com/w0rp/ale/commit/a82ead0dc151e63c7bbf8d04712a42b2d5b85f12

The workaround appears to be adding the following to your .vimrc:

let g:ale_ruby_rubocop_options = '--force-exclusions'

The real question is, should this be the default behavior? I think so since it's the default in the linter.

/cc @w0rp @CGamesPlay

[w0rp]

That option is now always given to the command, so remove it from your settings.

[jpowell]

@w0rp I think there's a misunderstanding. It's always passed in the linter, but not the fixer.

https://github.com/w0rp/ale/blob/master/autoload/ale/fixers/rubocop.vim

Thoughts?

[w0rp]

I'll reopen this so someone can do the same thing for the fixer then.

[w0rp]

Still not done yet, may never be fixed.

[spiderpug]

I still have the problem mentioned above:

Assume you have the following project structure:

# ls -l
Gemfile
.rubocop.yml

Rubocop rules:

# .rubocop.yml
AllCops:
  TargetRubyVersion: 2.3
  Exclude:
    - 'db/schema.rb'
    - '**/Gemfile'

Saving Gemfile will invoke Rubocop using:

['/bin/zsh', '-c', '''rubocop'' --config ''/Users/xxx/project/.rubocop.yml'' --auto-correct --force-exclusion  < ''/var/folders/8c/p_0dk3qd50qczv5zryxb5tb80000gn/T/nvimDYA4Jm/3/Gemfile''']

and the Gemfile will still be updated by Rubocop, even if excluded by config.

According to the rubocop manual the exlusion list is checked relative to the config file, which is broken here because of temp file usage. Only option left is to make use of the stdin option and parse rubocop's output (instead of relying on temp file modification)

monkeypatch

I was able to monkeypatch the rubocop plugin by invoking it like this:

diff --git a/autoload/ale/fixers/rubocop.vim b/autoload/ale/fixers/rubocop.vim
index 33ba688..0b59150 100644
--- a/autoload/ale/fixers/rubocop.vim
+++ b/autoload/ale/fixers/rubocop.vim
@@ -9,12 +9,17 @@ function! ale#fixers#rubocop#GetCommand(buffer) abort
     return ale#handlers#ruby#EscapeExecutable(l:executable, 'rubocop')
     \   . (!empty(l:config) ? ' --config ' . ale#Escape(l:config) : '')
     \   . (!empty(l:options) ? ' ' . l:options : '')
-    \   . ' --auto-correct --force-exclusion %t'
+    \   . ' --auto-correct --force-exclusion --stdin %s --format quiet -o /dev/null'
 endfunction

 function! ale#fixers#rubocop#Fix(buffer) abort
     return {
     \   'command': ale#fixers#rubocop#GetCommand(a:buffer),
-    \   'read_temporary_file': 1,
+    \   'process_with': 'ale#fixers#rubocop#ProcessRubocopOutput',
     \}
 endfunction
+
+function! ale#fixers#rubocop#ProcessRubocopOutput(buffer, output) abort
+    " skip the first line as it contains '=========='
+    return a:output[1:]
+endfunction

[jpowell]

@spiderpug a novel idea! Are there any side effects with other fixers / linters by processing this outside the context of a tempfile? How's the performance?

[spiderpug]

I'm not sure! This change affects the fixer only and I use rubocop exclusively. The tempfile is still used to provide input to rubocop, but the result is taken straight from stdout. I have no knowledge of the internals of Ale: if the tempfile is used to chain fixers / linters, it should be broken with this approach. If Ale receives the output and prepares a new tempfile for the next fixer, it would still work.

Also, I think it's not platform compatible as it's using /dev/null to suppress output we don't need. Another possibility would be to process the output and discard all lines up to (and including) the separator. I'm not familiar with vimscript so I couldn't figure this out.

Honestly I couldn't feel a difference in performance. Is there a way to provide some actual numbers?

[jpowell]

/cc @w0rp

Do you have quick answers to questions above about chaining results from fixers / linters? Are there other examples that use stdin successfully as opposed to the tempfile approach that you know of?

[w0rp]

It depends on the tool. Some work better with one or the other.

If someone wants to create a pull request with essentially the patch above and some tests, I'll have a look at it when I can. We might need a --version check if only recent versions of rubocop support the options above.