Merge pull request rails#45498 from HParker/deprecate-rewhere-on-merge

Deprecate passing `rewhere` to `merge`

Author: kamipo

activerecord/lib/active_record/relation/spawn_methods.rb

@@ -42,6 +42,21 @@ def merge(other, *rest)
 
     def merge!(other, *rest) # :nodoc:
       options = rest.extract_options!
+
+      if options.key?(:rewhere)
+        if options[:rewhere]
+          ActiveSupport::Deprecation.warn(<<-MSG.squish)
+            Specifying `Relation#merge(rewhere: true)` is deprecated, as that has now been
+            the default since Rails 7.0. Setting the rewhere option will error in Rails 7.2
+          MSG
+        else
+          ActiveSupport::Deprecation.warn(<<-MSG.squish)
+            `Relation#merge(rewhere: false)` is deprecated without replacement,
+            and will be removed in Rails 7.2
+          MSG
+        end
+      end
+
       if other.is_a?(Hash)
         Relation::HashMerger.new(self, other, options[:rewhere]).merge
       elsif other.is_a?(Relation)

activerecord/test/cases/relation/merging_test.rb

@@ -24,29 +24,41 @@ def test_merge_in_clause
 
     assert_equal [mary], david_and_mary.merge(Author.where(id: mary))
     assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary))
-    assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    assert_deprecated do
+      assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    end
 
     assert_equal [bob],  david_and_mary.merge(Author.where(id: bob))
     assert_equal [bob],  david_and_mary.merge(Author.rewhere(id: bob))
-    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+    assert_deprecated do
+      assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+    end
 
     assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]))
-    assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]), rewhere: true)
+    assert_deprecated do
+      assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]), rewhere: true)
+    end
 
     assert_equal [mary, bob], david_and_mary.merge(mary_and_bob)
-    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    end
     assert_equal [mary], david_and_mary.and(mary_and_bob)
     assert_equal authors, david_and_mary.or(mary_and_bob)
 
     assert_equal [david, mary], mary_and_bob.merge(david_and_mary)
-    assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+    assert_deprecated do
+      assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+    end
     assert_equal [mary], david_and_mary.and(mary_and_bob)
     assert_equal authors, david_and_mary.or(mary_and_bob)
 
     david_and_bob = Author.where(id: david).or(Author.where(name: "Bob"))
 
     assert_equal [david], david_and_mary.merge(david_and_bob)
-    assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true)
+    end
     assert_equal [david], david_and_mary.and(david_and_bob)
     assert_equal authors, david_and_mary.or(david_and_bob)
   end
@@ -62,29 +74,41 @@ def test_merge_between_clause
 
     assert_equal [mary], david_and_mary.merge(Author.where(id: mary))
     assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary))
-    assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    assert_deprecated do
+      assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    end
 
     assert_equal [bob], david_and_mary.merge(Author.where(id: bob))
     assert_equal [bob],  david_and_mary.merge(Author.rewhere(id: bob))
-    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+    assert_deprecated do
+      assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+    end
 
     assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]))
-    assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]), rewhere: true)
+    assert_deprecated do
+      assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]), rewhere: true)
+    end
 
     assert_equal [mary, bob], david_and_mary.merge(mary_and_bob)
-    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    end
     assert_equal [mary], david_and_mary.and(mary_and_bob)
     assert_equal authors, david_and_mary.or(mary_and_bob)
 
     assert_equal [david, mary], mary_and_bob.merge(david_and_mary)
-    assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+    assert_deprecated do
+      assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+    end
     assert_equal [mary], david_and_mary.and(mary_and_bob)
     assert_equal authors, david_and_mary.or(mary_and_bob)
 
     david_and_bob = Author.where(id: david).or(Author.where(name: "Bob"))
 
     assert_equal [david], david_and_mary.merge(david_and_bob)
-    assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true)
+    end
     assert_equal [david], david_and_mary.and(david_and_bob)
     assert_equal authors, david_and_mary.or(david_and_bob)
   end
@@ -100,30 +124,42 @@ def test_merge_or_clause
 
     assert_equal [mary], david_and_mary.merge(Author.where(id: mary))
     assert_equal [mary], david_and_mary.merge(Author.rewhere(id: mary))
-    assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    assert_deprecated do
+      assert_equal [mary], david_and_mary.merge(Author.where(id: mary), rewhere: true)
+    end
 
     assert_equal [bob], david_and_mary.merge(Author.where(id: bob))
     assert_equal [bob],  david_and_mary.merge(Author.rewhere(id: bob))
-    assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+    assert_deprecated do
+      assert_equal [bob],  david_and_mary.merge(Author.where(id: bob), rewhere: true)
+    end
 
     assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]))
-    assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]), rewhere: true)
+    assert_deprecated do
+      assert_equal [david, bob], mary_and_bob.merge(Author.where(id: [david, bob]), rewhere: true)
+    end
 
 
     assert_equal [mary, bob], david_and_mary.merge(mary_and_bob)
-    assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true)
+    end
     assert_equal [mary], david_and_mary.and(mary_and_bob)
     assert_equal authors, david_and_mary.or(mary_and_bob)
 
     assert_equal [david, mary], mary_and_bob.merge(david_and_mary)
