Merge pull request #41911 from Shopify/simplify-proxy-call

Allow to pass the method signature when defining attribute methods

activemodel/lib/active_model/attribute_methods.rb

@@ -67,6 +67,7 @@ module ActiveModel
 
     NAME_COMPILABLE_REGEXP = /\A[a-zA-Z_]\w*[!?=]?\z/
     CALL_COMPILABLE_REGEXP = /\A[a-zA-Z_]\w*[!?]?\z/
+    FORWARD_PARAMETERS = "*args"
 
     included do
       class_attribute :attribute_aliases, instance_writer: false, default: {}
@@ -105,8 +106,8 @@ module ActiveModel
       #   person.name          # => "Bob"
       #   person.clear_name
       #   person.name          # => nil
-      def attribute_method_prefix(*prefixes)
+      def attribute_method_prefix(*prefixes, parameters: nil)
-        self.attribute_method_matchers += prefixes.map! { |prefix| AttributeMethodMatcher.new prefix: prefix }
+        self.attribute_method_matchers += prefixes.map! { |prefix| AttributeMethodMatcher.new(prefix: prefix, parameters: parameters) }
         undefine_attribute_methods
       end
 
@@ -140,8 +141,8 @@ module ActiveModel
       #   person.name = 'Bob'
       #   person.name          # => "Bob"
       #   person.name_short?   # => true
-      def attribute_method_suffix(*suffixes)
+      def attribute_method_suffix(*suffixes, parameters: nil)
-        self.attribute_method_matchers += suffixes.map! { |suffix| AttributeMethodMatcher.new suffix: suffix }
+        self.attribute_method_matchers += suffixes.map! { |suffix| AttributeMethodMatcher.new(suffix: suffix, parameters: parameters) }
         undefine_attribute_methods
       end
 
@@ -177,7 +178,7 @@ module ActiveModel
       #   person.reset_name_to_default!
       #   person.name                         # => 'Default Name'
       def attribute_method_affix(*affixes)
-        self.attribute_method_matchers += affixes.map! { |affix| AttributeMethodMatcher.new prefix: affix[:prefix], suffix: affix[:suffix] }
+        self.attribute_method_matchers += affixes.map! { |affix| AttributeMethodMatcher.new(**affix) }
         undefine_attribute_methods
       end
 
@@ -211,7 +212,7 @@ module ActiveModel
           attribute_method_matchers.each do |matcher|
             matcher_new = matcher.method_name(new_name).to_s
             matcher_old = matcher.method_name(old_name).to_s
-            define_proxy_call owner, matcher_new, matcher_old
+            define_proxy_call(owner, matcher_new, matcher_old, matcher.parameters)
           end
         end
       end
@@ -296,7 +297,7 @@ module ActiveModel
               if respond_to?(generate_method, true)
                 send(generate_method, attr_name.to_s, owner: owner)
               else
-                define_proxy_call owner, method_name, matcher.target, attr_name.to_s
+                define_proxy_call(owner, method_name, matcher.target, matcher.parameters, attr_name.to_s)
               end
             end
           end
@@ -405,35 +406,47 @@ module ActiveModel
         # Define a method `name` in `mod` that dispatches to `send`
         # using the given `extra` args. This falls back on `define_method`
         # and `send` if the given names cannot be compiled.
-        def define_proxy_call(code_generator, name, target, *extra)
+        def define_proxy_call(code_generator, name, target, parameters, *call_args)
-          defn = if NAME_COMPILABLE_REGEXP.match?(name)
+          mangled_name = name
-            "def #{name}(*args)"
+          unless NAME_COMPILABLE_REGEXP.match?(name)
-          else
+            mangled_name = "__temp__#{name.unpack1("h*")}"
-            "define_method(:'#{name}') do |*args|"
           end
 
-          extra = (extra.map!(&:inspect) << "*args").join(", ")
+          call_args.map!(&:inspect)
+          call_args << parameters if parameters
 
           body = if CALL_COMPILABLE_REGEXP.match?(target)
-            "self.#{target}(#{extra})"
+            "self.#{target}(#{call_args.join(", ")})"
           else
-            "send(:'#{target}', #{extra})"
+            call_args.unshift(":'#{target}'")
+            "send(#{call_args.join(", ")})"
           end
 
           code_generator <<
-            defn <<
+            "def #{mangled_name}(#{parameters || ''})" <<
             body <<
-            "end" <<
+            "end"
-            "ruby2_keywords(:'#{name}')"
+
+          if parameters == FORWARD_PARAMETERS
+            code_generator << "ruby2_keywords(:'#{mangled_name}')"
+          end
+
+          if mangled_name != name
+            code_generator <<
+              "alias_method(:'#{name}', :'#{mangled_name}')" <<
+              "remove_method(:'#{mangled_name}')"
+          end
         end
 
         class AttributeMethodMatcher #:nodoc:
-          attr_reader :prefix, :suffix, :target
+          attr_reader :prefix, :suffix, :target, :parameters
 
           AttributeMethodMatch = Struct.new(:target, :attr_name)
 
-          def initialize(options = {})
+          def initialize(prefix: "", suffix: "", parameters: nil)
-            @prefix, @suffix = options.fetch(:prefix, ""), options.fetch(:suffix, "")
+            @prefix = prefix
+            @suffix = suffix
+            @parameters = parameters.nil? ? FORWARD_PARAMETERS : parameters
             @regex = /^(?:#{Regexp.escape(@prefix)})(.*)(?:#{Regexp.escape(@suffix)})$/
             @target = "#{@prefix}attribute#{@suffix}"
             @method_name = "#{prefix}%s#{suffix}"

activemodel/lib/active_model/attributes.rb

@@ -9,7 +9,7 @@ module ActiveModel
     include ActiveModel::AttributeMethods
 
     included do
-      attribute_method_suffix "="
+      attribute_method_suffix "=", parameters: "value"
       class_attribute :attribute_types, :_default_attributes, instance_accessor: false
       self.attribute_types = Hash.new(Type.default_value)
       self._default_attributes = AttributeSet.new({})

activemodel/lib/active_model/dirty.rb

@@ -123,10 +123,11 @@ module ActiveModel
     include ActiveModel::AttributeMethods
 
     included do
-      attribute_method_suffix "_changed?", "_change", "_will_change!", "_was"
+      attribute_method_suffix "_previously_changed?", "_changed?", parameters: "**options"
-      attribute_method_suffix "_previously_changed?", "_previous_change", "_previously_was"
+      attribute_method_suffix "_change", "_will_change!", "_was", parameters: false
-      attribute_method_affix prefix: "restore_", suffix: "!"
+      attribute_method_suffix "_previous_change", "_previously_was", parameters: false
-      attribute_method_affix prefix: "clear_", suffix: "_change"
+      attribute_method_affix prefix: "restore_", suffix: "!", parameters: false
+      attribute_method_affix prefix: "clear_", suffix: "_change", parameters: false
     end
 
     def initialize_dup(other) # :nodoc:

activemodel/test/models/topic.rb

@@ -6,7 +6,7 @@ class Topic
   include ActiveModel::AttributeMethods
   include ActiveSupport::NumberHelper
 
-  attribute_method_suffix "_before_type_cast"
+  attribute_method_suffix "_before_type_cast", parameters: false
   define_attribute_method :price
 
   def self._validates_default_keys

activerecord/lib/active_record/attribute_methods/before_type_cast.rb

@@ -29,8 +29,8 @@ module ActiveRecord
       extend ActiveSupport::Concern
 
       included do
-        attribute_method_suffix "_before_type_cast", "_for_database"
+        attribute_method_suffix "_before_type_cast", "_for_database", parameters: false
-        attribute_method_suffix "_came_from_user?"
+        attribute_method_suffix "_came_from_user?", parameters: false
       end
 
       # Returns the value of the attribute identified by +attr_name+ before

activerecord/lib/active_record/attribute_methods/dirty.rb

@@ -17,13 +17,13 @@ module ActiveRecord
         class_attribute :partial_writes, instance_writer: false, default: true
 
         # Attribute methods for "changed in last call to save?"
-        attribute_method_affix(prefix: "saved_change_to_", suffix: "?")
+        attribute_method_affix(prefix: "saved_change_to_", suffix: "?", parameters: "**options")
-        attribute_method_prefix("saved_change_to_")
+        attribute_method_prefix("saved_change_to_", parameters: false)
-        attribute_method_suffix("_before_last_save")
+        attribute_method_suffix("_before_last_save", parameters: false)
 
         # Attribute methods for "will change if I call save?"
-        attribute_method_affix(prefix: "will_save_change_to_", suffix: "?")
+        attribute_method_affix(prefix: "will_save_change_to_", suffix: "?", parameters: "**options")
-        attribute_method_suffix("_change_to_be_saved", "_in_database")
+        attribute_method_suffix("_change_to_be_saved", "_in_database", parameters: false)
       end
 
       # <tt>reload</tt> the record and clears changed attributes.

activerecord/lib/active_record/attribute_methods/query.rb

@@ -6,7 +6,7 @@ module ActiveRecord
       extend ActiveSupport::Concern
 
       included do
-        attribute_method_suffix "?"
+        attribute_method_suffix "?", parameters: false
       end
 
       def query_attribute(attr_name)

activerecord/lib/active_record/attribute_methods/write.rb

@@ -6,7 +6,7 @@ module ActiveRecord
       extend ActiveSupport::Concern
 
       included do
-        attribute_method_suffix "="
+        attribute_method_suffix "=", parameters: "value"
       end
 
       module ClassMethods # :nodoc: