Skip to content

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

Open
Copilot wants to merge 21 commits into
mainfrom
copilot/fix-invalid-property-name-compilation
Open

Fix invalid shadow property names in precompiled queries and JSON discriminator metadata#38458
Copilot wants to merge 21 commits into
mainfrom
copilot/fix-invalid-property-name-compilation

Conversation

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Precompiled queries derive some generated C# variable names from model metadata such as property names. Shadow properties can be given arbitrary names that are not valid C# identifiers (e.g. "NOT VALID !!!1"), producing uncompilable generated code whenever the query needs to access property metadata — for instance, a value converter on a shadow property.

modelBuilder.Entity<Blog>()
    .Property<string>("NOT VALID !!!1")
    .HasConversion<int>(x => 0, x => "");
// Generated: var nOT VALID !!!1Property = ...  // CS1003: Syntax error

Changes

  • Sanitize lifted-constant variable names in LiftableConstantProcessor.LiftConstants: when a lifted constant's parameter name is not a valid identifier, the base name falls back to "unknown" (instead of being sanitized in PrecompiledQueryCodeGenerator). The existing uniquification and parameter-remapping path then keeps declarations and references in sync across the lifted constant expressions and the query executor.
  • Shared identifier check: a simple, strict IsValidIdentifier helper on ModelValidator (only letters, ASCII digits and underscore, with no leading digit) is reused by both the model validator and LiftConstants. The helper is now public with internal annotation and is also used by CSharpHelper.Identifier for final generated-identifier validation, while CSharpHelper keeps its per-character filtering allocation-free.
  • New model-validation warning ShadowPropertyNameNotValidIdentifierWarning: fires when a shadow property name is not a valid identifier, surfaced through the standard diagnostics infrastructure (CoreEventId, CoreLoggerExtensions, LoggingDefinitions, CoreStrings).
  • JSON discriminator follow-up: the Cosmos discriminator convention now uses EF Core's default discriminator property name, and only applies an embedded discriminator JSON name when one is explicitly configured. The same explicit embedded-discriminator JSON name is also used for discriminator JSON property names in relational JSON columns, including complex-type discriminators. For design-time roundtripping, snapshots now generate HasJsonPropertyName(...) on discriminator properties whenever the JSON property name differs from the EF property name.
  • Tests: a reproduction precompiled-query test (relational base + Sqlite), two ModelValidatorTest unit tests (warns on invalid name, silent on valid name), updated Cosmos tests covering discriminator property/JSON-name behavior, a new relational test covering JSON owned/entity and complex discriminator JSON property names, snapshot baseline updates for relational JSON complex discriminators, updated Cosmos inheritance SQL baselines, refreshed Cosmos compiled-model baselines, and Cosmos Northwind fixture updates to align explicit discriminator configuration with the default discriminator property name plus embedded-discriminator JSON mapping. Public API additions reflected in EFCore.baseline.json.

Notes for reviewers

  • The warning escalates to an error under the default test diagnostics, so the repro context suppresses it via ConfigureWarnings(...Ignore(...)) passed through InitializeNonSharedTest (DbContext pooling forbids OnConfiguring).
  • Model snapshots skip ModelValidator.Validate, so the warning does not fire for snapshots.

Copilot AI and others added 3 commits June 18, 2026 17:35
…on + test

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix precompiled query compilation for invalid shadow property names Fix precompiled query generation for shadow property names that are not valid C# identifiers Jun 18, 2026
Copilot AI requested a review from AndriySvyryd June 18, 2026 17:52
Comment thread src/EFCore/Infrastructure/ModelValidator.cs Outdated
Comment thread src/EFCore/Properties/CoreStrings.resx Outdated
Comment thread src/EFCore.Design/Query/Internal/PrecompiledQueryCodeGenerator.cs Outdated
…te warning message

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI changed the title Fix precompiled query generation for shadow property names that are not valid C# identifiers Fix precompiled query generation for shadow property names that are not valid identifiers Jun 19, 2026
Copilot AI requested a review from AndriySvyryd June 19, 2026 02:31
Comment thread src/EFCore/Infrastructure/ModelValidator.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a precompiled-query code generation failure when lifted-constant variable names are derived from shadow property names that aren’t valid C# identifiers. It adds identifier validation (with a new warning) and ensures lifted constants fall back to a safe variable base name so generated code compiles.

Changes:

  • Update lifted-constant naming to fall back to "unknown" when the derived parameter name is not a valid identifier.
  • Add a new model-validation warning for shadow property names that are not valid identifiers, wired through EF Core diagnostics infrastructure.
  • Add tests covering both the new warning behavior and the precompiled-query repro.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs Adds unit tests asserting the new shadow-property-name warning behavior.
test/EFCore.Relational.Specification.Tests/Query/AdHocPrecompiledQueryRelationalTestBase.cs Adds a precompiled-query repro for invalid shadow property names (suppresses the new warning in the test context).
src/EFCore/Query/LiftableConstantProcessor.cs Ensures lifted constant variable names fall back to "unknown" when invalid identifiers are encountered.
src/EFCore/Properties/CoreStrings.resx Adds the new warning message text.
src/EFCore/Properties/CoreStrings.Designer.cs Adds the generated resource accessor for the new warning.
src/EFCore/Infrastructure/ModelValidator.cs Adds shadow-property name validation and an IsValidIdentifier helper used for both validation and lifting.
src/EFCore/EFCore.baseline.json Updates the API baseline for the newly added diagnostics members.
src/EFCore/Diagnostics/LoggingDefinitions.cs Adds a logging definition slot for the new warning.
src/EFCore/Diagnostics/CoreLoggerExtensions.cs Adds the logger extension method for the new warning event.
src/EFCore/Diagnostics/CoreEventId.cs Adds the new event id for the warning.
Files not reviewed (1)
  • src/EFCore/Properties/CoreStrings.Designer.cs: Generated file

Comment thread src/EFCore/Diagnostics/CoreEventId.cs
Comment thread src/EFCore/Infrastructure/ModelValidator.cs
Comment thread src/EFCore/Properties/CoreStrings.resx
…ecks

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@AndriySvyryd AndriySvyryd marked this pull request as ready for review June 19, 2026 06:28
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner June 19, 2026 06:28
Copilot AI review requested due to automatic review settings June 19, 2026 06:28
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 18:23
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 18:27
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 18:30
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 18:34
Copilot AI changed the title Fix invalid shadow property names in precompiled queries and Cosmos discriminator metadata Fix invalid shadow property names in precompiled queries and JSON discriminator metadata Jun 21, 2026
Copilot AI requested a review from AndriySvyryd June 21, 2026 18:35
Comment thread src/EFCore.Relational/Design/AnnotationCodeGenerator.cs Outdated
Comment thread src/EFCore/Infrastructure/ModelValidator.cs Outdated
Comment thread src/EFCore.Cosmos/Metadata/Conventions/CosmosDiscriminatorConvention.cs Outdated
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 19:11
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 19:15
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 21, 2026 19:20
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI requested review from AndriySvyryd and Copilot and removed request for Copilot June 21, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants