Merge pull request #2618 from ruby-grape/ruby3_handling_argument_delegation

Modernize argument delegation for Ruby 3+ compatibility

Author: dblock

CHANGELOG.md

@@ -23,6 +23,7 @@
 * [#2615](https://github.com/ruby-grape/grape/pull/2615): Remove manual toc and tod danger check - [@alexanderadam](https://github.com/alexanderadam).
 * [#2612](https://github.com/ruby-grape/grape/pull/2612): Avoid multiple mount pollution - [@alexanderadam](https://github.com/alexanderadam).
 * [#2617](https://github.com/ruby-grape/grape/pull/2617): Migrate from `ActiveSupport::Configurable` to `Dry::Configurable` - [@ericproulx](https://github.com/ericproulx).
+* [#2618](https://github.com/ruby-grape/grape/pull/2618): Modernize argument delegation for Ruby 3+ compatibility - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

UPGRADING.md

@@ -3,6 +3,18 @@ Upgrading Grape
 
 ### Upgrading to >= 3.0.0
 
+#### Ruby 3+ Argument Delegation Modernization
+
+Grape has been modernized to use Ruby 3+'s preferred argument delegation patterns. This change replaces `args.extract_options!` with explicit `**kwargs` parameters throughout the codebase.
+
+- All DSL methods now use explicit keyword arguments (`**kwargs`) instead of extracting options from mixed argument lists
+- Method signatures are now more explicit and follow Ruby 3+ best practices
+- The `active_support/core_ext/array/extract_options` dependency has been removed
+
+This is a modernization effort that improves code quality while maintaining full backward compatibility.
+
+See [#2618](https://github.com/ruby-grape/grape/pull/2618) for more information.
+
 #### Configuration API Migration from ActiveSupport::Configurable to Dry::Configurable
 
 Grape has migrated from `ActiveSupport::Configurable` to `Dry::Configurable` for its configuration system since its [deprecated](https://github.com/rails/rails/blob/1cdd190a25e483b65f1f25bbd0f13a25d696b461/activesupport/lib/active_support/configurable.rb#L3-L7).

lib/grape.rb

@@ -6,11 +6,11 @@
 require 'active_support/version'
 require 'active_support/isolated_execution_state'
 require 'active_support/core_ext/array/conversions'
-require 'active_support/core_ext/array/extract_options'
 require 'active_support/core_ext/array/wrap'
 require 'active_support/core_ext/enumerable'
 require 'active_support/core_ext/hash/conversions'
 require 'active_support/core_ext/hash/deep_merge'
+require 'active_support/core_ext/hash/deep_transform_values'
 require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/hash/indifferent_access'
 require 'active_support/core_ext/hash/keys'

lib/grape/api.rb

@@ -30,15 +30,6 @@ class << self
       # NOTE: This will only be called on an API directly mounted on RACK
       def_delegators :base_instance, :new, :configuration, :call, :compile!
 
-      # When inherited, will create a list of all instances (times the API was mounted)
-      # It will listen to the setup required to mount that endpoint, and replicate it on any new instance
-      def inherited(api)
-        super
-
-        api.initial_setup(self == Grape::API ? Grape::API::Instance : @base_instance)
-        api.override_all_methods!
-      end
-
       # Initialize the instance variables on the remountable class, and the base_instance
       # an instance that will be used to create the set up but will not be mounted
       def initial_setup(base_instance_parent)
@@ -51,8 +42,8 @@ def initial_setup(base_instance_parent)
       # Redefines all methods so that are forwarded to add_setup and be recorded
       def override_all_methods!
         (base_instance.methods - Class.methods - NON_OVERRIDABLE).each do |method_override|
-          define_singleton_method(method_override) do |*args, &block|
-            add_setup(method: method_override, args: args, block: block)
+          define_singleton_method(method_override) do |*args, **kwargs, &block|
+            add_setup(method: method_override, args: args, kwargs: kwargs, block: block)
           end
         end
       end
@@ -86,6 +77,15 @@ def mount_instance(configuration: nil)
 
       private
 
+      # When inherited, will create a list of all instances (times the API was mounted)
+      # It will listen to the setup required to mount that endpoint, and replicate it on any new instance
+      def inherited(api)
+        super
+
+        api.initial_setup(self == Grape::API ? Grape::API::Instance : @base_instance)
+        api.override_all_methods!
+      end
+
       # Replays the set up to produce an API as defined in this class, can be called
       # on classes that inherit from Grape::API
       def replay_setup_on(instance)
@@ -95,7 +95,7 @@ def replay_setup_on(instance)
       end
 
       # Adds a new stage to the set up require to get a Grape::API up and running
-      def add_setup(step)
+      def add_setup(**step)
         @setup << step
         last_response = nil
         @instances.each do |instance|
@@ -119,22 +119,23 @@ def refresh_mount_step
         end
       end
 
-      def replay_step_on(instance, method:, args:, block:)
-        return if skip_immediate_run?(instance, args)
+      def replay_step_on(instance, method:, args:, kwargs:, block:)
+        return if skip_immediate_run?(instance, args, kwargs)
 
         eval_args = evaluate_arguments(instance.configuration, *args)
-        response = instance.__send__(method, *eval_args, &block)
-        if skip_immediate_run?(instance, [response])
+        eval_kwargs = kwargs.deep_transform_values { |v| evaluate_arguments(instance.configuration, v).first }
+        response = instance.__send__(method, *eval_args, **eval_kwargs, &block)
+        if skip_immediate_run?(instance, [response], kwargs)
           response
         else
           evaluate_arguments(instance.configuration, response).first
         end
       end
 
       # Skips steps that contain arguments to be lazily executed (on re-mount time)
-      def skip_immediate_run?(instance, args)
+      def skip_immediate_run?(instance, args, kwargs)
         instance.base_instance? &&
-          (any_lazy?(args) || args.any? { |arg| arg.is_a?(Hash) && any_lazy?(arg.values) })
+          (any_lazy?(args) || args.any? { |arg| arg.is_a?(Hash) && any_lazy?(arg.values) } || any_lazy?(kwargs.values))
       end
 
       def any_lazy?(args)

lib/grape/api/instance.rb

@@ -61,11 +61,6 @@ def compile
           @instance ||= new # rubocop:disable Naming/MemoizedInstanceVariableName
         end
 
-        # Wipe the compiled API so we can recompile after changes were made.
-        def change!
-          @instance = nil
-        end
-
         # This is the interface point between Rack and Grape; it accepts a request
         # from Rack and ultimately returns an array of three values: the status,
         # the headers, and the body. See [the rack specification]
@@ -101,12 +96,6 @@ def recognize_path(path)
 
         protected
 
-        def inherited(subclass)
-          super
-          subclass.reset!
-          subclass.logger logger.clone
-        end
-
         def inherit_settings(other_settings)
           top_level_setting.inherit_from other_settings.point_in_time_copy
 
@@ -119,6 +108,19 @@ def inherit_settings(other_settings)
 
           reset_routes!
         end
+
+        # Wipe the compiled API so we can recompile after changes were made.
+        def change!
+          @instance = nil
+        end
+
+        private
+
+        def inherited(subclass)
+          super
+          subclass.reset!
+          subclass.logger logger.clone
+        end
       end
 
       attr_reader :router

lib/grape/dsl/inside_route.rb

@@ -337,8 +337,7 @@ def stream(value = nil)
       #       with: API::Entities::User,
       #       admin: current_user.admin?
       #   end
-      def present(*args)
-        options = args.count > 1 ? args.extract_options! : {}
+      def present(*args, **options)
         key, object = if args.count == 2 && args.first.is_a?(Symbol)
                         args
                       else

lib/grape/dsl/parameters.rb

@@ -53,9 +53,8 @@ def build_with(build_with)
       #         Collection.page(params[:page]).per(params[:per_page])
       #       end
       #     end
-      def use(*names)
+      def use(*names, **options)
         named_params = @api.inheritable_setting.namespace_stackable_with_hash(:named_params) || {}
-        options = names.extract_options!
         names.each do |name|
           params_block = named_params.fetch(name) do
             raise "Params :#{name} not found!"
@@ -123,29 +122,23 @@ def use(*names)
       #         requires :name, type: String
       #       end
       #     end
-      def requires(*attrs, &block)
-        orig_attrs = attrs.clone
-
-        opts = attrs.extract_options!.clone
+      def requires(*attrs, **opts, &block)
         opts[:presence] = { value: true, message: opts[:message] }
         opts = @group.deep_merge(opts) if instance_variable_defined?(:@group) && @group
 
         if opts[:using]
           require_required_and_optional_fields(attrs.first, opts)
         else
           validate_attributes(attrs, opts, &block)
-          block ? new_scope(orig_attrs, &block) : push_declared_params(attrs, opts.slice(:as))
+          block ? new_scope(attrs, opts, &block) : push_declared_params(attrs, opts.slice(:as))
         end
       end
 
       # Allow, but don't require, one or more parameters for the current
       #   endpoint.
       # @param (see #requires)
       # @option (see #requires)
-      def optional(*attrs, &block)
-        orig_attrs = attrs.clone
-
-        opts = attrs.extract_options!.clone
+      def optional(*attrs, **opts, &block)
         type = opts[:type]
         opts = @group.deep_merge(opts) if instance_variable_defined?(:@group) && @group
 
@@ -160,7 +153,7 @@ def optional(*attrs, &block)
         else
           validate_attributes(attrs, opts, &block)
 
-          block ? new_scope(orig_attrs, true, &block) : push_declared_params(attrs, opts.slice(:as))
+          block ? new_scope(attrs, opts, true, &block) : push_declared_params(attrs, opts.slice(:as))
         end
       end
 

lib/grape/dsl/request_response.rb

@@ -93,14 +93,13 @@ def default_error_status(new_status = nil)
       #   @option options [Boolean] :rescue_subclasses Also rescue subclasses of exception classes
       #   @param [Proc] handler Execution proc to handle the given exception as an
       #     alternative to passing a block.
-      def rescue_from(*args, &block)
+      def rescue_from(*args, **options, &block)
         if args.last.is_a?(Proc)
           handler = args.pop
         elsif block
           handler = block
         end
 
-        options = args.extract_options!
         raise ArgumentError, 'both :with option and block cannot be passed' if block && options.key?(:with)
 
         handler ||= extract_with(options)

lib/grape/dsl/routing.rb

@@ -22,9 +22,8 @@ module Routing
       #     end
       #   end
       #
-      def version(*args, &block)
+      def version(*args, **options, &block)
         if args.any?
-          options = args.extract_options!
           options = options.reverse_merge(using: :path)
           requested_versions = args.flatten.map(&:to_s)
 
@@ -165,8 +164,7 @@ def route(methods, paths = ['/'], route_options = {}, &block)
       end
 
       Grape::HTTP_SUPPORTED_METHODS.each do |supported_method|
-        define_method supported_method.downcase do |*args, &block|
-          options = args.extract_options!
+        define_method supported_method.downcase do |*args, **options, &block|
           paths = args.first || ['/']
           route(supported_method, paths, options, &block)
         end

lib/grape/error_formatter/base.rb

@@ -58,6 +58,8 @@ def format_structured_message(_structured_message)
           raise NotImplementedError
         end
 
+        private
+
         def inherited(klass)
           super
           ErrorFormatter.register(klass)