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.

Sign up for GitHub

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

Unmark AutoCorrect: false from Rails/UniqBeforePluck #580

Merged
merged 1 commit into from
Nov 10, 2021
Merged

Unmark AutoCorrect: false from Rails/UniqBeforePluck #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_unmark_autocorrect_false_from_rails_uniq_before_pluck.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#580](https://github.com/rubocop/rubocop-rails/pull/580): Unmark `AutoCorrect: false` from `Rails/UniqBeforePluck`. ([@koic][])
3 changes: 1 addition & 2 deletions config/default.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,13 +820,12 @@ Rails/UniqBeforePluck:
Description: 'Prefer the use of uniq or distinct before pluck.'
Enabled: true
VersionAdded: '0.40'
VersionChanged: '2.8'
VersionChanged: '2.13'
EnforcedStyle: conservative
SupportedStyles:
- conservative
- aggressive
SafeAutoCorrect: false
AutoCorrect: false

Rails/UniqueValidationWithoutIndex:
Description: 'Uniqueness validation should have a unique index on the database column.'
Expand Down
6 changes: 1 addition & 5 deletions docs/modules/ROOT/pages/cops_rails.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4875,7 +4875,7 @@ end
| Yes
| Yes (Unsafe)
| 0.40
| 2.8
| 2.13
|===

Prefer the use of distinct, before pluck instead of after.
Expand Down Expand Up @@ -4940,10 +4940,6 @@ Model.distinct.pluck(:id)
| EnforcedStyle
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side observation.

Examples are outdated, too.

# this will ~return a Relation~ that pluck is called on
Model.where(cond: true).pluck(:id).uniq

check:

> x=User.where('1 = 1').pluck(:id);
SELECT "users"."id" FROM "users" WHERE (1 = 1)
> x.class
=> Array

uniq will be called on an array.

# an association on an instance will ~return a CollectionProxy~
instance.assoc.pluck(:id).uniq

It will return an Array.

# good
Model.distinct.pluck(:id)

I understand the reasoning, but the usage of id breaks it.

Model.distinct.pluck(:favourite_food)

would make more sense.
I believe the usage of id in those examples is more confusing than helpful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. That point can open a PR separately. Can you open a PR if you like?

| `conservative`
| `conservative`, `aggressive`

| AutoCorrect
| `false`
| Boolean
|===

== Rails/UniqueValidationWithoutIndex
Expand Down