`WhereClause#predicates` does not need to be public

The only place it was accessed was in tests. Many of them have another
way that they can test their behavior, that doesn't involve reaching
into internals as far as they did. `AssociationScopeTest` is testing a
situation where the where clause would have one bind param per
predicate, so it can just ignore the predicates entirely. The where
chain test was primarly duplicating the logic tested on `WhereClause`
directly, so I instead just make sure it calls the appropriate method
which is fully tested in isolation.

10 files changed, 45 insertions, 125 deletions

diff --git a/activerecord/test/cases/associations/association_scope_test.rb b/activerecord/test/cases/associations/association_scope_test.rb
index dd26a85e44..fc5dbd0890 100644
--- a/activerecord/test/cases/associations/association_scope_test.rb
+++ b/activerecord/test/cases/associations/association_scope_test.rb
@@ -8,12 +8,7 @@ module ActiveRecord
       test 'does not duplicate conditions' do
         scope = AssociationScope.scope(Author.new.association(:welcome_posts),
                                         Author.connection)
-        wheres = scope.where_clause.predicates.map(&:right)
         binds = scope.where_clause.binds.map(&:last)
-        wheres.reject! { |node|
-          Arel::Nodes::BindParam === node
-        }
-        assert_equal wheres.uniq, wheres
         assert_equal binds.uniq, binds
       end
     end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 5ef84562ad..19f4384e83 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -316,16 +316,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   # would be convenient), because this would cause that scope to be applied to any callbacks etc.
   def test_build_and_create_should_not_happen_within_scope
     car = cars(:honda)
-    scoped_count = car.foo_bulbs.where_clause.predicates.count
+    scope = car.foo_bulbs.where_values_hash
 
     bulb = car.foo_bulbs.build
-    assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
+    assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
 
     bulb = car.foo_bulbs.create
-    assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
+    assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
 
     bulb = car.foo_bulbs.create!
-    assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
+    assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
   end
 
   def test_no_sql_should_be_fired_if_association_already_loaded
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index c02509db46..3b6a4038cd 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -237,16 +237,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
 
   def test_build_and_create_should_not_happen_within_scope
     pirate = pirates(:blackbeard)
-    scoped_count = pirate.association(:foo_bulb).scope.where_clause.predicates.count
+    scope = pirate.association(:foo_bulb).scope.where_values_hash
 
     bulb = pirate.build_foo_bulb
-    assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
+    assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
 
     bulb = pirate.create_foo_bulb
-    assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
+    assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
 
     bulb = pirate.create_foo_bulb!
-    assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
+    assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
   end
 
   def test_create_association
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index c88dc58950..de358114ab 100644
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -238,7 +238,7 @@ class AssociationProxyTest < ActiveRecord::TestCase
   end
 
   def test_scoped_allows_conditions
-    assert developers(:david).projects.merge!(where: 'foo').where_clause.predicates.include?('foo')
+    assert developers(:david).projects.merge(where: 'foo').to_sql.include?('foo')
   end
 
   test "getting a scope from an association" do
diff --git a/activerecord/test/cases/relation/where_chain_test.rb b/activerecord/test/cases/relation/where_chain_test.rb
index 00c9a001c0..27bbd80f79 100644
--- a/activerecord/test/cases/relation/where_chain_test.rb
+++ b/activerecord/test/cases/relation/where_chain_test.rb
@@ -11,22 +11,11 @@ module ActiveRecord
       @name = 'title'
     end
 
-    def test_not_eq
+    def test_not_inverts_where_clause
       relation = Post.where.not(title: 'hello')
+      expected_where_clause = Post.where(title: 'hello').where_clause.invert
 
-      assert_equal 1, relation.where_clause.predicates.length
-
-      value = relation.where_clause.predicates.first
-      bind  = relation.where_clause.binds.first
-
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
-      assert_equal 'hello', bind.last
-    end
-
-    def test_not_null
-      expected = Post.arel_table[@name].not_eq(nil)
-      relation = Post.where.not(title: nil)
-      assert_equal([expected], relation.where_clause.predicates)
+      assert_equal expected_where_clause, relation.where_clause
     end
 
     def test_not_with_nil
@@ -35,146 +24,81 @@ module ActiveRecord
       end
     end
 
-    def test_not_in
-      expected = Post.arel_table[@name].not_in(%w[hello goodbye])
-      relation = Post.where.not(title: %w[hello goodbye])
-      assert_equal([expected], relation.where_clause.predicates)
-    end
-
     def test_association_not_eq
-      expected = Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new)
+      expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new))
       relation = Post.joins(:comments).where.not(comments: {title: 'hello'})
-      assert_equal(expected.to_sql, relation.where_clause.predicates.first.to_sql)
+      assert_equal(expected.to_sql, relation.where_clause.ast.to_sql)
     end
 
     def test_not_eq_with_preceding_where
       relation = Post.where(title: 'hello').where.not(title: 'world')
