Skip to content
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

Need a cop to enforce disabling comment syntax #11069

Closed
knutsenm opened this issue Oct 12, 2022 · 1 comment · Fixed by #11073
Closed

Need a cop to enforce disabling comment syntax #11069

knutsenm opened this issue Oct 12, 2022 · 1 comment · Fixed by #11073
Labels

Comments

@knutsenm
Copy link

Is your feature request related to a problem? Please describe.

Lint/RedundantCopDisableDirective autocorrect can cause a syntax error if it removes a disabling comment that is followed by extra text:

% cat test4.rb 
# frozen_string_literal: true

puts 'Hello, world!' # rubocop:disable Cop/DNE - does not exist

% ruby test4.rb
Hello, world!

% rubocop --autocorrect test4.rb
Inspecting 1 file
E

Offenses:

test4.rb:3:22: W: [Corrected] Lint/RedundantCopDisableDirective: Unnecessary disabling of Cop/DNE (unknown cop).
puts 'Hello, world!' # rubocop:disable Cop/DNE - does not exist
                     ^^^^^^^^^^^^^^^^^^^^^^^^^
test4.rb:3:28: E: Lint/Syntax: unexpected token kNOT
(Using Ruby 3.0 parser; configure using TargetRubyVersion parameter, under AllCops)
puts 'Hello, world!'- does not exist
                           ^^^

1 file inspected, 2 offenses detected, 1 offense corrected

% cat test4.rb 
# frozen_string_literal: true

puts 'Hello, world!'- does not exist

% rubocop -V
1.36.0 (using Parser 3.1.2.1, rubocop-ast 1.21.0, running on ruby 3.0.4) [x86_64-darwin21]

Describe the solution you'd like

The docs do say

Do not write anything other than cop name in the disabling comment.

but since there is the potential to cause a syntax error on autocorrect that is supposed to be safe, perhaps there should be a cop to enforce that?

Alternatively, autocorrect could remove the entire comment line beginning with the directive; that would be safe.

Or, autocorrect could retain the # when comment text remains. There is something of a precedent for this: #9823 allows a comment to precede the directive.

Describe alternatives you've considered

See above.

Additional context

See above.

@koic koic added the bug label Oct 13, 2022
koic added a commit to koic/rubocop that referenced this issue Oct 13, 2022
…opDisableDirective`

Fix rubocop#11069.

This PR fixes an incorrect autocorrect for `Lint/RedundantCopDisableDirective`
when disable directive contains free comment.
bbatsov pushed a commit that referenced this issue Oct 13, 2022
…leDirective`

Fix #11069.

This PR fixes an incorrect autocorrect for `Lint/RedundantCopDisableDirective`
when disable directive contains free comment.
@knutsenm
Copy link
Author

Love it; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants