Readability pass: guard clauses + small refactors across lib/

CHANGELOG.md

@@ -45,6 +45,7 @@
 * [#2733](https://github.com/ruby-grape/grape/pull/2733): Drop the dead `active_support/core_ext/hash/reverse_merge` require; call `ActiveSupport::HashWithIndifferentAccess.new(...)` directly at call sites - [@ericproulx](https://github.com/ericproulx).
 * [#2734](https://github.com/ruby-grape/grape/pull/2734): Extract `options_route_enabled` from the Endpoint options Hash into a dedicated `attr_accessor` - [@ericproulx](https://github.com/ericproulx).
 * [#2736](https://github.com/ruby-grape/grape/pull/2736): Collapse `Endpoint#run_validators` rescue branches via `ValidationErrors` flatten; `ValidationErrors#initialize` keyword renamed `errors:` → `exceptions:` - [@ericproulx](https://github.com/ericproulx).
+* [#2741](https://github.com/ruby-grape/grape/pull/2741): Readability pass: guard clauses and small extractions across `lib/` - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/api.rb

@@ -14,7 +14,7 @@ class Boolean
       private_constant :VALUES
 
       def self.build(val)
-        return nil unless VALUES.include?(val)
+        return unless VALUES.include?(val)
 
         new
       end

lib/grape/dsl/entity.rb

@@ -49,29 +49,32 @@ def present(*args, root: nil, with: nil, **options)
       # @param object [Object] the object to locate the Entity class for
       # @return [Class] the located Entity class, or nil if none is found
       def entity_class_for_obj(object)
-        object_class =
-          if object.respond_to?(:klass)
-            object.klass
-          elsif object.respond_to?(:first)
-            object.first.class
-          else
-            object.class
-          end
+        klass = object_class(object)
 
         representations = inheritable_setting.namespace_stackable_with_hash(:representations)
         if representations
-          potential = object_class.ancestors.detect { |potential| representations.key?(potential) }
+          potential = klass.ancestors.detect { |potential| representations.key?(potential) }
           return representations[potential] if potential && representations[potential]
         end
 
-        return unless object_class.const_defined?(:Entity)
+        return unless klass.const_defined?(:Entity)
 
-        entity = object_class.const_get(:Entity)
+        entity = klass.const_get(:Entity)
         entity if entity.respond_to?(:represent)
       end
 
       private
 
+      # Resolves the class used to look up the Entity for +object+.
+      # @param object [Object] the object to represent.
+      # @return [Class] the object's collection element class, wrapped class, or its own class.
+      def object_class(object)
+        return object.klass if object.respond_to?(:klass)
+        return object.first.class if object.respond_to?(:first)
+
+        object.class
+      end
+
       # @param entity_class [Class] the entity class to use for representation.
       # @param object [Object] the object to represent.
       # @param options [Hash] additional options forwarded to the entity's `represent` call.

lib/grape/dsl/inside_route.rb

@@ -60,23 +60,11 @@ def redirect(url, permanent: false, body: nil)
       #
       # @param status [Integer] The HTTP Status Code to return for this request.
       def status(status = nil)
-        case status
-        when Symbol
-          raise ArgumentError, "Status code :#{status} is invalid." unless Rack::Utils::SYMBOL_TO_STATUS_CODE.key?(status)
+        return @status || default_status if status.nil?
 
+        case status
+        when Symbol, Integer
           @status = Rack::Utils.status_code(status)
-        when Integer
-          @status = status
-        when nil
-          return @status if @status
-
-          if request.post?
-            201
-          elsif request.delete?
-            @body.present? ? 200 : 204
-          else
-            200
-          end
         else
           raise ArgumentError, 'Status code must be Integer or Symbol.'
         end
@@ -120,7 +108,6 @@ def body(value = nil)
       #
       #   DELETE /12 # => 204 No Content, ""
       def return_no_content
-        status 204
         body false
       end
 
@@ -165,16 +152,7 @@ def stream(value = nil)
 
         return @stream if value.nil?
 
-        stream_body =
-          if value.is_a?(String)
-            Grape::ServeStream::FileBody.new(value)
-          elsif value.respond_to?(:each)
-            value
-          else
-            raise ArgumentError, 'Stream object must respond to :each.'
-          end
-
-        @stream = Grape::ServeStream::StreamResponse.new(stream_body)
+        @stream = Grape::ServeStream::StreamResponse.new(stream_body(value))
       end
 
       # Returns route information for the current request.
@@ -200,6 +178,25 @@ def api_format(format)
       def context
         self
       end
+
+      private
+
+      # Wraps a stream +value+ into a body that responds to +:each+.
+      def stream_body(value)
+        return Grape::ServeStream::FileBody.new(value) if value.is_a?(String)
+
+        raise ArgumentError, 'Stream object must respond to :each.' unless value.respond_to?(:each)
+
+        value
+      end
+
+      # The default HTTP status when none has been set explicitly.
+      def default_status
+        return 201 if request.post?
+        return 204 if request.delete? && @body.blank?
+
+        200
+      end
     end
   end
 end

lib/grape/dsl/logger.rb

@@ -8,11 +8,9 @@ module Logger
       # @param logger [Object] the new logger to use
       def logger(logger = nil)
         global_settings = inheritable_setting.global
-        if logger
-          global_settings[:logger] = logger
-        else
-          global_settings[:logger] || global_settings[:logger] = ::Logger.new($stdout)
-        end
+        return global_settings[:logger] = logger if logger
+
+        global_settings[:logger] || global_settings[:logger] = ::Logger.new($stdout)
       end
     end
   end

lib/grape/dsl/request_response.rb

@@ -148,7 +148,7 @@ def extract_handler(args, with:, block:)
 
         case with
         when Proc, Symbol then with
-        when String       then with.to_sym
+        when String then with.to_sym
         else raise ArgumentError, "with: #{with.class}, expected Symbol, String or Proc"
         end
       end

lib/grape/exceptions/error_response.rb

@@ -30,10 +30,14 @@ def self.from_exception(exception)
       # own middleware or `rescue_from` handlers.
       def self.coerce(input)
         case input
-        when ErrorResponse           then input
-        when Grape::Exceptions::Base then from_exception(input)
-        when Hash                    then new(**input.slice(:status, :message, :headers, :backtrace, :original_exception))
-        else                              new
+        when ErrorResponse
+          input
+        when Grape::Exceptions::Base
+          from_exception(input)
+        when Hash
+          new(**input.slice(:status, :message, :headers, :backtrace, :original_exception))
+        else
+          new
         end
       end
     end

lib/grape/formatter/serializable_hash.rb

@@ -19,15 +19,11 @@ def serializable?(object)
         end
 
         def serialize(object)
-          if object.respond_to? :serializable_hash
-            object.serializable_hash
-          elsif array_serializable?(object)
-            object.map(&:serializable_hash)
-          elsif object.is_a?(Hash)
-            object.transform_values { |v| serialize(v) }
-          else
-            object
-          end
+          return object.serializable_hash if object.respond_to?(:serializable_hash)
+          return object.map(&:serializable_hash) if array_serializable?(object)
+          return object.transform_values { |v| serialize(v) } if object.is_a?(Hash)
+
+          object
         end
 
         def array_serializable?(object)

lib/grape/middleware/base.rb

@@ -79,13 +79,10 @@ def merge_headers(response)
       end
 
       def merge_default_options(options)
-        if respond_to?(:default_options)
-          default_options.deep_merge(options)
-        elsif self.class.const_defined?(:DEFAULT_OPTIONS)
-          self.class::DEFAULT_OPTIONS.deep_merge(options)
-        else
-          options
-        end
+        return default_options.deep_merge(options) if respond_to?(:default_options)
+        return self.class::DEFAULT_OPTIONS.deep_merge(options) if self.class.const_defined?(:DEFAULT_OPTIONS)
+
+        options
       end
 
       def try_scrub(obj)

lib/grape/middleware/error.rb

@@ -165,13 +165,12 @@ def run_rescue_handler(handler, error, endpoint, redispatched: false)
       def redispatch(error, endpoint, already_redispatched)
         return framework_default(endpoint) if already_redispatched
 
-        if (registered = registered_rescue_handler(error.class))
-          run_rescue_handler(registered, error, endpoint, redispatched: true)
-        elsif error.is_a?(Grape::Exceptions::Base)
-          run_rescue_handler(method(:error_response), error, endpoint, redispatched: true)
-        else
-          safe_default(error, endpoint)
-        end
+        registered = registered_rescue_handler(error.class)
+
+        return run_rescue_handler(registered, error, endpoint, redispatched: true) if registered
+        return run_rescue_handler(method(:error_response), error, endpoint, redispatched: true) if error.is_a?(Grape::Exceptions::Base)
+
+        safe_default(error, endpoint)
       end
 
       # The unrecognised-error path. Exposes the original exception on

lib/grape/middleware/formatter.rb

@@ -35,11 +35,9 @@ def after
 
         status, headers, bodies = *@app_response
 
-        if Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status)
-          [status, headers, []]
-        else
-          build_formatted_response(status, headers, bodies)
-        end
+        return [status, headers, []] if Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status)
+
+        build_formatted_response(status, headers, bodies)
       end
 
       private
@@ -136,11 +134,9 @@ def read_body_input?
 
       def negotiate_content_type
         fmt = format_from_extension || query_params['format'] || format || format_from_header || default_format
-        if content_type_for(fmt)
-          env[Grape::Env::API_FORMAT] = fmt.to_sym
-        else
-          throw :error, Grape::Exceptions::ErrorResponse.new(status: 406, message: "The requested format '#{fmt}' is not supported.")
-        end
+        return env[Grape::Env::API_FORMAT] = fmt.to_sym if content_type_for(fmt)
+
+        throw :error, Grape::Exceptions::ErrorResponse.new(status: 406, message: "The requested format '#{fmt}' is not supported.")
       end
 
       def format_from_extension

lib/grape/router/pattern.rb

@@ -39,15 +39,11 @@ def extract_capture(format, version, requirements)
       end
 
       def build_path_from_pattern(pattern, anchor)
-        if pattern.end_with?('*path')
-          pattern.dup.insert(pattern.rindex('/') + 1, '?')
-        elsif anchor
-          pattern
-        elsif pattern.end_with?('/')
-          "#{pattern}?*path"
-        else
-          "#{pattern}/?*path"
-        end
+        return pattern.dup.insert(pattern.rindex('/') + 1, '?') if pattern.end_with?('*path')
+        return pattern if anchor
+        return "#{pattern}?*path" if pattern.end_with?('/')
+
+        "#{pattern}/?*path"
       end
 
       def map_str(value)

lib/grape/serve_stream/sendfile_response.rb

@@ -6,11 +6,9 @@ module ServeStream
     # for using Rack::SendFile middleware
     class SendfileResponse < Rack::Response
       def respond_to?(method_name, include_all = false)
-        if method_name == :to_path
-          @body.respond_to?(:to_path, include_all)
-        else
-          super
-        end
+        return @body.respond_to?(:to_path, include_all) if method_name == :to_path
+
+        super
       end
 
       def to_path

lib/grape/validations/params_scope.rb

@@ -314,12 +314,9 @@ def new_group_scope(group, &)
       # push_declared_params cannot be called on the frozen scope later.
       def configure_declared_params
         push_renamed_param(full_path, @element_renamed) if @element_renamed
+        return @parent.push_declared_params [{ @element => @declared_params }] if nested?
 
-        if nested?
-          @parent.push_declared_params [{ @element => @declared_params }]
-        else
-          @api.inheritable_setting.namespace_stackable[:declared_params] = @declared_params
-        end
+        @api.inheritable_setting.namespace_stackable[:declared_params] = @declared_params
       ensure
         @declared_params = nil
       end

lib/grape/validations/types/primitive_coercer.rb

@@ -15,7 +15,7 @@ def initialize(type, strict: false)
 
         def call(val)
           return InvalidValue.new if reject?(val)
-          return nil if val.nil? || treat_as_nil?(val)
+          return if val.nil? || treat_as_nil?(val)
 
           super
         end

spec/grape/dsl/inside_route_spec.rb

@@ -137,7 +137,7 @@ def header(key = nil, val = nil)
 
     it 'raises error if unknow symbol is passed' do
       expect { subject.status :foo_bar }
-        .to raise_error(ArgumentError, 'Status code :foo_bar is invalid.')
+        .to raise_error(ArgumentError, 'Unrecognized status code :foo_bar')
     end
 
     it 'accepts unknown Integer status codes' do