Skip to content

Remove duplicate definitions causing method-redefined warnings (#1434) - #1677

Merged
scarroll32 merged 1 commit into
activerecord-hackery:mainfrom
ryoya1122:fix/issue-1434-method-redefined-warnings
Sep 21, 2026
Merged

scarroll32 merged 1 commit into
activerecord-hackery:mainfrom
ryoya1122:fix/issue-1434-method-redefined-warnings

Conversation

@ryoya1122

Copy link
Copy Markdown
Contributor

Closes #1434.

Problem

Loading the gem with warnings enabled (e.g. ruby -W or RSpec under -w) emits three "method redefined" warnings:

lib/ransack/nodes/condition.rb:291: warning: method redefined; discarding old arel_predicate
lib/ransack/nodes/condition.rb:220: warning: previous definition of arel_predicate was here
lib/ransack/nodes/grouping.rb:29: warning: method redefined; discarding old conditions
lib/ransack/context.rb:7: warning: method redefined; discarding old arel_visitor

The reporter saw the arel_predicate one in #1434; the same root cause produces two more.

Cause

Three methods were defined twice in the same class. In each case the first definition was dead code — it was shadowed by a later real implementation:

File Dead definition Live definition
lib/ransack/nodes/condition.rb def arel_predicate; raise "not implemented"; end (was at L220) actual implementation at L291
lib/ransack/nodes/grouping.rb attr_reader :conditions (was at L4) def conditions; @conditions ||= []; end at L29 (lazy-inits the array)
lib/ransack/context.rb second attr_reader :arel_visitor (was at L7) already covered by the multi-symbol attr_reader one line earlier

Fix

Delete the dead definitions. No behavioural change — the live implementations were already the ones in effect at runtime.

Verification

Before:

$ bundle exec ruby -W -r./lib/ransack -e 'puts "loaded"'
lib/ransack/nodes/condition.rb:291: warning: method redefined; discarding old arel_predicate
lib/ransack/nodes/condition.rb:220: warning: previous definition of arel_predicate was here
lib/ransack/nodes/grouping.rb:29: warning: method redefined; discarding old conditions
lib/ransack/context.rb:7: warning: method redefined; discarding old arel_visitor
loaded

After:

$ bundle exec ruby -W -r./lib/ransack -e 'puts "loaded"'
loaded

(Two unrelated "assigned but unused variable" warnings in adapters/active_record/context.rb remain — different category, out of scope for #1434.)

Compatibility

The three deleted lines were dead code, shadowed by the live definitions next to them. Behaviour is identical before and after; the only difference is the absence of the warnings on load. Diff is -6 / +0.

Full suite: 514 examples, 0 failures, 1 pending on v5.0.0 + this branch (SQLite, ActiveRecord 7.2.3.1, Ruby 3.4.9).

Targets v5.0.0 per #1640.

…erecord-hackery#1434)

Three methods were defined twice in the same class, causing Ruby to
emit 'method redefined; discarding old ...' warnings whenever the gem
is required with warnings enabled. The first definition in each pair
was dead code: it was shadowed by a later, real implementation.

* `Ransack::Nodes::Condition#arel_predicate` was defined as a stub
  raising "not implemented" and re-defined further down with the
  actual implementation.
* `Ransack::Nodes::Grouping#conditions` had an `attr_reader` that
  was immediately shadowed by an explicit `def` doing lazy init.
* `Ransack::Context#arel_visitor` was listed in two consecutive
  `attr_reader` calls.

Dropping the dead definitions removes the warnings without any
behavioural change (the live implementations were already the ones in
effect).
@ryoya1122
ryoya1122 force-pushed the fix/issue-1434-method-redefined-warnings branch from a8359df to c732757 Compare June 1, 2026 15:27
@scarroll32
scarroll32 deleted the branch activerecord-hackery:main September 21, 2026 09:40
@scarroll32 scarroll32 closed this Sep 21, 2026
@scarroll32 scarroll32 reopened this Sep 21, 2026
@scarroll32
scarroll32 changed the base branch from v5.0.0 to main September 21, 2026 09:41
@scarroll32

Copy link
Copy Markdown
Member

Note

Comment from Claude (Claude Code), acting on behalf of @scarroll32.

Thanks @ryoya1122 — this is a model bug report and fix: the three shadowed definitions really were dead code, and the write-up made it trivial to confirm.

Retargeted from v5.0.0 to main: the two branches have been consolidated (#1647), so main is now the single line of development for 5.0.0.

Verified locally merged into current main — Rails 8.1.3 / Ruby 3.4.9 / SQLite: 517 examples, 0 failures, 1 pending, and 526/0 with your other three PRs stacked.

Merging with an admin override. This branch predates #1667, so it doesn't contain the Rails 8.1 / Ruby 3.3–4.0 CI matrix and those required contexts can never report on it; every job the branch does define is green across SQLite, MySQL, PostgreSQL and PostGIS.

@scarroll32
scarroll32 merged commit 9748305 into activerecord-hackery:main Sep 21, 2026
24 checks passed
This was referenced Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warning Method Redefined Discarding Old Require

2 participants