Ale: ALEFix deletes rubocop comments

[nyarly]

Information

VIM version
NVIM v0.2.2
Build type: Release

Operating System:
NAME=NixOS
ID=nixos
VERSION="18.03.131807.489a14add9a (Impala)"

:ALEInfo

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 = 0
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 = 1
let g:ale_linter_aliases = {}
let g:ale_linters = {'ruby': 'all'}
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 = 'on_save'
let g:ale_pattern_options = {}
let g:ale_pattern_options_enabled = 0
let g:ale_set_balloons = 0
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:
(executable check - failure) brakeman
(executable check - failure) rails_best_practices
(executable check - failure) reek
(finished - exit code 1) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/home/judson/dev/mezzo/app/services/adp_service/base.rb'' < ''/run/user/1000/nvim49pDqE/1288/base.rb''']
<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.57.2","ruby_engine":"ruby","ruby_version":"2.3.6","ruby_patchlevel":"384","ruby_platform":"x86_64-linux"},"files":[{"path":"app/services/adp_service/base.rb","offenses":[
{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":14,"start_column":7,"
last_line":14,"last_column":10,"length":4,"line":14,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output"
,"corrected":false,"location":{"start_line":21,"start_column":7,"last_line":21,"last_column":10,"length":4,"line":21,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout.
Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":23,"start_column":7,"last_line":23,"last_column":10,"length":4,"line":23,"column":7}},{"severit
y":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":25,"start_column":7,"last_line
":25,"last_column":10,"length":4,"line":25,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","correct
ed":false,"location":{"start_line":32,"start_column":7,"last_line":32,"last_column":10,"length":4,"line":32,"column":7}},{"severity":"warning","message":"Lint/UnneededCopEnableDirective: Unnecessary enabl
ing of Rails/Output.","cop_name":"Lint/UnneededCopEnableDirective","corrected":false,"location":{"start_line":35,"start_column":22,"last_line":35,"last_column":33,"length":12,"line":35,"column":22}}]}],"s
ummary":{"offense_count":6,"target_file_count":1,"inspected_file_count":1}}
<<<OUTPUT ENDS>>>
(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/run/user/1000/nvim49pDqE/1289/base.rb''']
<<<OUTPUT STARTS>>>
Ignoring msgpack-1.2.2 because its extensions are not built. Try: gem pristine msgpack --version 1.2.2
<<<OUTPUT ENDS>>>
(finished - exit code 0) ['/bin/sh', '-c', '''rubocop'' --config ''/home/judson/dev/mezzo/.rubocop.yml'' --auto-correct ''/run/user/1000/nvim49pDqE/1292/base.rb''']
(executable check - failure) brakeman
(executable check - failure) rails_best_practices
(executable check - failure) reek
(finished - exit code 1) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/home/judson/dev/mezzo/app/services/adp_service/base.rb'' < ''/run/user/1000/nvim49pDqE/1293/base.rb''']
<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.57.2","ruby_engine":"ruby","ruby_version":"2.3.6","ruby_patchlevel":"384","ruby_platform":"x86_64-linux"},"files":[{"path":"app/services/adp_service/base.rb","offenses":[
{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":14,"start_column":7,"
last_line":14,"last_column":10,"length":4,"line":14,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output"
,"corrected":false,"location":{"start_line":21,"start_column":7,"last_line":21,"last_column":10,"length":4,"line":21,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout.
Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":23,"start_column":7,"last_line":23,"last_column":10,"length":4,"line":23,"column":7}},{"severit
y":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":25,"start_column":7,"last_line
":25,"last_column":10,"length":4,"line":25,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","correct
ed":false,"location":{"start_line":32,"start_column":7,"last_line":32,"last_column":10,"length":4,"line":32,"column":7}},{"severity":"warning","message":"Lint/UnneededCopEnableDirective: Unnecessary enabl
ing of Rails/Output.","cop_name":"Lint/UnneededCopEnableDirective","corrected":false,"location":{"start_line":35,"start_column":22,"last_line":35,"last_column":33,"length":12,"line":35,"column":22}}]}],"s
ummary":{"offense_count":6,"target_file_count":1,"inspected_file_count":1}}
<<<OUTPUT ENDS>>>
(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/run/user/1000/nvim49pDqE/1294/base.rb''']
<<<OUTPUT STARTS>>>
Ignoring msgpack-1.2.2 because its extensions are not built. Try: gem pristine msgpack --version 1.2.2
<<<OUTPUT ENDS>>>
(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 ''/home/judson/dev/mezzo/app/services/adp_service/base.rb'' < ''/run/user/1000/nvim49pDqE/1297/base.rb''']
<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.57.2","ruby_engine":"ruby","ruby_version":"2.3.6","ruby_patchlevel":"384","ruby_platform":"x86_64-linux"},"files":[{"path":"app/services/adp_service/base.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 ''/run/user/1000/nvim49pDqE/1298/base.rb''']
<<<OUTPUT STARTS>>>
Ignoring msgpack-1.2.2 because its extensions are not built. Try: gem pristine msgpack --version 1.2.2
<<<OUTPUT ENDS>>>
(finished - exit code 0) ['/bin/sh', '-c', '''rubocop'' --config ''/home/judson/dev/mezzo/.rubocop.yml'' --auto-correct ''/run/user/1000/nvim49pDqE/1301/base.rb''']
(executable check - failure) reek
(finished - exit code 1) ['/bin/sh', '-c', '''rubocop'' --format json --force-exclusion  --stdin ''/home/judson/dev/mezzo/app/services/adp_service/base.rb'' < ''/run/user/1000/nvim49pDqE/1302/base.rb''']
<<<OUTPUT STARTS>>>
{"metadata":{"rubocop_version":"0.57.2","ruby_engine":"ruby","ruby_version":"2.3.6","ruby_patchlevel":"384","ruby_platform":"x86_64-linux"},"files":[{"path":"app/services/adp_service/base.rb","offenses":[
{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":14,"start_column":7,"
last_line":14,"last_column":10,"length":4,"line":14,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output"
,"corrected":false,"location":{"start_line":21,"start_column":7,"last_line":21,"last_column":10,"length":4,"line":21,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout.
Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":23,"start_column":7,"last_line":23,"last_column":10,"length":4,"line":23,"column":7}},{"severit
y":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","corrected":false,"location":{"start_line":25,"start_column":7,"last_line
":25,"last_column":10,"length":4,"line":25,"column":7}},{"severity":"convention","message":"Rails/Output: Do not write to stdout. Use Rails's logger if you want to log.","cop_name":"Rails/Output","correct
ed":false,"location":{"start_line":32,"start_column":7,"last_line":32,"last_column":10,"length":4,"line":32,"column":7}},{"severity":"warning","message":"Lint/UnneededCopEnableDirective: Unnecessary enabl
ing of Rails/Output.","cop_name":"Lint/UnneededCopEnableDirective","corrected":false,"location":{"start_line":35,"start_column":22,"last_line":35,"last_column":33,"length":12,"line":35,"column":22}}]}],"s
ummary":{"offense_count":6,"target_file_count":1,"inspected_file_count":1}}
<<<OUTPUT ENDS>>>
(finished - exit code 0) ['/bin/sh', '-c', '''ruby'' -w -c -T1 ''/run/user/1000/nvim49pDqE/1303/base.rb''']
<<<OUTPUT STARTS>>>
Ignoring msgpack-1.2.2 because its extensions are not built. Try: gem pristine msgpack --version 1.2.2
<<<OUTPUT ENDS>>>

What went wrong

I have a file with

# rubocop:disable Rails/Output

If I run :ALEFix or enable ale_fix_on_save, that line is deleted. I've confirmed that it is not deleted by running rubocop --auto-correct by itself.

Reproducing the bug

1. Create a ruby file with the above Rubocop control comment and it's corresponding rubocop:enable
2. Run :ALEFix
3. Observe that the comment is deleted.

[w0rp]

I don't know too much about Ruby programs, but check if the executable is different, try running the exact same command in your terminal, apart from using a different temporary filename, and try updating ALE. I looked on Google quickly, and rubocop can delete comments for disabling rules if it thinks they aren't needed.

[promisedlandt]

I have added details on why this happens in the linked issue. I don't think this is fixable in ALE unfortunately.

[w0rp]

It looks like this is something that will have to be fixed in rubocop. I suggested adding options similar to the options ESLint uses.

[Mange]

From looking at https://github.com/rubocop-hq/rubocop/issues/2502, I think there might be other workarounds one can use here in ALE.

1. Stop using a temporary file and fix the original file.
2. Read the stdout output and filter away the diagnostics. Done in other formatters:
   
   
   
   • https://github.com/sbdchd/neoformat/pull/49
   
   
   • https://github.com/kimihito/dotfiles/commit/222a14370662334381bfc691701f08182405b3ab
   
   

I tested approach 2 in a terminal and the config problems caused by the file having the wrong filename disappears.

[LandonSchropp]

I'm also having this issue. Is there any update on this?

[LandonSchropp]

@promisedlandt @w0rp Would you guys mind adding a little detail about why this can't be fixed in ALE? I'm not observing this behavior when running Rubocop directly.

[Mange]

I think it's because ALE doesn't save the file, run the command on the file, and then reloads the file in vim.

Instead it writes the buffer to a temporary file and runs on that, then replaces the buffer contents with the results.

This is because it should be possible to lint/fix without saving first.

[hsanson]

This PR #3230 changes rubocop fixer to use stdin instead of temporary files. In my preliminary tests this seems to fix the problem.