-    assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+    assert_deprecated do
+      assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true)
+    end
     assert_equal [mary], david_and_mary.and(mary_and_bob)
     assert_equal authors, david_and_mary.or(mary_and_bob)
 
     david_and_bob = Author.where(id: david).or(Author.where(name: "Bob"))
 
     assert_equal [david], david_and_mary.merge(david_and_bob)
-    assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true)
+    end
     assert_equal [david], david_and_mary.and(david_and_bob)
     assert_equal authors, david_and_mary.or(david_and_bob)
   end
@@ -136,10 +172,14 @@ def test_merge_not_in_clause
     assert_equal [david], non_mary_and_bob
 
     assert_equal [david], Author.where(id: david).merge(non_mary_and_bob)
-    assert_equal [david], Author.where(id: david).merge(non_mary_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david], Author.where(id: david).merge(non_mary_and_bob, rewhere: true)
+    end
 
     assert_equal [david], Author.where(id: mary).merge(non_mary_and_bob)
-    assert_equal [david], Author.where(id: mary).merge(non_mary_and_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david], Author.where(id: mary).merge(non_mary_and_bob, rewhere: true)
+    end
   end
 
   def test_merge_not_range_clause
@@ -150,10 +190,14 @@ def test_merge_not_range_clause
     assert_equal [david, mary], less_than_bob
 
     assert_equal [david, mary], Author.where(id: david).merge(less_than_bob)
-    assert_equal [david, mary], Author.where(id: david).merge(less_than_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david, mary], Author.where(id: david).merge(less_than_bob, rewhere: true)
+    end
 
     assert_equal [david, mary], Author.where(id: mary).merge(less_than_bob)
-    assert_equal [david, mary], Author.where(id: mary).merge(less_than_bob, rewhere: true)
+    assert_deprecated do
+      assert_equal [david, mary], Author.where(id: mary).merge(less_than_bob, rewhere: true)
+    end
   end
 
   def test_merge_doesnt_duplicate_same_clauses
@@ -174,15 +218,19 @@ def test_merge_doesnt_duplicate_same_clauses
       end
 
       assert_sql(/WHERE \(#{Regexp.escape(author_id)} IN \('1'\)\)\z/) do
-        assert_equal [david], only_david.merge(only_david, rewhere: true)
+        assert_deprecated do
+          assert_equal [david], only_david.merge(only_david, rewhere: true)
+        end
       end
     else
       assert_sql(/WHERE \(#{Regexp.escape(author_id)} IN \(1\)\)\z/) do
         assert_equal [david], only_david.merge(only_david)
       end
 
       assert_sql(/WHERE \(#{Regexp.escape(author_id)} IN \(1\)\)\z/) do
-        assert_equal [david], only_david.merge(only_david, rewhere: true)
+        assert_deprecated do
+          assert_equal [david], only_david.merge(only_david, rewhere: true)
+        end
       end
     end
   end
@@ -207,7 +255,9 @@ def test_relation_merging_with_arel_equalities_keeps_last_equality
     devs = Developer.where(salary_attr.eq(80000)).merge(Developer.where(salary_attr.eq(9000)))
     assert_equal [developers(:poor_jamis)], devs.to_a
 
-    devs = Developer.where(salary_attr.eq(80000)).merge(Developer.where(salary_attr.eq(9000)), rewhere: true)
+    assert_deprecated do
+      devs = Developer.where(salary_attr.eq(80000)).merge(Developer.where(salary_attr.eq(9000)), rewhere: true)
+    end
     assert_equal [developers(:poor_jamis)], devs.to_a
 
     devs = Developer.where(salary_attr.eq(80000)).rewhere(salary_attr.eq(9000))
@@ -221,7 +271,9 @@ def test_relation_merging_with_arel_equalities_keeps_last_equality_with_non_attr
     devs = Developer.where(abs_salary.eq(80000)).merge(Developer.where(abs_salary.eq(9000)))
     assert_equal [developers(:poor_jamis)], devs.to_a
 
-    devs = Developer.where(abs_salary.eq(80000)).merge(Developer.where(abs_salary.eq(9000)), rewhere: true)
+    assert_deprecated do
+      devs = Developer.where(abs_salary.eq(80000)).merge(Developer.where(abs_salary.eq(9000)), rewhere: true)
+    end
     assert_equal [developers(:poor_jamis)], devs.to_a
 
     devs = Developer.where(abs_salary.eq(80000)).rewhere(abs_salary.eq(9000))
@@ -347,6 +399,24 @@ def test_merging_duplicated_annotations
       Post.annotate("bar").merge(Post.annotate("foo")).merge(posts).to_a
     end
   end
+
+  def test_rewhere_true_is_deprecated
+    message = <<-MSG.squish
+      Specifying `Relation#merge(rewhere: true)` is deprecated
+    MSG
+    assert_deprecated(message) do
+      Author.where(id: 1).merge(Author.where(id: 2), rewhere: true)
+    end
+  end
+
+  def test_rewhere_false_is_deprecated
+    message = <<-MSG.squish
+      Relation#merge(rewhere: false)` is deprecated without replacement
+    MSG
+    assert_deprecated(message) do
+      Author.where(id: 1).merge(Author.where(id: 2), rewhere: false)
+    end
+  end
 end
 
 class MergingDifferentRelationsTest < ActiveRecord::TestCase