Address some Ruby warnings (#1995)

* Address some Ruby warnings

* Add warnings enablement to spec_helper.rb to see future warnings
* Use instance_variable_defined? to clean up not initialized warnings
* Add 'inherited' to NON_OVERRIDABLE list
* Add multi_xml to test dependencies in Gemfile to address spec failure

* Add changelog entry

* expand changelog entry

* adjust changelog

* remove unnecessary multi_xml

Author: nbeyer

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

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

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

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

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
 

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

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

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

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! }