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 `?` / `: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.
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 |