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

Detect opportunities for ||= (double pipe equals) #3965

Closed
agrimm opened this issue Jan 24, 2017 · 4 comments · Fixed by #4648
Closed

Detect opportunities for ||= (double pipe equals) #3965

agrimm opened this issue Jan 24, 2017 · 4 comments · Fixed by #4648

Comments

@agrimm
Copy link
Contributor

agrimm commented Jan 24, 2017

RuboCop doesn't detect when code can be written using ||=. Should it do so?

The Ruby Style Guide recommends this coding style: https://github.com/bbatsov/ruby-style-guide#double-pipe-for-uninit


Expected behavior

Given code like

def my_method(arr)
  x = arr.find(&:even?)

  x = x ? x : 42
  "Result is #{x}"
end

puts my_method([3, 5, 2])

I would expect RuboCop to tell me to use x ||= 42 rather than the ternary operator

Actual behavior

RuboCop doesn't make a complaint.

RuboCop version

$ rubocop -V
[warning skipped]
0.42.0 (using Parser 2.3.1.2, running on ruby 2.4.0 x86_64-darwin14)
@Drenmi
Copy link
Collaborator

Drenmi commented Jan 24, 2017

Makes perfect sense to me. 😀

@rrosenblum
Copy link
Contributor

Other possible formats that should be checked for would be

x = x || some_value
x = some_value unless x
x = some_value unless defined?(x)

@Drenmi
Copy link
Collaborator

Drenmi commented Jan 31, 2017

Think it's worth adding:

x = some_value if x.nil?

@rrosenblum
Copy link
Contributor

If we add support for x = some_value unless defined?(x) or x = some_value if x.nil?, then I think they should be configurable. ||= is going to assign the value if the variable is falsy. In most cases, then change should be safe, but there are times where you would not want to assign the variable if the variable is false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants