From b2cd66ff9cee6d799b7fd8a0f33e7e0ee458cb0f Mon Sep 17 00:00:00 2001
From: Prem Sichanugrist <s@sikachu.com>
Date: Sun, 17 Jul 2011 19:30:11 -0400
Subject: [PATCH] Show a deprecation warning if user passing less number of
 argument in the dynamic finder

The previous behavior was unintentional, and some people was relying on it. In the next version of Rails, the dynamic finder will always expecting the number of arguments to be equal or greater (so you can still pass the options to it.) It will raise the ArgumentError otherwise.
---
 activerecord/CHANGELOG                      |  2 ++
 activerecord/lib/active_record/base.rb      | 15 +++++++++++++++
 activerecord/test/cases/finder_test.rb      |  8 ++++++++
 activerecord/test/cases/named_scope_test.rb | 15 +++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 4c58664a19..3d7b814daa 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *Rails 3.1.0 (unreleased)*
 
+* Active Record's dynamic finder will now show a deprecation warning if you passing in less number of arguments than what you call in method signature. This behavior will raise ArgumentError in the next version of Rails [Prem Sichanugrist]
+
 * Deprecated the AssociationCollection constant. CollectionProxy is now the appropriate constant
   to use, though be warned that this is not really a public API.
 
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 83c9d37140..c7e35029bd 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -23,6 +23,7 @@ require 'active_support/core_ext/module/delegation'
 require 'active_support/core_ext/module/introspection'
 require 'active_support/core_ext/object/duplicable'
 require 'active_support/core_ext/object/blank'
+require 'active_support/deprecation'
 require 'arel'
 require 'active_record/errors'
 require 'active_record/log_subscriber'
@@ -1056,6 +1057,13 @@ module ActiveRecord #:nodoc:
           if match = DynamicFinderMatch.match(method_id)
             attribute_names = match.attribute_names
             super unless all_attributes_exists?(attribute_names)
+            if arguments.size < attribute_names.size
+              ActiveSupport::Deprecation.warn(
+                "Calling dynamic finder with less number of arguments than the number of attributes in " \
+                "method name is deprecated and will raise an ArguementError in the next version of Rails. " \
+                "Please passing `nil' to the argument you want it to be nil."
+              )
+            end
             if match.finder?
               options = arguments.extract_options!
               relation = options.any? ? scoped(options) : scoped
@@ -1066,6 +1074,13 @@ module ActiveRecord #:nodoc:
           elsif match = DynamicScopeMatch.match(method_id)
             attribute_names = match.attribute_names
             super unless all_attributes_exists?(attribute_names)
+            if arguments.size < attribute_names.size
+              ActiveSupport::Deprecation.warn(
+                "Calling dynamic scope with less number of arguments than the number of attributes in " \
+                "method name is deprecated and will raise an ArguementError in the next version of Rails. " \
+                "Please passing `nil' to the argument you want it to be nil."
+              )
+            end
             if match.scope?
               self.class_eval <<-METHOD, __FILE__, __LINE__ + 1
                 def self.#{method_id}(*args)                                    # def self.scoped_by_user_name_and_password(*args)
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 3d2a03d2b9..528ee0ce0a 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -666,6 +666,10 @@ class FinderTest < ActiveRecord::TestCase
     assert_nil Topic.find_by_title_and_author_name("The First Topic", "Mary")
   end
 
+  def test_find_by_two_attributes_but_passing_only_one
+    assert_deprecated { Topic.find_by_title_and_author_name("The First Topic") }
+  end
+
   def test_find_last_by_one_attribute
     assert_equal Topic.last, Topic.find_last_by_title(Topic.last.title)
     assert_nil Topic.find_last_by_title("A title with no matches")
@@ -859,6 +863,10 @@ class FinderTest < ActiveRecord::TestCase
     assert !sig38.persisted?
   end
 
+  def test_find_or_initialize_from_two_attributes_but_passing_only_one
+    assert_deprecated { Topic.find_or_initialize_by_title_and_author_name("Another topic") }
+  end
+
   def test_find_or_initialize_from_one_aggregate_attribute
     new_customer = Customer.find_or_initialize_by_balance(Money.new(123))
     assert_equal 123, new_customer.balance.amount
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 34188e4915..0cc73e0409 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -489,6 +489,17 @@ end
 class DynamicScopeTest < ActiveRecord::TestCase
   fixtures :posts
 
+  def setup
+    # Ensure we start with a clean model with no generated class method
+    Post.methods.select{ |c| c =~ /scoped_by_/ }.each do |method_name|
+      Post.class_eval <<-RUBY
+        class << self
+          remove_method :#{method_name}
+        end
+      RUBY
+    end
+  end
+
   def test_dynamic_scope
     assert_equal Post.scoped_by_author_id(1).find(1), Post.find(1)
     assert_equal Post.scoped_by_author_id_and_title(1, "Welcome to the weblog").first, Post.find(:first, :conditions => { :author_id => 1, :title => "Welcome to the weblog"})
@@ -499,4 +510,8 @@ class DynamicScopeTest < ActiveRecord::TestCase
     Developer.scoped_by_created_at(nil)
     assert_present Developer.methods.grep(/scoped_by_created_at/)
   end
+
+  def test_dynamic_scope_with_less_number_of_arguments
+    assert_deprecated { Post.scoped_by_author_id_and_title(1) }
+  end
 end