+      expected_where_clause =
+        Post.where(title: 'hello').where_clause +
+        Post.where(title: 'world').where_clause.invert
 
-      value = relation.where_clause.predicates.first
-      bind  = relation.where_clause.binds.first
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality
-      assert_equal 'hello', bind.last
-
-      value = relation.where_clause.predicates.last
-      bind  = relation.where_clause.binds.last
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
-      assert_equal 'world', bind.last
+      assert_equal expected_where_clause, relation.where_clause
     end
 
     def test_not_eq_with_succeeding_where
       relation = Post.where.not(title: 'hello').where(title: 'world')
+      expected_where_clause =
+        Post.where(title: 'hello').where_clause.invert +
+        Post.where(title: 'world').where_clause
 
-      value = relation.where_clause.predicates.first
-      bind  = relation.where_clause.binds.first
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
-      assert_equal 'hello', bind.last
-
-      value = relation.where_clause.predicates.last
-      bind  = relation.where_clause.binds.last
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality
-      assert_equal 'world', bind.last
-    end
-
-    def test_not_eq_with_string_parameter
-      expected = Arel::Nodes::Not.new("title = 'hello'")
-      relation = Post.where.not("title = 'hello'")
-      assert_equal([expected], relation.where_clause.predicates)
-    end
-
-    def test_not_eq_with_array_parameter
-      expected = Arel::Nodes::Not.new("title = 'hello'")
-      relation = Post.where.not(['title = ?', 'hello'])
-      assert_equal([expected], relation.where_clause.predicates)
+      assert_equal expected_where_clause, relation.where_clause
     end
 
     def test_chaining_multiple
       relation = Post.where.not(author_id: [1, 2]).where.not(title: 'ruby on rails')
+      expected_where_clause =
+        Post.where(author_id: [1, 2]).where_clause.invert +
+        Post.where(title: 'ruby on rails').where_clause.invert
 
-      expected = Post.arel_table['author_id'].not_in([1, 2])
-      assert_equal(expected, relation.where_clause.predicates[0])
-
-      value = relation.where_clause.predicates[1]
-      bind  = relation.where_clause.binds.first
-
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
-      assert_equal 'ruby on rails', bind.last
+      assert_equal expected_where_clause, relation.where_clause
     end
 
     def test_rewhere_with_one_condition
       relation = Post.where(title: 'hello').where(title: 'world').rewhere(title: 'alone')
+      expected = Post.where(title: 'alone')
 
-      assert_equal 1, relation.where_clause.predicates.size
-      value = relation.where_clause.predicates.first
-      bind = relation.where_clause.binds.first
-      assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality
-      assert_equal 'alone', bind.last
+      assert_equal expected.where_clause, relation.where_clause
     end
 
     def test_rewhere_with_multiple_overwriting_conditions
       relation = Post.where(title: 'hello').where(body: 'world').rewhere(title: 'alone', body: 'again')
+      expected = Post.where(title: 'alone', body: 'again')
 
-      assert_equal 2, relation.where_clause.predicates.size
-
-      value = relation.where_clause.predicates.first
-      bind = relation.where_clause.binds.first
-      assert_bound_ast value, Post.arel_table['title'], Arel::Nodes::Equality
-      assert_equal 'alone', bind.last
-
-      value = relation.where_clause.predicates[1]
-      bind = relation.where_clause.binds[1]
-      assert_bound_ast value, Post.arel_table['body'], Arel::Nodes::Equality
-      assert_equal 'again', bind.last
-    end
-
-    def assert_bound_ast value, table, type
-      assert_equal table, value.left
-      assert_kind_of type, value
-      assert_kind_of Arel::Nodes::BindParam, value.right
+      assert_equal expected.where_clause, relation.where_clause
     end
 
     def test_rewhere_with_one_overwriting_condition_and_one_unrelated
       relation = Post.where(title: 'hello').where(body: 'world').rewhere(title: 'alone')
+      expected = Post.where(body: 'world', title: 'alone')
 
-      assert_equal 2, relation.where_clause.predicates.size
-
-      value = relation.where_clause.predicates.first
-      bind  = relation.where_clause.binds.first
-
-      assert_bound_ast value, Post.arel_table['body'], Arel::Nodes::Equality
-      assert_equal 'world', bind.last
-
-      value = relation.where_clause.predicates.second
-      bind  = relation.where_clause.binds.second
-
-      assert_bound_ast value, Post.arel_table['title'], Arel::Nodes::Equality
-      assert_equal 'alone', bind.last
+      assert_equal expected.where_clause, relation.where_clause
     end
 
     def test_rewhere_with_range
       relation = Post.where(comments_count: 1..3).rewhere(comments_count: 3..5)
 
