Skip to content

Commit a4d280f

Browse files
committed
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.
1 parent 6e6eb41 commit a4d280f

4 files changed

Lines changed: 132 additions & 61 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
#### Features
44

5-
* [#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).
65
* Your contribution here.
76

87
#### Fixes
98

109
* Your contribution here.
1110
* [#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).
11+
* [#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).
1212

1313
### 1.0.4 (2026-04-17)
1414

lib/grape_entity/entity.rb

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -534,29 +534,37 @@ def exec_with_object(options, &block)
534534
end
535535

536536
def ensure_block_arity!(block)
537-
# MRI currently always includes "( &:foo )" for symbol-to-proc wrappers.
538-
# If this format changes in a new Ruby version, this logic must be updated.
539-
origin_method_name = block.to_s.scan(/(?<=\(&:)[^)]+(?=\))/).first&.to_sym
540-
return unless origin_method_name
541-
542-
unless object.respond_to?(origin_method_name, true)
543-
raise ArgumentError, <<~MSG
544-
Cannot use `&:#{origin_method_name}` because that method is not defined in the object.
545-
MSG
546-
end
537+
# Strict anchor to match MRI Proc#to_s format for symbol-to-proc: #<Proc:0x0...(&:method_name) (lambda)>
538+
match = block.to_s.match(/\A#<Proc:(?:0x)?\h+\(&:(?<name>.+)\) \(lambda\)>\z/)
539+
return unless match # Unrecognized format -> bail safe rather than misidentify
547540

548-
# Ensure that the function does not require any positional args
549-
# (functions defined using `delegate` or `method_missing` take an arg of `*rest`
550-
arity = object.method(origin_method_name).arity
551-
required_positional_arg_count = arity >= 0 ? arity : -arity - 1
552-
return if required_positional_arg_count.zero?
541+
origin_method_name = match[:name].to_sym
542+
required_positional_arg_count, variadic = positional_arity_for(origin_method_name)
543+
return unless required_positional_arg_count
544+
545+
expected_suffix = required_positional_arg_count == 1 ? 'argument' : 'arguments'
546+
expected_suffix += ' or more' if variadic
553547

554548
raise ArgumentError, <<~MSG
555-
Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} argument#{'s' if required_positional_arg_count != 1}.
556-
Symbol‐to‐proc shorthand only works for zero‐argument methods.
549+
Cannot use `&:#{origin_method_name}` because that method expects #{required_positional_arg_count} #{expected_suffix}.
550+
Symbol-to-proc shorthand only works for methods that can be called with no arguments.
557551
MSG
558552
end
559553

554+
def positional_arity_for(method_name)
555+
origin_method = object.method(method_name)
556+
return nil if origin_method.parameters.any? { |type, _| type == :keyreq }
557+
558+
arity = origin_method.arity
559+
required_positional_arg_count = arity >= 0 ? arity : -arity - 1
560+
return nil if required_positional_arg_count.zero?
561+
562+
[required_positional_arg_count, arity.negative?]
563+
rescue NameError
564+
# Delegation wrappers and method_missing proxies may not expose a Method; let Ruby raise natively at call time.
565+
nil
566+
end
567+
560568
def symbol_to_proc_wrapper?(block)
561569
params = block.parameters
562570

spec/grape_entity/entity_spec.rb

Lines changed: 105 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,14 @@ def method_with_optional_args_as_a_splat(*_optional_args)
420420
'result'
421421
end
422422

423+
def method_with_optional_kwargs_as_a_splat(**_kwargs)
424+
'result'
425+
end
426+
427+
def method_with_optional_args_and_kwargs_as_a_splat(*_args, **_kwargs)
428+
'result'
429+
end
430+
423431
def method_with_required_args_and_an_optional_splat(_required_arg, *_optional_args)
424432
'result'
425433
end
@@ -431,6 +439,12 @@ def method_with_multiple_args(_object, _options)
431439
def raises_argument_error
432440
raise ArgumentError, 'something different'
433441
end
442+
443+
private
444+
445+
def private_method_with_required_arg(_required_arg)
446+
'result'
447+
end
434448
end
435449

436450
class SomeObjectWithMethodMissing
@@ -521,6 +535,38 @@ def delegate_object
521535
value = subject.represent(object).value_for(:that_method_without_args_again)
522536
expect(value).to eq('result')
523537
end
538+
539+
it 'raises a curated `ArgumentError` for private methods with required arguments' do
540+
subject.expose :that_private_method_with_required_arg, &:private_method_with_required_arg
541+
542+
object = SomeObject.new
543+
544+
expect do
545+
subject.represent(object).value_for(:that_private_method_with_required_arg)
546+
end.to raise_error ArgumentError, include('method expects 1 argument.')
547+
end
548+
549+
it 'treats a source-less lambda shaped like symbol-to-proc as a one-argument exposure block' do
550+
block = lambda do |object, *args|
551+
raise ArgumentError, "expected no trailing args, got #{args.inspect}" unless args.empty?
552+
553+
object.method_without_args
554+
end
555+
556+
def block.source_location
557+
nil
558+
end
559+
560+
def block.to_s
561+
'#<Proc:0x123456 (lambda)>'
562+
end
563+
564+
subject.expose :that_method_without_args, &block
565+
566+
object = SomeObject.new
567+
568+
expect(subject.represent(object).value_for(:that_method_without_args)).to eq('result')
569+
end
524570
end
525571

526572
context 'with block passed in via & that references a method with optional args' do
@@ -530,14 +576,10 @@ def delegate_object
530576

531577
object = SomeObject.new
532578

533-
value = subject.represent(object).value_for(:method_with_only_optional_args)
534-
expect(value).to be_nil
535-
536-
value = subject.represent(object).value_for(:that_method_with_only_optional_args)
537-
expect(value).to eq('result')
538-
539-
value = subject.represent(object).value_for(:that_method_with_only_optional_args_again)
540-
expect(value).to eq('result')
579+
expect(subject.represent(object).serializable_hash).to eq(
580+
that_method_with_only_optional_args: 'result',
581+
that_method_with_only_optional_args_again: 'result'
582+
)
541583
end
542584

543585
it 'raises an `ArgumentError` if there are required arguments' do
@@ -548,11 +590,11 @@ def delegate_object
548590

549591
expect do
550592
subject.represent(object).value_for(:that_method_with_required_and_optional_args)
551-
end.to raise_error ArgumentError, include('method expects 1 argument.')
593+
end.to raise_error ArgumentError, include('method expects 1 argument or more.')
552594

553595
expect do
554596
subject.represent(object).value_for(:that_method_with_required_and_optional_args_again)
555-
end.to raise_error ArgumentError, include('(given 0, expected 1..3)')
597+
end.to raise_error ArgumentError, /given 0, expected 1/
556598
end
557599
end
558600

@@ -563,14 +605,10 @@ def delegate_object
563605

564606
object = SomeObject.new
565607

566-
value = subject.represent(object).value_for(:method_with_optional_args_as_a_splat)
567-
expect(value).to be_nil
568-
569-
value = subject.represent(object).value_for(:that_method_with_optional_args_as_a_splat)
570-
expect(value).to eq('result')
571-
572-
value = subject.represent(object).value_for(:that_method_with_optional_args_as_a_splat_again)
573-
expect(value).to eq('result')
608+
expect(subject.represent(object).serializable_hash).to eq(
609+
that_method_with_optional_args_as_a_splat: 'result',
610+
that_method_with_optional_args_as_a_splat_again: 'result'
611+
)
574612
end
575613

576614
it 'raises an `ArgumentError` if there are required arguments' do
@@ -581,29 +619,53 @@ def delegate_object
581619

582620
expect do
583621
subject.represent(object).value_for(:that_method_with_required_args_and_an_optional_splat)
584-
end.to raise_error ArgumentError, include('method expects 1 argument.')
622+
end.to raise_error ArgumentError, include('method expects 1 argument or more.')
585623

586624
expect do
587625
subject.represent(object).value_for(:that_method_with_required_args_and_an_optional_splat_again)
588-
end.to raise_error ArgumentError, include('(given 0, expected 1+)')
626+
end.to raise_error ArgumentError, /given 0, expected 1/
589627
end
590628
end
591629

592-
context 'with block passed in via & that references a method implemented using `method_missing`' do
630+
context 'with block passed in via & that references a method with optional kwargs as a splat' do
593631
specify do
632+
subject.expose :that_method_with_optional_kwargs_as_a_splat, &:method_with_optional_kwargs_as_a_splat
633+
subject.expose :method_with_optional_kwargs_as_a_splat, as: :that_method_with_optional_kwargs_as_a_splat_again
634+
635+
object = SomeObject.new
636+
637+
expect(subject.represent(object).serializable_hash).to eq(
638+
that_method_with_optional_kwargs_as_a_splat: 'result',
639+
that_method_with_optional_kwargs_as_a_splat_again: 'result'
640+
)
641+
end
642+
end
643+
644+
context 'with block passed in via & that references a method with optional args and kwargs as a splat' do
645+
specify do
646+
subject.expose :that_method_with_optional_args_and_kwargs_as_a_splat, &:method_with_optional_args_and_kwargs_as_a_splat
647+
subject.expose :method_with_optional_args_and_kwargs_as_a_splat, as: :that_method_with_optional_args_and_kwargs_as_a_splat_again
648+
649+
object = SomeObject.new
650+
651+
expect(subject.represent(object).serializable_hash).to eq(
652+
that_method_with_optional_args_and_kwargs_as_a_splat: 'result',
653+
that_method_with_optional_args_and_kwargs_as_a_splat_again: 'result'
654+
)
655+
end
656+
end
657+
658+
context 'with block passed in via & that references a method implemented using `method_missing`' do
659+
specify 'succeeds for a zero-required-arg method_missing-backed method' do
594660
subject.expose :that_method_without_args, &:method_without_args
595661
subject.expose :method_without_args, as: :that_method_without_args_again
596662

597663
object = SomeObjectWithMethodMissing.new
598664

599-
value = subject.represent(object).value_for(:method_without_args)
600-
expect(value).to be_nil
601-
602-
value = subject.represent(object).value_for(:that_method_without_args)
603-
expect(value).to eq('result')
604-
605-
value = subject.represent(object).value_for(:that_method_without_args_again)
606-
expect(value).to eq('result')
665+
expect(subject.represent(object).serializable_hash).to eq(
666+
that_method_without_args: 'result',
667+
that_method_without_args_again: 'result'
668+
)
607669
end
608670

609671
it 'raises an `ArgumentError` if there are required arguments' do
@@ -612,31 +674,29 @@ def delegate_object
612674

613675
object = SomeObjectWithMethodMissing.new
614676

677+
# NOTE: ensure_block_arity! cannot pierce delegation wrappers (arity -1);
678+
# Ruby raises a native error at call time instead of the curated grape-entity message.
615679
expect do
616680
subject.represent(object).value_for(:that_method_with_args)
617-
end.to raise_error ArgumentError, include('(given 0, expected 1)')
681+
end.to raise_error ArgumentError, /given 0, expected 1/
618682

619683
expect do
620684
subject.represent(object).value_for(:that_method_with_args_again)
621-
end.to raise_error ArgumentError, include('(given 0, expected 1)')
685+
end.to raise_error ArgumentError, /given 0, expected 1/
622686
end
623687
end
624688

625689
context 'with block passed in via & that references a method implemented using `delegate`' do
626-
specify do
690+
specify 'succeeds for a zero-required-arg delegate-backed method' do
627691
subject.expose :that_method_without_args, &:method_without_args
628692
subject.expose :method_without_args, as: :that_method_without_args_again
629693

630694
object = SomeObjectWithDelegation.new
631695

632-
value = subject.represent(object).value_for(:method_without_args)
633-
expect(value).to be_nil
634-
635-
value = subject.represent(object).value_for(:that_method_without_args)
636-
expect(value).to eq('result')
637-
638-
value = subject.represent(object).value_for(:that_method_without_args_again)
639-
expect(value).to eq('result')
696+
expect(subject.represent(object).serializable_hash).to eq(
697+
that_method_without_args: 'result',
698+
that_method_without_args_again: 'result'
699+
)
640700
end
641701

642702
it 'raises an `ArgumentError` if there are required arguments' do
@@ -645,13 +705,15 @@ def delegate_object
645705

646706
object = SomeObjectWithDelegation.new
647707

708+
# NOTE: ensure_block_arity! cannot pierce delegation wrappers (arity -1);
709+
# Ruby raises a native error at call time instead of the curated grape-entity message.
648710
expect do
649711
subject.represent(object).value_for(:that_method_with_args)
650-
end.to raise_error ArgumentError, include('(given 0, expected 1)')
712+
end.to raise_error ArgumentError, /given 0, expected 1/
651713

652714
expect do
653715
subject.represent(object).value_for(:that_method_with_args_again)
654-
end.to raise_error ArgumentError, include('(given 0, expected 1)')
716+
end.to raise_error ArgumentError, /given 0, expected 1/
655717
end
656718
end
657719

@@ -680,7 +742,7 @@ def delegate_object
680742

681743
expect do
682744
subject.represent(object).value_for(:that_undefined_method)
683-
end.to raise_error ArgumentError, include('method is not defined in the object')
745+
end.to raise_error NoMethodError
684746
end
685747
end
686748

spec/spec_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
# This works around the hash extensions not being automatically included in ActiveSupport < 4
77
require 'active_support/version'
8+
require 'active_support/core_ext/module/delegation'
89
require 'active_support/core_ext/hash' if ActiveSupport::VERSION &&
910
ActiveSupport::VERSION::MAJOR &&
1011
ActiveSupport::VERSION::MAJOR < 4

0 commit comments

Comments
 (0)