From 5d86e32ae2c62efe61fd55ca21737a59e99db07d Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 25 May 2021 15:53:54 +0200 Subject: [PATCH 1/2] Fix delegation in ActiveModel::Type.lookup * Without the change the new test fails like this: Failure: ActiveModel::TypeTest#test_registering_a_new_type [test/cases/type_test.rb:21]: Expected: # Actual: # * (*args, **kwargs)-delegation is not correct on Ruby 2.7 unless the target always accepts keyword arguments (not the case for `Struct.new(:args).new`). See https://eregon.me/blog/2021/02/13/correct-delegation-in-ruby-2-27-3.html --- activemodel/CHANGELOG.md | 2 +- activemodel/lib/active_model/type.rb | 5 +++-- activemodel/test/cases/type_test.rb | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index d899f9602c658..af13b78a8e67d 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,4 +1,4 @@ -* Fix delegation in ActiveModel::Type::Registry#lookup +* Fix delegation in ActiveModel::Type::Registry#lookup and ActiveModel::Type.lookup Passing a last positional argument `{}` would be incorrectly considered as keyword argument. diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index 735d0e7f57a91..0dc7b5b32391a 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -30,9 +30,10 @@ def register(type_name, klass = nil, &block) registry.register(type_name, klass, &block) end - def lookup(*args, **kwargs) # :nodoc: - registry.lookup(*args, **kwargs) + def lookup(*args) # :nodoc: + registry.lookup(*args) end + ruby2_keywords(:lookup) def default_value # :nodoc: @default_value ||= Value.new diff --git a/activemodel/test/cases/type_test.rb b/activemodel/test/cases/type_test.rb index 70f3f6052e691..43951e0259c4c 100644 --- a/activemodel/test/cases/type_test.rb +++ b/activemodel/test/cases/type_test.rb @@ -18,6 +18,7 @@ class TypeTest < ActiveModel::TestCase ActiveModel::Type.register(:foo, type) assert_equal type.new(:arg), ActiveModel::Type.lookup(:foo, :arg) + assert_equal type.new({}), ActiveModel::Type.lookup(:foo, {}) end end end From 191ee5eec93074e2c109fea5770c54e973901a1b Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 25 May 2021 16:06:15 +0200 Subject: [PATCH 2/2] Prefer (...) over ruby2_keywords for ActiveModel::Type.lookup * (...) is already used in several places in Rails and looks nicer. --- activemodel/lib/active_model/type.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index 0dc7b5b32391a..3a0e8abcc3f43 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -30,10 +30,9 @@ def register(type_name, klass = nil, &block) registry.register(type_name, klass, &block) end - def lookup(*args) # :nodoc: - registry.lookup(*args) + def lookup(...) # :nodoc: + registry.lookup(...) end - ruby2_keywords(:lookup) def default_value # :nodoc: @default_value ||= Value.new