Presence: check before assigning empty string

A previous commit updated the presence matcher so that when it is
testing invalid values for the attribute in question, it now considers
an empty string as one of those values. However, a string is not always
a valid value for an attribute. For instance, an attribute that is
decorated with `serialize` where the serialization class is Array cannot
be set to a string at all, and doing so will raise an error immediately.

With that in mind, this commit adds checks to ensure that it is safe to
try an empty string.

lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb

@@ -239,7 +239,7 @@ validation for you? Instead of using `validate_presence_of`, try
           else
             values = []
 
-            if !association_being_validated?
+            if attribute_accepts_string_values?
               values << ''
             end
 
@@ -316,12 +316,16 @@ validation for you? Instead of using `validate_presence_of`, try
         end
 
         def belongs_to_association_being_validated?
-          association_being_validated? &&
+          !!association_reflection &&
             association_reflection.macro == :belongs_to
         end
 
-        def association_being_validated?
+        def attribute_accepts_string_values?
-          !!association_reflection
+          !association_reflection && (
+            !attribute_type.respond_to?(:coder) ||
+            !attribute_type.coder ||
+            attribute_type.coder.object_class == String
+          )
         end
 
         def association_name
@@ -337,6 +341,14 @@ validation for you? Instead of using `validate_presence_of`, try
             model.reflect_on_association(@attribute)
         end
 
+        def attribute_type
+          if model.respond_to?(:attribute_types)
+            model.attribute_types[@attribute.to_s]
+          else
+            LegacyAttributeType.new(model, @attribute)
+          end
+        end
+
         def presence_validation_exists_on_attribute?
           model._validators.include?(@attribute)
         end
@@ -344,6 +356,25 @@ validation for you? Instead of using `validate_presence_of`, try
         def model
           @subject.class
         end
+
+        class LegacyAttributeType
+          def initialize(model, attribute_name)
+            @model = model
+            @attribute_name = attribute_name
+          end
+
+          def coder
+            if model.respond_to?(:serialized_attributes)
+              ActiveSupport::Deprecation.silence do
+                model.serialized_attributes[attribute_name.to_s]
+              end
+            end
+          end
+
+          private
+
+          attr_reader :model, :attribute_name
+        end
       end
     end
   end

spec/support/unit/helpers/active_model_versions.rb

@@ -28,5 +28,9 @@ module UnitTests
     def active_model_supports_strict?
       active_model_version >= 3.2
     end
+
+    def active_model_supports_full_attributes_api?
+      active_model_version >= '5.2'
+    end
   end
 end

spec/support/unit/helpers/model_builder.rb

@@ -56,13 +56,9 @@ module UnitTests
       def define_active_model_class(class_name, options = {}, &block)
         attribute_names = options.delete(:accessors) { [] }
 
-        columns = attribute_names.reduce({}) do |hash, attribute_name|
-          hash.merge(attribute_name => nil)
-        end
-
         UnitTests::ModelCreationStrategies::ActiveModel.call(
           class_name,
-          columns,
+          attribute_names,
           options,
           &block
         )

spec/support/unit/model_creation_strategies/active_model.rb

@@ -1,13 +1,19 @@
 module UnitTests
   module ModelCreationStrategies
     class ActiveModel
-      def self.call(name, columns = {}, options = {}, &block)
+      def self.call(name, attribute_names = [], options = {}, &block)
-        new(name, columns, options, &block).call
+        new(name, attribute_names, options, &block).call
       end
 
-      def initialize(name, columns = {}, options = {}, &block)
+      def initialize(name, attribute_names = [], options = {}, &block)
         @name = name
-        @columns = columns
+        @attribute_names =
+          if attribute_names.is_a?(Hash)
+            # mimicking columns
+            attribute_names.keys
+          else
+            attribute_names
+          end
         @options = options
         @model_customizers = []
 
@@ -22,7 +28,9 @@ module UnitTests
 
       def call
         ClassBuilder.define_class(name, Model).tap do |model|
-          model.columns = columns.keys
+          attribute_names.each do |attribute_name|
+            model.attribute(attribute_name)
+          end
 
           model_customizers.each do |block|
             run_block(model, block)
@@ -30,12 +38,10 @@ module UnitTests
         end
       end
 
-      protected
-
-      attr_reader :name, :columns, :model_customizers, :options
-
       private
 
+      attr_reader :name, :attribute_names, :model_customizers, :options
+
       def run_block(model, block)
         if block
           if block.arity == 0
@@ -46,29 +52,40 @@ module UnitTests
         end
       end
 
-      class Model
+      module PoorMansAttributes
-        class << self
+        extend ActiveSupport::Concern
-          def columns
-            @_columns ||= []
-          end
 
-          def columns=(columns)
+        included do
-            existing_columns = self.columns
+          class_attribute :attribute_names
-            new_columns = columns - existing_columns
 
-            @_columns += new_columns
+          self.attribute_names = Set.new
+        end
 
+        module ClassMethods
+          def attribute(name)
             include attributes_module
 
-            attributes_module.module_eval do
+            name = name.to_sym
-              new_columns.each do |column|
-                define_method(column) do
-                  attributes[column]
-                end
 
