Fix presence matcher against an AM record

.rubocop.yml

@@ -43,6 +43,8 @@ Lint/ParenthesesAsGroupedExpression:
   Enabled: false
 Lint/RequireParentheses:
   Enabled: false
+Lint/SafeNavigationChain:
+  Enabled: false
 Lint/UnderscorePrefixedVariableName:
   Enabled: false
 Lint/Void:
@@ -124,6 +126,8 @@ Style/DoubleNegation:
   Enabled: false
 Style/EachWithObject:
   Enabled: false
+Style/EmptyElse:
+  Enabled: false
 Style/EmptyLiteral:
   Enabled: false
 Style/Encoding:

lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb

@@ -170,7 +170,7 @@ def does_not_match?(subject)
               allows_and_double_checks_value_of!(value)
             end
           else
-            (expects_to_allow_nil? && !allows_value_of(nil)) ||
+            (expects_to_allow_nil? && disallows_value_of(nil)) ||
               disallowed_values.any? do |value|
                 allows_original_or_typecast_value?(value)
               end
@@ -234,7 +234,7 @@ def disallows_original_or_typecast_value?(value)
         end
 
         def disallowed_values
-          if collection?
+          if collection_association?
             [Array.new]
           elsif attachment?
             [nil]
@@ -253,16 +253,6 @@ def disallowed_values
           end
         end
 
-        def collection?
-          if association_reflection
-            [:has_many, :has_and_belongs_to_many].include?(
-              association_reflection.macro,
-            )
-          else
-            false
-          end
-        end
-
         def should_add_footnote_about_belongs_to?
           belongs_to_association_being_validated? &&
             presence_validation_exists_on_attribute?
@@ -318,15 +308,32 @@ def belongs_to_association_configured_to_be_required?
         end
 
         def belongs_to_association_being_validated?
-          !!association_reflection &&
-            association_reflection.macro == :belongs_to
+          association? && association_reflection.macro == :belongs_to
         end
 
         def attribute_accepts_string_values?
-          !association_reflection && (
-            !attribute_type.respond_to?(:coder) ||
-            !attribute_type.coder ||
-            attribute_type.coder.object_class == String
+          if association? || attachment?
+            false
+          elsif attribute_serializer
+            attribute_serializer.object_class == String
+          else
+            attribute_type.try(:type) == :string
+          end
+        end
+
+        def association?
+          association_reflection.present?
+        end
+
+        def collection_association?
+          association? && association_reflection.macro.in?(
+            [:has_many, :has_and_belongs_to_many],
+          )
+        end
+
+        def attachment?
+          model_has_associations?(
+            ["#{@attribute}_attachment", "#{@attribute}_attachments"]
           )
         end
 
@@ -339,25 +346,25 @@ def association_options
         end
 
         def association_reflection
-          model.respond_to?(:reflect_on_association) &&
-            model.reflect_on_association(@attribute)
+          model.try(:reflect_on_association, @attribute)
         end
 
-        def attachment?
-          model.respond_to?(:reflect_on_association) &&
-            model_has_associations?(["#{@attribute}_attachment", "#{@attribute}_attachments"])
+        def model_has_associations?(associations)
+          associations.any? do |association|
+            !!model.try(:reflect_on_association, association)
+          end
         end
 
-        def attribute_type
-          if model.respond_to?(:attribute_types)
-            model.attribute_types[@attribute.to_s]
+        def attribute_serializer
+          if attribute_type.respond_to?(:coder)
+            attribute_type.coder
           else
-            LegacyAttributeType.new(model, @attribute)
+            nil
           end
         end
 
-        def model_has_associations?(associations)
-          associations.any? { |association| !!model.reflect_on_association(association) }
+        def attribute_type
+          RailsShim.attribute_type_for(model, @attribute)
         end
 
         def presence_validation_exists_on_attribute?
@@ -367,25 +374,6 @@ def presence_validation_exists_on_attribute?
         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

lib/shoulda/matchers/rails_shim.rb

@@ -156,6 +156,14 @@ def secure_password_module
           nil
         end
 
+        def attribute_type_for(model, attribute_name)
+          if supports_full_attributes_api?(model)
+            model.attribute_types[attribute_name.to_s]
+          else
+            LegacyAttributeType.new(model, attribute_name)
+          end
+        end
+
         private
 
         def simply_generate_validation_message(
@@ -179,6 +187,30 @@ def simply_generate_validation_message(
             { default: default_translation_keys }.merge(options)
           I18n.translate(primary_translation_key, translate_options)
         end
+
+        def supports_full_attributes_api?(model)
+          defined?(::ActiveModel::Attributes) &&
+            model.respond_to?(:attribute_types)
+        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/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb

@@ -109,10 +109,17 @@
         expect(record).to matcher
       end
 
+      blank_value =
+        if active_model_supports_full_attributes_api?
+          ''
+        else
+          nil
+        end
+
       message = <<-MESSAGE
 Expected Example to validate that :attr cannot be empty/falsy, but this
 could not be proved.
-  After setting :attr to ‹""›, the matcher expected the Example to be
+  After setting :attr to ‹#{blank_value.inspect}›, the matcher expected the Example to be
   invalid, but it was valid instead.
       MESSAGE
 
@@ -178,6 +185,28 @@
           end
         end
       end
+
+      context 'when the attribute has not been configured with a type' do
+        context 'and it is assumed to be something other than a string' do
+          it 'still works' do
+            record = active_model_object_validating_presence_of(:user) do
+              attribute :user
+
+              validate :validate_user_has_email, if: :user
+
+              private
+
+              def validate_user_has_email
+                if !user.email
+                  errors.add(:base, 'user does not have an email')
+                end
+              end
+            end
+
+            expect(record).to validate_presence_of(:user)
+          end
+        end
+      end
     end
 
     def model_creator
@@ -196,7 +225,7 @@ def model_creator
       message = <<-MESSAGE
 Expected Example to validate that :attr cannot be empty/falsy, but this
 could not be proved.
-  After setting :attr to ‹""›, the matcher expected the Example to be
+  After setting :attr to ‹nil›, the matcher expected the Example to be
   invalid, but it was valid instead.
       MESSAGE
 
@@ -817,10 +846,17 @@ def record_belonging_to(
         expect(validating_presence(strict: false)).to matcher.strict
       end
 
+      blank_value =
+        if active_model_supports_full_attributes_api?
+          ''
+        else
+          nil
+        end
+
       message = <<-MESSAGE
 Expected Example to validate that :attr cannot be empty/falsy, raising a
 validation exception on failure, but this could not be proved.
-  After setting :attr to ‹""›, the matcher expected the Example to be
+  After setting :attr to ‹#{blank_value.inspect}›, the matcher expected the Example to be
   invalid and to raise a validation exception, but the record produced
   validation errors instead.
       MESSAGE
@@ -909,14 +945,23 @@ def foo=(value)
 
           assertion = -> { expect(record).not_to matcher.allow_nil }
 
-          expect(&assertion).to fail_with_message(<<-MESSAGE)
+          if active_model_supports_full_attributes_api?
+            expect(&assertion).to fail_with_message(<<-MESSAGE)
 Expected Example not to validate that :attr cannot be empty/falsy, but
 this could not be proved.
   After setting :attr to ‹""›, the matcher expected the Example to be
   valid, but it was invalid instead, producing these validation errors:
 
   * attr: ["can't be blank"]
-          MESSAGE
+            MESSAGE
+          else
+            expect(&assertion).to fail_with_message(<<-MESSAGE)
+Expected Example not to validate that :attr cannot be empty/falsy, but
+this could not be proved.
+  After setting :attr to ‹nil›, the matcher expected the Example to be
+  invalid, but it was valid instead.
+            MESSAGE
+          end
         end
       end
 
@@ -949,27 +994,52 @@ def foo=(value)
     end
 
     context 'when validating a model without a presence validator' do
-      it 'does not match in the positive' do
-        record = without_validating_presence
+      if active_model_supports_full_attributes_api?
+        it 'does not match in the positive' do
+          record = without_validating_presence
 
-        assertion = lambda do
-          expect(record).to matcher.allow_nil
-        end
+          assertion = lambda do
+            expect(record).to matcher.allow_nil
+          end
 
-        message = <<-MESSAGE
+          message = <<-MESSAGE
 Expected Example to validate that :attr cannot be empty/falsy, but this
 could not be proved.
   After setting :attr to ‹""›, the matcher expected the Example to be
   invalid, but it was valid instead.
-        MESSAGE
+          MESSAGE
 
-        expect(&assertion).to fail_with_message(message)
-      end
+          expect(&assertion).to fail_with_message(message)
+        end
 
-      it 'matches in the negative' do
-        record = without_validating_presence
+        it 'matches in the negative' do
+          record = without_validating_presence
 
-        expect(record).not_to matcher.allow_nil
+          expect(record).not_to matcher.allow_nil
+        end
+      else
+        it 'matches in the positive' do
+          record = without_validating_presence
+
+          expect(record).to matcher.allow_nil
+        end
+
+        it 'does not match in the negative' do
+          record = without_validating_presence
+
+          assertion = lambda do
+            expect(record).not_to matcher.allow_nil
+          end
+
+          message = <<-MESSAGE
+Expected Example not to validate that :attr cannot be empty/falsy, but
+this could not be proved.
+  After setting :attr to ‹nil›, the matcher expected the Example to be
+  invalid, but it was valid instead.
+          MESSAGE
+
+          expect(&assertion).to fail_with_message(message)
+        end
       end
     end
   end