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

[Fix #10375] Defer auto-correction of nested unless/else #10461

Merged
merged 1 commit into from Mar 20, 2022

Conversation

jonas054
Copy link
Collaborator

@jonas054 jonas054 commented Mar 19, 2022

An unless/else nested inside another unless/else would cause rewrite clobbering when auto-correcting both offenses. By ignoring the node after correcting it and skipping auto-correction for inner nodes with the same offense, we avoid this problem.

The auto-correct loop will pick up the remaining offense and fix it. No test case is added for this particular aspect, but it's standard RuboCop behavior.

An unless/else nested inside another unless/else would cause rewrite clobbering
when auto-correcting both offenses. By ignoring the node after correcting it
and skipping auto-correction for inner nodes with the same offense, we avoid
this problem.

The auto-correct loop will pick up the remaining offense and fix it. No test
case is added for this particular aspect, but it's standard RuboCop behavior.
@koic koic merged commit b901a9f into rubocop:master Mar 20, 2022
34 checks passed
@koic
Copy link
Member

koic commented Mar 20, 2022

This looks good! Thank you!

@jonas054 jonas054 deleted the 10375_fix_unless_else_autocorrect branch Mar 20, 2022
koic added a commit to koic/rubocop that referenced this issue Apr 8, 2022
Fixes rubocop#10510.

This PR fixes an error for `Style/SingleArgumentDig`
when using multiple `dig` in a method chain.

And this is the same solution as rubocop#10461.
bbatsov pushed a commit that referenced this issue Apr 9, 2022
Fixes #10510.

This PR fixes an error for `Style/SingleArgumentDig`
when using multiple `dig` in a method chain.

And this is the same solution as #10461.
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.

None yet

2 participants