-                define_method("#{column}=") do |value|
+            if (
-                  attributes[column] = value
+              attribute_names.include?(name) &&
-                end
+              attributes_module.instance_methods.include?(name)
+            )
+              attributes_module.module_eval do
+                remove_method(name)
+                remove_method("#{name}=")
+              end
+            end
+
+            self.attribute_names += [name]
+
+            attributes_module.module_eval do
+              define_method(name) do
+                attributes[name]
+              end
+
+              define_method("#{name}=") do |value|
+                attributes[name] = value
               end
             end
           end
@@ -80,8 +97,6 @@ module UnitTests
           end
         end
 
-        include ::ActiveModel::Model
-
         attr_reader :attributes
 
         def initialize(attributes = {})
@@ -92,7 +107,7 @@ module UnitTests
           middle = '%s:0x%014x%s' % [
             self.class,
             object_id * 2,
-            ' ' + inspected_attributes.join(' ')
+            ' ' + inspected_attributes.join(' '),
           ]
 
           "#<#{middle.strip}>"
@@ -101,11 +116,21 @@ module UnitTests
         private
 
         def inspected_attributes
-          self.class.columns.map do |key, value|
+          self.class.attribute_names.map do |name|
-            "#{key}: #{attributes[key].inspect}"
+            "#{name}: #{attributes[name].inspect}"
           end
         end
       end
+
+      class Model
+        include ::ActiveModel::Model
+
+        if defined?(::ActiveModel::Attributes)
+          include ::ActiveModel::Attributes
+        else
+          include PoorMansAttributes
+        end
+      end
     end
   end
 end

spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb

@@ -55,6 +55,50 @@ this could not be proved.
 
       expect(&assertion).to fail_with_message(message)
     end
+
+    context 'when the attribute is decorated with serialize' do
+      context 'and the type is a string' do
+        it 'still works' do
+          record = record_validating_presence_of(:traits) do
+            serialize :traits, String
+          end
+
+          expect(record).to validate_presence_of(:traits)
+        end
+      end
+
+      context 'and the type is not a string' do
+        it 'still works' do
+          record = record_validating_presence_of(:traits) do
+            serialize :traits, Array
+          end
+
+          expect(record).to validate_presence_of(:traits)
+        end
+      end
+    end
+
+    context 'when the column backing the attribute is a scalar, but not a string' do
+      it 'still works' do
+        record = record_validating_presence_of(
+          :pinned_on,
+          column_options: { type: :date },
+        )
+
+        expect(record).to validate_presence_of(:pinned_on)
+      end
+    end
+
+    context 'when the column backing the attribute is an array' do
+      it 'still works' do
+        record = record_validating_presence_of(
+          :possible_meeting_dates,
+          column_options: { type: :date, array: true },
+        )
+
+        expect(record).to validate_presence_of(:possible_meeting_dates)
+      end
+    end
   end
 
   context 'a model without a presence validation' do
@@ -112,6 +156,30 @@ could not be proved.
       }
     )
 
+    if active_model_supports_full_attributes_api?
+      context 'when the attribute has been configured with a type' do
+        context 'and it is a string' do
+          it 'works' do
+            record = active_model_object_validating_presence_of(:age) do
+              attribute :age, :string
+            end
+
+            expect(record).to validate_presence_of(:age)
+          end
+        end
+
+        context 'and it is not a string' do
+          it 'still works' do
+            record = active_model_object_validating_presence_of(:age) do
+              attribute :age, :time
+            end
+
+            expect(record).to validate_presence_of(:age)
+          end
+        end
+      end
+    end
+
     def model_creator
       :active_model
     end
@@ -120,7 +188,9 @@ could not be proved.
   context 'an ActiveModel class without a presence validation' do
     it 'rejects with the correct failure message' do
       assertion = lambda do
-        expect(active_model).to matcher
+        record = plain_active_model_object_with(:attr, model_name: 'Example')
+
+        expect(record).to matcher
       end
 
       message = <<-MESSAGE
@@ -826,22 +896,58 @@ could not be proved.
     validate_presence_of(:attr)
   end
 
-  def validating_presence(options = {})
+  def record_validating_presence_of(
-    define_model :example, attr: :string do
+    attribute_name = :attr,
-      validates_presence_of :attr, options
+    column_options: { type: :string },
-    end.new
+    **options,
+    &block
+  )
+    model = define_model 'Example', attribute_name => column_options do
+      validates_presence_of(attribute_name, options)
+
+      if block
+        class_eval(&block)
+      end
+    end
+
+    model.new
   end
+  alias_method :validating_presence, :record_validating_presence_of
 
   def without_validating_presence
     define_model(:example, attr: :string).new
   end
 
-  def active_model(&block)
+  def active_model_object_validating_presence_of(
-    define_active_model_class('Example', accessors: [:attr], &block).new
+    attribute_name = :attr,
-  end
+    **options,
+    &block
+  )
+    plain_active_model_object_with(attribute_name, **options) do
+      validates_presence_of(attribute_name)
 
-  def active_model_validating_presence
+      if block
-    active_model { validates_presence_of :attr }
+        class_eval(&block)
+      end
+    end
+  end
+  alias_method :active_model_validating_presence,
+    :active_model_object_validating_presence_of
+
+  def plain_active_model_object_with(
+    attribute_name = :attr,
+    model_name: 'Example',
+    **options,
+    &block
+  )
+    model = define_active_model_class(
+      model_name,
+      accessors: [attribute_name],
+      **options,
+      &block
+    )
+
+    model.new
   end
 
   def has_many_children(options = {})