diff --git a/CHANGELOG.md b/CHANGELOG.md index bb110d91bf..12e674d8e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ * Your contribution here. #### Fixes + +* [#1995](https://github.com/ruby-grape/grape/pull/1995): Fix: "undefined instance variables" and "method redefined" warnings - [@nbeyer](https://github.com/nbeyer). * [#1994](https://github.com/ruby-grape/grape/pull/1993): Fix typos in README - [@bellmyer](https://github.com/bellmyer). * [#1993](https://github.com/ruby-grape/grape/pull/1993): Lazy join allow header - [@ericproulx](https://github.com/ericproulx). * [#1987](https://github.com/ruby-grape/grape/pull/1987): Re-add exactly_one_of mutually exclusive error message - [@ZeroInputCtrl](https://github.com/ZeroInputCtrl). @@ -13,6 +15,7 @@ * [#1971](https://github.com/ruby-grape/grape/pull/1971): Fix BigDecimal coercion - [@FlickStuart](https://github.com/FlickStuart). * [#1968](https://github.com/ruby-grape/grape/pull/1968): Fix args forwarding in Grape::Middleware::Stack#merge_with for ruby 2.7.0 - [@dm1try](https://github.com/dm1try). * [#1988](https://github.com/ruby-grape/grape/pull/1988): Refactored the full_messages method and stop overriding full_message - [@hosseintoussi](https://github.com/hosseintoussi). + ### 1.3.0 (2020/01/11) #### Features diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 7df189d35f..29f1c5786d 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -8,7 +8,7 @@ module Grape # should subclass this class in order to build an API. class API # Class methods that we want to call on the API rather than on the API object - NON_OVERRIDABLE = (Class.new.methods + %i[call call! configuration compile!]).freeze + NON_OVERRIDABLE = (Class.new.methods + %i[call call! configuration compile! inherited]).freeze class << self attr_accessor :base_instance, :instances diff --git a/lib/grape/dsl/helpers.rb b/lib/grape/dsl/helpers.rb index 0d3c7e481d..bbd2ed3ba3 100644 --- a/lib/grape/dsl/helpers.rb +++ b/lib/grape/dsl/helpers.rb @@ -94,7 +94,7 @@ def api_changed(new_api) protected def process_named_params - return unless @named_params && @named_params.any? + return unless instance_variable_defined?(:@named_params) && @named_params && @named_params.any? api.namespace_stackable(:named_params, @named_params) end end diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 5943b1f73c..9bd6f3516f 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -182,12 +182,12 @@ def status(status = nil) when Integer @status = status when nil - return @status if @status + return @status if instance_variable_defined?(:@status) && @status case request.request_method.to_s.upcase when Grape::Http::Headers::POST 201 when Grape::Http::Headers::DELETE - if @body.present? + if instance_variable_defined?(:@body) && @body.present? 200 else 204 @@ -238,7 +238,7 @@ def body(value = nil) @body = '' status 204 else - @body + instance_variable_defined?(:@body) ? @body : nil end end @@ -272,7 +272,7 @@ def file(value = nil) warn '[DEPRECATION] Argument as FileStreamer-like object is deprecated. Use path to file instead.' @file = Grape::ServeFile::FileResponse.new(value) else - @file + instance_variable_defined?(:@file) ? @file : nil end end @@ -331,11 +331,12 @@ def present(*args) end representation = { root => representation } if root + if key - representation = (@body || {}).merge(key => representation) - elsif entity_class.present? && @body + representation = (body || {}).merge(key => representation) + elsif entity_class.present? && body raise ArgumentError, "Representation of type #{representation.class} cannot be merged." unless representation.respond_to?(:merge) - representation = @body.merge(representation) + representation = body.merge(representation) end body representation @@ -387,7 +388,7 @@ def entity_class_for_obj(object, options) def entity_representation_for(entity_class, object, options) embeds = { env: env } embeds[:version] = env[Grape::Env::API_VERSION] if env[Grape::Env::API_VERSION] - entity_class.represent(object, **embeds.merge(options)) + entity_class.represent(object, embeds.merge(options)) end end end diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index bb5e4034ad..9d393fd934 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -127,7 +127,7 @@ def requires(*attrs, &block) opts = attrs.extract_options!.clone opts[:presence] = { value: true, message: opts[:message] } - opts = @group.merge(opts) if @group + opts = @group.merge(opts) if instance_variable_defined?(:@group) && @group if opts[:using] require_required_and_optional_fields(attrs.first, opts) @@ -146,7 +146,7 @@ def optional(*attrs, &block) opts = attrs.extract_options!.clone type = opts[:type] - opts = @group.merge(opts) if @group + opts = @group.merge(opts) if instance_variable_defined?(:@group) && @group # check type for optional parameter group if attrs && block_given? @@ -243,8 +243,8 @@ def map_params(params, element) # @return hash of parameters relevant for the current scope # @api private def params(params) - params = @parent.params(params) if @parent - params = map_params(params, @element) if @element + params = @parent.params(params) if instance_variable_defined?(:@parent) && @parent + params = map_params(params, @element) if instance_variable_defined?(:@element) && @element params end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index ecbbb40ce7..ac7e9e85c7 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -51,7 +51,7 @@ def version(*args, &block) end end - @versions.last unless @versions.nil? + @versions.last if instance_variable_defined?(:@versions) && @versions end # Define a root URL prefix for your entire API. @@ -163,6 +163,8 @@ def route(methods, paths = ['/'], route_options = {}, &block) # end # end def namespace(space = nil, options = {}, &block) + @namespace_description = nil unless instance_variable_defined?(:@namespace_description) && @namespace_description + if space || block_given? within_namespace do previous_namespace_description = @namespace_description diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 108043d50a..43abeffbe6 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -935,7 +935,7 @@ class DummyFormatClass it 'adds a before filter to current and child namespaces only' do subject.get '/' do - "root - #{@foo}" + "root - #{instance_variable_defined?(:@foo) ? @foo : nil}" end subject.namespace :blah do before { @foo = 'foo' } @@ -3677,12 +3677,13 @@ def before end end context ':serializable_hash' do - before(:each) do - class SerializableHashExample - def serializable_hash - { abc: 'def' } - end + class SerializableHashExample + def serializable_hash + { abc: 'def' } end + end + + before(:each) do subject.format :serializable_hash end it 'instance' do diff --git a/spec/grape/validations/instance_behaivour_spec.rb b/spec/grape/validations/instance_behaivour_spec.rb index 9db070d95e..9f2038dce3 100644 --- a/spec/grape/validations/instance_behaivour_spec.rb +++ b/spec/grape/validations/instance_behaivour_spec.rb @@ -6,7 +6,7 @@ let(:validator_type) do Class.new(Grape::Validations::Base) do def validate_param!(_attr_name, _params) - if @instance_variable + if instance_variable_defined?(:@instance_variable) && @instance_variable raise Grape::Exceptions::Validation.new(params: ['params'], message: 'This should never happen') end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 18fae8d8e5..db3cf8b7b9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -35,6 +35,7 @@ def read_chunks(body) config.include Spec::Support::Helpers config.raise_errors_for_deprecations! config.filter_run_when_matching :focus + config.warnings = true config.before(:each) { Grape::Util::InheritableSetting.reset_global! }