Coerce empty string to nil for all Primitive types except String (#2067)

* Reorder PrimitiveCoercerSpec contexts to be alphabetical

* Coerce empty string to nil for all Primitive types except String

This matches the behavior in v1.2.5 with the exception of Symbol. In
v1.2.5 an empty string was coerced to an empty symbol (:''), now it is
coerced to nil.

* Add integration specs for empty string param coercion

These specs mirror the specs for nil params. Exceptions are made for
non-primitive types based on their current behavior.

Author: petekinnecom

CHANGELOG.md

@@ -9,6 +9,7 @@
 #### Fixes
 
 * Your contribution here.
+* [#2067](https://github.com/ruby-grape/grape/pull/2067): Coerce empty string to nil for all primitive types except String - [@petekinnecom](https://github.com/petekinnecom).
 * [#2064](https://github.com/ruby-grape/grape/pull/2064): Fix Ruby 2.7 deprecation warning in `Grape::Middleware::Base#initialize` - [@skarger](https://github.com/skarger).
 
 ### 1.3.3 (2020/05/23)

lib/grape/validations/types/primitive_coercer.rb

@@ -37,7 +37,6 @@ 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 == ''
 
           super
         end
@@ -60,7 +59,7 @@ def reject?(val)
         # absence of a value and coerces it into nil. See a discussion there
         # https://github.com/ruby-grape/grape/pull/2045
         def treat_as_nil?(val)
-          val == '' && type == Grape::API::Boolean
+          val == '' && type != String
         end
       end
     end

spec/grape/validations/types/primitive_coercer_spec.rb

@@ -8,6 +8,18 @@
   subject { described_class.new(type, strict) }
 
   describe '#call' do
+    context 'BigDecimal' do
+      let(:type) { BigDecimal }
+
+      it 'coerces to BigDecimal' do
+        expect(subject.call(5)).to eq(BigDecimal(5))
+      end
+
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
+      end
+    end
+
     context 'Boolean' do
       let(:type) { Grape::API::Boolean }
 
@@ -32,19 +44,63 @@
       end
     end
 
+    context 'DateTime' do
+      let(:type) { DateTime }
+
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
+      end
+    end
+
+    context 'Float' do
+      let(:type) { Float }
+
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
+      end
+    end
+
+    context 'Integer' do
+      let(:type) { Integer }
+
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
+      end
+    end
+
+    context 'Numeric' do
+      let(:type) { Numeric }
+
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
+      end
+    end
+
+    context 'Time' do
+      let(:type) { Time }
+
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
+      end
+    end
+
     context 'String' do
       let(:type) { String }
 
       it 'coerces to String' do
         expect(subject.call(10)).to eq('10')
       end
+
+      it 'does not coerce an empty string to nil' do
+        expect(subject.call('')).to eq('')
+      end
     end
 
-    context 'BigDecimal' do
-      let(:type) { BigDecimal }
+    context 'Symbol' do
+      let(:type) { Symbol }
 
-      it 'coerces to BigDecimal' do
-        expect(subject.call(5)).to eq(BigDecimal(5))
+      it 'coerces an empty string to nil' do
+        expect(subject.call('')).to be_nil
       end
     end
 

spec/grape/validations/validators/coerce_spec.rb

@@ -497,6 +497,92 @@ def self.parsed?(value)
           end
         end
       end
+
+      context 'empty string' do
+        context 'primitive types' do
+          (Grape::Validations::Types::PRIMITIVES - [String]).each do |type|
+            it "is coerced to nil for type #{type}" do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/empty_string' do
+                params[:param].class
+              end
+
+              get '/empty_string', param: ''
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+
+          it 'is not coerced to nil for type String' do
+            subject.params do
+              requires :param, type: String
+            end
+            subject.get '/empty_string' do
+              params[:param].class
+            end
+
+            get '/empty_string', param: ''
+            expect(last_response.status).to eq(200)
+            expect(last_response.body).to eq('String')
+          end
+        end
+
+        context 'structures types' do
+          (Grape::Validations::Types::STRUCTURES - [Hash]).each do |type|
+            it "is coerced to nil for type #{type}" do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/empty_string' do
+                params[:param].class
+              end
+
+              get '/empty_string', param: ''
+              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.keys - [File, Rack::Multipart::UploadedFile]).each do |type|
+            it "is coerced to nil for type #{type}" do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/empty_string' do
+                params[:param].class
+              end
+
+              get '/empty_string', param: ''
+              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 "is coerced to nil for type #{type}" do
+                subject.params do
+                  requires :param, type: type
+                end
+                subject.get '/empty_string' do
+                  params[:param].class
+                end
+
+                get '/empty_string', param: ''
+                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