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

Comment deletion in if/unless modifier lines #11482

Closed
bellebaum opened this issue Jan 23, 2023 · 0 comments · Fixed by #11644
Closed

Comment deletion in if/unless modifier lines #11482

bellebaum opened this issue Jan 23, 2023 · 0 comments · Fixed by #11644
Labels

Comments

@bellebaum
Copy link

When examining long lines with if or unless modifiers and subsequent comments,
RuboCop with the -A flag deletes the comment, leaving the surrounding code as is.

This problem can easily go unnoticed and is (to me at least) unexpected.


Steps to reproduce the problem

Using the default configuration (i.e. no custom 'rubocop.yml'),
create a file with the following line:

some_statement if some_quite_long_condition # The condition might have been long, but this comment is longer. In fact, it is too long for Rubocop

Then run rubocop -A on the file.

Expected behavior

Detect that the comment is causing the line to be too long, and place it somewhere sensible.

For example:

# The condition might have been long, but this comment is longer. In fact, it is too long for Rubocop
some_statement if some_quite_long_condition

Actual behavior

RoboCop seems to blame the if modifier to cause the long line.

test.rb:3:16: C: [Corrected] Style/IfUnlessModifier: Modifier form of if makes the line too long.
some_statement if some_quite_long_condition # The condition might have been long, but this comment is longer. In fact, it is too long for Rubocop

It proceeds to transform it into the following:

if some_quite_long_condition
  some_statement
end # The condition might have been long, but this comment is longer. In fact, it is too long for Rubocop

Next, it dislikes the comment after end, so it presumably removes it (I have not yet confirmed that the comment is removed in this step).

test.rb:5:5: C: [Corrected] Style/CommentedKeyword: Do not place comments on the same line as the end keyword.
end # The condition might have been long, but this comment is longer. In fact, it is too long for Rubocop

With the comment out of the way, RuboCop prefers the modifier form of if:

test.rb:3: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 some_quite_long_condition

The final result is the initial line with the comment missing. No other lines have been touched.

some_statement if some_quite_long_condition

RuboCop version

1.43.0 (using Parser 3.2.0.0, rubocop-ast 1.24.1, running on ruby 3.0.5) [x86_64-linux]

For debugging purposes, here is the (anonymized) --debug header from my tests:

For $PWD: Default configuration from /usr/lib/ruby/gems/3.0.0/gems/rubocop-1.43.0/config/default.yml
Use parallel by default.
Skipping parallel inspection: only a single file needs inspection
Inspecting 1 file
Scanning $PWD/test.rb
Loading cache from $HOME/.cache/rubocop_cache/3fb3810be52a0588e38fe598cd2408c05c5458f4/6d7a3b621ca1730e03accd938619e4bdab66cfb1/69258fadcfb00690acd86d208048f4cb9810aca7

I am using Arch Linux. If helpful, here is the script to build and package rubocop.

@koic koic added the bug label Jan 24, 2023
nobuyo added a commit to nobuyo/rubocop that referenced this issue Mar 2, 2023
…` when the modifier form expression has long comment

Update lib/rubocop/cop/style/if_unless_modifier.rb

Co-authored-by: Koichi ITO <koic.ito@gmail.com>
bbatsov pushed a commit that referenced this issue Mar 3, 2023
…the modifier form expression has long comment

Update lib/rubocop/cop/style/if_unless_modifier.rb

Co-authored-by: Koichi ITO <koic.ito@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants