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 methodsexecute, 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 methodswhere, 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