Warn when uniquifying an FK column name

Fixes #20435

src/EFCore/Design/Internal/CSharpRuntimeAnnotationCodeGenerator.cs

@@ -83,6 +83,7 @@ public virtual void Generate(IProperty property, CSharpRuntimeAnnotationCodeGene
                 annotations.Remove(CoreAnnotationNames.ValueConverterType);
                 annotations.Remove(CoreAnnotationNames.ValueComparer);
                 annotations.Remove(CoreAnnotationNames.ValueComparerType);
+                annotations.Remove(CoreAnnotationNames.PreUniquificationName);
             }
 
             GenerateSimpleAnnotations(parameters);

src/EFCore/Diagnostics/CoreEventId.cs

@@ -114,6 +114,7 @@ private enum Id
             PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning,
             RequiredAttributeOnSkipNavigation,
             AmbiguousEndRequiredWarning,
+            ShadowForeignKeyPropertyCreated,
 
             // ChangeTracking events
             DetectChangesStarting = CoreBaseId + 800,
@@ -473,6 +474,22 @@ private static EventId MakeModelValidationId(Id id)
         /// </summary>
         public static readonly EventId ShadowPropertyCreated = MakeModelValidationId(Id.ShadowPropertyCreated);
 
+        /// <summary>
+        ///     <para>
+        ///         A foreign key property was created in shadow state because a conflicting property with the simple name for
+        ///         this foreign key exists in the entity type, but is either not mapped, is already used for another relationship,
+        ///         or is incompatible with the associated primary key type. See https://aka.ms/efcore-relationships for information
+        ///         on mapping relationships in EF Core.
+        ///     </para>
+        ///     <para>
+        ///         This event is in the <see cref="DbLoggerCategory.Model" /> category.
+        ///     </para>
+        ///     <para>
+        ///         This event uses the <see cref="UniquifiedPropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+        ///     </para>
+        /// </summary>
+        public static readonly EventId ShadowForeignKeyPropertyCreated = MakeModelValidationId(Id.ShadowForeignKeyPropertyCreated);
+
         /// <summary>
         ///     <para>
         ///         An index was not created as the properties are already covered.

src/EFCore/Diagnostics/CoreLoggerExtensions.cs

@@ -1113,6 +1113,43 @@ public static void RedundantAddServicesCallWarning(
             }
         }
 
+        /// <summary>
+        ///     Logs for the <see cref="CoreEventId.ShadowForeignKeyPropertyCreated" /> event.
+        /// </summary>
+        /// <param name="diagnostics"> The diagnostics logger to use. </param>
+        /// <param name="property"> The property. </param>
+        /// <param name="basePropertyName"> The property name that was uniquified. </param>
+        public static void ShadowForeignKeyPropertyCreated(
+            this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
+            IProperty property,
+            string basePropertyName)
+        {
+            var definition = CoreResources.LogShadowForeignKeyPropertyCreated(diagnostics);
+
+            if (diagnostics.ShouldLog(definition))
+            {
+                definition.Log(diagnostics, property.DeclaringEntityType.DisplayName(), property.Name, basePropertyName);
+            }
+
+            if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+            {
+                var eventData = new UniquifiedPropertyEventData(
+                    definition,
+                    ShadowForeignKeyPropertyCreated,
+                    property,
+                    basePropertyName);
+
+                diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+            }
+        }
+
+        private static string ShadowForeignKeyPropertyCreated(EventDefinitionBase definition, EventData payload)
+        {
+            var d = (EventDefinition<string, string, string>)definition;
+            var p = (UniquifiedPropertyEventData)payload;
+            return d.GenerateMessage(p.Property.DeclaringEntityType.DisplayName(), p.Property.Name, p.BasePropertyName);
+        }
+
         /// <summary>
         ///     Logs for the <see cref="CoreEventId.ShadowPropertyCreated" /> event.
         /// </summary>

src/EFCore/Diagnostics/LoggingDefinitions.cs

@@ -404,6 +404,15 @@ public abstract class LoggingDefinitions
         [EntityFrameworkInternal]
         public EventDefinitionBase? LogShadowPropertyCreated;
 
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        [EntityFrameworkInternal]
+        public EventDefinitionBase? LogShadowForeignKeyPropertyCreated;
+
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore/Diagnostics/UniquifiedPropertyEventData.cs

@@ -0,0 +1,38 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Diagnostics;
+using Microsoft.EntityFrameworkCore.Metadata;
+
+namespace Microsoft.EntityFrameworkCore.Diagnostics
+{
+    /// <summary>
+    ///     A <see cref="DiagnosticSource" /> event payload class for events that have
+    ///     a property that has been uniquified.
+    /// </summary>
+    public class UniquifiedPropertyEventData : PropertyEventData
+    {
+        /// <summary>
+        ///     Constructs the event payload.
+        /// </summary>
+        /// <param name="eventDefinition"> The event definition. </param>
+        /// <param name="messageGenerator"> A delegate that generates a log message for this event. </param>
+        /// <param name="property"> The property. </param>
+        /// <param name="basePropertyName"> The property name that was uniquified. </param>
+        public UniquifiedPropertyEventData(
+            EventDefinitionBase eventDefinition,
+            Func<EventDefinitionBase, EventData, string> messageGenerator,
+            IReadOnlyProperty property,
+            string basePropertyName)
+            : base(eventDefinition, messageGenerator, property)
+        {
+            BasePropertyName = basePropertyName;
+        }
+
+        /// <summary>
+        ///     The property name that was uniquified.
+        /// </summary>
+        public virtual string BasePropertyName { get; }
+    }
+}

