RevEng: Generate lambda with good identifiers (#21946)

Resolves #21520
dotnet · Aug 5, 2020 · f7c3ce4 · f7c3ce4
1 parent 9f2d1dc
commit f7c3ce4
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 16 deletions.
diff --git a/src/EFCore.Design/Design/ICSharpHelper.cs b/src/EFCore.Design/Design/ICSharpHelper.cs
@@ -33,16 +33,18 @@ public interface ICSharpHelper
         ///     Generates a property accessor lambda.
         /// </summary>
         /// <param name="properties"> The property names. </param>
+        /// <param name="lambdaIdentifier"> The identifier to use for parameter in the lambda. </param>
         /// <returns> The lambda. </returns>
-        string Lambda([NotNull] IReadOnlyList<string> properties);
+        string Lambda([NotNull] IReadOnlyList<string> properties, [CanBeNull] string lambdaIdentifier = null);
 
         /// <summary>
         ///     Generates a property accessor lambda.
         /// </summary>
         /// <param name="properties"> The properties. </param>
+        /// <param name="lambdaIdentifier"> The identifier to use for parameter in the lambda. </param>
         /// <returns> The lambda. </returns>
-        string Lambda([NotNull] IEnumerable<IProperty> properties)
-            => Lambda(properties.Select(p => p.Name).ToList());
+        string Lambda([NotNull] IEnumerable<IProperty> properties, [CanBeNull] string lambdaIdentifier = null)
+            => Lambda(properties.Select(p => p.Name).ToList(), lambdaIdentifier);
 
         /// <summary>
         ///     Generates a multidimensional array literal.

diff --git a/src/EFCore.Design/Design/Internal/CSharpHelper.cs b/src/EFCore.Design/Design/Internal/CSharpHelper.cs
@@ -176,23 +176,27 @@ public CSharpHelper([NotNull] IRelationalTypeMappingSource relationalTypeMapping
         ///     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 virtual string Lambda(IReadOnlyList<string> properties)
+        public virtual string Lambda(IReadOnlyList<string> properties, string lambdaIdentifier)
         {
             Check.NotNull(properties, nameof(properties));
+            Check.NullButNotEmpty(lambdaIdentifier, nameof(lambdaIdentifier));
 
+            lambdaIdentifier ??= "x";
             var builder = new StringBuilder();
-            builder.Append("x => ");
+            builder.Append(lambdaIdentifier);
+            builder.Append(" => ");
 
             if (properties.Count == 1)
             {
                 builder
-                    .Append("x.")
+                    .Append(lambdaIdentifier)
+                    .Append(".")
                     .Append(properties[0]);
             }
             else
             {
                 builder.Append("new { ");
-                builder.AppendJoin(", ", properties.Select(p => "x." + p));
+                builder.AppendJoin(", ", properties.Select(p => $"{lambdaIdentifier}.{p}"));
                 builder.Append(" }");
             }
 

diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
@@ -470,7 +470,7 @@ private void GenerateKey(IKey key, IEntityType entityType, bool useDataAnnotatio
                 }
             }
 
-            var lines = new List<string> { $".{nameof(EntityTypeBuilder.HasKey)}({_code.Lambda(key.Properties)})" };
+            var lines = new List<string> { $".{nameof(EntityTypeBuilder.HasKey)}({_code.Lambda(key.Properties, "e")})" };
 
             if (explicitName)
             {
@@ -542,7 +542,7 @@ private void GenerateIndex(IIndex index)
 
             var lines = new List<string> {
                 $".{nameof(EntityTypeBuilder.HasIndex)}" +
-                $"({_code.Lambda(index.Properties)}, " +
+                $"({_code.Lambda(index.Properties, "e")}, " +
                 $"{_code.Literal(index.GetDatabaseName())})" };
             annotations.Remove(RelationalAnnotationNames.Name);
 
@@ -560,7 +560,7 @@ private void GenerateIndex(IIndex index)
 
         private void GenerateProperty(IProperty property, bool useDataAnnotations)
         {
-            var lines = new List<string> { $".{nameof(EntityTypeBuilder.Property)}(e => e.{property.Name})" };
+            var lines = new List<string> { $".{nameof(EntityTypeBuilder.Property)}({_code.Lambda(new [] { property.Name }, "e")})" };
 
             var annotations = _annotationCodeGenerator
                 .FilterIgnoredAnnotations(property.GetAnnotations())
@@ -701,13 +701,13 @@ private void GenerateRelationship(IForeignKey foreignKey, bool useDataAnnotation
                 lines.Add(
                     $".{nameof(ReferenceReferenceBuilder.HasPrincipalKey)}"
                     + (foreignKey.IsUnique ? $"<{foreignKey.PrincipalEntityType.DisplayName()}>" : "")
-                    + $"({_code.Lambda(foreignKey.PrincipalKey.Properties)})");
+                    + $"({_code.Lambda(foreignKey.PrincipalKey.Properties, "p")})");
             }
 
             lines.Add(
                 $".{nameof(ReferenceReferenceBuilder.HasForeignKey)}"
                 + (foreignKey.IsUnique ? $"<{foreignKey.DeclaringEntityType.DisplayName()}>" : "")
-                + $"({_code.Lambda(foreignKey.Properties)})");
+                + $"({_code.Lambda(foreignKey.Properties, "d")})");
 
             var defaultOnDeleteAction = foreignKey.IsRequired
                 ? DeleteBehavior.Cascade

diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs
@@ -251,7 +251,8 @@ public void ModelInDifferentNamespaceDbContext_works()
         {
             var modelGenerationOptions = new ModelCodeGenerationOptions
             {
-                ContextNamespace = "TestNamespace", ModelNamespace = "AnotherNamespaceOfModel"
+                ContextNamespace = "TestNamespace",
+                ModelNamespace = "AnotherNamespaceOfModel"
             };
 
             const string entityInAnotherNamespaceTypeName = "EntityInAnotherNamespace";
@@ -484,10 +485,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
         {
             modelBuilder.Entity<EntityWithIndexes>(entity =>
             {
-                entity.HasIndex(x => new { x.A, x.B }, ""IndexOnAAndB"")
+                entity.HasIndex(e => new { e.A, e.B }, ""IndexOnAAndB"")
                     .IsUnique();
 
-                entity.HasIndex(x => new { x.B, x.C }, ""IndexOnBAndC"")
+                entity.HasIndex(e => new { e.B, e.C }, ""IndexOnBAndC"")
                     .HasFilter(""Filter SQL"")
                     .HasAnnotation(""AnnotationName"", ""AnnotationValue"");
 
@@ -568,7 +569,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
         {
             modelBuilder.Entity<EntityWithIndexes>(entity =>
             {
-                entity.HasIndex(x => new { x.B, x.C }, ""IndexOnBAndC"")
+                entity.HasIndex(e => new { e.B, e.C }, ""IndexOnBAndC"")
                     .HasFilter(""Filter SQL"")
                     .HasAnnotation(""AnnotationName"", ""AnnotationValue"");
 
@@ -589,6 +590,105 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                     Assert.Equal(2, model.FindEntityType("TestNamespace.EntityWithIndexes").GetIndexes().Count()));
         }
 
+        [ConditionalFact]
+        public void Entity_lambda_uses_correct_identifiers()
+        {
+            Test(
+                modelBuilder =>
+                {
+                    modelBuilder.Entity("PrincipalEntity", b =>
+                    {
+                        b.Property<int>("Id");
+                        b.Property<int>("PrincipalId");
+                        b.Property<int>("AlternateId");
+                        b.HasKey("AlternateId");
+                    });
+                    modelBuilder.Entity("DependentEntity", b =>
+                    {
+                        b.Property<int>("Id");
+                        b.Property<int>("DependentId");
+                        b.HasOne("PrincipalEntity", "NavigationToPrincipal")
+                            .WithOne("NavigationToDependent")
+                            .HasForeignKey("DependentEntity", "DependentId")
+                            .HasPrincipalKey("PrincipalEntity", "PrincipalId");
+                    });
+                },
+                new ModelCodeGenerationOptions { UseDataAnnotations = false },
+                code =>
+                {
+                    Assert.Equal(
+                        @"using System;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Metadata;
+
+#nullable disable
+
+namespace TestNamespace
+{
+    public partial class TestDbContext : DbContext
+    {
+        public TestDbContext()
+        {
+        }
+
+        public TestDbContext(DbContextOptions<TestDbContext> options)
+            : base(options)
+        {
+        }
+
+        public virtual DbSet<DependentEntity> DependentEntity { get; set; }
+        public virtual DbSet<PrincipalEntity> PrincipalEntity { get; set; }
+
+        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
+        {
+            if (!optionsBuilder.IsConfigured)
+            {
+#warning "
+                        + DesignStrings.SensitiveInformationWarning
+                        + @"
+                optionsBuilder.UseSqlServer(""Initial Catalog=TestDatabase"");
+            }
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            modelBuilder.Entity<DependentEntity>(entity =>
+            {
+                entity.HasIndex(e => e.DependentId, ""IX_DependentEntity_DependentId"")
+                    .IsUnique();
+
+                entity.Property(e => e.Id).UseIdentityColumn();
+
+                entity.Property(e => e.DependentId).ValueGeneratedNever();
+
+                entity.HasOne(d => d.NavigationToPrincipal)
+                    .WithOne(p => p.NavigationToDependent)
+                    .HasPrincipalKey<PrincipalEntity>(p => p.PrincipalId)
+                    .HasForeignKey<DependentEntity>(d => d.DependentId);
+            });
+
+            modelBuilder.Entity<PrincipalEntity>(entity =>
+            {
+                entity.HasKey(e => e.AlternateId);
+
+                entity.Property(e => e.AlternateId).UseIdentityColumn();
+
+                entity.Property(e => e.Id).ValueGeneratedNever();
+            });
+
+            OnModelCreatingPartial(modelBuilder);
+        }
+
+        partial void OnModelCreatingPartial(ModelBuilder modelBuilder);
+    }
+}
+",
+                        code.ContextFile.Code,
+                        ignoreLineEndingDifferences: true);
+                },
+                model => { });
+                }
+
         private class TestCodeGeneratorPlugin : ProviderCodeGeneratorPlugin
         {
             public override MethodCallCodeFragment GenerateProviderOptions()