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

Style/NegatedIfElseCondition bugs with specific conditions #9731

Closed
edouard-per-angusta opened this issue Apr 22, 2021 · 1 comment
Closed
Assignees
Labels

Comments

@edouard-per-angusta
Copy link

The Style/NegatedIfElseCondition cop bugs with very specific conditions with error Parser::Source::TreeRewriter detected clobbering.

1 error occurred:
An error occurred while Style/NegatedIfElseCondition cop was inspecting ...

Steps to reproduce the problem

Create a file with the code below and run rubocop on it:

if !a_condition
  # comment
  flag = false
  do_a
end

if !another_condition
  # comment
  flag = false
  do_b
else
  do_c
end

This seems very specific since it works if you remove the else block, if you remove the comments...

RuboCop version

$ bundle exec rubocop -V
1.12.1 (using Parser 3.0.1.0, rubocop-ast 1.4.1, running on ruby 2.6.7 x86_64-darwin20)
  - rubocop-performance 1.9.2
  - rubocop-rails 2.9.1
  - rubocop-rspec 2.2.0
@koic koic added the bug label Apr 22, 2021
@dvandersluis dvandersluis self-assigned this May 1, 2021
@dvandersluis
Copy link
Member

dvandersluis commented May 1, 2021

This is an interesting bug indeed. I found an actual issue in Style/NegatedIfElseCondition that I have a fix for, where branches aren't being swapped properly when they are only a single statement.

However, there's also a second issue with how the AST is associated with comments that is being tripped for this specific example because there are two identical nodes (the flag = false lines) that each have an attached comment. The code to swap the if-else branches gets a range for each branch that includes comments, but because of rubocop/rubocop-ast#179, in your specific example it considers this entire part of the code to be the "if branch":

if !a_condition
  # comment            # <== start
  flag = false         #
  do_a                 #
end                    #
                       #
if !another_condition  #
  # comment            #
  flag = false         #
  do_b                 # <== end
else
  do_c
end

This is what's causing the clobbering error.

dvandersluis added a commit that referenced this issue May 2, 2021
[Fix #9731] Fix two autocorrection issues for `Style/NegatedIfElseCondition`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants