From c8f7a22f7a1a02b4640e804a602fe3e2d8f6428f Mon Sep 17 00:00:00 2001 From: Chau Hong Linh Date: Thu, 27 Jul 2023 11:40:04 -0500 Subject: [PATCH 1/6] Fix the method Grape::Entity#exec_with_object to work with Ruby 3. --- lib/grape_entity/entity.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index 9f01031..2cd7fdf 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -519,7 +519,7 @@ def serializable_hash(runtime_options = {}) end def exec_with_object(options, &block) - if block.parameters.count == 1 + if block.parameters.count == 1 || block.parameters == [[:req], [:rest]] instance_exec(object, &block) else instance_exec(object, options, &block) @@ -528,6 +528,7 @@ def exec_with_object(options, &block) # it handles: https://github.com/ruby/ruby/blob/v3_0_0_preview1/NEWS.md#language-changes point 3, Proc # accounting for expose :foo, &:bar if e.is_a?(ArgumentError) && block.parameters == [[:req], [:rest]] + Rails.logger.error("***** ERROR in exec_with_object: #{e.message}\n#{e.backtrace.join("\n")}") raise Grape::Entity::Deprecated.new e.message, 'in ruby 3.0' end From 7bf1cd5472d77fadceeaee7316f6cfef6bfee47b Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Sun, 1 Jun 2025 04:07:56 +0200 Subject: [PATCH 2/6] Fix(Entity): Improve block arity detection for `Symbol#to_proc` Addresses `ArgumentError`s when using `&:method_name` with `expose`, a scenario particularly affected by Ruby 3.0's stricter argument handling for procs. The previous arity check, including the condition `block.parameters == [[:req], [:rest]]`, was not consistently reliable for these cases. This change introduces `determine_block_arity` which: 1. Attempts to parse the original method name from `Proc#to_s` for blocks likely created via `&:method_name`. 2. If the method name is found and the object responds to it, this logic uses the *actual arity of the original method* to correctly determine how `instance_exec` should be called. 3. If the original method name can't be determined, it falls back to using `block.parameters.size`. This ensures methods exposed via `&:method_name` are called with the correct arguments (i.e., with or without `options`), resolving the `ArgumentError`s and removing the need for the previous `rescue` logic. --- lib/grape_entity/entity.rb | 25 +++++++++++++++---------- spec/grape_entity/entity_spec.rb | 28 ++++++++-------------------- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index 2cd7fdf..9833035 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -518,21 +518,26 @@ def serializable_hash(runtime_options = {}) root_exposure.serializable_value(self, opts) end + def determine_block_arity(block) + # As from Ruby 3.0, the block parameters are always equal to `[[:req], [:rest]]` + # In that case the method name could be extracted from the block to find the original method arity. + origin_method_name = block.to_s.scan(/(?<=\(&:)[^)]+(?=\))/).first&.to_sym + + if origin_method_name && object.respond_to?(origin_method_name, true) + object.method(origin_method_name).parameters.size + else + block.parameters.size + end + end + def exec_with_object(options, &block) - if block.parameters.count == 1 || block.parameters == [[:req], [:rest]] + block_arity = determine_block_arity(block) + + if block_arity == 0 instance_exec(object, &block) else instance_exec(object, options, &block) end - rescue StandardError => e - # it handles: https://github.com/ruby/ruby/blob/v3_0_0_preview1/NEWS.md#language-changes point 3, Proc - # accounting for expose :foo, &:bar - if e.is_a?(ArgumentError) && block.parameters == [[:req], [:rest]] - Rails.logger.error("***** ERROR in exec_with_object: #{e.message}\n#{e.backtrace.join("\n")}") - raise Grape::Entity::Deprecated.new e.message, 'in ruby 3.0' - end - - raise e end def exec_with_attribute(attribute, &block) diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 42fe0f1..14bdcb2 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -423,28 +423,16 @@ def raises_argument_error end context 'with block passed in via &' do - if RUBY_VERSION.start_with?('3') - specify do - subject.expose :that_method_without_args, &:method_without_args - subject.expose :method_without_args, as: :that_method_without_args_again - - object = SomeObject.new - expect do - subject.represent(object).value_for(:that_method_without_args) - end.to raise_error Grape::Entity::Deprecated - - value2 = subject.represent(object).value_for(:that_method_without_args_again) - expect(value2).to eq('result') - end - else - specify do - subject.expose :that_method_without_args_again, &:method_without_args + specify do + subject.expose :that_method_without_args, &:method_without_args + subject.expose :method_without_args, as: :that_method_without_args_again - object = SomeObject.new + object = SomeObject.new + value1 = subject.represent(object).value_for(:that_method_without_args) + expect(value1).to eq('result') - value2 = subject.represent(object).value_for(:that_method_without_args_again) - expect(value2).to eq('result') - end + value2 = subject.represent(object).value_for(:that_method_without_args_again) + expect(value2).to eq('result') end end end From f59125703e86c500c376a8815c1362e0453d609e Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Sun, 1 Jun 2025 04:14:52 +0200 Subject: [PATCH 3/6] Fix rubocop offense --- lib/grape_entity/entity.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index 9833035..ad435e2 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -533,7 +533,7 @@ def determine_block_arity(block) def exec_with_object(options, &block) block_arity = determine_block_arity(block) - if block_arity == 0 + if block_arity.zero? instance_exec(object, &block) else instance_exec(object, options, &block) From 1c893f629a44aba7954d90122e6634ee857e3bd9 Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Sun, 1 Jun 2025 14:27:45 +0200 Subject: [PATCH 4/6] Fix(Entity): Enforce zero-arity for Symbol#to_proc in exec_with_object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruby 3.0’s stricter arity rules meant `&:method_name` blocks could be called with wrong args, causing errors. The old check (`parameters == [[:req],[:rest]]` + `parameters.size`) was unreliable. This update: - Adds `symbol_to_proc_wrapper?` to detect pure `&:method_name` Procs (checks `lambda?`, `source_location.nil?`, and `parameters == [[:req],[:rest]]`). - Introduces `determine_block_arity`, which parses the method name from `block.to_s`; if the object responds to it, it uses `object.method(name).arity`, otherwise falls back to `block.arity`. - In `exec_with_object`, symbol-to-proc wrappers are required to have zero arity (raising `ArgumentError` otherwise), while regular Procs use `block.arity` to decide between `instance_exec(object)` and `instance_exec(object, options)`. This removes rescue logic and ensures `&:method_name` is only used for zero-argument methods. --- lib/grape_entity/entity.rb | 41 ++++++++++++++++++++++---------- spec/grape_entity/entity_spec.rb | 31 ++++++++++++++++++++---- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index ad435e2..7d8b035 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -518,28 +518,43 @@ def serializable_hash(runtime_options = {}) root_exposure.serializable_value(self, opts) end - def determine_block_arity(block) - # As from Ruby 3.0, the block parameters are always equal to `[[:req], [:rest]]` - # In that case the method name could be extracted from the block to find the original method arity. - origin_method_name = block.to_s.scan(/(?<=\(&:)[^)]+(?=\))/).first&.to_sym - - if origin_method_name && object.respond_to?(origin_method_name, true) - object.method(origin_method_name).parameters.size + def exec_with_object(options, &block) + if symbol_to_proc_wrapper?(block) + arity = determine_block_arity(block) + + if arity.positive? + raise ArgumentError, <<~MSG + Symbol to proc wrapper blocks must have zero arguments, but got #{arity}. + Expose methods with as: or use a block with no parameters. + MSG + end else - block.parameters.size + arity = block.arity end - end - - def exec_with_object(options, &block) - block_arity = determine_block_arity(block) - if block_arity.zero? + if arity.zero? instance_exec(object, &block) else instance_exec(object, options, &block) end end + def determine_block_arity(block) + origin_method_name = block.to_s.scan(/(?<=\(&:)[^)]+(?=\))/).first&.to_sym + block_arity = block.arity + + return block_arity unless origin_method_name + return block_arity unless object.respond_to?(origin_method_name, true) + + object.method(origin_method_name).arity + end + + def symbol_to_proc_wrapper?(block) + block.lambda? && + block.source_location.nil? && + block.parameters == [[:req], [:rest]] + end + def exec_with_attribute(attribute, &block) instance_exec(delegate_attribute(attribute), &block) end diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index 14bdcb2..aea31ef 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -393,6 +393,14 @@ def method_without_args 'result' end + def method_with_one_arg(_object) + 'result' + end + + def method_with_multiple_args(_object, _options) + 'result' + end + def raises_argument_error raise ArgumentError, 'something different' end @@ -428,11 +436,26 @@ def raises_argument_error subject.expose :method_without_args, as: :that_method_without_args_again object = SomeObject.new - value1 = subject.represent(object).value_for(:that_method_without_args) - expect(value1).to eq('result') + 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 + end + + context 'with block passed in via &' do + specify do + subject.expose :that_method_with_one_arg, &:method_with_one_arg + subject.expose :that_method_with_multple_args, &:method_with_multiple_args + + object = SomeObject.new - value2 = subject.represent(object).value_for(:that_method_without_args_again) - expect(value2).to eq('result') + expect do + subject.represent(object).value_for(:that_method_with_one_arg) + end.to raise_error ArgumentError, match(/blocks must have zero arguments/) + expect do + subject.represent(object).value_for(:that_method_with_multple_args) + end.to raise_error ArgumentError, match(/blocks must have zero arguments/) end end end From 48155a5c5fd8a9a9790e978d95c1e102922d53fc Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Sun, 1 Jun 2025 14:59:45 +0200 Subject: [PATCH 5/6] Correct error message --- lib/grape_entity/entity.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index 7d8b035..90c1f4c 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -525,7 +525,8 @@ def exec_with_object(options, &block) if arity.positive? raise ArgumentError, <<~MSG Symbol to proc wrapper blocks must have zero arguments, but got #{arity}. - Expose methods with as: or use a block with no parameters. + Use a regular block with parameters or access presented instance (`object`) + and the top-level entity options (`options`) directly. MSG end else From bd89ba9dd5737b89c2c5cd6604e27275fada94df Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Sun, 1 Jun 2025 20:38:32 +0200 Subject: [PATCH 6/6] Fix(Entity): Validate `&:method_name` arity and method presence Ensure symbol-to-proc exposures only work for zero-argument methods: - Raise if the method is undefined on the object - Raise if the method expects one or more arguments - Fall back to `Proc#arity` for regular blocks --- lib/grape_entity/entity.rb | 48 ++++++++++++++++++-------------- spec/grape_entity/entity_spec.rb | 22 +++++++++++++-- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/lib/grape_entity/entity.rb b/lib/grape_entity/entity.rb index 90c1f4c..38b2701 100644 --- a/lib/grape_entity/entity.rb +++ b/lib/grape_entity/entity.rb @@ -519,19 +519,11 @@ def serializable_hash(runtime_options = {}) end def exec_with_object(options, &block) - if symbol_to_proc_wrapper?(block) - arity = determine_block_arity(block) - - if arity.positive? - raise ArgumentError, <<~MSG - Symbol to proc wrapper blocks must have zero arguments, but got #{arity}. - Use a regular block with parameters or access presented instance (`object`) - and the top-level entity options (`options`) directly. - MSG - end - else - arity = block.arity - end + arity = if symbol_to_proc_wrapper?(block) + ensure_block_arity!(block) + else + block.arity + end if arity.zero? instance_exec(object, &block) @@ -540,20 +532,34 @@ def exec_with_object(options, &block) end end - def determine_block_arity(block) + 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 - block_arity = block.arity + return 0 unless origin_method_name - return block_arity unless origin_method_name - return block_arity unless object.respond_to?(origin_method_name, true) + 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 - object.method(origin_method_name).arity + arity = object.method(origin_method_name).arity + return 0 if arity.zero? + + 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. + MSG end def symbol_to_proc_wrapper?(block) - block.lambda? && - block.source_location.nil? && - block.parameters == [[:req], [:rest]] + params = block.parameters + + return false unless block.lambda? && block.source_location.nil? + return false unless params.size >= 2 + + params[0].first == :req && params[1].first == :rest end def exec_with_attribute(attribute, &block) diff --git a/spec/grape_entity/entity_spec.rb b/spec/grape_entity/entity_spec.rb index aea31ef..7139fec 100644 --- a/spec/grape_entity/entity_spec.rb +++ b/spec/grape_entity/entity_spec.rb @@ -436,8 +436,13 @@ def raises_argument_error subject.expose :method_without_args, as: :that_method_without_args_again object = SomeObject.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 @@ -452,10 +457,23 @@ def raises_argument_error expect do subject.represent(object).value_for(:that_method_with_one_arg) - end.to raise_error ArgumentError, match(/blocks must have zero arguments/) + end.to raise_error ArgumentError, match(/method expects 1 argument/) + expect do subject.represent(object).value_for(:that_method_with_multple_args) - end.to raise_error ArgumentError, match(/blocks must have zero arguments/) + end.to raise_error ArgumentError, match(/method expects 2 arguments/) + end + end + + context 'with symbol-to-proc passed in via &' do + specify do + subject.expose :that_undefined_method, &:unknown_method + + object = SomeObject.new + + expect do + subject.represent(object).value_for(:that_undefined_method) + end.to raise_error ArgumentError, match(/method is not defined in the object/) end end end