Skip to content

Rails 4.0 patterns: Class.scoped + FINDER_SQL fix correction - #104

Merged
etagwerker merged 4 commits into
mainfrom
feature/rails-40-patterns-pr3
Jul 12, 2026
Merged

Rails 4.0 patterns: Class.scoped + FINDER_SQL fix correction#104
etagwerker merged 4 commits into
mainfrom
feature/rails-40-patterns-pr3

Conversation

@JuanVqz

@JuanVqz JuanVqz commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

Two AR scope/finder updates.

  • Class.scoped — new entry. Two forms: bare Model.scoped (replace with Model.all or Model.where(nil) depending on chaining) and Model.scoped(hash) (per-key mapping to chained relation methods). Both deprecated in 4.0, removed in 4.1; bridged by bundled activerecord-deprecated_finders on 4.0. Regex excludes unrelated APIs sharing the substring (unscoped, with_scope, reorder, reset_scope, RSpec deprecation_stream).
  • FINDER_SQL fix correction — previous fix said "Rewrite using standard associations." Pre-bump rewrite breaks Rails 3.2 boot: has_many :name, -> { where(...) }, class_name: ... raises ArgumentError: wrong number of arguments (given 3, expected 1..2) on 3.2.22.5. Fix now defers to dual-boot with NextRails.next?, preserves the common accepts_nested_attributes_for stub idiom, and documents the FK gotcha when converting to -> { none } scope form.

Test plan

  • bin/validate-patterns rails-upgrade/detection-scripts/patterns/rails-40-patterns.yml clean.
  • Reviewer spot-checks .scoped\b regex against representative call sites (especially unscoped/reorder exclusions).

@JuanVqz JuanVqz self-assigned this May 26, 2026
@JuanVqz
JuanVqz requested a review from etagwerker May 26, 2026 00:12
JuanVqz added 2 commits July 3, 2026 20:40
The previous fix said "Rewrite using standard associations with
scopes or custom query methods." Operators following that advice
would write the Rails-4 canonical form
`has_many :name, -> { where(...) }, class_name: ...`, but the
lambda-as-second-arg signature on has_many raises ArgumentError on
Rails 3.2:

  wrong number of arguments (given 3, expected 1..2)

Verified empirically on Rails 3.2.22.5. A naive pre-bump rewrite
breaks 3.2 boot.

Updated fix:

- Defer to dual-boot and branch the association definition with
  NextRails.next?. The Rails-4 form lives behind the guard; the
  Rails-3 finder_sql form stays as the else branch.

- Preserves a common 3.2 idiom — `finder_sql: "SELECT id FROM x
  WHERE id IS NULL"` as an always-empty stub for
  accepts_nested_attributes_for — by keeping the intent intact in
  the dual-boot branch.

- Documents the FK gotcha when converting to `-> { none }`:
  scope-form restores Rails' default FK predicate against
  <owner_class_name>_id. For self-referential associations or any
  case where the implied FK column does not exist, accessing the
  association raises an unknown-column SQL error even when the
  scope returns no rows. Pin foreign_key: and primary_key: to
  existing columns on both sides.

Explanation now leads with the 3.2-side ArgumentError so operators
recognize the failure mode before attempting the rewrite.
Two related Rails-3-only deprecations share the .scoped name and
were both missing from OSS detection:

- Bare Model.scoped — returns a base Relation. Replaced by
  Model.all (when used terminally) or Model.where(nil) (when
  followed by further chaining).

- Model.scoped(conditions:, include:, order:, ...) — the with-args
  ad-hoc scope constructor. Replaced by chaining each hash key as
  its own relation method.

Both deprecated in Rails 4.0, removed in 4.1. On Rails 4.0 the
bundled activerecord-deprecated_finders gem keeps both forms
working, so the rewrite is mandatory before the 4.0 → 4.1 hop but
not before the 3.2 → 4.0 hop. Bridge gem coverage of Class.scoped
is referenced in the entry.

Fix lays out both forms:

  Form A1 (terminal):   Model.scoped         -> Model.all
  Form A2 (chained):    Model.scoped.joins() -> Model.where(nil).joins()
  Form B (hash args):   per-key mapping documented in the fix
                        (conditions/include/order/joins/select/
                        limit/offset/group/having/readonly)

All replacement forms work on Rails 3.2 and 4.0+, so the rewrites
are backwards-compatible.

The exclude regex filters unrelated APIs that share the substring:
unscoped, with_scope, reorder (contains "scope"), reset_scope, and
RSpec's deprecation_stream references.

Classified medium_priority / kind: deprecation — Rails 4.0 emits
warnings but the app still boots and tests still pass.
@etagwerker
etagwerker force-pushed the feature/rails-40-patterns-pr3 branch from 52a3509 to dd2753d Compare July 4, 2026 00:41
etagwerker and others added 2 commits July 12, 2026 09:12
The pattern only matched `Receiver.scoped`, missing the bare implicit-self
form (`scope :foo, -> { scoped.joins(:bar) }`) — which is both the common
Rails 3 idiom and the fix's own documented example. Widen to `\bscoped\b`.

The exclude (`unscoped|with_scope|reorder|reset_scope|deprecation_stream`)
never matched `\.scoped\b` in the first place, so it did nothing useful;
being line-level it actively suppressed real hits that shared a line with
those substrings (e.g. `Model.scoped.reorder(:id)`). Set exclude to "".
Word boundaries already reject unscoped / default_scoped / scoped_ids /
x_scoped, so no exclude is needed.

Add the required CLASS_SCOPED expectations fixture (4 match / 4 no_match),
including the bare-lambda and .scoped.reorder cases that regressions would
have hidden. Update explanation and fix false-positive notes that
referenced the removed exclude.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…erns-pr3

# Conflicts:
#	rails-upgrade/detection-scripts/patterns/rails-40-patterns.expectations.yml

@etagwerker etagwerker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuanVqz LGTM

@etagwerker
etagwerker merged commit 08bb7ae into main Jul 12, 2026
1 check passed
@etagwerker
etagwerker deleted the feature/rails-40-patterns-pr3 branch July 12, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants