Fix invalid shadow property names in precompiled queries and JSON discriminator metadata#38458
Open
Copilot wants to merge 21 commits into
Open
Fix invalid shadow property names in precompiled queries and JSON discriminator metadata#38458Copilot wants to merge 21 commits into
Copilot wants to merge 21 commits into
Conversation
…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
AndriySvyryd
approved these changes
Jun 19, 2026
…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
There was a problem hiding this comment.
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
…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>
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>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
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
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>
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.Changes
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 inPrecompiledQueryCodeGenerator). The existing uniquification and parameter-remapping path then keeps declarations and references in sync across the lifted constant expressions and the query executor.IsValidIdentifierhelper onModelValidator(only letters, ASCII digits and underscore, with no leading digit) is reused by both the model validator andLiftConstants. The helper is now public with internal annotation and is also used byCSharpHelper.Identifierfor final generated-identifier validation, whileCSharpHelperkeeps its per-character filtering allocation-free.ShadowPropertyNameNotValidIdentifierWarning: fires when a shadow property name is not a valid identifier, surfaced through the standard diagnostics infrastructure (CoreEventId,CoreLoggerExtensions,LoggingDefinitions,CoreStrings).HasJsonPropertyName(...)on discriminator properties whenever the JSON property name differs from the EF property name.ModelValidatorTestunit 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 inEFCore.baseline.json.Notes for reviewers
ConfigureWarnings(...Ignore(...))passed throughInitializeNonSharedTest(DbContext pooling forbidsOnConfiguring).ModelValidator.Validate, so the warning does not fire for snapshots.