Simplify/fix implementation of default scopes

The previous implementation was necessary in order to support stuff
like:

    class Post < ActiveRecord::Base
      default_scope where(published: true)
      scope :ordered, order("created_at")
    end

If we didn't evaluate the default scope at the last possible moment
before sending the SQL to the database, it would become impossible to
do:

    Post.unscoped.ordered

This is because the default scope would already be bound up in the
"ordered" scope, and therefore wouldn't be removed by the
"Post.unscoped" part.

In 4.0, we have deprecated all "eager" forms of scopes. So now you must
write:

    class Post < ActiveRecord::Base
      default_scope { where(published: true) }
      scope :ordered, -> { order("created_at") }
    end

This prevents the default scope getting bound up inside the "ordered"
scope, which means we can now have a simpler/better/more natural
implementation of default scoping.

A knock on effect is that some things that didn't work properly now do.
For example it was previously impossible to use #except to remove a part
of the default scope, since the default scope was evaluated after the
call to #except.

12 files changed, 31 insertions, 74 deletions

diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index ee62298793..3fbdf9eb0b 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -122,11 +122,7 @@ module ActiveRecord
       # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
       # through association's scope)
       def target_scope
-        all = klass.all
-        scope = AssociationRelation.new(klass, klass.arel_table, self)
-        scope.merge! all
-        scope.default_scoped = all.default_scoped?
-        scope
+        AssociationRelation.new(klass, klass.arel_table, self).merge!(klass.all)
       end
 
       # Loads the \target if needed and returns it.
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 7cdb5ba5b3..f836d45d0e 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -33,7 +33,6 @@ module ActiveRecord
       def initialize(klass, association) #:nodoc:
         @association = association
         super klass, klass.arel_table
-        self.default_scoped = true
         merge! association.scope(nullify: false)
       end
 
diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb
index 82588905c6..d31f7432cd 100644
--- a/activerecord/lib/active_record/associations/preloader/association.rb
+++ b/activerecord/lib/active_record/associations/preloader/association.rb
@@ -98,7 +98,6 @@ module ActiveRecord
 
         def build_scope
           scope = klass.unscoped
-          scope.default_scoped = true
 
           values         = reflection_scope.values
           preload_values = preload_scope.values
@@ -113,7 +112,7 @@ module ActiveRecord
             scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name })
           end
 
-          scope
+          klass.default_scoped.merge(scope)
         end
       end
     end
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index 35f29b37a2..bd99997e7f 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -15,7 +15,7 @@ module ActiveRecord
           scope = super
           chain[1..-1].each do |reflection|
             scope.merge!(
-              reflection.klass.all.with_default_scope.
+              reflection.klass.all.
                 except(:select, :create_with, :includes, :preload, :joins, :eager_load)
             )
           end
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index d37471e9ad..e6bfa2a969 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -17,17 +17,14 @@ module ActiveRecord
     include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation
 
     attr_reader :table, :klass, :loaded
-    attr_accessor :default_scoped
     alias :model :klass
     alias :loaded? :loaded
-    alias :default_scoped? :default_scoped
 
     def initialize(klass, table, values = {})
-      @klass             = klass
-      @table             = table
-      @values            = values
-      @loaded            = false
-      @default_scoped    = false
+      @klass  = klass
+      @table  = table
+      @values = values
+      @loaded = false
     end
 
     def initialize_copy(other)
@@ -313,7 +310,7 @@ module ActiveRecord
       stmt.table(table)
       stmt.key = table[primary_key]
 
-      if with_default_scope.joins_values.any?
+      if joins_values.any?
         @klass.connection.join_to_update(stmt, arel)
       else
         stmt.take(arel.limit)
@@ -438,7 +435,7 @@ module ActiveRecord
         stmt = Arel::DeleteManager.new(arel.engine)
         stmt.from(table)
 
-        if with_default_scope.joins_values.any?
+        if joins_values.any?
           @klass.connection.join_to_delete(stmt, arel, table[primary_key])
         else
           stmt.wheres = arel.constraints
@@ -512,12 +509,11 @@ module ActiveRecord
     #   User.where(name: 'Oscar').where_values_hash
     #   # => {name: "Oscar"}
     def where_values_hash
