From a6621912fb55328ab2ebf1dc8c5505d07f17eb60 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 6 Sep 2024 20:36:17 +0200 Subject: [PATCH] alias_attribute: handle user defined source methods `alias_attribute` used to define a "jump method", e.g. `alias_attribute :foo, :bar` was pretty much a macro to generate ```ruby def foo bar end ``` This is convienient because it's easy, it doesn't impose an order of declaration or anything like that. But it's also much less efficient than a true `alias_method`. It also used to cause cache size explosion which we fixed in https://github.com/rails/rails/pull/52118, but making it behave like Ruby's `alias_method`, by doing a real alias. But this breaks some expectations (literally from the documentation): ```ruby attr_accessor :name attribute_method_suffix '_short?' define_attribute_methods :name alias_attribute :nickname, :name ``` Here we're not supposed to alias a generated method, but a user defined one. So this assumption can only hold for Active Record, not Active Model. --- .../lib/active_model/attribute_methods.rb | 11 +++-- .../test/cases/attribute_methods_test.rb | 40 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 521907a443fbe..4e42d864e3f81 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -215,7 +215,12 @@ def eagerly_generate_alias_attribute_methods(new_name, old_name) # :nodoc: end def generate_alias_attribute_methods(code_generator, new_name, old_name) - define_attribute_method(old_name, _owner: code_generator, as: new_name) + ActiveSupport::CodeGenerator.batch(code_generator, __FILE__, __LINE__) do |owner| + attribute_method_patterns.each do |pattern| + alias_attribute_method_definition(code_generator, pattern, new_name, old_name) + end + attribute_method_patterns_cache.clear + end end def alias_attribute_method_definition(code_generator, pattern, new_name, old_name) # :nodoc: @@ -228,7 +233,7 @@ def alias_attribute_method_definition(code_generator, pattern, new_name, old_nam call_args = [] call_args << parameters if parameters - define_call(code_generator, method_name, target_name, mangled_name, parameters, call_args, namespace: :alias_attribute) + define_call(code_generator, method_name, target_name, mangled_name, parameters, call_args, namespace: :alias_attribute, as: method_name) end # Is +new_name+ an alias? @@ -441,7 +446,7 @@ def build_mangled_name(name) mangled_name = name unless NAME_COMPILABLE_REGEXP.match?(name) - mangled_name = "__temp__#{name.unpack1("h*")}" + mangled_name = :"__temp__#{name.unpack1("h*")}" end mangled_name diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb index 6d72854bc5fa8..349aeec376548 100644 --- a/activemodel/test/cases/attribute_methods_test.rb +++ b/activemodel/test/cases/attribute_methods_test.rb @@ -371,4 +371,44 @@ def attribute?(name) assert_equal :model_1, NameClash::Model1.new.x_changed? assert_equal :model_2, NameClash::Model2.new.x_changed? end + + test "alias attribute respects user defined method" do + model = Class.new do + include ActiveModel::AttributeMethods + + attr_accessor :name + define_attribute_methods :name + + alias_attribute :nickname, :name + + def initialize(name) + @name = name + end + end + + instance = model.new("George") + assert_equal "George", instance.name + assert_equal "George", instance.nickname + end + + test "alias attribute respects user defined method in parent classes" do + model = Class.new do + include ActiveModel::AttributeMethods + + attr_accessor :name + define_attribute_methods :name + + def initialize(name) + @name = name + end + end + + subclass = Class.new(model) do + alias_attribute :nickname, :name + end + + instance = subclass.new("George") + assert_equal "George", instance.name + assert_equal "George", instance.nickname + end end