Skip to content

Fix false positives in Rails/Pluck for non-AR/Array receivers - #1628

Open
dfl wants to merge 1 commit into
rubocop:masterfrom
dfl:fix_pluck_false_positives_with_allowed_receivers
Open

Fix false positives in Rails/Pluck for non-AR/Array receivers#1628
dfl wants to merge 1 commit into
rubocop:masterfrom
dfl:fix_pluck_false_positives_with_allowed_receivers

Conversation

@dfl

@dfl dfl commented May 21, 2026

Copy link
Copy Markdown

Summary

Adds an AllowedReceivers config option to Rails/Pluck that skips the cop when any method in the receiver's call chain matches the list. The default list includes the most common Nokogiri traversal methods (xpath, css, search, at_xpath, at_css).

Motivation

Rails/Pluck rewrites .map { |el| el[:key] } to .pluck(:key) without inspecting the receiver. This produces broken autocorrections when the receiver isn't an ActiveRecord::Relation or Array — for example, Nokogiri:

# before autocorrect (works)
doc.xpath('//item').map { |node| node['name'] }

# after autocorrect (NoMethodError: undefined method `pluck` for Nokogiri::XML::NodeSet)
doc.xpath('//item').pluck('name')

The cop is documented as @safety: unsafe, but Nokogiri is widespread enough that a default-on guard seems worth it. Other rubocop-rails cops (Rails/RedundantActiveRecordAllMethod, Rails/DynamicFindBy, Rails/SaveBang) already use an AllowedReceivers config for similar reasons.

Behavior

  • AllowedReceivers is matched against any send/csend node anywhere in the receiver chain, by method name. This handles both immediate (page.css('.row').map { ... }) and chained (doc.xpath('//x').first_thing.map { ... }) receivers.
  • Users can extend the list for other libraries that expose [] on non-Array collections (HappyMapper, Mongoid documents, etc.) without further code changes.

Changes

  • lib/rubocop/cop/rails/pluck.rb: add allowed_receiver? guard and early-return in on_block.
  • config/default.yml: add AllowedReceivers default for Rails/Pluck.
  • spec/rubocop/cop/rails/pluck_spec.rb: 4 new examples covering immediate, chained, safe-nav, and a negative (AR chain still flagged).
  • changelog/...: changelog entry.

Test plan

  • bundle exec rspec spec/rubocop/cop/rails/pluck_spec.rb — 40/40 passing
  • bundle exec rspec — full suite green (modulo the changelog PR-number format check, which will pass once the PR number is filled in)
  • Reviewer confirms AllowedReceivers default list matches what the team is comfortable shipping by default

@koic

koic commented Jul 9, 2026

Copy link
Copy Markdown
Member

@dfl There may be differing opinions about the default values for AllowedReceivers, but let's move forward with this for now. Can you squash your commits into one?

Add an `AllowedReceivers` config option to `Rails/Pluck` that skips the
cop when any method in the receiver chain matches the list. Defaults to
Nokogiri traversal methods (`xpath`, `css`, `search`, `at_xpath`,
`at_css`) so that, e.g., `doc.xpath('//item').map { |n| n['name'] }` is
no longer autocorrected to a `pluck` call that the receiver does not
support.
@dfl
dfl force-pushed the fix_pluck_false_positives_with_allowed_receivers branch from c2bda70 to dbc781d Compare July 9, 2026 02:31
@dfl

dfl commented Jul 9, 2026

Copy link
Copy Markdown
Author

squashed ✅

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 this pull request may close these issues.

2 participants