diff --git a/CHANGELOG.md b/CHANGELOG.md index 102494a87..0e7f622e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ * [#2697](https://github.com/ruby-grape/grape/pull/2697): Extract `Grape::Util::PathNormalizer` from `Grape::Router`; `Grape::Router.normalize_path` is now a deprecated alias - [@ericproulx](https://github.com/ericproulx). * [#2696](https://github.com/ruby-grape/grape/pull/2696): Reduce per-request allocations on the request hot path; migrate middleware options to `attr_reader` and freeze `@options` post-init - [@ericproulx](https://github.com/ericproulx). * [#2693](https://github.com/ruby-grape/grape/pull/2693): Introduce `Grape::Exceptions::ErrorResponse` value object to replace the implicit-schema Hash thrown via `throw` - [@ericproulx](https://github.com/ericproulx). +* [#2701](https://github.com/ruby-grape/grape/pull/2701): Replace `.tap` usages in `lib/` with explicit local variables - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 9e31322cd..90824e607 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -67,11 +67,11 @@ def configure # depending on where the endpoint is mounted. Use with care, if you find yourself using configuration # too much, you may actually want to provide a new API rather than remount it. def mount_instance(configuration: nil) - Class.new(@base_parent).tap do |instance| - instance.configuration = Grape::Util::EndpointConfiguration.new(configuration || {}) - instance.base = self - replay_setup_on(instance) - end + instance = Class.new(@base_parent) + instance.configuration = Grape::Util::EndpointConfiguration.new(configuration || {}) + instance.base = self + replay_setup_on(instance) + instance end private diff --git a/lib/grape/dsl/helpers.rb b/lib/grape/dsl/helpers.rb index cfcb07c15..a4626c4a3 100644 --- a/lib/grape/dsl/helpers.rb +++ b/lib/grape/dsl/helpers.rb @@ -45,9 +45,8 @@ def include_new_modules(modules) def include_block(block) return unless block - Module.new.tap do |mod| - make_inclusion(mod) { mod.class_eval(&block) } - end + mod = Module.new + make_inclusion(mod) { mod.class_eval(&block) } end def make_inclusion(mod, &) @@ -57,10 +56,10 @@ def make_inclusion(mod, &) end def include_all_in_scope - Module.new.tap do |mod| - namespace_stackable(:helpers).each { |mod_to_include| mod.include mod_to_include } - change! - end + mod = Module.new + namespace_stackable(:helpers).each { |mod_to_include| mod.include mod_to_include } + change! + mod end def define_boolean_in_mod(mod) diff --git a/lib/grape/dsl/settings.rb b/lib/grape/dsl/settings.rb index 730c8e7e7..bc62dbbd1 100644 --- a/lib/grape/dsl/settings.rb +++ b/lib/grape/dsl/settings.rb @@ -11,19 +11,25 @@ module Settings # Fetch our top-level settings, which apply to all endpoints in the API. def top_level_setting - @top_level_setting ||= Grape::Util::InheritableSetting.new.tap do |setting| - # Doesn't try to inherit settings from +Grape::API::Instance+ which also responds to - # +inheritable_setting+, however, it doesn't contain any user-defined settings. - # Otherwise, it would lead to an extra instance of +Grape::Util::InheritableSetting+ - # in the chain for every endpoint. - setting.inherit_from superclass.inheritable_setting if defined?(superclass) && superclass.respond_to?(:inheritable_setting) && superclass != Grape::API::Instance - end + return @top_level_setting if @top_level_setting + + @top_level_setting = Grape::Util::InheritableSetting.new + # Doesn't try to inherit settings from +Grape::API::Instance+ which also responds to + # +inheritable_setting+, however, it doesn't contain any user-defined settings. + # Otherwise, it would lead to an extra instance of +Grape::Util::InheritableSetting+ + # in the chain for every endpoint. + @top_level_setting.inherit_from superclass.inheritable_setting if defined?(superclass) && superclass.respond_to?(:inheritable_setting) && superclass != Grape::API::Instance + @top_level_setting end # Fetch our current inheritable settings, which are inherited by # nested scopes but not shared across siblings. def inheritable_setting - @inheritable_setting ||= Grape::Util::InheritableSetting.new.tap { |new_settings| new_settings.inherit_from top_level_setting } + return @inheritable_setting if @inheritable_setting + + @inheritable_setting = Grape::Util::InheritableSetting.new + @inheritable_setting.inherit_from top_level_setting + @inheritable_setting end def global_setting(key, value = nil) diff --git a/lib/grape/error_formatter/txt.rb b/lib/grape/error_formatter/txt.rb index 7c0c75918..0c66eacde 100644 --- a/lib/grape/error_formatter/txt.rb +++ b/lib/grape/error_formatter/txt.rb @@ -5,16 +5,16 @@ module ErrorFormatter class Txt < Base def self.format_structured_message(structured_message) message = structured_message[:message] || Grape::Json.dump(structured_message) - Array.wrap(message).tap do |final_message| - if structured_message.key?(:backtrace) - final_message << 'backtrace:' - final_message.concat(structured_message[:backtrace]) - end - if structured_message.key?(:original_exception) - final_message << 'original exception:' - final_message << structured_message[:original_exception] - end - end.join("\r\n ") + final_message = Array.wrap(message) + if structured_message.key?(:backtrace) + final_message << 'backtrace:' + final_message.concat(structured_message[:backtrace]) + end + if structured_message.key?(:original_exception) + final_message << 'original exception:' + final_message << structured_message[:original_exception] + end + final_message.join("\r\n ") end end end diff --git a/lib/grape/middleware/auth/base.rb b/lib/grape/middleware/auth/base.rb index 019dfefbc..931d67fc2 100644 --- a/lib/grape/middleware/auth/base.rb +++ b/lib/grape/middleware/auth/base.rb @@ -8,9 +8,8 @@ def initialize(app, **options) super return unless options.key?(:type) - @auth_strategy = Grape::Middleware::Auth::Strategies[options[:type]].tap do |auth_strategy| - raise Grape::Exceptions::UnknownAuthStrategy.new(strategy: options[:type]) unless auth_strategy - end + @auth_strategy = Grape::Middleware::Auth::Strategies[options[:type]] + raise Grape::Exceptions::UnknownAuthStrategy.new(strategy: options[:type]) unless @auth_strategy end def call!(env) diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index ca412b65a..cf8b1f62c 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -128,7 +128,7 @@ def rescue_handler_for_any_class(klass) end def run_rescue_handler(handler, error, endpoint) - handler = endpoint.public_method(handler) if handler.instance_of?(Symbol) + handler = endpoint.public_method(handler) if handler.is_a?(Symbol) response = catch(:error) do handler.arity.zero? ? endpoint.instance_exec(&handler) : endpoint.instance_exec(error, &handler) end diff --git a/lib/grape/middleware/stack.rb b/lib/grape/middleware/stack.rb index 6d6bbcf1b..8d9b93e11 100644 --- a/lib/grape/middleware/stack.rb +++ b/lib/grape/middleware/stack.rb @@ -83,12 +83,10 @@ def merge_with(middleware_specs) # @return [Rack::Builder] the builder object with our middlewares applied def build - Rack::Builder.new.tap do |builder| - others.shift(others.size).each { |m| merge_with(m) } - middlewares.each do |m| - m.build(builder) - end - end + builder = Rack::Builder.new + others.shift(others.size).each { |m| merge_with(m) } + middlewares.each { |m| m.build(builder) } + builder end # @description Add middlewares with :use operation to the stack. Store others with :insert_* operation for later diff --git a/lib/grape/path.rb b/lib/grape/path.rb index 0dd1fd148..fba75a34f 100644 --- a/lib/grape/path.rb +++ b/lib/grape/path.rb @@ -31,13 +31,13 @@ def build_suffix(raw_path, raw_namespace, settings) end def build_parts(raw_path, raw_namespace, settings) - [].tap do |parts| - add_part(parts, settings[:mount_path]) - add_part(parts, settings[:root_prefix]) - parts << VERSION_SEGMENT if uses_path_versioning?(settings) - add_part(parts, raw_namespace) - add_part(parts, raw_path) - end + parts = [] + add_part(parts, settings[:mount_path]) + add_part(parts, settings[:root_prefix]) + parts << VERSION_SEGMENT if uses_path_versioning?(settings) + add_part(parts, raw_namespace) + add_part(parts, raw_path) + parts end def add_part(parts, value) diff --git a/lib/grape/util/base_inheritable.rb b/lib/grape/util/base_inheritable.rb index c70ad20f1..d686a9e2f 100644 --- a/lib/grape/util/base_inheritable.rb +++ b/lib/grape/util/base_inheritable.rb @@ -28,14 +28,9 @@ def initialize_copy(other) end def keys - if new_values.any? - inherited_values.keys.tap do |combined| - combined.concat(new_values.keys) - combined.uniq! - end - else - inherited_values.keys - end + return inherited_values.keys if new_values.empty? + + (inherited_values.keys + new_values.keys).uniq end def key?(name) diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index cae002774..e74ab269d 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -63,19 +63,19 @@ def inherit_from(parent) # changed via #inherit_from, it will copy that inheritence to any copies # which were made. def point_in_time_copy - self.class.new.tap do |new_setting| - point_in_time_copies << new_setting - new_setting.point_in_time_copies = [] - - new_setting.namespace = namespace.clone - new_setting.namespace_inheritable = namespace_inheritable.clone - new_setting.namespace_stackable = namespace_stackable.clone - new_setting.namespace_reverse_stackable = namespace_reverse_stackable.clone - new_setting.route = route.clone - new_setting.api_class = api_class - - new_setting.inherit_from(parent) - end + new_setting = self.class.new + point_in_time_copies << new_setting + new_setting.point_in_time_copies = [] + + new_setting.namespace = namespace.clone + new_setting.namespace_inheritable = namespace_inheritable.clone + new_setting.namespace_stackable = namespace_stackable.clone + new_setting.namespace_reverse_stackable = namespace_reverse_stackable.clone + new_setting.route = route.clone + new_setting.api_class = api_class + + new_setting.inherit_from(parent) + new_setting end # Resets the instance store of per-route settings. diff --git a/lib/grape/validations/params_documentation.rb b/lib/grape/validations/params_documentation.rb index 0d4e94313..22d55f21e 100644 --- a/lib/grape/validations/params_documentation.rb +++ b/lib/grape/validations/params_documentation.rb @@ -18,23 +18,24 @@ def document_params(attrs, validations, type = nil, values = nil, except_values private def extract_details(validations, type, values, except_values) - {}.tap do |details| - details[:required] = validations.key?(:presence) - details[:type] = TypeCache[type] if type - details[:values] = values if values - details[:except_values] = except_values if except_values - details[:default] = validations[:default] if validations.key?(:default) - if validations.key?(:length) - details[:min_length] = validations[:length][:min] if validations[:length].key?(:min) - details[:max_length] = validations[:length][:max] if validations[:length].key?(:max) - end + details = {} + details[:required] = validations.key?(:presence) + details[:type] = TypeCache[type] if type + details[:values] = values if values + details[:except_values] = except_values if except_values + details[:default] = validations[:default] if validations.key?(:default) + if validations.key?(:length) + details[:min_length] = validations[:length][:min] if validations[:length].key?(:min) + details[:max_length] = validations[:length][:max] if validations[:length].key?(:max) + end - desc = validations.delete(:desc) || validations.delete(:description) - details[:desc] = desc if desc + desc = validations.delete(:desc) || validations.delete(:description) + details[:desc] = desc if desc - documentation = validations.delete(:documentation) - details[:documentation] = documentation if documentation - end + documentation = validations.delete(:documentation) + details[:documentation] = documentation if documentation + + details end class TypeCache < Grape::Util::Cache