Class: RuboCop::Cop::DevDoc::Rails::AvoidRawSql

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

Overview

Avoid raw SQL in query methods and ‘connection.execute`.

## Rationale Raw SQL bypasses Rails’ adapter abstraction, hides typos in column and table names (a misspelled column silently returns nothing instead of raising ‘NameError`), is non-reversible in migrations, and ships no audit trail for why raw SQL was chosen over the Rails idiom. Every raw-SQL site should be a deliberate, reviewable choice — not a default.

Prefer, in order:

  • The hash/array form: ‘where(status: “active”)`, `where(status: [“active”, “pending”])`.

  • Parameterized fragments: ‘where(“col = ?”, val)`, `where(“col = :sym”, sym: val)`. The `?` / `:name` placeholder binds the value safely and the column name is the only user-controlled surface.

  • Arel nodes for database-specific constructs that the hash form can’t express.

    ❌ Raw SQL — typo in ‘statu` returns nothing, silently User.where(“statu = ’active’”)

    ✔️ Hash form — typo raises ‘ArgumentError` at the query layer User.where(status: “active”)

    ✔️ Parameterized fragment — value bound, column name spelled User.where(“status = ?”, status)

## Scope The cop covers any string argument to:

  • **Execution methods** — ‘execute`, `exec_query`, `exec_update`, `exec_delete`, `exec_insert`, `find_by_sql`, `select_rows`, `select_values`, `select_one`, `select_all`, `update_sql`, `insert_sql`, `delete_sql`, `update_all`, `delete_all`.

  • **Query fragment methods** — ‘where`, `having`, `order`, `reorder`, `joins`, `select`, `group`, `from`, `lock`, `pluck`.

  • **‘Arel.sql`** — the explicit opt-in to raw SQL (see below).

Applies everywhere — application code AND migrations. A migration’s ‘execute “UPDATE …”` is the same smell as an app code `where(“…”)`; both bypass the adapter and offer no audit trail.

## ‘Arel.sql` Always flagged. `Arel.sql` exists specifically to bypass Rails’ raw-SQL protection — reaching for it should require justification, not be a reflex. If a fragment is genuinely needed, disable inline with a reason; otherwise express it as an Arel AST node or a parameterized query.

## Counter updates (‘update_all(“col = col + 1”)`) For **single records**, use `Model.increment_counter(:col, id)` or `Model.update_counters(id, col: 1)` — both generate parameterized SQL and surface typos in the column name. Neither is flagged by `DevDoc/Migration/AvoidBypassingValidation` (they are the Rails-blessed atomic-counter primitives).

For **bulk counter updates**, the only clean option is ‘find_each { |m| Model.increment_counter(:col, m.id) }` — N queries, slow on large tables, but free of the validation-bypass smell. If the N-query cost is genuinely unacceptable, `update_all(“col = col + 1”)` requires disabling BOTH this cop AND `DevDoc/Migration/AvoidBypassingValidation` with reasons — the friction is the audit trail (locking implications, idempotency on re-runs, and the like are worth a second look).

❌ Single-record counter update via raw SQL
User.where(id: id).update_all("login_count = login_count + 1")

✔️ Parameterized — generates safe SQL
User.increment_counter(:login_count, id)

## Exception Genuine raw-SQL needs — database-specific DDL (‘CREATE INDEX CONCURRENTLY`, `ALTER TYPE … ADD VALUE`), complex joins that don’t fit Arel, analytic queries with CTEs — go through an inline disable comment with a brief reason. The friction is the feature: every raw-SQL site ends up in code review with an articulated justification.

## Migration backfills are NOT an exception A common AI-agent reflex is to justify ‘execute “UPDATE users SET role = 0 …”` with “models can drift, so raw SQL is safer.” That justification is rejected here. The data-integrity cost of bypassing validations and callbacks outweighs the (rare, usually catchable) risk of a migration failing because a model changed.

The correct backfill pattern goes through the model with ‘save!` — `DevDoc/Migration/AvoidBypassingValidation` enforces this, and `best_practices/backend/en/02_migration.md` shows the shape:

✔️ Model through the migration — validations run, callbacks fire
User.where(role: nil).find_each do |user|
  user.role = 0
  user.save!
end

If existing rows would fail validation, fix the data first rather than bypassing validation — that’s the signal, not an obstacle.

NOTE: Parameterized fragments are detected by the presence of a ‘?` or `:name` placeholder plus at least one additional argument. A `?` appearing in literal SQL that isn’t a placeholder (‘where(“name LIKE ’%?%‘”)`) is a residual false negative; inline-disable if it surfaces.

NOTE: Interpolated strings (‘where(“col = ’#val‘”)`) are ALWAYS flagged — they cannot be parameterized by construction and are a SQL-injection risk. Use the `?` placeholder form.

NOTE: Method calls returning strings (‘where(some_helper)`) are not flagged — the cop only inspects literal strings. Helper methods that wrap raw SQL should themselves carry the disable.

Examples:

# bad — raw SQL
execute "UPDATE users SET role = 0 WHERE role IS NULL"
User.where("status = 'active'")
User.where("status = '#{status}'")
User.order("created_at")
User.joins("INNER JOIN items ON items.user_id = users.id")
User.update_all("login_count = login_count + 1")
User.select("DISTINCT status")
Arel.sql("NULLS LAST")

# good — hash / array / symbol form
User.where(status: "active")
User.where(status: ["active", "pending"])
User.order(:created_at)
User.joins(:items)
User.update_all(status: "active")
User.select(:status)

# good — parameterized fragment
User.where("status = ?", status)
User.where("status = :sym", sym: status)
User.find_by_sql("SELECT * FROM users WHERE id = ?", id)

# good — counter update on a single record
User.increment_counter(:login_count, id)

Constant Summary collapse

MSG =
'Avoid raw SQL — prefer the hash/array form, a ' \
'parameterized fragment, or an Arel node. Disable with ' \
'a reason when raw SQL is genuinely required.'.freeze
EXECUTION_METHODS =
%i[
  execute exec_query exec_update exec_delete exec_insert
  find_by_sql select_rows select_values select_one select_all
  update_sql insert_sql delete_sql update_all delete_all
].freeze
FRAGMENT_METHODS =
%i[
  where having order reorder joins select group from lock pluck
].freeze
SQL_METHODS =
(EXECUTION_METHODS + FRAGMENT_METHODS).freeze
RESTRICT_ON_SEND =
(SQL_METHODS + [:sql]).freeze

Instance Method Summary collapse

Instance Method Details

#on_send(node) ⇒ Object



172
173
174
175
176
177
178
# File 'lib/rubocop/cop/dev_doc/rails/avoid_raw_sql.rb', line 172

def on_send(node)
  if arel_sql?(node)
    check_arel_sql(node)
  elsif SQL_METHODS.include?(node.method_name)
    check_sql_method(node)
  end
end