Fix precompiled query generation for shadow property names that are not valid identifiers

src/EFCore.Design/Design/Internal/CSharpHelper.cs

@@ -1633,72 +1633,8 @@ public virtual string Expression(
     }
 
     private static bool IsIdentifierStartCharacter(char ch)
-    {
-        if (ch < 'a')
-        {
-            return ch is >= 'A' and (<= 'Z' or '_');
-        }
-
-        if (ch <= 'z')
-        {
-            return true;
-        }
-
-        return ch > '\u007F' && IsLetterChar(CharUnicodeInfo.GetUnicodeCategory(ch));
-    }
+        => char.IsLetter(ch) || ch == '_';
 
     private static bool IsIdentifierPartCharacter(char ch)
-    {
-        if (ch < 'a')
-        {
-            return (ch < 'A'
-                    ? ch is >= '0' and <= '9'
-                    : ch <= 'Z')
-                || ch == '_';
-        }
-
-        if (ch <= 'z')
-        {
-            return true;
-        }
-
-        if (ch <= '\u007F')
-        {
-            return false;
-        }
-
-        var cat = CharUnicodeInfo.GetUnicodeCategory(ch);
-        if (IsLetterChar(cat))
-        {
-            return true;
-        }
-
-        switch (cat)
-        {
-            case UnicodeCategory.DecimalDigitNumber:
-            case UnicodeCategory.ConnectorPunctuation:
-            case UnicodeCategory.NonSpacingMark:
-            case UnicodeCategory.SpacingCombiningMark:
-            case UnicodeCategory.Format:
-                return true;
-        }
-
-        return false;
-    }
-
-    private static bool IsLetterChar(UnicodeCategory cat)
-    {
-        switch (cat)
-        {
-            case UnicodeCategory.UppercaseLetter:
-            case UnicodeCategory.LowercaseLetter:
-            case UnicodeCategory.TitlecaseLetter:
-            case UnicodeCategory.ModifierLetter:
-            case UnicodeCategory.OtherLetter:
-            case UnicodeCategory.LetterNumber:
-                return true;
-        }
-
-        return false;
-    }
+        => char.IsLetter(ch) || char.IsAsciiDigit(ch) || ch == '_';
 }

src/EFCore/Diagnostics/CoreEventId.cs

@@ -129,6 +129,7 @@ private enum Id
         NoEntityTypeConfigurationsWarning = CoreBaseId + 632,
         AccidentalEntityType = CoreBaseId + 633,
         AccidentalComplexPropertyCollection = CoreBaseId + 634,
+        ShadowPropertyNameNotValidIdentifierWarning = CoreBaseId + 635,
 
         // ChangeTracking events
         DetectChangesStarting = CoreBaseId + 800,
@@ -750,6 +751,20 @@ private static EventId MakeModelValidationId(Id id)
     /// </remarks>
     public static readonly EventId AccidentalComplexPropertyCollection = MakeModelValidationId(Id.AccidentalComplexPropertyCollection);
 
+    /// <summary>
+    ///     A shadow property has a name that is not a valid identifier, which can cause issues in generated code.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This event is in the <see cref="DbLoggerCategory.Model.Validation" /> category.
+    ///     </para>
+    ///     <para>
+    ///         This event uses the <see cref="PropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+    ///     </para>
+    /// </remarks>
+    public static readonly EventId ShadowPropertyNameNotValidIdentifierWarning =
+        MakeModelValidationId(Id.ShadowPropertyNameNotValidIdentifierWarning);
+
     /// <summary>
     ///     The <see cref="RequiredAttribute" /> on the collection navigation property was ignored.
     /// </summary>

src/EFCore/Diagnostics/CoreLoggerExtensions.cs

@@ -1432,6 +1432,40 @@ private static string ShadowPropertyCreated(EventDefinitionBase definition, Even
         return d.GenerateMessage(p.Property.DeclaringType.DisplayName(), p.Property.Name);
     }
 
