From 43052fd13291bb89f843c35062e92d5ed339d7a2 Mon Sep 17 00:00:00 2001 From: Marc Rohloff Date: Fri, 8 May 2026 09:40:15 -0600 Subject: [PATCH 1/6] Feature: Handle symbol-to-proc wrappers (`&:`) that refer to a method created using delegation or method missing These methods have an arity of -1 --- CHANGELOG.md | 2 ++ lib/grape_entity/entity.rb | 3 +- spec/grape_entity/entity_spec.rb | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb5463..37c4930 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ### Next Release +* [#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` + #### Features * Your contribution here. diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index bd36d20..d0d40ff 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -546,7 +546,8 @@ def ensure_block_arity!(block) end arity = object.method(origin_method_name).arity - return if arity.zero? + # functions defined using `delegate` or `method_missing` have an arity of -1 + return if arity <= 0 raise ArgumentError, <<~MSG Cannot use `&:#{origin_method_name}` because that method expects #{arity} argument#{'s' if arity != 1}. diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 3d8a74a..7c2250c 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -400,6 +400,14 @@ class BogusEntity < Grape::Entity describe 'blocks' do class SomeObject + class SomeObjectDelegate + def method_using_delegation + 'delegated-result' + end + end + + delegate :method_using_delegation, to: :delegate_object + def method_without_args 'result' end @@ -415,6 +423,23 @@ def method_with_multiple_args(_object, _options) def raises_argument_error raise ArgumentError, 'something different' end + + def method_missing(method, ...) + return 'missing-result' if method.to_sym == :method_using_missing + + super + end + + def delegate_object + @delegate_object ||= SomeObjectDelegate.new + end + + private + + def respond_to_missing?(method, include_private = false) + method.to_sym == :method_using_missing || + super + end end describe 'with block passed in' do @@ -459,6 +484,28 @@ def raises_argument_error end end + context 'with block passed in via & that uses `missing_method`' do + specify do + subject.expose :using_missing, &:method_using_missing + + object = SomeObject.new + expect(object.method(:method_using_missing).arity).to eq(-1) + value = subject.represent(object).value_for(:using_missing) + expect(value).to eq('missing-result') + end + end + + context 'with block passed in via & that uses `delegate`' do + specify do + subject.expose :using_delegation, &:method_using_delegation + + object = SomeObject.new + expect(object.method(:method_using_delegation).arity).to eq(-1) + value = subject.represent(object).value_for(:using_delegation) + expect(value).to eq('delegated-result') + end + end + context 'with block passed in via &' do specify do subject.expose :that_method_with_one_arg, &:method_with_one_arg From a892626c1cd54c7d315f8d94ded8cc8a33deba7d Mon Sep 17 00:00:00 2001 From: Marc Rohloff Date: Sat, 9 May 2026 15:28:46 -0600 Subject: [PATCH 2/6] Fix CI issues --- CHANGELOG.md | 2 +- CONTRIBUTING.md | 2 +- spec/grape_entity/entity_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37c4930..907c1ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ### Next Release -* [#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` +* [#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` - [@marcrohloff](https://github.com/marcrohloff). #### Features diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6023a84..655e94e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,7 @@ Implement your feature or bug fix. Ruby style is enforced with [Rubocop](https://github.com/bbatsov/rubocop), run `bundle exec rubocop` and fix any style issues highlighted. -Make sure that `bundle exec rake` completes without errors. +Make sure that `bundle exec rake` and `rubocop` completes without errors. #### Write Documentation diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 7c2250c..2df0b65 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -436,7 +436,7 @@ def delegate_object private - def respond_to_missing?(method, include_private = false) + def respond_to_missing?(method, include_private = false) # rubocop:disable Style/OptionalBooleanParameter method.to_sym == :method_using_missing || super end From 99edb4c361a4b52d10a6176bcbddcd79672799b2 Mon Sep 17 00:00:00 2001 From: Marc Rohloff Date: Wed, 13 May 2026 15:36:02 -0600 Subject: [PATCH 3/6] PR recommendations Add argumenterror specs --- CHANGELOG.md | 3 +- CONTRIBUTING.md | 2 +- lib/grape_entity/entity.rb | 10 +- spec/grape_entity/entity_spec.rb | 201 +++++++++++++++++++++++++++---- 4 files changed, 183 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 907c1ff..9a9bc17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,8 @@ ### Next Release -* [#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` - [@marcrohloff](https://github.com/marcrohloff). - #### Features +* [#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` - [@marcrohloff](https://github.com/marcrohloff). * Your contribution here. #### Fixes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 655e94e..6023a84 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,7 @@ Implement your feature or bug fix. Ruby style is enforced with [Rubocop](https://github.com/bbatsov/rubocop), run `bundle exec rubocop` and fix any style issues highlighted. -Make sure that `bundle exec rake` and `rubocop` completes without errors. +Make sure that `bundle exec rake` completes without errors. #### Write Documentation diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index d0d40ff..feedab9 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 @@ -545,12 +545,14 @@ def ensure_block_arity!(block) MSG end + # Ensure that the function does not require any positional args + # (functions defined using `delegate` or `method_missing` take an arg of `*rest` arity = object.method(origin_method_name).arity - # functions defined using `delegate` or `method_missing` have an arity of -1 - return if arity <= 0 + required_positional_arg_count = arity >= 0 ? arity : -arity - 1 + return if required_positional_arg_count.zero? raise ArgumentError, <<~MSG - Cannot use `&:#{origin_method_name}` because that method expects #{arity} argument#{'s' if arity != 1}. + Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} #{'argument'.pluralize(required_positional_arg_count)}. Symbol‐to‐proc shorthand only works for zero‐argument methods. MSG end diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 2df0b65..44b35df 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -400,19 +400,27 @@ class BogusEntity < Grape::Entity describe 'blocks' do class SomeObject - class SomeObjectDelegate - def method_using_delegation - 'delegated-result' - end + def method_without_args + 'result' end - delegate :method_using_delegation, to: :delegate_object + def method_with_one_arg(_object) + 'result' + end - def method_without_args + def method_with_only_optional_args(_optional1 = 1, _optional2 = 2) 'result' end - def method_with_one_arg(_object) + 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_required_args_and_an_optional_splat(_required_arg, *_optional_argds) 'result' end @@ -423,25 +431,56 @@ def method_with_multiple_args(_object, _options) def raises_argument_error raise ArgumentError, 'something different' end + end + class SomeObjectWithMethodMissing def method_missing(method, ...) - return 'missing-result' if method.to_sym == :method_using_missing + 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 - super + def method_without_args_impl + 'result' end - def delegate_object - @delegate_object ||= SomeObjectDelegate.new + 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_using_missing || + 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 specify do subject.expose :that_method_without_args do |object| @@ -484,25 +523,135 @@ def respond_to_missing?(method, include_private = false) # rubocop:disable Style end end - context 'with block passed in via & that uses `missing_method`' do - specify do - subject.expose :using_missing, &:method_using_missing + context 'with block passed in via & that references a method with optional args' do + it 'succeeds if there 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 + + value = subject.represent(object).value_for(:method_with_only_optional_args) + expect(value).to be_nil + + value = subject.represent(object).value_for(:that_method_with_only_optional_args) + expect(value).to eq('result') + + value = subject.represent(object).value_for(:that_method_with_only_optional_args_again) + expect(value).to eq('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(object.method(:method_using_missing).arity).to eq(-1) - value = subject.represent(object).value_for(:using_missing) - expect(value).to eq('missing-result') + + 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, include('(given 0, expected 1..3)') end end - context 'with block passed in via & that uses `delegate`' do + context 'with block passed in via & that references a method with optional args as a splat' do specify do - subject.expose :using_delegation, &:method_using_delegation + 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 + + value = subject.represent(object).value_for(:method_with_optional_args_as_a_splat) + expect(value).to be_nil + + value = subject.represent(object).value_for(:that_method_with_optional_args_as_a_splat) + expect(value).to eq('result') + + value = subject.represent(object).value_for(:that_method_with_optional_args_as_a_splat_again) + expect(value).to eq('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(object.method(:method_using_delegation).arity).to eq(-1) - value = subject.represent(object).value_for(:using_delegation) - expect(value).to eq('delegated-result') + + 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.') + + expect do + subject.represent(object).value_for(:that_method_with_required_args_and_an_optional_splat_again) + end.to raise_error ArgumentError, include('(given 0, expected 1+)') + end + end + + context 'with block passed in via & that references a method implemented using `method_missing`' do + specify do + subject.expose :that_method_without_args, &:method_without_args + subject.expose :method_without_args, as: :that_method_without_args_again + + object = SomeObjectWithMethodMissing.new + + value = subject.represent(object).value_for(:method_without_args) + expect(value).to be_nil + + value = subject.represent(object).value_for(:that_method_without_args) + expect(value).to eq('result') + + value = subject.represent(object).value_for(:that_method_without_args_again) + expect(value).to eq('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 + + expect do + subject.represent(object).value_for(:that_method_with_args) + end.to raise_error ArgumentError, include('(given 0, expected 1)') + + expect do + subject.represent(object).value_for(:that_method_with_args_again) + end.to raise_error ArgumentError, include('(given 0, expected 1)') + end + end + + context 'with block passed in via & that references a method implemented using `delegate`' do + specify do + subject.expose :that_method_without_args, &:method_without_args + subject.expose :method_without_args, as: :that_method_without_args_again + + object = SomeObjectWithDelegation.new + + value = subject.represent(object).value_for(:method_without_args) + expect(value).to be_nil + + value = subject.represent(object).value_for(:that_method_without_args) + expect(value).to eq('result') + + value = subject.represent(object).value_for(:that_method_without_args_again) + expect(value).to eq('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 + + expect do + subject.represent(object).value_for(:that_method_with_args) + end.to raise_error ArgumentError, include('(given 0, expected 1)') + + expect do + subject.represent(object).value_for(:that_method_with_args_again) + end.to raise_error ArgumentError, include('(given 0, expected 1)') end end @@ -515,11 +664,11 @@ def respond_to_missing?(method, include_private = false) # rubocop:disable Style 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 @@ -531,7 +680,7 @@ def respond_to_missing?(method, include_private = false) # rubocop:disable Style 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 ArgumentError, include('method is not defined in the object') end end From 6e6eb414feb15e813c75989edcd496599b9e26e5 Mon Sep 17 00:00:00 2001 From: Marc Rohloff Date: Fri, 15 May 2026 11:13:38 -0600 Subject: [PATCH 4/6] Include PR recommendations --- lib/grape_entity/entity.rb | 2 +- spec/grape_entity/entity_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index feedab9..5e169b0 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -552,7 +552,7 @@ def ensure_block_arity!(block) return if required_positional_arg_count.zero? raise ArgumentError, <<~MSG - Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} #{'argument'.pluralize(required_positional_arg_count)}. + Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} argument#{'s' if required_positional_arg_count != 1}. Symbol‐to‐proc shorthand only works for zero‐argument methods. MSG end diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 44b35df..fb44a30 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -420,7 +420,7 @@ def method_with_optional_args_as_a_splat(*_optional_args) 'result' end - def method_with_required_args_and_an_optional_splat(_required_arg, *_optional_argds) + def method_with_required_args_and_an_optional_splat(_required_arg, *_optional_args) 'result' end From a4d280ffdd04f988f6c02284bca59851b92decb5 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Tue, 2 Jun 2026 15:38:57 +0200 Subject: [PATCH 5/6] Harden &: arity check: fix variadic math, kwargs, extract helper, add specs Fixes arity math for variadic methods, skips methods with required keyword arguments, extracts positional_arity_for for clarity, and adds specs for optional kwargs, splats, private methods, delegation, and a canary for the Proc#to_s regex. Also pins the ActiveSupport delegation require in spec_helper and moves the CHANGELOG entry under Fixes. --- CHANGELOG.md | 2 +- lib/grape_entity/entity.rb | 42 +++++---- spec/grape_entity/entity_spec.rb | 148 ++++++++++++++++++++++--------- spec/spec_helper.rb | 1 + 4 files changed, 132 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a9bc17..e258d03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,13 +2,13 @@ #### Features -* [#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` - [@marcrohloff](https://github.com/marcrohloff). * Your contribution here. #### Fixes * 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` - [@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 5e169b0..40ef314 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -534,29 +534,37 @@ 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 - # Ensure that the function does not require any positional args - # (functions defined using `delegate` or `method_missing` take an arg of `*rest` - arity = object.method(origin_method_name).arity - required_positional_arg_count = arity >= 0 ? arity : -arity - 1 - return if required_positional_arg_count.zero? + origin_method_name = match[:name].to_sym + required_positional_arg_count, variadic = positional_arity_for(origin_method_name) + return unless required_positional_arg_count + + expected_suffix = required_positional_arg_count == 1 ? 'argument' : 'arguments' + expected_suffix += ' or more' if variadic raise ArgumentError, <<~MSG - Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} argument#{'s' if required_positional_arg_count != 1}. - Symbol‐to‐proc shorthand only works for zero‐argument methods. + Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} #{expected_suffix}. + Symbol-to-proc shorthand only works for methods that can be called with no arguments. MSG end + def positional_arity_for(method_name) + origin_method = object.method(method_name) + return nil if origin_method.parameters.any? { |type, _| type == :keyreq } + + arity = origin_method.arity + required_positional_arg_count = arity >= 0 ? arity : -arity - 1 + return nil if required_positional_arg_count.zero? + + [required_positional_arg_count, arity.negative?] + rescue NameError + # Delegation wrappers and method_missing proxies may not expose a Method; let Ruby raise natively at call time. + nil + 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 fb44a30..8519eeb 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -420,6 +420,14 @@ 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_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 @@ -431,6 +439,12 @@ 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 @@ -521,6 +535,38 @@ def delegate_object 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 @@ -530,14 +576,10 @@ def delegate_object object = SomeObject.new - value = subject.represent(object).value_for(:method_with_only_optional_args) - expect(value).to be_nil - - value = subject.represent(object).value_for(:that_method_with_only_optional_args) - expect(value).to eq('result') - - value = subject.represent(object).value_for(:that_method_with_only_optional_args_again) - expect(value).to eq('result') + 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 @@ -548,11 +590,11 @@ def delegate_object expect do subject.represent(object).value_for(:that_method_with_required_and_optional_args) - end.to raise_error ArgumentError, include('method expects 1 argument.') + end.to raise_error ArgumentError, include('method expects 1 argument or more.') expect do subject.represent(object).value_for(:that_method_with_required_and_optional_args_again) - end.to raise_error ArgumentError, include('(given 0, expected 1..3)') + end.to raise_error ArgumentError, /given 0, expected 1/ end end @@ -563,14 +605,10 @@ def delegate_object object = SomeObject.new - value = subject.represent(object).value_for(:method_with_optional_args_as_a_splat) - expect(value).to be_nil - - value = subject.represent(object).value_for(:that_method_with_optional_args_as_a_splat) - expect(value).to eq('result') - - value = subject.represent(object).value_for(:that_method_with_optional_args_as_a_splat_again) - expect(value).to eq('result') + 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 @@ -581,29 +619,53 @@ def delegate_object 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.') + 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, include('(given 0, expected 1+)') + end.to raise_error ArgumentError, /given 0, expected 1/ end end - context 'with block passed in via & that references a method implemented using `method_missing`' do + 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 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 - value = subject.represent(object).value_for(:method_without_args) - expect(value).to be_nil - - value = subject.represent(object).value_for(:that_method_without_args) - expect(value).to eq('result') - - value = subject.represent(object).value_for(:that_method_without_args_again) - expect(value).to eq('result') + 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 @@ -612,31 +674,29 @@ def delegate_object object = SomeObjectWithMethodMissing.new + # NOTE: ensure_block_arity! cannot pierce delegation wrappers (arity -1); + # 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, include('(given 0, expected 1)') + 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, include('(given 0, expected 1)') + 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 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 - value = subject.represent(object).value_for(:method_without_args) - expect(value).to be_nil - - value = subject.represent(object).value_for(:that_method_without_args) - expect(value).to eq('result') - - value = subject.represent(object).value_for(:that_method_without_args_again) - expect(value).to eq('result') + 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 @@ -645,13 +705,15 @@ def delegate_object object = SomeObjectWithDelegation.new + # NOTE: ensure_block_arity! cannot pierce delegation wrappers (arity -1); + # 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, include('(given 0, expected 1)') + 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, include('(given 0, expected 1)') + end.to raise_error ArgumentError, /given 0, expected 1/ end end @@ -680,7 +742,7 @@ def delegate_object expect do subject.represent(object).value_for(:that_undefined_method) - end.to raise_error ArgumentError, include('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 From ec9c91d5b1eeafb6b269eb1a0c0fae31fc291a8d Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Tue, 2 Jun 2026 17:08:28 +0200 Subject: [PATCH 6/6] Use parameters over arity, add curated error for required kwargs Switches from Method#arity math to Method#parameters inspection for more precise required-arg detection. Required keyword arguments now produce a curated ArgumentError instead of falling through to Ruby's native error. Adds a spec for the required-kwargs case and sharpens the delegation and method_missing NOTE comments. --- CHANGELOG.md | 2 +- lib/grape_entity/entity.rb | 36 +++++++++++++++++++++++--------- spec/grape_entity/entity_spec.rb | 28 +++++++++++++++++++------ 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e258d03..9c27973 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +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` - [@marcrohloff](https://github.com/marcrohloff). +* [#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 40ef314..4b409c1 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -539,32 +539,48 @@ def ensure_block_arity!(block) return unless match # Unrecognized format -> bail safe rather than misidentify origin_method_name = match[:name].to_sym - required_positional_arg_count, variadic = positional_arity_for(origin_method_name) + required_positional_arg_count, required_keyword_arg_count, variadic_positional = + arity_requirement_for(origin_method_name) return unless required_positional_arg_count - expected_suffix = required_positional_arg_count == 1 ? 'argument' : 'arguments' - expected_suffix += ' or more' if variadic + 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 #{required_positional_arg_count} #{expected_suffix}. + 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 positional_arity_for(method_name) + def arity_requirement_for(method_name) origin_method = object.method(method_name) - return nil if origin_method.parameters.any? { |type, _| type == :keyreq } + parameters = origin_method.parameters - arity = origin_method.arity - required_positional_arg_count = arity >= 0 ? arity : -arity - 1 - return nil if required_positional_arg_count.zero? + 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, arity.negative?] + [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 8519eeb..aa49785 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -424,6 +424,10 @@ 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 @@ -570,7 +574,7 @@ def block.to_s end context 'with block passed in via & that references a method with optional args' do - it 'succeeds if there no required arguments' 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 @@ -590,7 +594,7 @@ def block.to_s expect do subject.represent(object).value_for(:that_method_with_required_and_optional_args) - end.to raise_error ArgumentError, include('method expects 1 argument or more.') + 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) @@ -641,6 +645,18 @@ def block.to_s 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 @@ -674,8 +690,8 @@ def block.to_s object = SomeObjectWithMethodMissing.new - # NOTE: ensure_block_arity! cannot pierce delegation wrappers (arity -1); - # Ruby raises a native error at call time instead of the curated grape-entity message. + # 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/ @@ -705,8 +721,8 @@ def block.to_s object = SomeObjectWithDelegation.new - # NOTE: ensure_block_arity! cannot pierce delegation wrappers (arity -1); - # Ruby raises a native error at call time instead of the curated grape-entity message. + # 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/