-      assert_equal 1, relation.where_clause.predicates.size
       assert_equal Post.where(comments_count: 3..5), relation
     end
 
     def test_rewhere_with_infinite_upper_bound_range
       relation = Post.where(comments_count: 1..Float::INFINITY).rewhere(comments_count: 3..5)
 
-      assert_equal 1, relation.where_clause.predicates.size
       assert_equal Post.where(comments_count: 3..5), relation
     end
 
     def test_rewhere_with_infinite_lower_bound_range
       relation = Post.where(comments_count: -Float::INFINITY..1).rewhere(comments_count: 3..5)
 
-      assert_equal 1, relation.where_clause.predicates.size
       assert_equal Post.where(comments_count: 3..5), relation
     end
 
     def test_rewhere_with_infinite_range
       relation = Post.where(comments_count: -Float::INFINITY..Float::INFINITY).rewhere(comments_count: 3..5)
 
-      assert_equal 1, relation.where_clause.predicates.size
       assert_equal Post.where(comments_count: 3..5), relation
     end
   end
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index 511d595f71..f3644154b6 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -112,11 +112,12 @@ class ActiveRecord::Relation
     end
 
     test "ast groups its predicates with AND" do
-      where_clause = WhereClause.new([
+      predicates = [
         table["id"].in([1, 2, 3]),
         table["name"].eq(bind_param),
-      ], [])
-      expected = Arel::Nodes::And.new(where_clause.predicates)
+      ]
+      where_clause = WhereClause.new(predicates, [])
+      expected = Arel::Nodes::And.new(predicates)
 
       assert_equal expected, where_clause.ast
     end
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index d75c5435c6..194faa9473 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -156,7 +156,7 @@ module ActiveRecord
       relation = Relation.new(FakeKlass, :b, nil)
       relation = relation.merge where: :lol, readonly: true
 
-      assert_equal [:lol], relation.where_clause.predicates
+      assert_equal Relation::WhereClause.new([:lol], []), relation.where_clause
       assert_equal true, relation.readonly_value
     end
 
@@ -191,7 +191,7 @@ module ActiveRecord
 
       relation = Relation.new(klass, :b, nil)
       relation.merge!(where: ['foo = ?', 'bar'])
-      assert_equal ['foo = bar'], relation.where_clause.predicates
+      assert_equal Relation::WhereClause.new(['foo = bar'], []), relation.where_clause
     end
 
     def test_merging_readonly_false
diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb
index d0835ba3e5..4137b20c4a 100644
--- a/activerecord/test/cases/scoping/default_scoping_test.rb
+++ b/activerecord/test/cases/scoping/default_scoping_test.rb
@@ -426,19 +426,19 @@ class DefaultScopingTest < ActiveRecord::TestCase
 
   test "additional conditions are ANDed with the default scope" do
     scope = DeveloperCalledJamis.where(name: "David")
-    assert_equal 2, scope.where_clause.predicates.length
+    assert_equal 2, scope.where_clause.ast.children.length
     assert_equal [], scope.to_a
   end
 
   test "additional conditions in a scope are ANDed with the default scope" do
     scope = DeveloperCalledJamis.david
-    assert_equal 2, scope.where_clause.predicates.length
+    assert_equal 2, scope.where_clause.ast.children.length
     assert_equal [], scope.to_a
   end
 
   test "a scope can remove the condition from the default scope" do
     scope = DeveloperCalledJamis.david2
-    assert_equal 1, scope.where_clause.predicates.length
-    assert_equal Developer.where(name: "David").map(&:id), scope.map(&:id)
+    assert_equal 1, scope.where_clause.ast.children.length
+    assert_equal Developer.where(name: "David"), scope
   end
 end
diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb
index 57c9ff6a8d..8cd94ebcc2 100644
--- a/activerecord/test/cases/scoping/named_scoping_test.rb
+++ b/activerecord/test/cases/scoping/named_scoping_test.rb
@@ -380,8 +380,8 @@ class NamedScopingTest < ActiveRecord::TestCase
   end
 
   def test_should_not_duplicates_where_values
-    where_values = Topic.where("1=1").scope_with_lambda.where_clause.predicates
-    assert_equal ["1=1"], where_values
+    relation = Topic.where("1=1")
+    assert_equal relation.where_clause, relation.scope_with_lambda.where_clause
   end
 
   def test_chaining_with_duplicate_joins
diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb
index 73760e628b..02b32abebf 100644
--- a/activerecord/test/cases/scoping/relation_scoping_test.rb
+++ b/activerecord/test/cases/scoping/relation_scoping_test.rb
@@ -184,7 +184,7 @@ class RelationScopingTest < ActiveRecord::TestCase
     rescue
     end
 
-    assert !Developer.all.where_clause.predicates.include?("name = 'Jamis'")
+    assert_not Developer.all.to_sql.include?("name = 'Jamis'"), "scope was not restored"
   end
 
   def test_default_scope_filters_on_joins