Deprecate eager-evaluated scopes.

Don't use this:

    scope :red, where(color: 'red')
    default_scope where(color: 'red')

Use this:

    scope :red, -> { where(color: 'red') }
    default_scope { where(color: 'red') }

The former has numerous issues. It is a common newbie gotcha to do
the following:

    scope :recent, where(published_at: Time.now - 2.weeks)

Or a more subtle variant:

    scope :recent, -> { where(published_at: Time.now - 2.weeks) }
    scope :recent_red, recent.where(color: 'red')

Eager scopes are also very complex to implement within Active
Record, and there are still bugs. For example, the following does
not do what you expect:

    scope :remove_conditions, except(:where)
    where(...).remove_conditions # => still has conditions

2 files changed, 31 insertions, 12 deletions

diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb
index 5f05d146f2..b0609a8c08 100644
--- a/activerecord/lib/active_record/scoping/default.rb
+++ b/activerecord/lib/active_record/scoping/default.rb
@@ -1,4 +1,5 @@
 require 'active_support/concern'
+require 'active_support/deprecation'
 
 module ActiveRecord
   module Scoping
@@ -51,7 +52,7 @@ module ActiveRecord
         # the model.
         #
         #   class Article < ActiveRecord::Base
-        #     default_scope where(:published => true)
+        #     default_scope { where(:published => true) }
         #   end
         #
         #   Article.all # => SELECT * FROM articles WHERE published = true
@@ -62,12 +63,6 @@ module ActiveRecord
         #   Article.new.published    # => true
         #   Article.create.published # => true
         #
-        # You can also use <tt>default_scope</tt> with a block, in order to have it lazily evaluated:
-        #
-        #   class Article < ActiveRecord::Base
-        #     default_scope { where(:published_at => Time.now - 1.week) }
-        #   end
-        #
         # (You can also pass any object which responds to <tt>call</tt> to the <tt>default_scope</tt>
         # macro, and it will be called when building the default scope.)
         #
@@ -75,8 +70,8 @@ module ActiveRecord
         # be merged together:
         #
         #   class Article < ActiveRecord::Base
-        #     default_scope where(:published => true)
-        #     default_scope where(:rating => 'G')
+        #     default_scope { where(:published => true) }
+        #     default_scope { where(:rating => 'G') }
         #   end
         #
         #   Article.all # => SELECT * FROM articles WHERE published = true AND rating = 'G'
@@ -94,6 +89,16 @@ module ActiveRecord
         #   end
         def default_scope(scope = {})
           scope = Proc.new if block_given?
+
+          if scope.is_a?(Relation) || !scope.respond_to?(:call)
+            ActiveSupport::Deprecation.warn(
+              "Calling #default_scope without a block is deprecated. For example instead " \
+              "of `default_scope where(color: 'red')`, please use " \
+              "`default_scope { where(color: 'red') }`. (Alternatively you can just redefine " \
+              "self.default_scope.)"
+            )
+          end
+
           self.default_scopes = default_scopes + [scope]
         end
 
@@ -106,7 +111,7 @@ module ActiveRecord
                 if scope.is_a?(Hash)
                   default_scope.apply_finder_options(scope)
                 elsif !scope.is_a?(Relation) && scope.respond_to?(:call)
-                  default_scope.merge(scope.call)
+                  default_scope.merge(unscoped { scope.call })
                 else
                   default_scope.merge(scope)
                 end
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index 2f266eff79..077e2d067e 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -3,6 +3,7 @@ require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/kernel/singleton_class'
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/class/attribute'
+require 'active_support/deprecation'
 
 module ActiveRecord
   # = Active Record Named \Scopes
@@ -171,11 +172,24 @@ module ActiveRecord
         #   Article.published.featured.latest_article
         #   Article.featured.titles
 
-        def scope(name, scope_options = {}, &block)
+        def scope(name, body = {}, &block)
           extension = Module.new(&block) if block
 
+          # Check body.is_a?(Relation) to prevent the relation actually being
+          # loaded by respond_to?
+          if body.is_a?(Relation) || !body.respond_to?(:call)
+            ActiveSupport::Deprecation.warn(
+              "Using #scope without passing a callable object is deprecated. For " \
+              "example `scope :red, where(color: 'red')` should be changed to " \
+              "`scope :red, -> { where(color: 'red') }`. There are numerous gotchas " \
+              "in the former usage and it makes the implementation more complicated " \
+              "and buggy. (If you prefer, you can just define a class method named " \
+              "`self.red`.)"
+            )
+          end
+
           singleton_class.send(:define_method, name) do |*args|
-            options = scope_options.respond_to?(:call) ? unscoped { scope_options.call(*args) } : scope_options
+            options = body.respond_to?(:call) ? unscoped { body.call(*args) } : body
             options = scoped.apply_finder_options(options) if options.is_a?(Hash)
 
             relation = scoped.merge(options)