src/EFCore/Infrastructure/ModelValidator.cs

@@ -1122,7 +1122,16 @@ protected virtual void LogShadowProperties(
                 {
                     if (property.IsImplicitlyCreated())
                     {
-                        logger.ShadowPropertyCreated((IProperty)property);
+                        var uniquifiedAnnotation = property.FindAnnotation(CoreAnnotationNames.PreUniquificationName);
+                        if (uniquifiedAnnotation != null
+                            && property.IsForeignKey())
+                        {
+                            logger.ShadowForeignKeyPropertyCreated((IProperty)property, (string)uniquifiedAnnotation.Value!);
+                        }
+                        else
+                        {
+                            logger.ShadowPropertyCreated((IProperty)property);
+                        }
                     }
                 }
             }

src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs

@@ -303,6 +303,7 @@ protected virtual void ProcessPropertyAnnotations(
                 annotations.Remove(CoreAnnotationNames.ValueConverterType);
                 annotations.Remove(CoreAnnotationNames.ValueComparer);
                 annotations.Remove(CoreAnnotationNames.ValueComparerType);
+                annotations.Remove(CoreAnnotationNames.PreUniquificationName);
             }
         }
 

src/EFCore/Metadata/Internal/CoreAnnotationNames.cs

@@ -225,6 +225,14 @@ public static class CoreAnnotationNames
         /// </summary>
         public const string ReadOnlyModel = "ReadOnlyModel";
 
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public const string PreUniquificationName = "PreUniquificationName";
+
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -319,6 +327,7 @@ public static class CoreAnnotationNames
             ProviderClrType,
             ModelDependencies,
             ReadOnlyModel,
+            PreUniquificationName,
             InverseNavigations,
             DerivedTypes,
             NavigationCandidates,

src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs

@@ -4466,6 +4466,14 @@ public virtual bool ShouldReuniquifyTemporaryProperties(ForeignKey foreignKey)
                                     return (false, null);
                                 }
 
+                                if (index > 0)
+                                {
+                                    propertyBuilder.HasAnnotation(
+                                        CoreAnnotationNames.PreUniquificationName,
+                                        keyModifiedBaseName,
+                                        ConfigurationSource.Convention);
+                                }
+
                                 if (clrType.IsNullableType())
                                 {
                                     propertyBuilder.IsRequired(isRequired, ConfigurationSource.Convention);

src/EFCore/Properties/CoreStrings.resx

@@ -967,6 +967,10 @@
     <value>An additional 'IServiceProvider' was created for internal use by Entity Framework. An existing service provider was not used due to the following configuration changes: {debugInfo}.</value>
     <comment>Debug CoreEventId.ServiceProviderDebugInfo string</comment>
   </data>
+  <data name="LogShadowForeignKeyPropertyCreated" xml:space="preserve">
+    <value>The foreign key property '{entityType}.{property}' was created in shadow state because a conflicting property with the simple name '{baseName}' exists in the entity type, but is either not mapped, is already used for another relationship, or is incompatible with the associated primary key type. See https://aka.ms/efcore-relationships for information on mapping relationships in EF Core.</value>
+    <comment>Warning CoreEventId.ShadowForeignKeyPropertyCreated string string</comment>
+  </data>
   <data name="LogShadowPropertyCreated" xml:space="preserve">
     <value>The property '{entityType}.{property}' was created in shadow state because there are no eligible CLR members with a matching name.</value>
     <comment>Debug CoreEventId.ShadowPropertyCreated string string</comment>

test/EFCore.Cosmos.FunctionalTests/EmbeddedDocumentsTest.cs

@@ -585,6 +585,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                             .WithOne(e => e.FuelTank)
                             .HasForeignKey<FuelTank>(e => e.VehicleName)
                             .OnDelete(DeleteBehavior.Restrict);
+                        eb.HasOne(e => e.Vehicle)
+                            .WithOne()
+                            .HasForeignKey<FuelTank>("VehicleName1");
                     });
 
                 modelBuilder.Entity<ContinuousCombustionEngine>();

test/EFCore.Specification.Tests/DataAnnotationTestBase.cs

