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

Inline comment on lines violating Style/IfUnlessModifier gets removed by auto-correct #9989

Closed
iv-nn opened this issue Aug 9, 2021 · 1 comment · Fixed by #9991
Closed
Labels

Comments

@iv-nn
Copy link

iv-nn commented Aug 9, 2021

Auto-correct for Style/IfUnlessModifier move any inline comment to the line of the end keyword, it's then removed by the auto-correct for Style/CommentedKeyword


Expected behavior

I think the best way to handle this is to move any inline comment to before the if/unless.

Actual behavior

puts "a line way way way way way way way way way way way way way way way way way way way way too long" if true # some comment

gets auto-corrected to:

puts "a line way way way way way way way way way way way way way way way way way way way way too long" if true

with the following output from rubocop --auto-correct:

Offenses:

test.rb:1:1: C: [Corrected] Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
if true
^^
test.rb:1:104: C: [Corrected] Style/IfUnlessModifier: Modifier form of if makes the line too long.
puts "a line way way way way way way way way way way way way way way way way way way way way too long" if true # some comment
                                                                                                       ^^
test.rb:1:107: W: Lint/LiteralAsCondition: Literal true appeared as a condition.
puts "a line way way way way way way way way way way way way way way way way way way way way too long" if true
                                                                                                          ^^^^
test.rb:3:5: C: [Corrected] Style/CommentedKeyword: Do not place comments on the same line as the end keyword.
end # some comment
    ^^^^^^^^^^^^^^

1 file inspected, 4 offenses detected, 3 offenses corrected

This is a special case because the corrected line becomes short enough to use if modifier, so it gets corrected back to an inline if.

Steps to reproduce the problem

Run rubocop --auto-correct on:

puts "a line way way way way way way way way way way way way way way way way way way way way too long" if true # some comment

RuboCop version

$ rubocop -V
1.18.3 (using Parser 3.0.1.1, rubocop-ast 1.7.0, running on ruby 2.7.4 x86_64-linux)
@koic koic added the bug label Aug 9, 2021
koic added a commit to koic/rubocop that referenced this issue Aug 9, 2021
Fixes rubocop#9989

This PR mark unsafe auto-correct for `Style/CommentedKeyword`
because it may remove meaningful comments.
@koic
Copy link
Member

koic commented Aug 9, 2021

The code auto-corrected by Style/IfUnlessModifier is below.

% bundle exec rubocop -a --only Style/IfUnlessModifier
(snip)

Offenses:

example.rb:1:104: C: [Corrected] Style/IfUnlessModifier: Modifier form of if makes the line too long.
puts "a line way way way way way way way way way way way way way way way way way way way way too long" if true # some comment
                                                                                                       ^^

1 file inspected, 1 offense detected, 1 offense corrected

% cat example.rb
if true
  puts "a line way way way way way way way way way way way way way way way way way way way way too long"
end # some comment

This code is uncommented by Style/CommentedKeyword. This is an expected detection of Style/CommentedKeyword, but this issue showed that auto-correction can remove meaningful comments. RuboCop wouldn't know whether this comment was an auto-correction move or the original comment. So, I opened #9991 to mark Style/CommentedKeyword as unsafe auto-correction. Is was my lack of consideration at #9243.

bbatsov pushed a commit that referenced this issue Aug 12, 2021
Fixes #9989

This PR mark unsafe auto-correct for `Style/CommentedKeyword`
because it may remove meaningful comments.
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