Class: RuboCop::Cop::DevDoc::Rails::NoBlockPredicateOnRelation

Inherits:
Base
  • Object
show all
Defined in:
lib/rubocop/cop/dev_doc/rails/no_block_predicate_on_relation.rb

Overview

Avoid block-form predicates on ActiveRecord relations.

Rationale

.count { }, .reject { }, .select { }, .any? { }, and .find { } accept a block, which silently converts the relation to an Array — every row is loaded into Ruby memory and filtered there, defeating database indexes and pagination.

Push the predicate into SQL with .where(...) or a model scope so the database does the filtering.

❌ Loads every pending record into memory before counting
pending_subscriptions.count { |s| !s.expired_for_context? }

✔️ Becomes a SQL COUNT via a scope
class Subscription < ApplicationRecord
scope :not_expired_for_context, ->(context) { ... }
end

pending_subscriptions.not_expired_for_context(context).count

Exception

Some predicates genuinely can't be expressed in SQL (decrypted attributes, non-trivial Ruby logic). For those, add a # rubocop:disable with a brief reason.

Receiver detection is precision-first (inclusion, not exclusion)

The cop only fires when the receiver chain PROVES it is a relation: it must contain a relation-returning method (where, joins, includes, order, limit, ... — see RELATION_RETURNING_METHODS) or a project-declared scope from AdditionalRelationMethods. Opaque receivers (locals, ivars, method params, bare association reads like user.posts) are NOT flagged.

This is a deliberate trade. The earlier exclusion-based design ("flag anything that isn't provably a non-relation") was dogfooded against a mature codebase and scored 21 offenses with ZERO true positives — params arrays, gem data, AST enumerators, and validators that must see unsaved in-memory records. A cop that is all noise teaches people to write disables, which is worse than the missed recall: a bare user.posts.any? { } slipping through is a perf nit; twenty reflex disables are a culture problem. Projects can recover recall selectively by listing their hot association/scope names in AdditionalRelationMethods.

Excluded receivers

Even within a proven relation chain, the cop skips receivers whose FINAL call is known to return a non-Relation (the collection is already materialised, so SQL push-down is no longer possible), plus obvious non-relations: array literals ([...]), hash literals ({...}), and screaming-case constants (e.g. PRICING_PLANS):

  • Array-returning — pluck, to_a, map, flatten, compact, uniq, sort, sort_by, reduce, inject, each_with_object, zip, take, drop, group_by, partition, tally, chunk_while, slice_when, etc.
  • Hash-returning — slice, except, merge, transform_values, transform_keys, to_h, compact_blank, with_indifferent_access.
  • Enumerator-returning — each_with_index, each_slice, each_cons, lazy, with_index, with_object. Calling these on a Relation forces eager loading; the next .select/etc. then operates on an in-memory Enumerator, so pushing into SQL is no longer possible.

Parenthesised receivers ((expr).select { ... }) are looked through to expr so the rules above still apply.

Excluded block shapes

Even when the receiver is opaque (a local/instance variable, method parameter, etc.), the block itself sometimes proves the element can't be an AR record:

  • 2+ block arguments|k, v|, |item, _index|, etc. AR relations only yield single records; multi-arg destructuring means the iterator is Hash#each, zip, or similar.
  • Symbol-key indexing on the block argarg[:foo] proves the element is a Hash (AR's [] is rarely called with literal symbol keys).
  • Single-char string indexing on the block argc[0] == '+' proves the element is a String.

Configuration

AdditionalRelationMethods (default []): per-project list of association/scope names the cop should treat as relation-returning. This is how a project recovers recall on its hot paths without re-opening the false-positive door. Example:

DevDoc/Rails/NoBlockPredicateOnRelation:
AdditionalRelationMethods:
  - accessible_organization_documents
  - active_subscriptions

AdditionalNonRelationMethods (default []): per-project list of method names that return non-Relation collections (e.g. a presenter factory) — a chain ENDING in one of these is skipped even when an earlier link is a relation method.

Examples:

# bad
pending_memberships.count { |m| !m.expired? }

# bad
user.posts.reject { |post| post.archived? }

# bad
Model.where(active: true).any? { |r| r.flagged? }

# good
pending_memberships.not_expired.count

# good (receiver returns Array — excluded)
user.posts.pluck(:title).reject(&:blank?)

# good (Hash#values — excluded)
PRICING_PLANS.reject { |_k, v| v.archived? }

Constant Summary collapse

MSG =
'`%<method>s` with a block loads every row into Ruby. ' \
'Push the predicate into SQL with `.where(...)` or a model scope.'.freeze
RESTRICT_ON_SEND =

filter/detect are the Ruby aliases of select/find — leaving them out would make the alias a zero-cost dodge.

%i[count reject select filter find detect any?].freeze
RELATION_RETURNING_METHODS =

Methods returning an ActiveRecord::Relation — a chain containing one of these PROVES the receiver is a relation (inclusion-based detection; see the docstring). merge, all and from are deliberately absent: Hash#merge is everyday Ruby, .all is also the API of non-AR clients (e.g. Stripe list endpoints), and ActiveSupport adds Array#from.

%i[
  where rewhere not joins left_joins left_outer_joins includes
  preload eager_load references order reorder group having limit
  offset distinct unscope unscoped lock readonly or
].freeze
NON_RELATION_RETURNING_METHODS =

Methods whose return value is known to be a non-Relation collection (Array, Hash, or Enumerator). When a .select/.reject/etc. with a block is chained onto a call to one of these, the block runs over the materialised collection — pushing into SQL isn't possible.

%i[
  pluck pluck_to_hash to_a to_ary values keys map flat_map collect
  collect_concat filter_map flatten compact uniq sort sort_by reverse
  reduce inject each_with_object split lines chars bytes zip take
  drop drop_while take_while group_by partition tally tally_by
  chunk_while slice_when slice except merge transform_values
  transform_keys to_h compact_blank with_indifferent_access index_by
  index_with each_with_index each_slice each_cons each_entry each_key
  each_value each_pair chunk slice_before slice_after lazy with_index
  with_object
].freeze

Instance Method Summary collapse

Instance Method Details

#on_send(node) ⇒ Object



155
156
157
158
159
160
161
162
163
# File 'lib/rubocop/cop/dev_doc/rails/no_block_predicate_on_relation.rb', line 155

def on_send(node)
  return unless node.block_literal? || symbol_block_pass?(node)
  return if node.receiver.nil?
  return unless relation_chain?(node.receiver)
  return if excluded_receiver?(node.receiver)
  return if excluded_block?(node)

  add_offense(node.loc.selector, message: format(MSG, method: node.method_name))
end