Repository navigation
Warn when collection properties are mapped as scalars - #39664
AndriySvyryd wants to merge 3 commits into
Conversation
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
| AccidentalEntityType = CoreBaseId + 633, | ||
| AccidentalComplexPropertyCollection = CoreBaseId + 634, | ||
| ShadowPropertyNameNotValidIdentifierWarning = CoreBaseId + 635, | ||
| AccidentalScalarCollection = CoreBaseId + 636, |
There was a problem hiding this comment.
@copilot Update progress by rewriting the PR summary using this template:
Fixes #{number}
**Description**
{Issue description}
**Customer impact**
{How does the reported issue affects customer's app? Are there workarounds?}
**How found**
{Was it customer reported or found during verification? How many customers are affected?}
**Regression**
{Is it a regression from a released version? Which one?}
**Testing**
{How the changes were tested}
**Risk**
{Low/Medium/High, argument why}
There was a problem hiding this comment.
I can’t directly edit the PR description with the available tools. Here is the revised summary text to use:
Fixes #39600
Description
Warn when a collection-typed property is explicitly configured as a scalar and has no value converter, suggesting PrimitiveCollection() to avoid later query translation failures. The warning excludes string, byte[], and converted properties, and supports standard warning configuration with property metadata in the diagnostic event.
Customer impact
Applications that configure primitive collections with Property() may encounter query translation failures without an indication of the configuration issue. The workaround is to configure the member with PrimitiveCollection().
How found
Reported by a customer via an Npgsql issue; the scenario was also reproduced with SQL Server. The number of affected customers is unknown.
Regression
The scalar-mapping behavior was introduced in EF Core 11 as an intentional breaking change. This PR adds diagnostic guidance; it does not change that mapping behavior.
Testing
Added model-validator tests for warning behavior and exclusions, plus SQL Server tests for failed scalar collection translation and successful primitive collection translation. The validate check passed; the main Copilot check was still running when checked.
Risk
Low. The change adds model-validation diagnostics for a narrowly defined configuration and follows the existing warning configuration path.
There was a problem hiding this comment.
🟢 Approval recommended
The supplied review evidence identifies no active issues for this change.
No open issues 🎉
1 file not reviewed
src/EFCore/Properties/CoreStrings.Designer.cs: Generated file
What changed in this PR
Adds model-validation diagnostics for collection-typed properties explicitly mapped as scalar properties, helping users identify when Property() should be replaced with PrimitiveCollection().
Changes:
- Emits
CoreEventId.AccidentalScalarCollectionfor explicitly scalar-mapped collection properties without converters, excluding strings,byte[], and primitive collections. - Adds localized warning resources, diagnostic logging integration, property metadata payloads, and public API baseline updates.
- Adds core and SQL Server tests covering warning behavior, configuration, exclusions, and query translation.
| File | Description |
|---|---|
| test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs | Tests scalar collection warnings, exclusions, complex and shadow properties, and warning configuration behavior. |
| test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs | Verifies diagnostics for failed scalar collection query translation and successful primitive collection translation. |
| src/EFCore/Properties/CoreStrings.resx | Adds the localized warning message and formatting metadata. |
| src/EFCore/Properties/CoreStrings.Designer.cs | Adds the generated resource accessor and event definition for the warning. |
| src/EFCore/Infrastructure/ModelValidator.cs | Detects explicitly configured collection properties that are mapped as scalars during model validation. |
| src/EFCore/EFCore.baseline.json | Updates the public API baseline for the new diagnostic event and logger extension. |
| src/EFCore/Diagnostics/LoggingDefinitions.cs | Adds storage for the new logging definition. |
| src/EFCore/Diagnostics/CoreLoggerExtensions.cs | Logs the warning and dispatches property metadata through diagnostic events. |
| src/EFCore/Diagnostics/CoreEventId.cs | Defines and documents the new AccidentalScalarCollection event. |
Review effort: Lite
In EF Core 11,
Property()maps a collection-typed member as a scalar. Subsequent collection queries can fail translation without identifying the configuration change.CoreEventId.AccidentalScalarCollectionwith guidance to usePrimitiveCollection(). The warning excludes strings,byte[], and properties with an explicit value converter.For example, configure a
Guid[]intended as a primitive collection with: