Improve model attribute accessor method names for backtraces

Ruby uses the original method name, so will show the __temp__ method
name in the backtrace. However, in the common case the method name
is compatible with the `def` keyword, so we can avoid the __temp__
method name in that case to improve the name shown in backtraces
or TracePoint#method_id.

activemodel/lib/active_model/attribute_methods.rb

@@ -474,5 +474,43 @@ module ActiveModel
       def _read_attribute(attr)
         __send__(attr)
       end
+
+      module AttrNames # :nodoc:
+        DEF_SAFE_NAME = /\A[a-zA-Z_]\w*\z/
+
+        # We want to generate the methods via module_eval rather than
+        # define_method, because define_method is slower on dispatch.
+        # Evaluating many similar methods may use more memory as the instruction
+        # sequences are duplicated and cached (in MRI).  define_method may
+        # be slower on dispatch, but if you're careful about the closure
+        # created, then define_method will consume much less memory.
+        #
+        # But sometimes the database might return columns with
+        # characters that are not allowed in normal method names (like
+        # 'my_column(omg)'. So to work around this we first define with
+        # the __temp__ identifier, and then use alias method to rename
+        # it to what we want.
+        #
+        # We are also defining a constant to hold the frozen string of
+        # the attribute name. Using a constant means that we do not have
+        # to allocate an object on each call to the attribute method.
+        # Making it frozen means that it doesn't get duped when used to
+        # key the @attributes in read_attribute.
+        def self.define_attribute_accessor_method(mod, attr_name, writer: false)
+          method_name = "#{attr_name}#{'=' if writer}"
+          if attr_name.ascii_only? && DEF_SAFE_NAME.match?(attr_name)
+            yield method_name, "'#{attr_name}'.freeze"
+          else
+            safe_name = attr_name.unpack1("h*")
+            const_name = "ATTR_#{safe_name}"
+            const_set(const_name, attr_name) unless const_defined?(const_name)
+            temp_method_name = "__temp__#{safe_name}#{'=' if writer}"
+            attr_name_expr = "::ActiveModel::AttributeMethods::AttrNames::#{const_name}"
+            yield temp_method_name, attr_name_expr
+            mod.send(:alias_method, method_name, temp_method_name)
+            mod.send(:undef_method, temp_method_name)
+          end
+        end
+      end
   end
 end

activemodel/lib/active_model/attributes.rb

@@ -29,17 +29,16 @@ module ActiveModel
       private
 
         def define_method_attribute=(name)
-          safe_name = name.unpack1("h*")
-          ActiveModel::AttributeMethods::AttrNames.set_name_cache safe_name, name
-
-          generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
-            def __temp__#{safe_name}=(value)
-              name = ::ActiveModel::AttributeMethods::AttrNames::ATTR_#{safe_name}
-              write_attribute(name, value)
-            end
-            alias_method #{(name + '=').inspect}, :__temp__#{safe_name}=
-            undef_method :__temp__#{safe_name}=
-          STR
+          ActiveModel::AttributeMethods::AttrNames.define_attribute_accessor_method(
+            generated_attribute_methods, name, writer: true,
+          ) do |temp_method_name, attr_name_expr|
+            generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1
+              def #{temp_method_name}(value)
+                name = #{attr_name_expr}
+                write_attribute(name, value)
+              end
+            RUBY
+          end
         end
 
         NO_DEFAULT_PROVIDED = Object.new # :nodoc:
@@ -97,15 +96,4 @@ module ActiveModel
         write_attribute(attribute_name, value)
       end
   end
-
-  module AttributeMethods #:nodoc:
-    AttrNames = Module.new {
-      def self.set_name_cache(name, value)
-        const_name = "ATTR_#{name}"
-        unless const_defined? const_name
-          const_set const_name, -value
-        end
-      end
-    }
-  end
 end

activerecord/lib/active_record/attribute_methods.rb

@@ -22,15 +22,6 @@ module ActiveRecord
       delegate :column_for_attribute, to: :class
     end
 
-    AttrNames = Module.new {
-      def self.set_name_cache(name, value)
-        const_name = "ATTR_#{name}"
-        unless const_defined? const_name
-          const_set const_name, -value
-        end
-      end
-    }
-
     RESTRICTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass)
 
     class GeneratedAttributeMethods < Module #:nodoc:

activerecord/lib/active_record/attribute_methods/read.rb

@@ -8,42 +8,19 @@ module ActiveRecord
       module ClassMethods # :nodoc:
         private
 
-          # We want to generate the methods via module_eval rather than
-          # define_method, because define_method is slower on dispatch.
-          # Evaluating many similar methods may use more memory as the instruction
-          # sequences are duplicated and cached (in MRI).  define_method may
-          # be slower on dispatch, but if you're careful about the closure
-          # created, then define_method will consume much less memory.
-          #
-          # But sometimes the database might return columns with
-          # characters that are not allowed in normal method names (like
-          # 'my_column(omg)'. So to work around this we first define with
-          # the __temp__ identifier, and then use alias method to rename
-          # it to what we want.
-          #
-          # We are also defining a constant to hold the frozen string of
-          # the attribute name. Using a constant means that we do not have
-          # to allocate an object on each call to the attribute method.
-          # Making it frozen means that it doesn't get duped when used to
-          # key the @attributes in read_attribute.
           def define_method_attribute(name)
-            safe_name = name.unpack1("h*")
-            temp_method = "__temp__#{safe_name}"
-
-            ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name
             sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key
 
-            generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
-              def #{temp_method}
-                #{sync_with_transaction_state}
-                name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name}
-                _read_attribute(name) { |n| missing_attribute(n, caller) }
-              end
-            STR
-
-            generated_attribute_methods.module_eval do
-              alias_method name, temp_method
-              undef_method temp_method
+            ActiveModel::AttributeMethods::AttrNames.define_attribute_accessor_method(
+              generated_attribute_methods, name
+            ) do |temp_method_name, attr_name_expr|
+              generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1
+                def #{temp_method_name}
+                  #{sync_with_transaction_state}
+                  name = #{attr_name_expr}
+                  _read_attribute(name) { |n| missing_attribute(n, caller) }
+                end
+              RUBY
             end
           end
       end

activerecord/lib/active_record/attribute_methods/write.rb

@@ -13,19 +13,19 @@ module ActiveRecord
         private
 
           def define_method_attribute=(name)
-            safe_name = name.unpack1("h*")
-            ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name
             sync_with_transaction_state = "sync_with_transaction_state" if name == primary_key
 
-            generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
-              def __temp__#{safe_name}=(value)
-                name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name}
-                #{sync_with_transaction_state}
-                _write_attribute(name, value)
-              end
-              alias_method #{(name + '=').inspect}, :__temp__#{safe_name}=
-              undef_method :__temp__#{safe_name}=
-            STR
+            ActiveModel::AttributeMethods::AttrNames.define_attribute_accessor_method(
+              generated_attribute_methods, name, writer: true,
+            ) do |temp_method_name, attr_name_expr|
+              generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1
+                def #{temp_method_name}(value)
+                  name = #{attr_name_expr}
+                  #{sync_with_transaction_state}
+                  _write_attribute(name, value)
+                end
+              RUBY
+            end
           end
       end