Class: RuboCop::Cop::DevDoc::Rails::AvoidOrderingById
- Inherits:
-
Base
- Object
- Base
- RuboCop::Cop::DevDoc::Rails::AvoidOrderingById
- Defined in:
- lib/rubocop/cop/dev_doc/rails/avoid_ordering_by_id.rb
Overview
Avoid ordering by ‘id` as the primary (sole or leftmost) sort.
## Rationale ‘order(id: …)` / `order(:id)` sorts by the primary key, which reflects insertion order (and, for UUIDs, random order) rather than a column the reader can reason about. It makes a list read as “most recent” by accident, hides the real ordering intent, and silently breaks the moment rows are imported out of sequence or the primary key type changes.
Order by a meaningful business column instead, and reach for ‘id` only as a secondary key to break ties deterministically:
❌ Primary key is not a sort the reader can reason about
Snapshot.order(id: :desc)
✔️ Business column carries the intent
Snapshot.order(created_at: :desc)
✔️ `id` as a deterministic tiebreaker — never the primary sort
Snapshot.order(created_at: :desc, id: :desc)
See ‘best_practices/backend/en/05_controller.md` — always `.order()` when displaying more than one record, and reserve `order(id: …)` for the rare case where id-ordering is genuinely intended.
## Scope Flags ‘order` and `reorder` where `:id` is the sole ordering column or the leftmost (primary) one, in hash form (`order(id: :desc)`), symbol form (`order(:id)`, `order(:id, :name)`), or array form (`order()`).
Only the FIRST argument is inspected — it is the primary sort; every later argument is a tiebreaker. Raw-SQL string ordering (‘order(“id”)`, `order(“id DESC”)`) is the domain of `DevDoc/Rails/AvoidRawSql`, so this cop defers the moment its leading argument is a string, an `Arel.sql(…)`, or anything other than a hash/symbol/array — the primary sort can’t be read statically, and flagging would risk a false positive on a tiebreaker.
NOTE: Each ‘order`/`reorder` call is inspected in isolation, so a chained form like `rel.order(:created_at).order(id: :desc)` is flagged even though Rails appends chained `order` calls — making `id` a secondary tiebreaker there. The deterministic-tiebreaker idiom this cop endorses is the single-call hash/symbol form (`order(created_at: :desc, id: :desc)`); prefer that, or disable inline with a reason when the chained form is genuinely intended.
## ‘id` as a secondary key is allowed `order(created_at: :desc, id: :desc)` is the standard pattern for deterministic ordering — `id` only breaks ties after the business column. The cop allows `:id` in any non-primary position:
✔️ `id` breaks ties only — primary sort is `created_at`
Snapshot.order(created_at: :desc, id: :desc)
Snapshot.order(:created_at, :id)
## Exception When ‘id` is genuinely the intended primary sort (rare — usually a sign something is special about the code), suppress the offense with an inline disable comment and a brief reason — e.g. an append-only audit log where `id` is creation order, so id-ordering is the real intent rather than an accident.
NOTE: A named scope or model method that wraps the offense hides it at the call site — ‘scope :newest, -> { order(id: :desc) }` is flagged in the model file, but callers of `.newest` are not. The cop flags the definition, which is where the decision belongs.
NOTE: Prefer ‘order(created_at: :desc)` for deterministic results — see `best_practices/backend/en/05_controller.md` #4. The rare legitimate `order(id:)` cases (see Exception above) —e.g. a test grabbing a record a controller just created, where `created_at` ties across one request and `id` is the only reliable insertion-order proxy — take an inline disable with a reason, not a blanket `Exclude` of `spec/**`/`test/**` (that silences accidental id-ordering in tests).
Constant Summary collapse
- MSG =
'Ordering by `id` as the primary sort exposes the primary ' \ "key's insertion order rather than a meaningful column. " \ 'Order by a business column (e.g. `created_at:`), or move ' \ '`id` to a secondary position as a deterministic tiebreaker. ' \ 'Disable with a reason when `id` is genuinely the intended ' \ 'primary sort.'.freeze
- RESTRICT_ON_SEND =
%i[order reorder].freeze
Instance Method Summary collapse
- #on_send(node) ⇒ Object (also: #on_csend)
Instance Method Details
#on_send(node) ⇒ Object Also known as: on_csend
106 107 108 109 110 111 112 |
# File 'lib/rubocop/cop/dev_doc/rails/avoid_ordering_by_id.rb', line 106 def on_send(node) first = first_ordering_column(node) return unless first return unless id_column?(first) add_offense(node.loc.selector) end |