Deprecate defining scopes with a callable (lambda, proc, etc) via the scope class method. Just define a class method yourself instead.

1 files changed, 60 insertions, 2 deletions

diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 60840e6958..f1df04950b 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -51,6 +51,14 @@ module ActiveRecord
       # The above calls to <tt>scope</tt> define class methods Shirt.red and Shirt.dry_clean_only. Shirt.red,
       # in effect, represents the query <tt>Shirt.where(:color => 'red')</tt>.
       #
+      # Note that this is simply 'syntactic sugar' for defining an actual class method:
+      #
+      #   class Shirt < ActiveRecord::Base
+      #     def self.red
+      #       where(:color => 'red')
+      #     end
+      #   end
+      #
       # Unlike <tt>Shirt.find(...)</tt>, however, the object returned by Shirt.red is not an Array; it
       # resembles the association object constructed by a <tt>has_many</tt> declaration. For instance,
       # you can invoke <tt>Shirt.red.first</tt>, <tt>Shirt.red.count</tt>, <tt>Shirt.red.where(:size => 'small')</tt>.
@@ -74,14 +82,34 @@ module ActiveRecord
       # then <tt>elton.shirts.red.dry_clean_only</tt> will return all of Elton's red, dry clean
       # only shirts.
       #
-      # Named \scopes can also be procedural:
+      # If you need to pass parameters to a scope, define it as a normal method:
       #
       #   class Shirt < ActiveRecord::Base
-      #     scope :colored, lambda {|color| where(:color => color) }
+      #     def self.colored(color)
+      #       where(:color => color)
+      #     end
       #   end
       #
       # In this example, <tt>Shirt.colored('puce')</tt> finds all puce shirts.
       #
+      # Note that scopes defined with \scope will be evaluated when they are defined, rather than
+      # when they are used. For example, the following would be incorrect:
+      #
+      #   class Post < ActiveRecord::Base
+      #     scope :recent, where('published_at >= ?', Time.now - 1.week)
+      #   end
+      #
+      # The example above would be 'frozen' to the <tt>Time.now</tt> value when the <tt>Post</tt>
+      # class was defined, and so the resultant SQL query would always be the same. The correct
+      # way to do this would be via a class method, which will re-evaluate the scope each time
+      # it is called:
+      #
+      #   class Post < ActiveRecord::Base
+      #     def self.recent
+      #       where('published_at >= ?', Time.now - 1.week)
+      #     end
+      #   end
+      #
       # Named \scopes can also have extensions, just as with <tt>has_many</tt> declarations:
       #
       #   class Shirt < ActiveRecord::Base
@@ -92,6 +120,18 @@ module ActiveRecord
       #     end
       #   end
       #
+      # The above could also be written as a class method like so:
+      #
+      #   class Shirt < ActiveRecord::Base
+      #     def self.red
+      #       where(:color => 'red').extending do
+      #         def dom_id
+      #           'red_shirts'
+      #         end
+      #       end
+      #     end
+      #   end
+      #
       # Scopes can also be used while creating/building a record.
       #
       #   class Article < ActiveRecord::Base
@@ -128,6 +168,24 @@ module ActiveRecord
         valid_scope_name?(name)
         extension = Module.new(&Proc.new) if block_given?
 
+        if !scope_options.is_a?(Relation) && scope_options.respond_to?(:call)
+          ActiveSupport::Deprecation.warn <<-WARN
+Passing a proc (or other object that responds to #call) to scope is deprecated. If you need your scope to be lazily evaluated, or takes parameters, please define it as a normal class method instead. For example, change this:
+
+class Post < ActiveRecord::Base
+  scope :unpublished, lambda { where('published_at > ?', Time.now) }
+end
+
+To this:
+
+class Post < ActiveRecord::Base
+  def self.unpublished
+    where('published_at > ?', Time.now)
+  end
+end
+          WARN
+        end
+
         scope_proc = lambda do |*args|
           options = scope_options.respond_to?(:call) ? scope_options.call(*args) : scope_options
           options = scoped.apply_finder_options(options) if options.is_a?(Hash)