Fix wrong autocorrect for Rails/FindByOrAssignmentMemoization #1516
Conversation
| expect_correction(<<~RUBY) | ||
| def foo | ||
| if defined?(@current_user) | ||
| @current_user |
There was a problem hiding this comment.
Indentation is wrong here but that was already happening and other cops will take care of it.
There was a problem hiding this comment.
I see. It’s fine to delegate this to another cop.
| RUBY | ||
| | ||
| expect_correction(<<~RUBY) | ||
| return @current_user if defined?(@current_user) |
There was a problem hiding this comment.
Overall, this looks good. However, when right_siblings is empty, wouldn’t it be simpler to return early like this?
There was a problem hiding this comment.
It's a bit more complicated than that I think. In a block for example it should also not return early if code follows:
foo do @bar ||= User.find_by(id: id) end something else There are some cases where you can do that, but I'm not certain yet which that would be. I think the way it is it should already handle the majority.
There was a problem hiding this comment.
Yeah, either way, this will definitely fix the mistake. Thank you!
| Thanks |
2 participants
Followup to 501e5d3
This type of autocorrect can only happen if the method contains no other code. Otherwise, it would exist early if followed by other code. Docs only contained example code for it happening inside of a method in that way, but the cop actually checks for the pattern regardless of context. In such cases, the longer form of autocorrect must happen.
Ref #1513
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.