Class: RuboCop::Cop::DevDoc::Rails::AvoidRawSql
- Inherits:
-
Base
- Object
- Base
- RuboCop::Cop::DevDoc::Rails::AvoidRawSql
- 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?/:nameplaceholder 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
statureturns nothing, silently User.where("statu = 'active'")✔️ Hash form — typo raises
ArgumentErrorat 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.
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 |