diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb5463..9c27973 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Your contribution here. * [#404](https://github.com/ruby-grape/grape-entity/pull/404): Drop `MultiJson` dependency, use `Hash#to_json` for ActiveSupport-aware serialization - [@numbata](https://github.com/numbata). +* [#406](https://github.com/ruby-grape/grape-entity/pull/406): Handle symbol-to-proc wrappers (`&:method_name`) where the method uses `delegate` or `method_missing`, and let unknown methods raise a native `NoMethodError` - [@marcrohloff](https://github.com/marcrohloff). ### 1.0.4 (2026-04-17) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index bd36d20..4b409c1 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -291,7 +291,7 @@ def self.documentation end # This allows you to declare a Proc in which exposures can be formatted with. - # It take a block with an arity of 1 which is passed as the value of the exposed attribute. + # It takes a block with a single argument which is passed as the value of the exposed attribute. # # @param name [Symbol] the name of the formatter # @param block [Proc] the block that will interpret the exposed attribute @@ -534,26 +534,53 @@ def exec_with_object(options, &block) end def ensure_block_arity!(block) - # MRI currently always includes "( &:foo )" for symbol-to-proc wrappers. - # If this format changes in a new Ruby version, this logic must be updated. - origin_method_name = block.to_s.scan(/(?<=\(&:)[^)]+(?=\))/).first&.to_sym - return unless origin_method_name - - unless object.respond_to?(origin_method_name, true) - raise ArgumentError, <<~MSG - Cannot use `&:#{origin_method_name}` because that method is not defined in the object. - MSG - end + # Strict anchor to match MRI Proc#to_s format for symbol-to-proc: # + match = block.to_s.match(/\A#.+)\) \(lambda\)>\z/) + return unless match # Unrecognized format -> bail safe rather than misidentify + + origin_method_name = match[:name].to_sym + required_positional_arg_count, required_keyword_arg_count, variadic_positional = + arity_requirement_for(origin_method_name) + return unless required_positional_arg_count - arity = object.method(origin_method_name).arity - return if arity.zero? + required_arguments = + required_arguments_summary(required_positional_arg_count, required_keyword_arg_count, variadic_positional) raise ArgumentError, <<~MSG - Cannot use `&:#{origin_method_name}` because that method expects #{arity} argument#{'s' if arity != 1}. - Symbol‐to‐proc shorthand only works for zero‐argument methods. + Cannot use `&:#{origin_method_name}` because that method expects #{required_arguments}. + Symbol-to-proc shorthand only works for methods that can be called with no arguments. MSG end + def arity_requirement_for(method_name) + origin_method = object.method(method_name) + parameters = origin_method.parameters + + required_positional_arg_count = parameters.count { |type, _| type == :req } + required_keyword_arg_count = parameters.count { |type, _| type == :keyreq } + return nil if required_positional_arg_count.zero? && required_keyword_arg_count.zero? + + [required_positional_arg_count, required_keyword_arg_count, parameters.any? { |type, _| type == :rest }] + rescue NameError + # Delegation wrappers and method_missing proxies may not expose a Method; let Ruby raise natively at call time. + nil + end + + def required_arguments_summary(required_positional_arg_count, required_keyword_arg_count, variadic_positional) + parts = [] + unless required_positional_arg_count.zero? + suffix = required_positional_arg_count == 1 ? 'argument' : 'arguments' + suffix += ' or more' if variadic_positional + parts << "#{required_positional_arg_count} #{suffix}" + end + unless required_keyword_arg_count.zero? + suffix = required_keyword_arg_count == 1 ? 'keyword argument' : 'keyword arguments' + parts << "#{required_keyword_arg_count} #{suffix}" + end + + parts.join(' and ') + end + def symbol_to_proc_wrapper?(block) params = block.parameters diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 3d8a74a..aa49785 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -408,6 +408,34 @@ def method_with_one_arg(_object) 'result' end + def method_with_only_optional_args(_optional1 = 1, _optional2 = 2) + 'result' + end + + def method_with_required_and_optional_args(_required_arg, _optional1 = 1, _optional2 = 2) + 'result' + end + + def method_with_optional_args_as_a_splat(*_optional_args) + 'result' + end + + def method_with_optional_kwargs_as_a_splat(**_kwargs) + 'result' + end + + def method_with_required_kwargs(_required:) + 'result' + end + + def method_with_optional_args_and_kwargs_as_a_splat(*_args, **_kwargs) + 'result' + end + + def method_with_required_args_and_an_optional_splat(_required_arg, *_optional_args) + 'result' + end + def method_with_multiple_args(_object, _options) 'result' end @@ -415,6 +443,60 @@ def method_with_multiple_args(_object, _options) def raises_argument_error raise ArgumentError, 'something different' end + + private + + def private_method_with_required_arg(_required_arg) + 'result' + end + end + + class SomeObjectWithMethodMissing + def method_missing(method, ...) + if method.to_sym == :method_without_args + method_without_args_impl(...) + elsif method.to_sym == :method_with_args + method_with_args_impl(...) + else + super + end + end + + def method_without_args_impl + 'result' + end + + def method_with_args_impl(_required_arg) + 'result' + end + + private + + def respond_to_missing?(method, include_private = false) # rubocop:disable Style/OptionalBooleanParameter + method.to_sym == :method_without_args || + method.to_sym == :method_with_args || + super + end + end + + class SomeObjectWithDelegation + class InnerDelegate + def method_without_args + 'result' + end + + def method_with_args(_required_arg) + 'result' + end + end + + delegate :method_without_args, + :method_with_args, + to: :delegate_object + + def delegate_object + @delegate_object ||= InnerDelegate.new + end end describe 'with block passed in' do @@ -457,6 +539,198 @@ def raises_argument_error value = subject.represent(object).value_for(:that_method_without_args_again) expect(value).to eq('result') end + + it 'raises a curated `ArgumentError` for private methods with required arguments' do + subject.expose :that_private_method_with_required_arg, &:private_method_with_required_arg + + object = SomeObject.new + + expect do + subject.represent(object).value_for(:that_private_method_with_required_arg) + end.to raise_error ArgumentError, include('method expects 1 argument.') + end + + it 'treats a source-less lambda shaped like symbol-to-proc as a one-argument exposure block' do + block = lambda do |object, *args| + raise ArgumentError, "expected no trailing args, got #{args.inspect}" unless args.empty? + + object.method_without_args + end + + def block.source_location + nil + end + + def block.to_s + '#' + end + + subject.expose :that_method_without_args, &block + + object = SomeObject.new + + expect(subject.represent(object).value_for(:that_method_without_args)).to eq('result') + end + end + + context 'with block passed in via & that references a method with optional args' do + it 'succeeds if there are no required arguments' do + subject.expose :that_method_with_only_optional_args, &:method_with_only_optional_args + subject.expose :method_with_only_optional_args, as: :that_method_with_only_optional_args_again + + object = SomeObject.new + + expect(subject.represent(object).serializable_hash).to eq( + that_method_with_only_optional_args: 'result', + that_method_with_only_optional_args_again: 'result' + ) + end + + it 'raises an `ArgumentError` if there are required arguments' do + subject.expose :that_method_with_required_and_optional_args, &:method_with_required_and_optional_args + subject.expose :method_with_required_and_optional_args, as: :that_method_with_required_and_optional_args_again + + object = SomeObject.new + + expect do + subject.represent(object).value_for(:that_method_with_required_and_optional_args) + end.to raise_error ArgumentError, include('method expects 1 argument.') + + expect do + subject.represent(object).value_for(:that_method_with_required_and_optional_args_again) + end.to raise_error ArgumentError, /given 0, expected 1/ + end + end + + context 'with block passed in via & that references a method with optional args as a splat' do + specify do + subject.expose :that_method_with_optional_args_as_a_splat, &:method_with_optional_args_as_a_splat + subject.expose :method_with_optional_args_as_a_splat, as: :that_method_with_optional_args_as_a_splat_again + + object = SomeObject.new + + expect(subject.represent(object).serializable_hash).to eq( + that_method_with_optional_args_as_a_splat: 'result', + that_method_with_optional_args_as_a_splat_again: 'result' + ) + end + + it 'raises an `ArgumentError` if there are required arguments' do + subject.expose :that_method_with_required_args_and_an_optional_splat, &:method_with_required_args_and_an_optional_splat + subject.expose :method_with_required_args_and_an_optional_splat, as: :that_method_with_required_args_and_an_optional_splat_again + + object = SomeObject.new + + expect do + subject.represent(object).value_for(:that_method_with_required_args_and_an_optional_splat) + end.to raise_error ArgumentError, include('method expects 1 argument or more.') + + expect do + subject.represent(object).value_for(:that_method_with_required_args_and_an_optional_splat_again) + end.to raise_error ArgumentError, /given 0, expected 1/ + end + end + + context 'with block passed in via & that references a method with optional kwargs as a splat' do + specify do + subject.expose :that_method_with_optional_kwargs_as_a_splat, &:method_with_optional_kwargs_as_a_splat + subject.expose :method_with_optional_kwargs_as_a_splat, as: :that_method_with_optional_kwargs_as_a_splat_again + + object = SomeObject.new + + expect(subject.represent(object).serializable_hash).to eq( + that_method_with_optional_kwargs_as_a_splat: 'result', + that_method_with_optional_kwargs_as_a_splat_again: 'result' + ) + end + end + + context 'with block passed in via & that references a method with required kwargs' do + it 'raises an `ArgumentError` if there are required keyword arguments' do + subject.expose :that_method_with_required_kwargs, &:method_with_required_kwargs + + object = SomeObject.new + + expect do + subject.represent(object).value_for(:that_method_with_required_kwargs) + end.to raise_error ArgumentError, include('method expects 1 keyword argument.') + end + end + + context 'with block passed in via & that references a method with optional args and kwargs as a splat' do + specify do + subject.expose :that_method_with_optional_args_and_kwargs_as_a_splat, &:method_with_optional_args_and_kwargs_as_a_splat + subject.expose :method_with_optional_args_and_kwargs_as_a_splat, as: :that_method_with_optional_args_and_kwargs_as_a_splat_again + + object = SomeObject.new + + expect(subject.represent(object).serializable_hash).to eq( + that_method_with_optional_args_and_kwargs_as_a_splat: 'result', + that_method_with_optional_args_and_kwargs_as_a_splat_again: 'result' + ) + end + end + + context 'with block passed in via & that references a method implemented using `method_missing`' do + specify 'succeeds for a zero-required-arg method_missing-backed method' do + subject.expose :that_method_without_args, &:method_without_args + subject.expose :method_without_args, as: :that_method_without_args_again + + object = SomeObjectWithMethodMissing.new + + expect(subject.represent(object).serializable_hash).to eq( + that_method_without_args: 'result', + that_method_without_args_again: 'result' + ) + end + + it 'raises an `ArgumentError` if there are required arguments' do + subject.expose :that_method_with_args, &:method_with_args + subject.expose :method_with_args, as: :that_method_with_args_again + + object = SomeObjectWithMethodMissing.new + + # NOTE: object.method(:method_with_args) is not available for this method_missing proxy, + # so Ruby raises a native error at call time instead of the curated grape-entity message. + expect do + subject.represent(object).value_for(:that_method_with_args) + end.to raise_error ArgumentError, /given 0, expected 1/ + + expect do + subject.represent(object).value_for(:that_method_with_args_again) + end.to raise_error ArgumentError, /given 0, expected 1/ + end + end + + context 'with block passed in via & that references a method implemented using `delegate`' do + specify 'succeeds for a zero-required-arg delegate-backed method' do + subject.expose :that_method_without_args, &:method_without_args + subject.expose :method_without_args, as: :that_method_without_args_again + + object = SomeObjectWithDelegation.new + + expect(subject.represent(object).serializable_hash).to eq( + that_method_without_args: 'result', + that_method_without_args_again: 'result' + ) + end + + it 'raises an `ArgumentError` if there are required arguments' do + subject.expose :that_method_with_args, &:method_with_args + subject.expose :method_with_args, as: :that_method_with_args_again + + object = SomeObjectWithDelegation.new + + # NOTE: delegation wrappers expose variable arity, so Ruby raises a native error + # at call time instead of the curated grape-entity message. + expect do + subject.represent(object).value_for(:that_method_with_args) + end.to raise_error ArgumentError, /given 0, expected 1/ + + expect do + subject.represent(object).value_for(:that_method_with_args_again) + end.to raise_error ArgumentError, /given 0, expected 1/ + end end context 'with block passed in via &' do @@ -468,11 +742,11 @@ def raises_argument_error expect do subject.represent(object).value_for(:that_method_with_one_arg) - end.to raise_error ArgumentError, match(/method expects 1 argument/) + end.to raise_error ArgumentError, include('method expects 1 argument.') expect do subject.represent(object).value_for(:that_method_with_multple_args) - end.to raise_error ArgumentError, match(/method expects 2 arguments/) + end.to raise_error ArgumentError, include('method expects 2 arguments.') end end @@ -484,7 +758,7 @@ def raises_argument_error expect do subject.represent(object).value_for(:that_undefined_method) - end.to raise_error ArgumentError, match(/method is not defined in the object/) + end.to raise_error NoMethodError end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b375bf5..0aee860 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,7 @@ # This works around the hash extensions not being automatically included in ActiveSupport < 4 require 'active_support/version' +require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/hash' if ActiveSupport::VERSION && ActiveSupport::VERSION::MAJOR && ActiveSupport::VERSION::MAJOR < 4