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

UniqBeforePluck should recommend Distinct on rails 5 #91

Closed
ghiculescu opened this issue Jul 21, 2019 · 0 comments · Fixed by #135
Closed

UniqBeforePluck should recommend Distinct on rails 5 #91

ghiculescu opened this issue Jul 21, 2019 · 0 comments · Fixed by #135

Comments

@ghiculescu
Copy link
Contributor

In Rails 5, uniq was deprecated in favour of distinct on ActiveRecord relations: https://edgeguides.rubyonrails.org/5_0_release_notes.html#active-record-deprecations

This means the documentation here is wrong: https://github.com/rubocop-hq/rubocop-rails/blob/master/lib/rubocop/cop/rails/uniq_before_pluck.rb#L23

Specifically, Model.uniq.pluck(:id) will raise (NoMethodError (undefined method uniq' for #Class:0x000000000b339d10)). Whereas Model.distinct.pluck(:id)` will still work.

My suggestions are:

  • Update docs to always recommend using distinct even if uniq was used beforehand.
  • Update the error message to always recommend distinct no matter what method was matched.

Happy to do a PR if everyone is okay with this.

santib added a commit to santib/rubocop-rails that referenced this issue Oct 7, 2019
santib added a commit to santib/rubocop-rails that referenced this issue Oct 7, 2019
santib added a commit to santib/rubocop-rails that referenced this issue Jun 5, 2020
santib added a commit to santib/rubocop-rails that referenced this issue Jun 5, 2020
santib added a commit to santib/rubocop-rails that referenced this issue Jun 5, 2020
santib added a commit to santib/rubocop-rails that referenced this issue Jun 7, 2020
santib added a commit to santib/rubocop-rails that referenced this issue Jun 7, 2020
santib added a commit to santib/rubocop-rails that referenced this issue Jun 7, 2020
@koic koic closed this as completed in #135 Jun 7, 2020
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 a pull request may close this issue.

1 participant