Skip to content

Break and return false in forced_brace_bounds? when Parent is a Binary #181

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

Conversation

weizheheng
Copy link

Fixes #122

@weizheheng weizheheng force-pushed the add-Binary-check-to-forced_brace_bounds branch from 8f8b47a to ec78306 Compare October 25, 2022 03:18
@weizheheng weizheheng closed this Oct 25, 2022
@weizheheng weizheheng reopened this Oct 25, 2022
@weizheheng
Copy link
Author

@kddnewton Sorry for opening another PR, here is the original PR. I messed up the original PR while trying to fix the idempotency_test. Unfortunately, the tests are still failing and I appreciate it if you could give me some guidance on fixing it.

Tried running the CI=true bundle exec rake and it gave me more errors as compared to the Github Action run. The error I on Github Action run are:

  • I am not sure why the rdoc/parser/ruby.rb and c.rb is raising syntax error.

CleanShot 2022-10-25 at 11 13 20@2x

@kddnewton
Copy link
Member

No worries @weizheheng I'll take a look.

@kddnewton
Copy link
Member

I did some more digging into this issue. The issue is this kind of expression:

1 while line.gsub!(/\t+/) do
    ' ' * (tab_width*$&.length - $`.length % tab_width)
  end  && $~

In this case, it will get flipped into the non-modifier form, as in:

while line.gsub!(/\t+/) do
        " " * (tab_width * $&.length - $`.length % tab_width)
      end && $~
  1
end

but with your PR it's going to use do..end. This means the do keyword is going to get confused with the optional do keyword of the while loop. Which means we need to do more than to just check if it's a Binary node.

@kddnewton
Copy link
Member

I'm going to close this for now since it seems like we won't be able to solve this.

@kddnewton kddnewton closed this Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block arguments in if statements uses {, triggering Rubocop
2 participants