Remove support to activerecord-deprecated_finders

16 files changed, 64 insertions, 130 deletions

diff --git a/RELEASING_RAILS.rdoc b/RELEASING_RAILS.rdoc
index 4aab3b35ba..ee5e6a91c1 100644
--- a/RELEASING_RAILS.rdoc
+++ b/RELEASING_RAILS.rdoc
@@ -33,7 +33,6 @@ after some refactoring or bug fix, so it is important to check if the following
 are working with the versions that will be released:
 
 * https://github.com/rails/protected_attributes
-* https://github.com/rails/activerecord-deprecated_finders
 
 Do not release red plugins tests.
 
diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index 947d61ee7b..88406740d8 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -1,5 +1,3 @@
-require 'active_support/core_ext/module/attribute_accessors'
-
 # This is the parent Association class which defines the variables
 # used by all associations.
 #
@@ -15,15 +13,10 @@ module ActiveRecord::Associations::Builder
   class Association #:nodoc:
     class << self
       attr_accessor :extensions
-      # TODO: This class accessor is needed to make activerecord-deprecated_finders work.
-      # We can move it to a constant in 5.0.
-      attr_accessor :valid_options
     end
     self.extensions = []
 
-    self.valid_options = [:class_name, :class, :foreign_key, :validate]
-
-    attr_reader :name, :scope, :options
+    VALID_OPTIONS = [:class_name, :class, :foreign_key, :validate] # :nodoc:
 
     def self.build(model, name, scope, options, &block)
       if model.dangerous_attribute_method?(name)
@@ -32,57 +25,60 @@ module ActiveRecord::Associations::Builder
                              "Please choose a different association name."
       end
 
-      builder = create_builder model, name, scope, options, &block
-      reflection = builder.build(model)
+      extension = define_extensions model, name, &block
+      reflection = create_reflection model, name, scope, options, extension
       define_accessors model, reflection
       define_callbacks model, reflection
       define_validations model, reflection
-      builder.define_extensions model
       reflection
     end
 
-    def self.create_builder(model, name, scope, options, &block)
+    def self.create_reflection(model, name, scope, options, extension = nil)
       raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol)
 
-      new(model, name, scope, options, &block)
-    end
-
-    def initialize(model, name, scope, options)
-      # TODO: Move this to create_builder as soon we drop support to activerecord-deprecated_finders.
       if scope.is_a?(Hash)
         options = scope
         scope   = nil
       end
 
-      # TODO: Remove this model argument as soon we drop support to activerecord-deprecated_finders.
-      @name    = name
-      @scope   = scope
-      @options = options
+      validate_options(options)
 
-      validate_options
+      scope = build_scope(scope, extension)
+
+      ActiveRecord::Reflection.create(macro, name, scope, options, model)
+    end
+
+    def self.build_scope(scope, extension)
+      new_scope = scope
 
       if scope && scope.arity == 0
-        @scope = proc { instance_exec(&scope) }
+        new_scope = proc { instance_exec(&scope) }
+      end
+
+      if extension
+        new_scope = wrap_scope new_scope, extension
       end
+
+      new_scope
     end
 
-    def build(model)
-      ActiveRecord::Reflection.create(macro, name, scope, options, model)
+    def self.wrap_scope(scope, extension)
+      scope
     end
 
-    def macro
+    def self.macro
       raise NotImplementedError
     end
 
-    def valid_options
-      Association.valid_options + Association.extensions.flat_map(&:valid_options)
+    def self.valid_options(options)
+      VALID_OPTIONS + Association.extensions.flat_map(&:valid_options)
     end
 
-    def validate_options
-      options.assert_valid_keys(valid_options)
+    def self.validate_options(options)
+      options.assert_valid_keys(valid_options(options))
     end
 
-    def define_extensions(model)
+    def self.define_extensions(model, name)
     end
 
     def self.define_callbacks(model, reflection)
@@ -133,8 +129,6 @@ module ActiveRecord::Associations::Builder
       raise NotImplementedError
     end
 
-    private
-
     def self.check_dependent_options(dependent)
       unless valid_dependent_options.include? dependent
         raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{dependent}"
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index 954ea3878a..d0ad57f9c6 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -1,10 +1,10 @@
 module ActiveRecord::Associations::Builder
   class BelongsTo < SingularAssociation #:nodoc:
-    def macro
+    def self.macro
       :belongs_to
     end
 
-    def valid_options
+    def self.valid_options(options)
       super + [:foreign_type, :polymorphic, :touch, :counter_cache]
     end
 
@@ -23,8 +23,6 @@ module ActiveRecord::Associations::Builder
       add_counter_cache_methods mixin
     end
 
-    private
-
     def self.add_counter_cache_methods(mixin)
       return if mixin.method_defined? :belongs_to_counter_cache_after_update
 
diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index bc15a49996..2ff67f904d 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -7,22 +7,11 @@ module ActiveRecord::Associations::Builder
 
     CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove]
 
-    def valid_options
+    def self.valid_options(options)
       super + [:table_name, :before_add,
                :after_add, :before_remove, :after_remove, :extend]
     end
 
-    attr_reader :block_extension
-
-    def initialize(model, name, scope, options)
-      super
-      @mod = nil
-      if block_given?
-        @mod = Module.new(&Proc.new)
-        @scope = wrap_scope @scope, @mod
-      end
-    end
-
     def self.define_callbacks(model, reflection)
       super
       name    = reflection.name
@@ -32,10 +21,11 @@ module ActiveRecord::Associations::Builder
       }
     end
 
-    def define_extensions(model)
-      if @mod
+    def self.define_extensions(model, name)
+      if block_given?
         extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension"
-        model.parent.const_set(extension_module_name, @mod)
+        extension = Module.new(&Proc.new)
+        model.parent.const_set(extension_module_name, extension)
       end
     end
 
@@ -78,9 +68,7 @@ module ActiveRecord::Associations::Builder
       CODE
     end
 
-    private
-
-    def wrap_scope(scope, mod)
+    def self.wrap_scope(scope, mod)
       if scope
         proc { |owner| instance_exec(owner, &scope).extending(mod) }
       else
diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
index 092b4ebd2f..93dc4ae118 100644
--- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -87,11 +87,11 @@ module ActiveRecord::Associations::Builder
       middle_name = [lhs_model.name.downcase.pluralize,
                      association_name].join('_').gsub(/::/, '_').to_sym
       middle_options = middle_options join_model
-      hm_builder = HasMany.create_builder(lhs_model,
-                                          middle_name,
-                                          nil,
-                                          middle_options)
-      hm_builder.build lhs_model
+
+      HasMany.create_reflection(lhs_model,
+                                middle_name,
+                                nil,
+                                middle_options)
     end
 
     private
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index 1b87f92170..1c1b47bd56 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -1,10 +1,10 @@
 module ActiveRecord::Associations::Builder
   class HasMany < CollectionAssociation #:nodoc:
-    def macro
+    def self.macro
       :has_many
     end
 
-    def valid_options
+    def self.valid_options(options)
       super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache, :join_table, :foreign_type]
     end
 
diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb
index 1387717396..64e9e6b334 100644
--- a/activerecord/lib/active_record/associations/builder/has_one.rb
+++ b/activerecord/lib/active_record/associations/builder/has_one.rb
@@ -1,10 +1,10 @@
 module ActiveRecord::Associations::Builder
   class HasOne < SingularAssociation #:nodoc:
-    def macro
+    def self.macro
       :has_one
     end
 
-    def valid_options
+    def self.valid_options(options)
       valid = super + [:as, :foreign_type]
       valid += [:through, :source, :source_type] if options[:through]
       valid
@@ -14,8 +14,6 @@ module ActiveRecord::Associations::Builder
       [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception]
     end
 
-    private
-
     def self.add_destroy_callbacks(model, reflection)
       super unless reflection.options[:through]
     end
diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb
index 6e6dd7204c..1369212837 100644
--- a/activerecord/lib/active_record/associations/builder/singular_association.rb
+++ b/activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -2,7 +2,7 @@
 
 module ActiveRecord::Associations::Builder
   class SingularAssociation < Association #:nodoc:
