Fix invalid shadow property names in precompiled queries and JSON discriminator metadata

src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs

@@ -83,11 +83,10 @@ private static void ProcessEntityType(IConventionEntityTypeBuilder entityTypeBui
         {
             return;
         }
-
         if (entityType.IsDocumentRoot())
         {
-            entityTypeBuilder.HasDiscriminator(entityType.Model.GetEmbeddedDiscriminatorName(), typeof(string))
-                ?.HasValue(entityType, entityType.ShortName());
+            var discriminator = HasDiscriminator(entityTypeBuilder);
+            discriminator?.HasValue(entityType, entityType.ShortName());
         }
         else
         {
@@ -125,7 +124,7 @@ public override void ProcessEntityTypeBaseTypeChanged(
         {
             if (entityType.IsDocumentRoot())
             {
-                entityTypeBuilder.HasDiscriminator(entityType.Model.GetEmbeddedDiscriminatorName(), typeof(string));
+                HasDiscriminator(entityTypeBuilder);
             }
         }
         else
@@ -137,14 +136,28 @@ public override void ProcessEntityTypeBaseTypeChanged(
                 return;
             }
 
-            var discriminator = rootType.Builder.HasDiscriminator(entityType.Model.GetEmbeddedDiscriminatorName(), typeof(string));
+            var discriminator = HasDiscriminator(rootType.Builder);
             if (discriminator != null)
             {
                 SetDefaultDiscriminatorValues(entityTypeBuilder.Metadata.GetDerivedTypesInclusive(), discriminator);
             }
         }
     }
 
+    private static IConventionDiscriminatorBuilder? HasDiscriminator(IConventionEntityTypeBuilder entityTypeBuilder)
+    {
+        var discriminator = entityTypeBuilder.HasDiscriminator(typeof(string));
+        var discriminatorProperty = discriminator?.EntityType.FindDiscriminatorProperty();
+        if (discriminatorProperty != null)
+        {
+            CosmosPropertyBuilderExtensions.ToJsonProperty(
+                discriminatorProperty.Builder,
+                (string?)entityTypeBuilder.Metadata.Model.FindAnnotation("EmbeddedDiscriminatorName")?.Value);
+        }
+
+        return discriminator;
+    }
+
     /// <inheritdoc />
     protected override void SetDefaultDiscriminatorValues(
         IEnumerable<IConventionEntityType> entityTypes,

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

@@ -297,19 +297,16 @@ private static string Identifier(string name, bool? capitalize)
             builder.Append(name[partStart..]);
         }
 
-        if (builder.Length == 0
-            || !IsIdentifierStartCharacter(builder[0]))
-        {
-            builder.Insert(0, '_');
-        }
-
         if (capitalize != null)
         {
             ChangeFirstLetterCase(builder, capitalize.Value);
         }
 
-        var identifier = builder.ToString();
-        return identifier;
+        var candidateIdentifier = builder.ToString();
+
+        return ModelValidator.IsValidIdentifier(candidateIdentifier)
+            ? candidateIdentifier
+            : "_" + candidateIdentifier;
     }
 
     private static void ChangeFirstLetterCase(StringBuilder builder, bool capitalize)
@@ -1633,72 +1630,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.Design/Migrations/Design/CSharpSnapshotGenerator.cs

@@ -579,6 +579,15 @@ private Dictionary<string, IAnnotation> GetAnnotations(IProperty property)
                 property.GetColumnName());
         }
 
+        if (!annotations.ContainsKey(RelationalAnnotationNames.JsonPropertyName)
+            && property.GetJsonPropertyName() is { } jsonPropertyName
+            && property.Name != jsonPropertyName)
+        {
+            annotations[RelationalAnnotationNames.JsonPropertyName] = new Annotation(
+                RelationalAnnotationNames.JsonPropertyName,
+                jsonPropertyName);
+        }
+
         return Dependencies.AnnotationCodeGenerator
             .FilterIgnoredAnnotations(annotations.Values)
             .ToDictionary(a => a.Name, a => a);

src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs

@@ -2071,7 +2071,7 @@ private static TValue ThrowReadValueException<TValue>(
             ?? (property.IsKey() || !property.DeclaringType.IsMappedToJson()
                 ? null
                 : property == property.DeclaringType.FindDiscriminatorProperty()
-                    ? "$type"
+                    ? (string?)property.DeclaringType.Model.FindAnnotation("EmbeddedDiscriminatorName")?.Value
                     : property.Name);
 
     /// <summary>

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,37 @@ 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>
+    [EntityFrameworkInternal]
+    public 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 use only letters, digits (except for the first character), 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.Cosmos.FunctionalTests/Query/Inheritance/InheritanceQueryCosmosFixture.cs

@@ -34,7 +34,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
 
         modelBuilder.Entity<Animal>().ToContainer("Animals");
         modelBuilder.Entity<Plant>().ToContainer("Plants");
-        modelBuilder.Entity<Plant>().Property<string>("Discriminator").ToJsonProperty("_type");
         modelBuilder.Entity<Country>().ToContainer("Countries");
         modelBuilder.Entity<Drink>().ToContainer("Drinks");
         modelBuilder.Entity<KiwiQuery>().ToContainer("Animals");