+    /// <summary>
+    ///     Logs for the <see cref="CoreEventId.ShadowPropertyNameNotValidIdentifierWarning" /> event.
+    /// </summary>
+    /// <param name="diagnostics">The diagnostics logger to use.</param>
+    /// <param name="property">The property.</param>
+    public static void ShadowPropertyNameNotValidIdentifierWarning(
+        this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
+        IProperty property)
+    {
+        var definition = CoreResources.LogShadowPropertyNameNotValidIdentifier(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(diagnostics, property.DeclaringType.DisplayName(), property.Name);
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new PropertyEventData(
+                definition,
+                ShadowPropertyNameNotValidIdentifier,
+                property);
+
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
+
+    private static string ShadowPropertyNameNotValidIdentifier(EventDefinitionBase definition, EventData payload)
+    {
+        var d = (EventDefinition<string, string>)definition;
+        var p = (PropertyEventData)payload;
+        return d.GenerateMessage(p.Property.DeclaringType.DisplayName(), p.Property.Name);
+    }
+
     /// <summary>
     ///     Logs for the <see cref="CoreEventId.CollectionWithoutComparer" /> event.
     /// </summary>

src/EFCore/Diagnostics/LoggingDefinitions.cs

@@ -825,4 +825,13 @@ public abstract class LoggingDefinitions
     /// </summary>
     [EntityFrameworkInternal]
     public EventDefinitionBase? LogQueryCompilationStarting;
+
+    /// <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? LogShadowPropertyNameNotValidIdentifier;
 }

src/EFCore/EFCore.baseline.json

@@ -3092,6 +3092,9 @@
         {
           "Member": "static readonly Microsoft.Extensions.Logging.EventId ShadowPropertyCreated"
         },
+        {
+          "Member": "static readonly Microsoft.Extensions.Logging.EventId ShadowPropertyNameNotValidIdentifierWarning"
+        },
         {
           "Member": "static readonly Microsoft.Extensions.Logging.EventId SkipCollectionChangeDetected"
         },
@@ -3355,6 +3358,9 @@
         {
           "Member": "static void ShadowPropertyCreated(this Microsoft.EntityFrameworkCore.Diagnostics.IDiagnosticsLogger<Microsoft.EntityFrameworkCore.DbLoggerCategory.Model.Validation> diagnostics, Microsoft.EntityFrameworkCore.Metadata.IProperty property);"
         },
+        {
+          "Member": "static void ShadowPropertyNameNotValidIdentifierWarning(this Microsoft.EntityFrameworkCore.Diagnostics.IDiagnosticsLogger<Microsoft.EntityFrameworkCore.DbLoggerCategory.Model.Validation> diagnostics, Microsoft.EntityFrameworkCore.Metadata.IProperty property);"
+        },
         {
           "Member": "static void SkipCollectionChangeDetected(this Microsoft.EntityFrameworkCore.Diagnostics.IDiagnosticsLogger<Microsoft.EntityFrameworkCore.DbLoggerCategory.ChangeTracking> diagnostics, Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry internalEntityEntry, Microsoft.EntityFrameworkCore.Metadata.ISkipNavigation navigation, System.Collections.Generic.ISet<object> added, System.Collections.Generic.ISet<object> removed);"
         },

src/EFCore/Infrastructure/ModelValidator.cs

@@ -174,6 +174,36 @@ protected virtual void ValidateProperty(
         ValidateTypeMapping(property, logger);
         ValidatePrimitiveCollection(property, logger);
         ValidateAutoLoaded(property, structuralType, logger);
+
+        if (property.IsShadowProperty()
+            && !IsValidIdentifier(property.Name))
+        {
+            logger.ShadowPropertyNameNotValidIdentifierWarning(property);
+        }
+    }
+
+    /// <summary>
+    ///     Returns <see langword="true" /> if the given name only uses letters, ASCII digits and underscores and does not start with a digit;
+    ///     that is, if it can be used as-is as an identifier in generated code.
+    /// </summary>
+    internal static bool IsValidIdentifier(string? name)
+    {
+        if (string.IsNullOrEmpty(name)
+            || (!char.IsLetter(name[0]) && name[0] != '_'))
+        {
+            return false;
+        }
+
+        for (var i = 1; i < name.Length; i++)
+        {
+            var ch = name[i];
+            if (!char.IsLetter(ch) && !char.IsAsciiDigit(ch) && ch != '_')
+            {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     /// <summary>

src/EFCore/Properties/CoreStrings.resx

@@ -1230,6 +1230,10 @@
     <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>
   </data>
+  <data name="LogShadowPropertyNameNotValidIdentifier" xml:space="preserve">
+    <value>The shadow property '{entityType}.{property}' has a name that is not a valid identifier. This can cause issues in generated code. Consider renaming the property to start with a letter or underscore, and use only letters, digits, and underscore.</value>
+    <comment>Warning CoreEventId.ShadowPropertyNameNotValidIdentifierWarning string string</comment>
+  </data>
   <data name="LogSkipCollectionChangeDetected" xml:space="preserve">
     <value>{addedCount} entities were added and {removedCount} entities were removed from skip navigation '{entityType}.{property}'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.</value>
     <comment>Debug CoreEventId.SkipCollectionChangeDetected int int string string</comment>

src/EFCore/Query/LiftableConstantProcessor.cs

@@ -126,7 +126,11 @@ public virtual Expression LiftConstants(Expression expression, ParameterExpressi
                 continue;
             }
 
-            var name = liftedConstant.Parameter.Name ?? "unknown";
+            // Lifted constant variable names are partly derived from model metadata (e.g. property names), which may not be valid C#
+            // identifiers (shadow properties can be given arbitrary names). Fall back to a generic name in that case.
+            var name = ModelValidator.IsValidIdentifier(liftedConstant.Parameter.Name)
+                ? liftedConstant.Parameter.Name!
+                : "unknown";
             var baseName = name;
             for (var j = 0; variableNames.Contains(name); j++)
             {

test/EFCore.Relational.Specification.Tests/Query/AdHocPrecompiledQueryRelationalTestBase.cs

@@ -387,6 +387,36 @@ public class InvalidNameNestedEntity
         public string Name2 { get; set; } = "";
     }
 
+    [Fact]
+    public virtual async Task Invalid_identifier_shadow_property_name()
+    {
+        var contextFactory = await InitializeNonSharedTest<InvalidShadowNameContext>(
+            onConfiguring: o => o.ConfigureWarnings(w => w.Ignore(CoreEventId.ShadowPropertyNameNotValidIdentifierWarning)));
+        var options = contextFactory.GetOptions();
+
+        await Test(
+            """
+await using var context = new AdHocPrecompiledQueryRelationalTestBase.InvalidShadowNameContext(dbContextOptions);
+var entities = await context.Entities.ToListAsync();
+""",
+            typeof(InvalidShadowNameContext),
+            options);
+    }
+
+    public class InvalidShadowNameContext(DbContextOptions options) : DbContext(options)
+    {
+        public DbSet<InvalidShadowNameEntity> Entities { get; set; } = null!;
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+            => modelBuilder.Entity<InvalidShadowNameEntity>()
+                .Property<string>("NOT VALID !!!1").HasConversion<int>(x => 0, x => "");
+    }
+
+    public class InvalidShadowNameEntity
+    {
+        public Guid Id { get; set; }
+    }
+
     #endregion
 
     protected TestSqlLoggerFactory TestSqlLoggerFactory

test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs

@@ -462,6 +462,40 @@ public virtual void Passes_on_shadow_primary_key_created_by_convention_in_depend
                 .GenerateMessage("A", "Key"), modelBuilder, LogLevel.Debug);
     }
 
+    [Fact]
+    public virtual void Warns_on_shadow_property_name_that_is_not_a_valid_identifier()
+    {
+        var modelBuilder = CreateConventionlessModelBuilder();
+        var model = modelBuilder.Model;
+
+        var entityType = model.AddEntityType(typeof(A));
+        SetPrimaryKey(entityType);
+        AddProperties(entityType);
+
+        entityType.AddProperty("NOT VALID !!!1", typeof(string));
+
+        VerifyWarning(
+            CoreResources.LogShadowPropertyNameNotValidIdentifier(new TestLogger<TestLoggingDefinitions>())
+                .GenerateMessage("A", "NOT VALID !!!1"), modelBuilder, LogLevel.Warning);
+    }
+
+    [Fact]
+    public virtual void Does_not_warn_on_shadow_property_with_valid_identifier_name()
+    {
+        var modelBuilder = CreateConventionlessModelBuilder();
+        var model = modelBuilder.Model;
+
+        var entityType = model.AddEntityType(typeof(A));
+        SetPrimaryKey(entityType);
+        AddProperties(entityType);
+
+        entityType.AddProperty("ValidName", typeof(string));
+
+        VerifyLogDoesNotContain(
+            CoreResources.LogShadowPropertyNameNotValidIdentifier(new TestLogger<TestLoggingDefinitions>())
+                .GenerateMessage("A", "ValidName"), modelBuilder);
+    }
+
     [Fact] // Issue #33484
     public virtual void Does_not_log_for_shadow_property_when_creating_indexer_property()
     {