-    def valid_options
+    def self.valid_options(options)
       super + [:dependent, :primary_key, :inverse_of, :required]
     end
 
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 16b1228b8a..f2c96e9a2a 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -218,11 +218,7 @@ module ActiveRecord
 
       # Count all records using SQL.  Construct options and pass them with
       # scope to the target class's +count+.
-      def count(column_name = nil, count_options = {})
-        # TODO: Remove count_options argument as soon we remove support to
-        # activerecord-deprecated_finders.
-        column_name, count_options = nil, column_name if column_name.is_a?(Hash)
-
+      def count(column_name = nil)
         relation = scope
         if association_scope.distinct_value
           # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index dc42b19a83..c22dc6e11e 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -688,10 +688,8 @@ module ActiveRecord
       #   #       #<Pet id: 2, name: "Spook", person_id: 1>,
       #   #       #<Pet id: 3, name: "Choo-Choo", person_id: 1>
       #   #    ]
-      def count(column_name = nil, options = {})
-        # TODO: Remove options argument as soon we remove support to
-        # activerecord-deprecated_finders.
-        @association.count(column_name, options)
+      def count(column_name = nil)
+        @association.count(column_name)
       end
 
       # Returns the size of the collection. If the collection hasn't been loaded,
diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb
index e94b74063e..b6dd6814db 100644
--- a/activerecord/lib/active_record/dynamic_matchers.rb
+++ b/activerecord/lib/active_record/dynamic_matchers.rb
@@ -1,10 +1,5 @@
 module ActiveRecord
   module DynamicMatchers #:nodoc:
-    # This code in this file seems to have a lot of indirection, but the indirection
-    # is there to provide extension points for the activerecord-deprecated_finders
-    # gem. When we stop supporting activerecord-deprecated_finders (from Rails 5),
-    # then we can remove the indirection.
-
     def respond_to?(name, include_private = false)
       if self == Base
         super
@@ -72,26 +67,14 @@ module ActiveRecord
         CODE
       end
 
-      def body
-        raise NotImplementedError
-      end
-    end
+      private
 
-    module Finder
-      # Extended in activerecord-deprecated_finders
       def body
-        result
-      end
-
-      # Extended in activerecord-deprecated_finders
-      def result
         "#{finder}(#{attributes_hash})"
       end
 
       # The parameters in the signature may have reserved Ruby words, in order
       # to prevent errors, we start each param name with `_`.
-      #
-      # Extended in activerecord-deprecated_finders
       def signature
         attribute_names.map { |name| "_#{name}" }.join(', ')
       end
@@ -109,7 +92,6 @@ module ActiveRecord
 
     class FindBy < Method
       Method.matchers << self
-      include Finder
 
       def self.prefix
         "find_by"
@@ -122,7 +104,6 @@ module ActiveRecord
 
     class FindByBang < Method
       Method.matchers << self
-      include Finder
 
       def self.prefix
         "find_by"
diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb
index 807c301596..b406da14dc 100644
--- a/activerecord/lib/active_record/null_relation.rb
+++ b/activerecord/lib/active_record/null_relation.rb
@@ -62,9 +62,7 @@ module ActiveRecord
       calculate :maximum, nil
     end
 
-    def calculate(operation, _column_name, _options = {})
-      # TODO: Remove _options argument as soon we remove support to
-      # activerecord-deprecated_finders.
+    def calculate(operation, _column_name)
       if [:count, :sum, :size].include? operation
         group_values.any? ? Hash.new : 0
       elsif [:average, :minimum, :maximum].include?(operation) && group_values.any?
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 71673324eb..1d4cb1a83b 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -35,21 +35,16 @@ module ActiveRecord
     #
     # Note: not all valid +select+ expressions are valid +count+ expressions. The specifics differ
     # between databases. In invalid cases, an error from the database is thrown.
-    def count(column_name = nil, options = {})
-      # TODO: Remove options argument as soon we remove support to
-      # activerecord-deprecated_finders.
-      column_name, options = nil, column_name if column_name.is_a?(Hash)
-      calculate(:count, column_name, options)
+    def count(column_name = nil)
+      calculate(:count, column_name)
     end
 
     # Calculates the average value on a given column. Returns +nil+ if there's
     # no row. See +calculate+ for examples with options.
     #
     #   Person.average(:age) # => 35.8
-    def average(column_name, options = {})
-      # TODO: Remove options argument as soon we remove support to
-      # activerecord-deprecated_finders.
-      calculate(:average, column_name, options)
+    def average(column_name)
+      calculate(:average, column_name)
     end
 
     # Calculates the minimum value on a given column. The value is returned
@@ -57,10 +52,8 @@ module ActiveRecord
     # +calculate+ for examples with options.
     #
     #   Person.minimum(:age) # => 7
-    def minimum(column_name, options = {})
-      # TODO: Remove options argument as soon we remove support to
-      # activerecord-deprecated_finders.
-      calculate(:minimum, column_name, options)
+    def minimum(column_name)
+      calculate(:minimum, column_name)
     end
 
     # Calculates the maximum value on a given column. The value is returned
@@ -68,10 +61,8 @@ module ActiveRecord
     # +calculate+ for examples with options.
     #
     #   Person.maximum(:age) # => 93
-    def maximum(column_name, options = {})
-      # TODO: Remove options argument as soon we remove support to
-      # activerecord-deprecated_finders.
-      calculate(:maximum, column_name, options)
+    def maximum(column_name)
+      calculate(:maximum, column_name)
     end
 
     # Calculates the sum of values on a given column. The value is returned
@@ -114,17 +105,15 @@ module ActiveRecord
     #   Person.group(:last_name).having("min(age) > 17").minimum(:age)
     #
     #   Person.sum("2 * age")
-    def calculate(operation, column_name, options = {})
-      # TODO: Remove options argument as soon we remove support to
-      # activerecord-deprecated_finders.
+    def calculate(operation, column_name)
       if column_name.is_a?(Symbol) && attribute_alias?(column_name)
         column_name = attribute_alias(column_name)
       end
 
       if has_include?(column_name)
-        construct_relation_for_association_calculations.calculate(operation, column_name, options)
+        construct_relation_for_association_calculations.calculate(operation, column_name)
       else
-        perform_calculation(operation, column_name, options)
+        perform_calculation(operation, column_name)
       end
     end
 
@@ -196,9 +185,7 @@ module ActiveRecord
       eager_loading? || (includes_values.present? && ((column_name && column_name != :all) || references_eager_loaded_tables?))
     end
 
-    def perform_calculation(operation, column_name, options = {})
-      # TODO: Remove options argument as soon we remove support to
-      # activerecord-deprecated_finders.
+    def perform_calculation(operation, column_name)
       operation = operation.to_s.downcase
 
       # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count)
diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb
index 9d373cd73b..b161cde335 100644
--- a/activerecord/test/cases/associations/extension_test.rb
+++ b/activerecord/test/cases/associations/extension_test.rb
@@ -76,7 +76,6 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase
   private
 
     def extend!(model)
-      builder = ActiveRecord::Associations::Builder::HasMany.new(model, :association_name, nil, {}) { }
-      builder.define_extensions(model)
+      ActiveRecord::Associations::Builder::HasMany.define_extensions(model, :association_name) { }
     end
 end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 0210ae6487..fb9258c038 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -358,7 +358,7 @@ class RelationTest < ActiveRecord::TestCase
   def test_null_relation_calculations_methods
     assert_no_queries(ignore_none: false) do
       assert_equal 0, Developer.none.count
-      assert_equal 0, Developer.none.calculate(:count, nil, {})
+      assert_equal 0, Developer.none.calculate(:count, nil)
       assert_equal nil, Developer.none.calculate(:average, 'salary')
     end
   end
diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md
index 476456eef7..e0a9add2be 100644
--- a/guides/source/active_record_querying.md
+++ b/guides/source/active_record_querying.md
@@ -1389,8 +1389,6 @@ NOTE: Remember that, if `find_by` returns more than one registry, it will take j
 Find or Build a New Object
 --------------------------
 
-NOTE: Some dynamic finders were deprecated in Rails 4.0 and removed in Rails 4.1. The best practice is to use Active Record scopes instead. You can find the deprecation gem at https://github.com/rails/activerecord-deprecated_finders
-
 It's common that you need to find a record or create it if it doesn't exist. You can do that with the `find_or_create_by` and `find_or_create_by!` methods.
 
 ### `find_or_create_by`