Add more model validation tests

src/EFCore/Infrastructure/ModelValidator.cs

@@ -875,7 +875,7 @@ protected virtual void ValidateTypeMappings(
  {
  _ = property.GetCurrentValueComparer(); // Will throw if there is no way to compare
  }
- 
+
  var providerComparer = property.GetProviderValueComparer();
  if (providerComparer == null)
  {

src/EFCore/Storage/CoreTypeMapping.cs

@@ -200,7 +200,7 @@ public virtual ValueComparer ProviderValueComparer
  this,
  static c => (c.Converter?.ProviderClrType ?? c.ClrType) == c.ClrType
  ? c.KeyComparer
- : ValueComparer.CreateDefault(c.Converter?.ProviderClrType ?? c.ClrType, favorStructuralComparisons: true));
+ : ValueComparer.CreateDefault(c.Converter!.ProviderClrType, favorStructuralComparisons: true));
 
  /// <summary>
  /// Returns a new copy of this type mapping with the given <see cref="ValueConverter" />

test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs

@@ -27,6 +27,39 @@ public override void Detects_key_property_which_cannot_be_compared()
  modelBuilder);
  }
 
+ public override void Detects_noncomparable_key_property_with_comparer()
+ {
+ var modelBuilder = CreateConventionModelBuilder();
+
+ modelBuilder.Entity<WithNonComparableKey>(
+ eb =>
+ {
+ eb.Property(e => e.Id).HasConversion(typeof(NotComparable), typeof(CustomValueComparer<NotComparable>));
+ eb.HasKey(e => e.Id);
+ });
+
+ VerifyError(
+ CoreStrings.PropertyNotMapped(nameof(NotComparable), nameof(WithNonComparableKey), nameof(WithNonComparableKey.Id)),
+ modelBuilder);
+ }
+
+ public override void Detects_noncomparable_key_property_with_provider_comparer()
+ {
+ var modelBuilder = CreateConventionModelBuilder();
+
+ modelBuilder.Entity<WithNonComparableKey>(
+ eb =>
+ {
+ eb.Property(e => e.Id).HasConversion(
+ typeof(CastingConverter<NotComparable, NotComparable>), null, typeof(CustomValueComparer<NotComparable>));
+ eb.HasKey(e => e.Id);
+ });
+
+ VerifyError(
+ CoreStrings.PropertyNotMapped(nameof(NotComparable), nameof(WithNonComparableKey), nameof(WithNonComparableKey.Id)),
+ modelBuilder);
+ }
+
  public override void Detects_unique_index_property_which_cannot_be_compared()
  {
  var modelBuilder = CreateConventionModelBuilder();
@@ -2613,7 +2646,7 @@ public virtual void Detects_keyless_entity_type_mapped_to_a_stored_procedure()
  RelationalStrings.StoredProcedureKeyless(nameof(Animal), "Animal_Insert"),
  modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_tableless_entity_type_mapped_to_some_stored_procedures()
  {
@@ -2723,7 +2756,7 @@ public virtual void Detects_unmatched_stored_procedure_result_columns_in_TPH()
  RelationalStrings.StoredProcedureResultColumnNotFound("Missing", nameof(Animal), "dbo.Update"),
  modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_duplicate_parameter()
  {
@@ -2738,7 +2771,7 @@ public virtual void Detects_duplicate_parameter()
  RelationalStrings.StoredProcedureDuplicateParameterName("Id", "Animal_Insert"),
  modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_duplicate_result_column()
  {
@@ -2822,7 +2855,7 @@ public virtual void Detects_delete_stored_procedure_result_columns_in_TPH()
  RelationalStrings.StoredProcedureResultColumnDelete(nameof(Animal), nameof(Animal.Name), "Delete"),
  modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_generated_properties_mapped_to_result_and_parameter()
  {
@@ -2838,7 +2871,7 @@ public virtual void Detects_generated_properties_mapped_to_result_and_parameter(
  RelationalStrings.StoredProcedureResultColumnParameterConflict(nameof(Animal), nameof(Animal.Name), "Animal_Update"),
  modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_generated_properties_mapped_to_original_and_current_parameter()
  {
@@ -2854,7 +2887,7 @@ public virtual void Detects_generated_properties_mapped_to_original_and_current_
  RelationalStrings.StoredProcedureOutputParameterConflict(nameof(Animal), nameof(Animal.Name), "Animal_Update"),
  modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_unmapped_concurrency_token()
  {

test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs

@@ -33,6 +33,50 @@ protected class WithNonComparableKey
  public NotComparable Id { get; set; }
  }
 
+ [ConditionalFact]
+ public virtual void Detects_noncomparable_key_property_with_comparer()
+ {
+ var modelBuilder = CreateConventionModelBuilder();
+
+ modelBuilder.Entity<WithNonComparableKey>(
+ eb =>
+ {
+ eb.Property(e => e.Id).HasConversion(typeof(NotComparable), typeof(CustomValueComparer<NotComparable>));
+ eb.HasKey(e => e.Id);
+ });
+
+ VerifyError(
+ CoreStrings.NonComparableKeyType(nameof(WithNonComparableKey), nameof(WithNonComparableKey.Id), nameof(NotComparable)),
+ modelBuilder);
+ }
+
+ [ConditionalFact]
+ public virtual void Detects_noncomparable_key_property_with_provider_comparer()
+ {
+ var modelBuilder = CreateConventionModelBuilder();
+
+ modelBuilder.Entity<WithNonComparableKey>(
+ eb =>
+ {
+ eb.Property(e => e.Id).HasConversion(
+ typeof(CastingConverter<NotComparable, NotComparable>), null, typeof(CustomValueComparer<NotComparable>));
+ eb.HasKey(e => e.Id);
+ });
+
+ VerifyError(
+ CoreStrings.NonComparableKeyTypes(
+ nameof(WithNonComparableKey), nameof(WithNonComparableKey.Id), nameof(NotComparable), nameof(NotComparable)),
+ modelBuilder);
+ }
+
+ public class CustomValueComparer<T> : ValueComparer<T> // Doesn't implement IComparer
+ {
+ public CustomValueComparer()
+ : base(false)
+ {
+ }
+ }
+
  [ConditionalFact]
  public virtual void Detects_unique_index_property_which_cannot_be_compared()
  {
@@ -1396,7 +1440,7 @@ public virtual void Detects_incompatible_discriminator_value()
 
  VerifyError(CoreStrings.DiscriminatorValueIncompatible("1", nameof(A), "int"), modelBuilder);
  }
- 
+
  [ConditionalFact]
  public virtual void Detects_missing_discriminator_value_on_base()
  {
@@ -1412,7 +1456,7 @@ public virtual void Detects_missing_discriminator_value_on_base()
 
  entityA.SetDiscriminatorProperty(entityA.AddProperty("D", typeof(int)));
  entityA.RemoveDiscriminatorValue();
- 
+
  entityC.SetDiscriminatorValue(1);
 
  VerifyError(CoreStrings.NoDiscriminatorValue(entityA.DisplayName()), modelBuilder);
@@ -1433,7 +1477,7 @@ public virtual void Detects_missing_discriminator_value_on_leaf()
 
  entityAbstract.SetDiscriminatorProperty(entityAbstract.AddProperty("D", typeof(int)));
  entityAbstract.SetDiscriminatorValue(0);
- 
+
  entityGeneric.RemoveDiscriminatorValue();
 
  VerifyError(CoreStrings.NoDiscriminatorValue(entityGeneric.DisplayName()), modelBuilder);

test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs

@@ -517,14 +517,14 @@ public override TestPropertyBuilder<TProperty> HasField(string fieldName)
  public override TestPropertyBuilder<TProperty> UsePropertyAccessMode(PropertyAccessMode propertyAccessMode)
  => Wrap(PropertyBuilder.UsePropertyAccessMode(propertyAccessMode));
 
- public override TestPropertyBuilder<TProperty> HasConversion<TProvider>()
- => Wrap(PropertyBuilder.HasConversion<TProvider>());
+ public override TestPropertyBuilder<TProperty> HasConversion<TConversion>()
+ => Wrap(PropertyBuilder.HasConversion<TConversion>());
 
- public override TestPropertyBuilder<TProperty> HasConversion<TProvider>(ValueComparer? valueComparer)
- => Wrap(PropertyBuilder.HasConversion<TProvider>(valueComparer));
+ public override TestPropertyBuilder<TProperty> HasConversion<TConversion>(ValueComparer? valueComparer)
+ => Wrap(PropertyBuilder.HasConversion<TConversion>(valueComparer));
 
- public override TestPropertyBuilder<TProperty> HasConversion<TProvider>(ValueComparer? valueComparer, ValueComparer? providerComparerType)
- => Wrap(PropertyBuilder.HasConversion<TProvider>(valueComparer, providerComparerType));
+ public override TestPropertyBuilder<TProperty> HasConversion<TConversion>(ValueComparer? valueComparer, ValueComparer? providerComparerType)
+ => Wrap(PropertyBuilder.HasConversion<TConversion>(valueComparer, providerComparerType));
 
  public override TestPropertyBuilder<TProperty> HasConversion<TProvider>(
  Expression<Func<TProperty, TProvider>> convertToProviderExpression,

test/EFCore.Tests/ModelBuilding/ModelBuilderNonGenericTest.cs

@@ -597,14 +597,14 @@ public override TestPropertyBuilder<TProperty> HasField(string fieldName)
  public override TestPropertyBuilder<TProperty> UsePropertyAccessMode(PropertyAccessMode propertyAccessMode)
  => Wrap(PropertyBuilder.UsePropertyAccessMode(propertyAccessMode));
 
- public override TestPropertyBuilder<TProperty> HasConversion<TProvider>()
- => Wrap(PropertyBuilder.HasConversion<TProvider>());
+ public override TestPropertyBuilder<TProperty> HasConversion<TConversion>()
+ => Wrap(PropertyBuilder.HasConversion(typeof(TConversion)));
 
- public override TestPropertyBuilder<TProperty> HasConversion<TProvider>(ValueComparer? valueComparer)
- => Wrap(PropertyBuilder.HasConversion<TProvider>(valueComparer));
+ public override TestPropertyBuilder<TProperty> HasConversion<TConversion>(ValueComparer? valueComparer)
+ => Wrap(PropertyBuilder.HasConversion(typeof(TConversion), valueComparer));
 
- public override TestPropertyBuilder<TProperty> HasConversion<TProvider>(ValueComparer? valueComparer, ValueComparer? providerComparerType)
- => Wrap(PropertyBuilder.HasConversion<TProvider>(valueComparer, providerComparerType));
+ public override TestPropertyBuilder<TProperty> HasConversion<TConversion>(ValueComparer? valueComparer, ValueComparer? providerComparerType)
+ => Wrap(PropertyBuilder.HasConversion(typeof(TConversion), valueComparer, providerComparerType));
 
  public override TestPropertyBuilder<TProperty> HasConversion<TProvider>(
  Expression<Func<TProperty, TProvider>> convertToProviderExpression,

test/EFCore.Tests/ModelBuilding/ModelBuilderTestBase.cs

@@ -400,9 +400,9 @@ public abstract TestPropertyBuilder<TProperty> HasValueGeneratorFactory<TFactory
  public abstract TestPropertyBuilder<TProperty> HasField(string fieldName);
  public abstract TestPropertyBuilder<TProperty> UsePropertyAccessMode(PropertyAccessMode propertyAccessMode);
 
- public abstract TestPropertyBuilder<TProperty> HasConversion<TProvider>();
- public abstract TestPropertyBuilder<TProperty> HasConversion<TProvider>(ValueComparer? valueComparer);
- public abstract TestPropertyBuilder<TProperty> HasConversion<TProvider>(ValueComparer? valueComparer, ValueComparer? providerComparerType);
+ public abstract TestPropertyBuilder<TProperty> HasConversion<TConversion>();
+ public abstract TestPropertyBuilder<TProperty> HasConversion<TConversion>(ValueComparer? valueComparer);
+ public abstract TestPropertyBuilder<TProperty> HasConversion<TConversion>(ValueComparer? valueComparer, ValueComparer? providerComparerType);
 
  public abstract TestPropertyBuilder<TProperty> HasConversion<TProvider>(
  Expression<Func<TProperty, TProvider>> convertToProviderExpression,