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

Jump to bottom

👎 on Rails/PluckId #31

Closed
miry opened this issue Feb 25, 2024 · 4 comments
Closed

👎 on Rails/PluckId #31

miry opened this issue Feb 25, 2024 · 4 comments

Comments

@miry
Copy link

miry commented Feb 25, 2024

I found Rails/PluckId could not recognise the cases when pluck is used for Enumerable.

https://apidock.com/rails/Enumerable/pluck

Example of the code:

[{ id: "David" }, { id: "Rafael" }, { id: "Aaron" }].pluck(:id)
@mjankowski
Copy link
Contributor

Just enabled the default standard rules (and by extension, Rails/PluckId cop) on a decent sized legacy project. It was about 50/50 on legit suggestions to use ids instead, and false positives coming out of Enumerable usage.

I think rubocop having this disabled by default in their default rules is a good precedent to follow for this one.

@searls
Copy link
Contributor

searls commented May 23, 2024

If a rule can't be applied consistently, let's turn it off. PR?

@miry miry mentioned this issue May 24, 2024
Closed
@miry
Copy link
Author

miry commented May 24, 2024

Prepared PR: #35

mjankowski added a commit to mjankowski/standard-rails that referenced this issue May 24, 2024
@mjankowski
Disable Rails/PluckId cop by default
352895f 
This cop was originally disabled with the original rubocop import: standardrb@71e48d5#diff-da3213a24b350fa386ff3bab0d6b239112408c3fe4f986cbd9d5236f7e0b61e4R458-R463

It was subsequently enabled during a mass enable commit: standardrb@24bc50f#diff-da3213a24b350fa386ff3bab0d6b239112408c3fe4f986cbd9d5236f7e0b61e4R248-R249

As discussed on this issue, the cop is unsafe by default and frequently gets false positives: rubocop/rubocop-rails#301

It is disabled by default in most recent rubocop release, for same reasons: https://github.com/rubocop/rubocop-rails/blob/v2.25.0/config/default.yml#L746-L751

Discussion of why to disable on: standardrb#31
@mjankowski mjankowski mentioned this issue May 24, 2024
Merged
@mjankowski
Copy link
Contributor

With #36 merged, this can be closed.

@searls searls closed this as completed May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Disable Rails/PluckId miry/standard-rails
3 participants
@mjankowski @miry @searls