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

New cop to replace conditional assignment with or-assignment #10325

Closed
fatkodima opened this issue Dec 27, 2021 · 4 comments · Fixed by #10593
Closed

New cop to replace conditional assignment with or-assignment #10325

fatkodima opened this issue Dec 27, 2021 · 4 comments · Fixed by #10593

Comments

@fatkodima
Copy link
Contributor

I've written the following code

def lock_retrier=(value)
  if value.nil?
    @lock_retrier = NullLockRetrier.new
  else
    @lock_retrier = value
  end
end

But then realized that this should be really written as

def lock_retrier=(value)
  @lock_retrier = value || NullLockRetrier.new
end

and wondering why rubocop hasn't suggested such change.

Quick search on the rails codebase via regexp revealed this:
https://github.com/rails/rails/blob/296ef7a17221e81881e38b51aa2b014d7a28bac5/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb#L75-L79

So I suggest implementing a new cop (not sure about its name).

This should also check for cases like

if value
  @lock_retrier = value
else
  @lock_retrier =  NullLockRetrier.new
end

etc.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 28, 2021

Seems like a good idea to me. I think we have a couple of similar cops already.

@marcospedro97
Copy link

Can I give it a try? Maybe we can call it something like Style/OrAssignment?

@fatkodima
Copy link
Contributor Author

@marcospedro97 Yes, please, go ahead 👍

@marcospedro97
Copy link

marcospedro97 commented Jan 15, 2022

I've found a cop with this name already, and it's behaviour is too different from the one we're talking about, thinking about naming it Style/SingleLineOrAssginment, there's also another with the name conditional assignment, also too diferent

nobuyo added a commit to nobuyo/rubocop that referenced this issue Apr 30, 2022
… the case that variable assignments in each branch
bbatsov pushed a commit that referenced this issue May 6, 2022
…se that variable assignments in each branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants