PR recommendations

Add argumenterror specs

Author: marcrohloff

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

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
 

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

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