Rails/PluckId is not excluding array collections

[beetlegius-jt]

Expected behavior

No alerts for arrays.

Actual behavior

Rails/PluckId raises an alert when using pluck(:id) with array collections.
Arrays does not support .ids, that only works for ActiveRecord::Relation collections.

Steps to reproduce the problem

I have this code:

array = [{ id: 1 }, { id: 2 }]
extracted_ids = array.pluck(:id)

Rubocop suggestion:

Rails/PluckId: Use ids instead of pluck(:id).
    extracted_ids = array.pluck(:id)
                          ^^^^^^^^^^

Updated code:

array = [{ id: 1 }, { id: 2 }]
extracted_ids = array.ids

Then it produces the following error:

NoMethodError: undefined method `ids' for [{:id=>1}, {:id=>2}]:Array

RuboCop version

bundle exec rubocop -V
0.88.0 (using Parser 2.7.1.4, rubocop-ast 0.2.0, running on ruby 2.7.1 x86_64-linux)

[koic]

The Rails/PluckId cop is already marked unsafe (Safe: No).
https://docs.rubocop.org/rubocop-rails/2.7/cops_rails.html#railspluckid

On the other hand, it is difficult to know a receiver type in the current static analysis implementation. So, It may be possible to consider disable the cop by default if there are many false alarms to users.

[beetlegius-jt]

The Rails/PluckId cop is already marked unsafe (Safe: No).
https://docs.rubocop.org/rubocop-rails/2.7/cops_rails.html#railspluckid

On the other hand, it is difficult to know a receiver type in the current static analysis implementation. So, It may be possible to consider disable the cop by default if there are many false alarms to users.

Sure, I've disabled the cop because otherwise I have to add a lot of disable comments when using pluck with array collections.

Maybe you can check if it responds to ids as a guard clause. 🤔 not sure if it's possible in the current implementation.

[koic]

Yeah, it would be redundant to introduce respond_to?(:ids) even when it is not needed :-)
It seems better to set Safe: false by default for this cop. Thank you for the feedback!

[beetlegius-jt]

Yeah, it would be redundant to introduce respond_to?(:ids) even when it is not needed :-)
It seems better to set Safe: false by default for this cop. Thank you for the feedback!

What do you mean? I have the cop disabled because otherwise it will not pass the CI rubocop check.
I mean, it's not an rubocop --auto-correct thing. There are "false alarms" when enabling this cop.

An alternative could be to add a rubocop:disable Rails/PluckId to every line that makes a pluck to an array, but there are ~ 15 occurrences in this project, and I think the same thing happens to most people.

[koic]

It seems better to set Safe: false by default for this cop.

Ah, I was typo. Correctly it is Enabled: false.

[koic]

I've opened the PR #307.

[inkstak]

Won't the same problem occur for Rails/PluckInWhere ?

Rails/PluckInWhere: Use select instead of pluck within where query method.
  Record.where(id: something.pluck(:whatever))
                             ^^^^^

something may be an array which doesn't respond to select.

[koic]

@inkstak Sure. There will be similar problems. On the other hand, the context is narrowed down to the condition of where. It is not yet predicted how many false positives will occur in this context. So, I would like to watch the situation as a different issue. Thank you for the feedback.

[koic]

Won't the same problem occur for Rails/PluckInWhere ?

Just FYI, I opened the PR #317 to resolve a false positive for Rails/PluckInWhere cop.

[mos-adebayo]

You can use the map method instead of pluck if the collection is an Array

array = [{ id: 1 }, { id: 2 }]
extracted_ids = array.map(&:id)

[inkstak]

@mos-adebayo It won't because Hash doesn't responds to id method.

• map is a Ruby method to iterate a method - in shorthand version - of each item of an enumerable
• pluck is a Rails method to iterate a key of each item

array = [{ id: 1 }, { id: 2 }]
array.pluck(:id)  # => [1, 2]
array.map(&:id)   # => undefined method `id' for {:id=>1}:Hash (NoMethodError)

Item = Data.define(:id)
array = [Item.new(1), Item.new(2)]
array.map(&:id)   #=>  [1, 2]
array.pluck(:id)  #=> undefined method `[]' for #<data Item id=1> (NoMethodError)