@@ -2500,6 +2500,7 @@ public virtual void OwnedEntityTypeAttribute_configures_all_references_as_owned(
             var modelBuilder = CreateModelBuilder();
             var model = modelBuilder.Model;
 
+            modelBuilder.Entity<Book>().Ignore(e => e.AlternateLabel);
             modelBuilder.Entity<Book>()
                 .HasOne(b => b.Label).WithOne(l => l.Book)
                 .HasForeignKey<BookLabel>(l => l.BookId);

test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryFixtureBase.cs

@@ -462,16 +462,19 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
             modelBuilder.Entity<InheritanceLeaf2>().Property(e => e.Id).ValueGeneratedNever();
 
             // FK name needs to be explicitly provided because issue #9310
-            modelBuilder.Entity<InheritanceBase2>().HasOne(e => e.Reference).WithOne().HasForeignKey<InheritanceBase1>("InheritanceBase2Id")
+            modelBuilder.Entity<InheritanceBase2>().HasOne(e => e.Reference).WithOne()
+                .HasForeignKey<InheritanceBase1>("InheritanceBase2Id")
                 .IsRequired(false);
-            modelBuilder.Entity<InheritanceBase2>().HasMany(e => e.Collection).WithOne();
+            modelBuilder.Entity<InheritanceBase2>().HasMany(e => e.Collection).WithOne()
+                .HasForeignKey("InheritanceBase2Id1");
 
             modelBuilder.Entity<InheritanceDerived1>().HasBaseType<InheritanceBase1>();
             modelBuilder.Entity<InheritanceDerived1>().HasOne(e => e.ReferenceSameType).WithOne()
                 .HasForeignKey<InheritanceLeaf1>("SameTypeReference_InheritanceDerived1Id").IsRequired(false);
             modelBuilder.Entity<InheritanceDerived1>().HasOne(e => e.ReferenceDifferentType).WithOne()
                 .HasForeignKey<InheritanceLeaf1>("DifferentTypeReference_InheritanceDerived1Id").IsRequired(false);
-            modelBuilder.Entity<InheritanceDerived1>().HasMany(e => e.CollectionSameType).WithOne().HasForeignKey("InheritanceDerived1Id")
+            modelBuilder.Entity<InheritanceDerived1>().HasMany(e => e.CollectionSameType).WithOne()
+                .HasForeignKey("InheritanceDerived1Id1")
                 .IsRequired(false);
             modelBuilder.Entity<InheritanceDerived1>().HasMany(e => e.CollectionDifferentType).WithOne()
                 .HasForeignKey("InheritanceDerived1Id").IsRequired(false);

test/EFCore.Specification.Tests/Query/ComplexNavigationsSharedTypeQueryFixtureBase.cs

@@ -67,9 +67,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
             modelBuilder.Entity<InheritanceLeaf2>().Property(e => e.Id).ValueGeneratedNever();
 
             // FK name needs to be explicitly provided because issue #9310
-            modelBuilder.Entity<InheritanceBase2>().HasOne(e => e.Reference).WithOne().HasForeignKey<InheritanceBase1>("InheritanceBase2Id")
+            modelBuilder.Entity<InheritanceBase2>().HasOne(e => e.Reference).WithOne()
+                .HasForeignKey<InheritanceBase1>("InheritanceBase2Id1")
                 .IsRequired(false);
-            modelBuilder.Entity<InheritanceBase2>().HasMany(e => e.Collection).WithOne();
+            modelBuilder.Entity<InheritanceBase2>().HasMany(e => e.Collection).WithOne()
+                .HasForeignKey("InheritanceBase2Id");
 
             modelBuilder.Entity<InheritanceDerived1>().HasBaseType<InheritanceBase1>();
             modelBuilder.Entity<InheritanceDerived1>().HasOne(e => e.ReferenceSameType).WithOne()

test/EFCore.Specification.Tests/Query/ManyToManyFieldsQueryFixtureBase.cs

@@ -288,6 +288,13 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                     r => r.HasOne<EntityTwo>().WithMany().HasForeignKey(e => e.TwoId),
                     l => l.HasOne<EntityOne>().WithMany().HasForeignKey(e => e.OneId));
 
+            modelBuilder.Entity<JoinOneToThreePayloadFull>(
+                b =>
+                {
+                    b.Property(e => e.OneId);
+                    b.Property(e => e.ThreeId);
+                });
+
             // Nav:6 Payload:Yes Join:Concrete Extra:None
             modelBuilder.Entity<EntityOne>()
                 .HasMany(e => e.ThreeSkipPayloadFull)
@@ -373,7 +380,12 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                             e.CompositeId2,
                             e.CompositeId3
                         }).IsRequired(),
-                    r => r.HasOne(x => x.Three).WithMany(x => x.JoinCompositeKeyFull).IsRequired());
+                    r => r.HasOne(x => x.Three).WithMany(x => x.JoinCompositeKeyFull).IsRequired(),
+                    b =>
+                    {
+                        b.Property(e => e.Id);
+                        b.Property(e => e.ThreeId);
+                    });
 
             // Nav:2 Payload:No Join:Shared Extra:Inheritance
             // TODO: Remove UsingEntity

test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs

@@ -5259,7 +5259,7 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder)
                 modelBuilder.Entity<Thing>().OwnsMany(
                     p => p.OwnedByThings, a =>
                     {
-                        a.WithOwner().HasForeignKey(e => e.ThingId);
+                        a.WithOwner(e => e.Thing).HasForeignKey(e => e.ThingId);
                         a.HasKey(e => e.OwnedByThingId);
                     });
             }