Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#### 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
Expand Down
9 changes: 6 additions & 3 deletions lib/grape_entity/entity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,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
Expand Down Expand Up @@ -547,11 +547,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
return if arity.zero?
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
Expand Down
202 changes: 199 additions & 3 deletions spec/grape_entity/entity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,22 @@ 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_required_args_and_an_optional_splat(_required_arg, *_optional_argds)
'result'
end

def method_with_multiple_args(_object, _options)
'result'
end
Expand All @@ -417,6 +433,54 @@ def raises_argument_error
end
end

class SomeObjectWithMethodMissing
def method_missing(method, ...)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than adding method_missing directly to SomeObject, could you extract the new fixtures into subclasses? That way the shared class stays clean and the existing tests (especially the "undefined method" one) don't depend on your
respond_to_missing? being correct to pass.

Something like:

SomeObjectWithMissing = Class.new(SomeObject) do
  def method_missing(method, ...)
    return 'missing-result' if method == :method_using_missing
    super
  end

  def respond_to_missing?(method, include_private = false)
    method == :method_using_missing || super
  end
end

SomeObjectWithDelegation = Class.new(SomeObject) do
  InnerDelegate = Class.new { def delegated_method = 'delegated-result' }
  delegate :delegated_method, to: :inner
  def inner = @inner ||= InnerDelegate.new
end

and specs kind of:

context 'with &: referencing a method_missing-backed method' do
  specify do
    subject.expose :using_missing, &:method_using_missing

    object = SomeObjectWithMissing.new
    expect(subject.represent(object).value_for(:using_missing)).to eq('missing-result')
  end
end

context 'with &: referencing a delegated method' do
  specify do
    subject.expose :using_delegation, &:delegated_method

    object = SomeObjectWithDelegation.new
    expect(subject.represent(object).value_for(:using_delegation)).to eq('delegated-result')
  end
end

with the regression case that pins the boundary:

context 'with &: referencing a method with required args and a splat (arity -2)' do
  specify do
    subject.expose :x, &:method_with_required_and_splat

    object = SomeObjectWithMissing.new  # or any object with such a method
    expect do
      subject.represent(object).value_for(:x)
    end.to raise_error(ArgumentError, match(/method expects/))
  end
end

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@numbata

I decided to add some more specs to test the negative cases. Something like the others tests. i..e

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

Getting the value of the first expose gives the expected exception message. But the second results in wrong number of arguments (given 0, expected 1) because it is processed through the delegator

Do you think that the second message should match the first message?

Copy link
Copy Markdown
Collaborator

@numbata numbata May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! My short subjective answer is: no, they don't need to match for this PR.

The ensure_block_arity! only guards the &:symbol path — the as: alias goes through different machinery and its error message is a separate concern. Aligning the two would be a worthwhile follow-up, but let's not expand scope here.

For the context: as: bypasses ensure_block_arity! not by skipping it directly, but because it never produces a proc at all, so the exposure type is DelegatorExposure (plain attribute delegation) rather than BlockExposure. Two completely different code paths.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the suggested changes and added several tests to verify that ArgumentError was raised when expected

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
specify do
subject.expose :that_method_without_args do |object|
Expand Down Expand Up @@ -459,6 +523,138 @@ def raises_argument_error
end
end

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 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 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

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 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

context 'with block passed in via &' do
specify do
subject.expose :that_method_with_one_arg, &:method_with_one_arg
Expand All @@ -468,11 +664,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

Expand All @@ -484,7 +680,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 ArgumentError, include('method is not defined in the object')
end
end

Expand Down