Add is_a?(Array) to coerce_nil because of Array with a type.

Add CHANGELOG.md

Add documentation in README.md
Add specs for future proof

return nil instead of InvalidValue

Remove Structures Implicit Default Value section

Not returning when param's value is nil

Remove default value for structures

Add specs for nil values and default values

Add non-empty value spec when params is nil

Typo change

Rubocop

Author: ericproulx

CHANGELOG.md

@@ -9,6 +9,7 @@
 #### Fixes
 
 * Your contribution here.
+* [#2040](https://github.com/ruby-grape/grape/pull/2040): Fix a regression with Array of type nil - [@ericproulx](https://github.com/ericproulx).
 
 ### 1.3.2 (2020/04/12)
 

lib/grape/validations/types/variant_collection_coercer.rb

@@ -33,7 +33,7 @@ def initialize(types, method = nil)
         #   the coerced result, or an instance
         #   of {InvalidValue} if the value could not be coerced.
         def call(value)
-          return InvalidValue.new unless value.is_a? Array
+          return unless value.is_a? Array
 
           value =
             if @method

lib/grape/validations/validators/coerce.rb

@@ -67,21 +67,12 @@ def valid_type?(val)
       end
 
       def coerce_value(val)
-        val.nil? ? coerce_nil(val) : converter.call(val)
+        converter.call(val)
       # Some custom types might fail, so it should be treated as an invalid value
       rescue StandardError
         Types::InvalidValue.new
       end
 
-      def coerce_nil(val)
-        # define default values for structures, the dry-types lib which is used
-        # for coercion doesn't accept nil as a value, so it would fail
-        return []      if type == Array
-        return Set.new if type == Set
-        return {}      if type == Hash
-        val
-      end
-
       # Type to which the parameter will be coerced.
       #
       # @return [Class]

lib/grape/validations/validators/default.rb

@@ -9,7 +9,6 @@ def initialize(attrs, options, required, scope, opts = {})
       end
 
       def validate_param!(attr_name, params)
-        return if params.key? attr_name
         params[attr_name] = if @default.is_a? Proc
                               @default.call
                             elsif @default.frozen? || !duplicatable?(@default)

spec/grape/validations/validators/coerce_spec.rb

@@ -424,6 +424,80 @@ def self.parsed?(value)
         expect(last_response.status).to eq(200)
         expect(last_response.body).to eq(integer_class_name)
       end
+
+      context 'nil values' do
+        context 'primitive types' do
+          Grape::Validations::Types::PRIMITIVES.each do |type|
+            it 'respects the nil value' do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/nil_value' do
+                params[:param].class
+              end
+
+              get '/nil_value', param: nil
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+        end
+
+        context 'structures types' do
+          Grape::Validations::Types::STRUCTURES.each do |type|
+            it 'respects the nil value' do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/nil_value' do
+                params[:param].class
+              end
+
+              get '/nil_value', param: nil
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+        end
+
+        context 'special types' do
+          Grape::Validations::Types::SPECIAL.each_key do |type|
+            it 'respects the nil value' do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/nil_value' do
+                params[:param].class
+              end
+
+              get '/nil_value', param: nil
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+
+          context 'variant-member-type collections' do
+            [
+              Array[Integer, String],
+              [Integer, String, Array[Integer, String]]
+            ].each do |type|
+              it 'respects the nil value' do
+                subject.params do
+                  requires :param, type: type
+                end
+                subject.get '/nil_value' do
+                  params[:param].class
+                end
+
+                get '/nil_value', param: nil
+                p last_response.body
+                expect(last_response.status).to eq(200)
+                expect(last_response.body).to eq('NilClass')
+              end
+            end
+          end
+        end
+      end
     end
 
     context 'using coerce_with' do
@@ -752,19 +826,6 @@ def self.parsed?(value)
         expect(last_response.body).to eq('String')
       end
 
-      it 'respects nil values' do
-        subject.params do
-          optional :a, types: [File, String]
-        end
-        subject.get '/' do
-          params[:a].class.to_s
-        end
-
-        get '/', a: nil
-        expect(last_response.status).to eq(200)
-        expect(last_response.body).to eq('NilClass')
-      end
-
       it 'fails when no coercion is possible' do
         subject.params do
           requires :a, types: [Boolean, Integer]

spec/grape/validations/validators/default_spec.rb

@@ -298,4 +298,124 @@ def app
       end
     end
   end
+
+  context 'optional with nil as value' do
+    subject do
+      Class.new(Grape::API) do
+        default_format :json
+      end
+    end
+
+    def app
+      subject
+    end
+
+    context 'primitive types' do
+      [
+        [Integer, 0],
+        [Integer, 42],
+        [Float, 0.0],
+        [Float, 4.2],
+        [BigDecimal, 0.0],
+        [BigDecimal, 4.2],
+        [Numeric, 0],
+        [Numeric, 42],
+        [Date, Date.today],
+        [DateTime, DateTime.now],
+        [Time, Time.now],
+        [Time, Time.at(0)],
+        [Grape::API::Boolean, false],
+        [String, ''],
+        [String, 'non-empty-string'],
+        [Symbol, :symbol],
+        [TrueClass, true],
+        [FalseClass, false]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            optional :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+
+    context 'structures types' do
+      [
+        [Hash, {}],
+        [Hash, { test: 'non-empty' }],
+        [Array, []],
+        [Array, ['non-empty']],
+        [Set, []],
+        [Set, [1]]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            optional :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+
+    context 'special types' do
+      [
+        [JSON, ''],
+        [JSON, { test: 'non-empty-json' }.to_json],
+        [Array[JSON], []],
+        [Array[JSON], [{ test: 'non-empty-json' }.to_json]],
+        [::File, ''],
+        [::File, { test: 'non-empty-json' }.to_json],
+        [Rack::Multipart::UploadedFile, ''],
+        [Rack::Multipart::UploadedFile, { test: 'non-empty-json' }.to_json]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            requires :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+
+    context 'variant-member-type collections' do
+      [
+        [Array[Integer, String], [0, '']],
+        [Array[Integer, String], [42, 'non-empty-string']],
+        [[Integer, String, Array[Integer, String]], [0, '', [0, '']]],
+        [[Integer, String, Array[Integer, String]], [42, 'non-empty-string', [42, 'non-empty-string']]]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            requires :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+  end
 end

spec/grape/validations/validators/regexp_spec.rb

@@ -98,9 +98,10 @@ def app
         expect(last_response.status).to eq(200)
       end
 
-      it 'accepts nil instead of array' do
+      it 'refuses nil instead of array' do
         get '/custom_message/regexp_with_array', names: nil
-        expect(last_response.status).to eq(200)
+        expect(last_response.status).to eq(400)
+        expect(last_response.body).to eq('{"error":"names can\'t be nil"}')
       end
     end
   end
@@ -153,9 +154,10 @@ def app
       expect(last_response.status).to eq(200)
     end
 
-    it 'accepts nil instead of array' do
+    it 'rejects nil instead of array' do
       get '/regexp_with_array', names: nil
-      expect(last_response.status).to eq(200)
+      expect(last_response.status).to eq(400)
+      expect(last_response.body).to eq('{"error":"names is invalid"}')
     end
   end
 

spec/grape/validations/validators/values_spec.rb

@@ -319,9 +319,10 @@ def app
       expect(last_response.status).to eq 200
     end
 
-    it 'allows for an optional param with a list of values' do
+    it 'refuses for an optional param with a list of values' do
       put('/optional_with_array_of_string_values', optional: nil)
-      expect(last_response.status).to eq 200
+      expect(last_response.status).to eq 400
+      expect(last_response.body).to eq({ error: 'optional is invalid' }.to_json)
     end
   end
 

spec/grape/validations_spec.rb

@@ -574,7 +574,7 @@ def validate_param!(attr_name, params)
         # NOTE: with body parameters in json or XML or similar this
         # should actually fail with: children[parents][name] is missing.
         expect(last_response.status).to eq(400)
-        expect(last_response.body).to eq('children[1][parents] is missing')
+        expect(last_response.body).to eq('children[1][parents] is missing, children[0][parents][1][name] is missing, children[0][parents][1][name] is empty')
       end
 
       it 'errors when a parameter is not present in array within array' do
@@ -615,7 +615,7 @@ def validate_param!(attr_name, params)
 
         get '/within_array', children: [name: 'Jay']
         expect(last_response.status).to eq(400)
-        expect(last_response.body).to eq('children[0][parents] is missing')
+        expect(last_response.body).to eq('children[0][parents] is missing, children[0][parents][0][name] is missing, children[0][parents][0][name] is empty')
       end
 
       it 'errors when param is not an Array' do
@@ -763,7 +763,7 @@ def validate_param!(attr_name, params)
         expect(last_response.status).to eq(200)
         put_with_json '/within_array', children: [name: 'Jay']
         expect(last_response.status).to eq(400)
-        expect(last_response.body).to eq('children[0][parents] is missing')
+        expect(last_response.body).to eq('children[0][parents] is missing, children[0][parents][0][name] is missing')
       end
     end
 
@@ -838,7 +838,7 @@ def validate_param!(attr_name, params)
       it 'does internal validations if the outer group is present' do
         get '/nested_optional_group', items: [{ key: 'foo' }]
         expect(last_response.status).to eq(400)
-        expect(last_response.body).to eq('items[0][required_subitems] is missing')
+        expect(last_response.body).to eq('items[0][required_subitems] is missing, items[0][required_subitems][0][value] is missing')
 
         get '/nested_optional_group', items: [{ key: 'foo', required_subitems: [{ value: 'bar' }] }]
         expect(last_response.status).to eq(200)
@@ -858,7 +858,7 @@ def validate_param!(attr_name, params)
       it 'handles validation within arrays' do
         get '/nested_optional_group', items: [{ key: 'foo' }]
         expect(last_response.status).to eq(400)
-        expect(last_response.body).to eq('items[0][required_subitems] is missing')
+        expect(last_response.body).to eq('items[0][required_subitems] is missing, items[0][required_subitems][0][value] is missing')
 
         get '/nested_optional_group', items: [{ key: 'foo', required_subitems: [{ value: 'bar' }] }]
         expect(last_response.status).to eq(200)