-      scope = with_default_scope
-      equalities = scope.where_values.grep(Arel::Nodes::Equality).find_all { |node|
+      equalities = where_values.grep(Arel::Nodes::Equality).find_all { |node|
         node.left.relation.name == table_name
       }
 
-      binds = Hash[scope.bind_values.find_all(&:first).map { |column, v| [column.name, v] }]
+      binds = Hash[bind_values.find_all(&:first).map { |column, v| [column.name, v] }]
       binds.merge!(Hash[bind_values.find_all(&:first).map { |column, v| [column.name, v] }])
 
       Hash[equalities.map { |where|
@@ -565,16 +561,6 @@ module ActiveRecord
       q.pp(self.to_a)
     end
 
-    def with_default_scope #:nodoc:
-      if default_scoped? && default_scope = klass.send(:build_default_scope)
-        default_scope = default_scope.merge(self)
-        default_scope.default_scoped = false
-        default_scope
-      else
-        self
-      end
-    end
-
     # Returns true if relation is blank.
     def blank?
       to_a.blank?
@@ -594,22 +580,16 @@ module ActiveRecord
     private
 
     def exec_queries
-      default_scoped = with_default_scope
-
-      if default_scoped.equal?(self)
-        @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
+      @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
 
-        preload = preload_values
-        preload +=  includes_values unless eager_loading?
-        preload.each do |associations|
-          ActiveRecord::Associations::Preloader.new(@records, associations).run
-        end
-
-        @records.each { |record| record.readonly! } if readonly_value
-      else
-        @records = default_scoped.to_a
+      preload = preload_values
+      preload +=  includes_values unless eager_loading?
+      preload.each do |associations|
+        ActiveRecord::Associations::Preloader.new(@records, associations).run
       end
 
+      @records.each { |record| record.readonly! } if readonly_value
+
       @loaded = true
       @records
     end
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 4becf3980d..f2d28db923 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -91,20 +91,14 @@ module ActiveRecord
     #
     #   Person.sum("2 * age")
     def calculate(operation, column_name, options = {})
-      relation = with_default_scope
-
       if column_name.is_a?(Symbol) && attribute_aliases.key?(column_name.to_s)
         column_name = attribute_aliases[column_name.to_s].to_sym
       end
 
-      if relation.equal?(self)
-        if has_include?(column_name)
-          construct_relation_for_association_calculations.calculate(operation, column_name, options)
-        else
-          perform_calculation(operation, column_name, options)
-        end
+      if has_include?(column_name)
+        construct_relation_for_association_calculations.calculate(operation, column_name, options)
       else
-        relation.calculate(operation, column_name, options)
+        perform_calculation(operation, column_name, options)
       end
     end
 
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 3ea3c33fcc..4ea13e2585 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -211,7 +211,6 @@ module ActiveRecord
         relation = relation.where(table[primary_key].eq(conditions)) if conditions != :none
       end
 
-      relation = relation.with_default_scope
       connection.select_value(relation.arel, "#{name} Exists", relation.bind_values)
     end
 
@@ -354,7 +353,7 @@ module ActiveRecord
       if loaded?
         @records.first
       else
-        @first ||= find_first_with_limit(with_default_scope.order_values, 1).first
+        @first ||= find_first_with_limit(order_values, 1).first
       end
     end
 
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index c114ea0c0d..6d047e0331 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -42,10 +42,6 @@ module ActiveRecord
       attr_reader :relation, :values, :other
 
       def initialize(relation, other)
-        if other.default_scoped? && other.klass != relation.klass
-          other = other.with_default_scope
-        end
-
         @relation = relation
         @values   = other.values
         @other    = other
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 0200fcf69b..05e92bea33 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -795,7 +795,7 @@ module ActiveRecord
 
     # Returns the Arel object associated with the relation.
     def arel
-      @arel ||= with_default_scope.build_arel
+      @arel ||= build_arel
     end
 
     # Like #arel, but ignores the default scope of the model.
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index de784f9f57..c63ae9c9fb 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -65,7 +65,6 @@ module ActiveRecord
 
       def relation_with(values) # :nodoc:
         result = Relation.new(klass, table, values)
-        result.default_scoped = default_scoped
         result.extend(*extending_values) if extending_values.any?
         result
       end
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index da73bead32..e7d5e6ce84 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -25,22 +25,18 @@ module ActiveRecord
           if current_scope
             current_scope.clone
           else
-            scope = relation
-            scope.default_scoped = true
-            scope
+            default_scoped
           end
         end
 
+        def default_scoped # :nodoc:
+          relation.merge(build_default_scope)
+        end
+
         # Collects attributes from scopes that should be applied when creating
         # an AR instance for the particular class this is called on.
         def scope_attributes # :nodoc:
-          if current_scope
-            current_scope.scope_for_create
-          else
-            scope = relation
-            scope.default_scoped = true
-            scope.scope_for_create
-          end
+          all.scope_for_create
         end
 
         # Are there default attributes associated with this scope?
diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb
index 0f69443839..5c7751b445 100644
--- a/activerecord/test/cases/scoping/default_scoping_test.rb
+++ b/activerecord/test/cases/scoping/default_scoping_test.rb
@@ -153,9 +153,8 @@ class DefaultScopingTest < ActiveRecord::TestCase
   end
 
   def test_order_to_unscope_reordering
-    expected = DeveloperOrderedBySalary.all.collect { |dev| [dev.name, dev.id] }
-    received = DeveloperOrderedBySalary.order('salary DESC, name ASC').reverse_order.unscope(:order).collect { |dev| [dev.name, dev.id] }
-    assert_equal expected, received
+    scope = DeveloperOrderedBySalary.order('salary DESC, name ASC').reverse_order.unscope(:order)
+    assert !(scope.to_sql =~ /order/i)
   end
 
   def test_unscope_reverse_order