From 71a5f3193c193d2f6ca382c13c4af97a450a9526 Mon Sep 17 00:00:00 2001 From: Kieron Lanning Date: Fri, 23 Oct 2026 23:05:17 +0100 Subject: [PATCH] fix: updated EF generation of nullable vs non-nullable based on vo --- CHANGELOG.md | 33 ++ README.md | 2 +- docs/Diagnostics.md | 10 + docs/Entity-Framework.md | 113 +++++++ docs/Getting-Started.md | 73 ++++- .../SKILL.md | 4 + skills/purview-value-objects-efcore/SKILL.md | 57 +++- skills/purview-value-objects/SKILL.md | 10 +- src/src/Sample/Models.cs | 40 ++- src/src/Sample/NullableScalarDemo.cs | 80 +++++ src/src/Sample/OrderDto.cs | 64 ++++ src/src/Sample/Program.cs | 2 + src/src/Sample/README.md | 10 +- .../ScalarNullabilityCodeFixProvider.cs | 210 ++++++++++++ .../AnalyzerReleases.Unshipped.md | 4 + .../ValueObjectDiagnosticAnalyzer.cs | 213 ++++++++++++ .../Common/DiagnosticLibrary.cs | 41 +++ .../Generators/ValueObjectSourceGenerator.cs | 3 +- .../Models/EFValueObjectDescriptors.cs | 5 +- .../Models/ScalarValueObjectModel.cs | 3 +- .../ReferencedEFValueObjectDiscovery.cs | 6 +- .../ValueObject/ScalarAttributeParser.cs | 20 ++ .../ScalarValueObjectEmitter.Equality.cs | 30 +- .../ScalarValueObjectEmitter.Mirroring.cs | 2 +- .../ValueObject/ScalarValueObjectEmitter.cs | 32 +- .../ScalarValueObjectModelBuilder.cs | 130 +++++--- .../ValueObjectEFRegistryEmitter.cs | 106 +++++- .../ValueObject/ValueObjectSymbolInspector.cs | 78 ++++- src/src/ValueObjects/Sdk/README.md | 2 +- .../Serialization/ScalarAttribute.cs | 3 +- .../Serialization/ScalarAttributeOfT.cs | 13 +- .../Serialization/ScalarOptionsAttribute.cs | 9 +- .../NullableScalarEntityFrameworkTests.cs | 306 ++++++++++++++++++ .../ScalarNullabilityDiagnosticTests.cs | 165 ++++++++++ .../Generators/ScalarBoundaryMappingTests.cs | 199 ++++++++++++ .../ScalarComparisonOperatorTests.cs | 227 +++++++++++++ .../Generators/ScalarHasValueTests.cs | 128 ++++++++ .../Generators/ScalarNullabilityTests.cs | 239 ++++++++++++++ .../Refactorings/CodeFixTestHarness.cs | 23 +- .../PackagedAnalyzerComponents.cs | 13 +- .../ScalarNullabilityCodeFixTests.cs | 111 +++++++ .../CompatibilityModels.cs | 66 +++- .../EntityFrameworkCompatibilityTests.cs | 141 +++++++- 43 files changed, 2927 insertions(+), 99 deletions(-) create mode 100644 src/src/Sample/NullableScalarDemo.cs create mode 100644 src/src/Sample/OrderDto.cs create mode 100644 src/src/SourceGenerator.Refactorings/ScalarNullabilityCodeFixProvider.cs create mode 100644 src/tests/SourceGenerator.IntegrationTests/Generators/NullableScalarEntityFrameworkTests.cs create mode 100644 src/tests/SourceGenerator.UnitTests/Analyzers/ScalarNullabilityDiagnosticTests.cs create mode 100644 src/tests/SourceGenerator.UnitTests/Generators/ScalarBoundaryMappingTests.cs create mode 100644 src/tests/SourceGenerator.UnitTests/Generators/ScalarComparisonOperatorTests.cs create mode 100644 src/tests/SourceGenerator.UnitTests/Generators/ScalarHasValueTests.cs create mode 100644 src/tests/SourceGenerator.UnitTests/Generators/ScalarNullabilityTests.cs create mode 100644 src/tests/SourceGenerator.UnitTests/Refactorings/ScalarNullabilityCodeFixTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index d272a4c..f620ac3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,39 @@ newest heading below is the version in development. Earlier releases are listed `public static TSelf Null()`, equivalent to `Hydrate(null)`, so the unknown value is explicit and does not depend on `Empty`. It is not generated when the author declares a member named `Null`, and non-nullable scalars are unchanged. +- **`HasValue` for null-capable scalars.** A nullable scalar, and any reference-backed scalar (whose + `Empty`/`Hydrate(null!)` value is null even when the domain contract says non-null), exposes + `public bool HasValue`. It reports whether the underlying value is null without validating or + normalizing, is not generated for a non-nullable value-type scalar, and is skipped when the author + declares `HasValue`. +- **Nullable scalars map to nullable Entity Framework Core columns.** The generated registry unwraps + `Nullable` before matching the converter, so a `NullableString?` property uses the generated + `ValueConverter` with no hand-written converter, and both `T` and `T?` + properties are mapped. `ScalarMapping` now carries the scalar's nullability; the registry marks a + mapped property optional where EF Core accepts it, never overrides an explicit `.IsRequired(...)` or + `[Required]`, and never marks a key nullable. EF Core cannot mark a non-nullable struct property + optional (`CannotBeNullable`), so a nullable scalar used as a non-nullable entity property stays a + required column; the new `VO1031` warning reports that at compile time. The Entity Framework guide + documents the `T?` shape and the two-scalar (`NonNullableString?` + nullable DTO scalar) pattern, + and `src/src/Sample` demonstrates it for `string`, `DateTimeOffset`, and `int`. +- **Nullability conflicts are reported specifically.** A `[Scalar(Nullable = true)]` combined with a + declared `Value` is `VO1028` (with a code fix offering three repairs: remove the property, convert to + the manual `[Scalar]` form, or remove only `Nullable = true`); explicit `Nullable = false` with a + nullable `Value` is `VO1029`; a declared `Value` whose type does not match the underlying type is + `VO1030`. The fully manual `[Scalar]` form with a nullable property remains valid. +- **`VO1031`** — a nullable scalar used as a non-nullable property on an entity or complex-type-mapped + `[ValueObject]`. Scoped to types the compilation can see being mapped, so DTO properties of nullable + scalars are not reported. + +### Changed + +- **Extraction is explicit for nullable scalars only.** A nullable scalar emits + `explicit operator TValue(TSelf)`, so `value == null`, `value != null`, `null == value`, and + comparisons with the underlying value are unambiguous against the record's synthesized + `(TSelf, TSelf)` equality, and they read the underlying value directly — no normalization or + validation. Extraction for a nullable scalar is `(TValue)value` or `value.Value`. A **non-nullable** + scalar keeps the implicit conversion, so `string raw = value;` and passing the value object where its + underlying primitive is expected continue to work. ## 1.1.0 diff --git a/README.md b/README.md index 40733f4..7154ff4 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Adds F#-style single-case types to C#. Mark a `partial` struct or class with `[S incremental source generator produces: - `Create` / `Hydrate` / `TryCreate` factories with `OnNormalize` normalization and `OnValidate` validation -- `Empty` instances, equality, comparison, `CompareTo`, `ToString` (including the underlying value's `IFormattable` and format overloads), and implicit conversions +- `Empty` instances, `HasValue` for null-capable scalars, equality, comparison, `CompareTo`, `ToString` (including the underlying value's `IFormattable` and format overloads), implicit conversions (with explicit extraction for nullable scalars) - The underlying value's standard interfaces — `IEquatable`, `ISpanFormattable`, `IUtf8SpanFormattable`, `IParsable`, `ISpanParsable`, and `IUtf8SpanParsable` — mirrored per type - JSON converters (scalar value objects serialize as their underlying value) - Contextual creation via `IContextualValueObject<,>` + `ValueObjectContext` diff --git a/docs/Diagnostics.md b/docs/Diagnostics.md index c4ed554..f25d815 100644 --- a/docs/Diagnostics.md +++ b/docs/Diagnostics.md @@ -22,6 +22,9 @@ These govern the declaration shape a value object must have for the generator to | `VO1008` | Error | `[Scalar]` and `[ValueObject]` are both applied to the same type. | Pick one. A scalar wraps a single primitive; a value object composes members. | | `VO1016` | Warning | A value object member is mutable. | Make the member `readonly`/`init`-only. A mutable member breaks immutability and makes equality unstable. | | `VO1022` | Error | A `[Scalar]` type declares a member with the name of the property the generator owns. | Remove the declaration, or use `[Scalar]` (without a type argument) to own the property yourself. | +| `VO1028` | Error | `[Scalar(Nullable = true)]` is combined with a declared `Value` property, so nullability has two sources. | Remove the declared `Value`; convert to the manual `[Scalar]` form; or remove `Nullable = true`. The code fix offers all three. | +| `VO1029` | Error | Explicit `Nullable = false` contradicts a declared nullable `Value` property. | Remove the declared `Value`, or use the manual `[Scalar]` form and own the property. | +| `VO1030` | Error | The automatic form's declared `Value` has a type that does not match the scalar's underlying type. | Remove the declaration, or use the manual `[Scalar]` form with a matching type. | ## ZodSharp validation rules @@ -61,6 +64,13 @@ See [Entity Framework integration](Entity-Framework.md) for the mapping model. | `VO1018` | Warning | EF complex mapping cannot convert one of the members. | Simplify the member, or map it explicitly. | | `VO1019` | Warning | EF complex-type mapping is requested but the project resolves to EF Core 7 or earlier. | Target EF Core 8+, or use a different `EntityFrameworkMapping`. | | `VO1021` | Warning | EF key value generation is unavailable for the value object. | Assign keys explicitly instead of relying on store generation. | +| `VO1031` | Warning | A nullable scalar is used as a non-nullable property on an entity or complex-type-mapped `[ValueObject]`. EF Core maps that to a required column and cannot persist the scalar's `Null()`. | Declare the property as `T?`, or use a non-nullable scalar behind a nullable property. See [Entity Framework](Entity-Framework.md#nullable-scalars-and-nullable-columns). | + +`VO1031` is scoped to types the compilation can see being mapped (a `DbSet` on a `DbContext`, an +`IEntityTypeConfiguration` target, or a complex-type-mapped `[ValueObject]`). Plain DTOs are not +reported: a DTO property of a nullable scalar is the supported way to represent the scalar's `Null()` +value. Entity types configured only in a referenced assembly, or built entirely at runtime, are not +discovered; the rule stays a warning and can be suppressed when the non-nullable shape is deliberate. ## Retired identifiers diff --git a/docs/Entity-Framework.md b/docs/Entity-Framework.md index 310264e..7c94c2e 100644 --- a/docs/Entity-Framework.md +++ b/docs/Entity-Framework.md @@ -140,6 +140,113 @@ What the mapping does: - Complex value objects with `[ValueObject(EFMapping = EntityFrameworkMapping.Json)]` map to a single JSON column using the generated JSON converter. +## Nullable scalars and nullable columns + +A scalar can represent null (`[Scalar(Nullable = true)]`, `[Scalar]`, or a manual +`[Scalar]` with a nullable `Value` property). Entity Framework Core derives a **column's** nullability +from the CLR property type, so the registry: + +- unwraps `Nullable` before matching the converter, so a `T?` property uses the generated `T` + converter — no hand-written `ValueConverter` is needed; +- marks a mapped property optional when the scalar is nullable (or the property is `T?`) **where EF + Core permits it**, never overrides an explicit `.IsRequired(...)`/`[Required]`, and never marks a key + nullable. + +| Entity property | Column | What a database `NULL` means | +| --- | --- | --- | +| `NullableString` (a non-nullable struct) | required | impossible; `VO1031` reports it | +| `NullableString?` | nullable | both a CLR `null` and `NullableString.Null()` | +| `NonNullableString?` (two-scalar pattern) | nullable | absence only; no value can collapse into it | + +`HasValue` is generated for a nullable scalar and for any reference-backed scalar, so +`dto.Email.HasValue` inspects the value without constructing one through `Create`. + +EF Core cannot mark a non-nullable struct property optional: `Property.SetIsNullable(true)` throws +`CannotBeNullable`. A nullable scalar on the non-nullable shape therefore stays a required column (and +`NullableString.Null()` cannot be persisted through it); `VO1031` reports that at compile time. The +fix is one of: + +1. declare the entity property as `T?`; or +2. keep a non-nullable scalar in the entity (`NonNullableString?`) and map the nullable DTO scalar at + the boundary. + +### Keeping "absent" distinct from "null" + +The two-scalar pattern is the stricter option: the entity stores a non-nullable scalar behind a +nullable property, so a database `NULL` can only mean "no value", and the domain's `Null()` value lives +in the DTO: + +```csharp +// Persistence scalar: never null, and Create canonicalises input. +[Scalar(Trim = true, Casing = StringCasing.LowerInvariant)] +public readonly partial record struct NonNullableString { } + +// Domain/DTO scalar: null is a value. +[Scalar(Nullable = true)] +public readonly partial record struct NullableString { } + +public sealed class Customer +{ + public NonNullableString? Email { get; set; } // nullable column, generated T converter +} + +public sealed record CustomerDto +{ + public NullableString Email { get; init; } // Null() = explicitly no value +} +``` + +Map at the boundary; `Hydrate` is the replay path (no re-validation), `Create` applies the +non-nullable scalar's normalization and validation: + +```csharp +static CustomerDto ToDto(Customer entity) => new() +{ + Email = entity.Email is null + ? NullableString.Null() + : NullableString.Hydrate(entity.Email.Value.Value), +}; + +static Customer ToEntity(CustomerDto dto) => new() +{ + Email = dto.Email.HasValue + ? NonNullableString.Create(dto.Email.Value!) + : (NonNullableString?)null, +}; +``` + +Because `HasValue` is a computed property, the compiler does not narrow `Value`; use `Value!` (or +`GetValueOrDefault()` for a value-type underlying) after the check. + +> **The cast on the null branch matters for a reference-backed scalar.** A bare `null` is convertible +> to `NonNullableString` through the generated implicit creation operator, so the conditional would +> produce a *populated* scalar whose `Value` is null instead of a null `Nullable`. Write +> `(NonNullableString?)null` (or branch with an `if`). A value-type-backed scalar (`int`, +> `DateTimeOffset`) does not have this problem because `null` is not convertible to its underlying +> type. + +The conversion can also be expressed with explicit operators declared in one scalar's partial: + +```csharp +[Scalar(Nullable = true)] +public readonly partial record struct NullableString +{ + public static explicit operator NullableString(NonNullableString? value) => + value is null ? Null() : Hydrate(value.Value.Value); + + public static explicit operator NonNullableString?(NullableString value) => + value.Value is null ? (NonNullableString?)null : NonNullableString.Create(value.Value); +} + +dto.Email = (NullableString)entity.Email; +entity.Email = (NonNullableString?)dto.Email; +``` + +The generator does not emit cross-scalar conversions: the pairing and the `Create`/`Hydrate` policy +belong to the boundary. The runnable [sample](../src/src/Sample) demonstrates the pattern for +`string`, `DateTimeOffset`, and `int`; the tests are in +`src/tests/ValueObjects.EFCompatibility.IntegrationTests`. + ## Queries — no `.Value` required Because scalar value objects convert to their underlying primitive column, queries compare the value object @@ -342,6 +449,9 @@ modelBuilder.Entity().HasQueryFilter(tenant => tenant.Id == Platform.Sys - A scalar value object is a **single column** of its provider primitive, so `dotnet ef migrations add` sees the primitive: renaming the value object's property does not change the schema, and changing the underlying type is a column type change you review like any other. +- A nullable scalar is a **nullable column only when the entity property is nullable** (`T?`): EF Core + derives a column's nullability from the CLR property type, not from the scalar's own ability to hold + null. A non-nullable struct property is a required column (`VO1031`), even for a nullable scalar. - A complex value object mapped as a complex type is a **set of columns** named after its members, and one mapped with `EFMapping = Json` is a **single JSON column**. Moving a value object between those shapes is a schema change: add a migration and consider the data path (a JSON column usually needs a data migration @@ -438,6 +548,9 @@ value-object provider assemblies referenced by EF consumers: property manually. - `VO1021` — `GenerateEFValueGenerator` was requested for a value object that is not Guid-backed, or whose Entity Framework converter is disabled; no value generator is emitted. +- `VO1031` — a nullable scalar is used as a non-nullable property on an entity or complex type. EF Core + maps that to a required column and cannot persist the scalar's `Null()`; declare the property as `T?` + (or use the two-scalar pattern above). ## Notes diff --git a/docs/Getting-Started.md b/docs/Getting-Started.md index c0b538b..dc1698c 100644 --- a/docs/Getting-Started.md +++ b/docs/Getting-Started.md @@ -42,7 +42,8 @@ The generator adds: - `EmailAddress.Hydrate(string)` – constructs without re-validating (for persisted data). - `EmailAddress.TryCreate(string, out EmailAddress)` – returns `false` instead of throwing. - `EmailAddress.Empty` – a default instance. -- Equality, comparison, `CompareTo`, `ToString`, implicit conversions, and a JSON converter. +- Equality, comparison, `CompareTo`, `ToString`, implicit conversions (explicit extraction for nullable + scalars), and a JSON converter. - The underlying value's standard interfaces (see [Formatting](#formatting) and [Standard interfaces](#standard-interfaces)) when it implements them. ```csharp @@ -73,7 +74,8 @@ public readonly partial record struct EmailAddress { } `[Scalar]` is the **manual** form: you declare the property. `[Scalar]` and `[Scalar(typeof(T))]` are the **automatic** form: the generator declares it. They are mutually exclusive for a given type — -declaring the property with an automatic form is an error (`VO1022`). +declaring the property with an automatic form is an error (`VO1022`, or the more specific +`VO1028`–`VO1030` when the declaration also asserts nullability). ```csharp // Automatic: the generator emits `public Guid Value { get; init; }`. @@ -126,9 +128,68 @@ A nullable scalar also exposes `Null()`, which creates the “unknown” value w `Nickname.Null()` is equivalent to `Nickname.Hydrate(null)`. Use `Create(null)` instead when `null` is input that should go through normalization and validation. +A nullable scalar — and any reference-backed scalar, whose `Empty`/`Hydrate(null!)` value is null even +when the domain contract says non-null — exposes `HasValue`, which reports whether the underlying value +is null without validating or normalizing: `Nickname.Null().HasValue` is `false`, +`Nickname.Create("x").HasValue` is `true`, and `Nickname.Empty.HasValue` is also `false`. A +non-nullable value-type scalar (for example `[Scalar]`) has no `HasValue`; it is always present. + The manual form is the more explicit option when you prefer the type and nullability to appear in a -property declaration. Both forms are supported, and neither is reported as a diagnostic. Other properties -are allowed in either form; only the scalar property (and the interface's `Value` forwarding) is reserved. +property declaration. Both forms are supported on their own; they cannot be combined. Declaring the +generator-owned property is reported as an error, with a specific rule for each nullability conflict: + +| Rule | When | +| --- | --- | +| `VO1022` | An automatic form declares a member with the generator-owned name (the plain duplicate). | +| `VO1028` | `Nullable = true` is combined with a declared `Value`: nullability has two sources. | +| `VO1029` | `Nullable = false` is explicit but the declared `Value` is nullable. | +| `VO1030` | The declared `Value` type does not match the scalar's underlying type. | + +`VO1028` has a code fix offering three repairs: remove the declared property; convert the attribute to +the manual `[Scalar]` form; or remove only `Nullable = true`. Other properties are allowed in either +form; only the scalar property (and the interface's `Value` forwarding) is reserved. + +When a nullable scalar is used as an Entity Framework Core property, declare the property as `T?`; see +[Entity Framework](Entity-Framework.md#nullable-scalars-and-nullable-columns). Using it as a +non-nullable entity property is reported as `VO1031`. + +### Comparing with `null` and the underlying value + +Equality comparisons work against the underlying value and against `null`, in either operand order: + +```csharp +NullableString value = NullableString.Null(); + +_ = value == null; // true +_ = null == value; // true +_ = value != null; // false + +NullableString name = NullableString.Create("value"); +_ = name == "value"; // true +_ = "value" == name; // true +_ = name == null; // false +``` + +`default(T)`, `Empty`, `Null()`, and `Create(null)` all compare equal to `null`, symmetrically. The +comparison reads the underlying value directly — it never calls `Create`, so it neither normalizes nor +validates. + +`is null` does not work for a non-nullable struct (CS0037); use `== null`/`!= null`. A **nullable** or +reference-backed scalar also exposes `HasValue`, which inspects the underlying value without constructing +one (and therefore without normalizing or validating), and is the cheap null check. Extraction is +**explicit for a nullable scalar**, so write `.Value` or a cast; a **non-nullable** scalar keeps the +implicit conversion, so `string raw = email;` continues to work: + +```csharp +string rawNullable = (string)name; // or name.Value +string rawRequired = email; // implicit for a non-nullable scalar +``` + +The conversion **from** the underlying value remains implicit for both, so +`NullableString value = "value";` still creates through `Create`. A non-nullable scalar does not +support `== null`: a value-type scalar has no conversion from `null`, and a non-nullable +reference-backed scalar is ambiguous between its primitive operator and the synthesized equality. Check +`!value.HasValue` when you need to inspect the wrapped value. ### Formatting @@ -272,6 +333,10 @@ Use `[ValueObjectDefaults]` to set generic defaults for the whole assembly. Ever Assembly defaults apply to every value object in the assembly; an option explicitly set on an individual `[Scalar]`/`[ValueObject]` attribute always overrides it. +`GenerateImplicitToPrimitive` keeps its name for compatibility. It emits an explicit extraction operator +for a nullable scalar (write `(TValue)value` or `.Value`) and an implicit one for a non-nullable scalar; +`GenerateImplicitFromPrimitive` controls the implicit creation conversion. + ### Setting defaults from MSBuild Every `[ValueObjectDefaults]` option also has a matching `ValueObjectsDefault` MSBuild property, so a diff --git a/skills/purview-value-objects-diagnostics/SKILL.md b/skills/purview-value-objects-diagnostics/SKILL.md index 8d03cf0..7f6b150 100644 --- a/skills/purview-value-objects-diagnostics/SKILL.md +++ b/skills/purview-value-objects-diagnostics/SKILL.md @@ -32,6 +32,9 @@ failure to the missing member. | `VO1008` | Error | `[Scalar]` and `[ValueObject]` are both applied to one type. | Pick one: a scalar wraps a single primitive, a value object composes members. | | `VO1016` | Warning | A value object member is mutable. | Make it `readonly`/`init`-only — a mutable member breaks immutability and makes equality unstable. | | `VO1022` | Error | A `[Scalar]` type declares a member with the name of the property the generator owns. | Remove the declaration, or use `[Scalar]` without a type argument and own the property yourself. | +| `VO1028` | Error | `[Scalar(Nullable = true)]` is combined with a declared `Value`, so nullability has two sources. | Remove the declared property; convert to the manual `[Scalar]` form; or remove `Nullable = true`. The code fix offers all three. | +| `VO1029` | Error | Explicit `Nullable = false` contradicts a declared nullable `Value`. | Remove the declared `Value`, or use the manual `[Scalar]` form. | +| `VO1030` | Error | The automatic form's declared `Value` type does not match the underlying type. | Remove the declaration, or use the manual `[Scalar]` form with a matching type. | ## String normalization rules @@ -64,6 +67,7 @@ effective configuration still raises `VO1023`/`VO1024` where those rules apply. | `VO1018` | Warning | EF complex mapping cannot convert one of the members. | Simplify the member, or map it explicitly. | | `VO1019` | Warning | EF complex-type mapping is requested but the project resolves to EF Core 7 or earlier. | Target EF Core 8+, or choose a different `EntityFrameworkMapping`. | | `VO1021` | Warning | EF key value generation is unavailable for the value object. | Assign keys explicitly rather than relying on store generation. | +| `VO1031` | Warning | A nullable scalar is used as a non-nullable property on an entity or complex-type-mapped `[ValueObject]`. EF Core maps that to a required column and cannot persist the scalar's `Null()`. | Declare the property as `T?`, or use a non-nullable scalar behind a nullable property. The rule only runs where EF Core is referenced and only for entity/complex types the compilation can see; DTOs are not reported. | ## Retired identifiers — never reused diff --git a/skills/purview-value-objects-efcore/SKILL.md b/skills/purview-value-objects-efcore/SKILL.md index cb57c12..8800c71 100644 --- a/skills/purview-value-objects-efcore/SKILL.md +++ b/skills/purview-value-objects-efcore/SKILL.md @@ -3,7 +3,7 @@ name: purview-value-objects-efcore description: >- Use when persisting Purview.ValueObjects with Entity Framework Core - ConfigureValueObjects, UseValueObjects, scalar-to-column conversion, complex-type versus JSON mapping, value objects as - keys and foreign keys, query translation, generated key values, migrations, and the VO1009-VO1021 + keys and foreign keys, query translation, generated key values, migrations, and the VO1009-VO1031 Entity Framework diagnostics. metadata: product: Purview.ValueObjects @@ -67,6 +67,58 @@ generated one, and the composite loses the provider tolerance that raw-primitive A plain `enum` property that is not a value object keeps EF Core's own enum mapping, untouched. +## Nullable scalars + +A nullable scalar (`Nullable = true`, `[Scalar]`, or a manually declared `T?` value) is still a +struct. The registry unwraps `Nullable` before matching, so a `T?` property uses the generated `T` +converter and needs no hand-written converter. It marks the property optional where EF Core accepts it, +never overrides an explicit `IsRequired(...)`/`[Required]`, and never marks a key nullable. + +| Entity property | Column | Database `NULL` means | +| --- | --- | --- | +| `NullableString` (non-nullable struct) | required | impossible — `VO1031` warns | +| `NullableString?` | nullable | both CLR `null` and `NullableString.Null()` | +| `NonNullableString?` | nullable | absence only (two-scalar pattern) | + +`HasValue` is generated for a nullable scalar and for any reference-backed scalar, so +`dto.Email.HasValue` is the non-validating check. + +EF Core cannot mark a non-nullable struct property optional (`CannotBeNullable`), so a nullable scalar +on the non-nullable shape stays a required column. Declare the entity property as `T?`, or keep a +non-nullable scalar on the entity and map the nullable DTO scalar at the boundary: + +```csharp +[Scalar(Nullable = true)] public readonly partial record struct NullableString { } +[Scalar(Trim = true)] public readonly partial record struct NonNullableString { } + +public sealed class Customer { public NonNullableString? Email { get; set; } } // nullable column +public sealed record CustomerDto { public NullableString Email { get; init; } } // Null() = no value + +static CustomerDto ToDto(Customer entity) => new() +{ + Email = entity.Email is null + ? NullableString.Null() + : NullableString.Hydrate(entity.Email.Value.Value), +}; + +static Customer ToEntity(CustomerDto dto) => new() +{ + Email = dto.Email.HasValue + ? NonNullableString.Create(dto.Email.Value!) + : (NonNullableString?)null, // see the note below +}; +``` + +`Hydrate` is the replay path (no re-validation); `Create` applies the non-nullable scalar's +normalization and validation. **For a reference-backed scalar the null branch needs the explicit cast.** +A bare `null` is convertible to `NonNullableString` through the implicit creation operator, so the +conditional would produce a populated scalar whose `Value` is null instead of a null `Nullable`. A +value-type-backed scalar (`int`, `DateTimeOffset`) does not have that problem. + +`Null()` on a `T?` property is a null provider value: it is written as SQL `NULL` and reads back as a +CLR `null`. Consumers can also declare their own explicit conversion operators in one scalar's partial; +the generator does not invent cross-scalar conversions. + ## Querying — no `.Value` Because a scalar converts to its primitive column, a predicate compares the value object directly: @@ -266,7 +318,8 @@ and later. `VO1009` (EF option set without an EF reference), `VO1010` (underlying type not mappable), `VO1017` (JSON mapping without the JSON converter), `VO1018` (a complex member cannot be converted), `VO1019` -(complex type on EF Core 7 or earlier) and `VO1021` (value generation unavailable) are the EF rules. +(complex type on EF Core 7 or earlier), `VO1021` (value generation unavailable) and `VO1031` (a +nullable scalar on a non-nullable mapped property) are the EF rules. The `purview-value-objects-diagnostics` skill carries the full `VO1xxx` catalogue. ## Where to look in the repository diff --git a/skills/purview-value-objects/SKILL.md b/skills/purview-value-objects/SKILL.md index abe5bdf..9636d32 100644 --- a/skills/purview-value-objects/SKILL.md +++ b/skills/purview-value-objects/SKILL.md @@ -74,7 +74,15 @@ public readonly partial record struct EmailAddress - `Nullable = true` makes the automatic property nullable for both reference and non-nullable value types (`[Scalar(Nullable = true)]` emits `DateTimeOffset? Value`). A nullable scalar round-trips JSON `null` and exposes `Null()`, which is `Hydrate(null)` for the unknown case and never - validates; use `Create(null)` when `null` is input that must be checked. + validates, plus `HasValue` for a non-validating null check; use `Create(null)` when `null` is input + that must be checked. +- The automatic form cannot also declare `Value`: that is `VO1022`, or `VO1028` when + `Nullable = true`, `VO1029` when explicit `Nullable = false` contradicts a nullable `Value`, and + `VO1030` when the declared type does not match. `VO1028` has a code fix offering all three repairs. +- A nullable scalar compares with `null` and with the underlying value in either order + (`value == null`, `null == value`, `value == "x"`). `is null` is impossible on a struct (`CS0037`). + Extraction is explicit for a nullable scalar (`(string?)value` or `value.Value`) and implicit for a + non-nullable one; creation stays implicit and goes through `Create`. - The default property name is `Value`; `ScalarAttribute.PropertyName` changes it. A `[Scalar]` type with no matching property is `VO1004`. - Prefer `readonly record struct`. Anything else is `VO1006`, because equality, immutability and diff --git a/src/src/Sample/Models.cs b/src/src/Sample/Models.cs index 92269c2..f137d1b 100644 --- a/src/src/Sample/Models.cs +++ b/src/src/Sample/Models.cs @@ -59,8 +59,35 @@ static partial void OnValidate(Guid value) } // A nullable reference scalar: the generator declares `public string? Value { get; init; }`. +// `Null()` is the domain value for "explicitly null" and is what a nullable column stores as SQL NULL. [Scalar(Nullable = true)] -readonly partial record struct Nickname { } +readonly partial record struct NullableNickname +{ + // Boundary mapping to the non-nullable persistence scalar. The explicit cast on the null branch is + // required: a bare `null` would otherwise convert through the implicit creation operator and become + // a populated scalar whose Value is null. + public static explicit operator NullableNickname(NonNullableNickname? value) => + value is null ? Null() : Hydrate(value.Value.Value); + + public static explicit operator NonNullableNickname?(NullableNickname value) => + value.Value is null ? (NonNullableNickname?)null : NonNullableNickname.Create(value.Value); +} + +// The non-nullable persistence counterpart. Create trims and lowercases; Hydrate replays as-is. +[Scalar(Trim = true, Casing = StringCasing.LowerInvariant)] +readonly partial record struct NonNullableNickname { } + +[Scalar(Nullable = true)] +readonly partial record struct NullableTimestamp { } + +[Scalar] +readonly partial record struct NonNullableTimestamp { } + +[Scalar] +readonly partial record struct NullableQuantity { } + +[Scalar] +readonly partial record struct NonNullableQuantity { } [ValueObject] readonly partial record struct Money @@ -107,6 +134,17 @@ sealed class Order public Money Total { get; set; } public OrderStatus Status { get; set; } + + // The direct nullable-scalar shape: the column is nullable because the CLR property is T?. + public NullableNickname? Nickname { get; set; } + + // The two-scalar shape: a non-nullable scalar behind a nullable property. The DTO keeps + // NullableNickname and maps at the boundary. + public NonNullableNickname? PreferredName { get; set; } + + public NonNullableTimestamp? DeliveredAt { get; set; } + + public NonNullableQuantity? Quantity { get; set; } } // Complex members configure normalization with [StringNormalize] instead of an OnNormalize hook. diff --git a/src/src/Sample/NullableScalarDemo.cs b/src/src/Sample/NullableScalarDemo.cs new file mode 100644 index 0000000..04afc67 --- /dev/null +++ b/src/src/Sample/NullableScalarDemo.cs @@ -0,0 +1,80 @@ +using Microsoft.EntityFrameworkCore; + +namespace Purview.ValueObjects.Sample; + +/// +/// Demonstrates the nullable-scalar shapes: a nullable scalar behind a T? property, the +/// two-scalar pattern (a non-nullable persistence scalar behind a nullable property), and the DTO +/// mapping between them. Entity Framework Core maps a T? property to a nullable column; a +/// non-nullable struct property stays required because EF Core cannot mark it optional. +/// +static class NullableScalarDemo +{ + public static async Task RunAsync(SampleDbContext db) + { + Console.WriteLine(); + Console.WriteLine("== Nullable scalars, nullable columns and DTO mapping =="); + + var entity = NewOrder("nullable@example.com", 10m, OrderStatusKind.Shipped); + entity.Nickname = NullableNickname.Create(" Mixed Case "); + entity.PreferredName = NonNullableNickname.Create(" Mixed Case "); + entity.DeliveredAt = NonNullableTimestamp.Create(DateTimeOffset.UtcNow); + entity.Quantity = NonNullableQuantity.Create(3); + + var dto = entity.ToDto(); + Console.WriteLine($"DTO PreferredName -> '{dto.PreferredName.Value}'"); + Console.WriteLine($"Entity PreferredName -> '{entity.PreferredName!.Value.Value}'"); + + var mappedBack = dto.ToEntity(); + Console.WriteLine($"DTO -> entity -> '{mappedBack.PreferredName!.Value.Value}' (Create trims/lowercases)"); + var castBack = (NonNullableNickname?)dto.PreferredName; + Console.WriteLine($"DTO -> cast -> '{castBack!.Value.Value}'"); + + var nullScalars = NewOrder("null@example.com", 5m, OrderStatusKind.Pending); + // A nullable scalar's Null() is a null provider value, so the nullable column stores NULL. + nullScalars.Nickname = NullableNickname.Null(); + + var nullScalarsDto = nullScalars.ToDto(); + Console.WriteLine($"Null() -> DTO PreferredName HasValue -> {nullScalarsDto.PreferredName.HasValue}"); + Console.WriteLine( + $"Null() -> DTO -> entity PreferredName is null -> {nullScalarsDto.ToEntity().PreferredName is null}" + ); + + db.Orders.AddRange(entity, nullScalars); + await db.SaveChangesAsync(); + db.ChangeTracker.Clear(); + + var stored = await db.Orders.SingleAsync(order => order.Id == entity.Id); + Console.WriteLine($"Stored PreferredName -> '{stored.PreferredName!.Value.Value}'"); + Console.WriteLine($"Stored DeliveredAt -> {stored.DeliveredAt!.Value.Value:O}"); + Console.WriteLine($"Stored Quantity -> {stored.Quantity!.Value.Value}"); + + var storedNulls = await db.Orders.SingleAsync(order => order.Id == nullScalars.Id); + Console.WriteLine($"Null() persisted as NULL, read back as null -> {storedNulls.Nickname is null}"); + + var orderType = db.Model.FindEntityType(typeof(Order))!; + foreach ( + var propertyName in new[] + { + nameof(Order.Nickname), + nameof(Order.PreferredName), + nameof(Order.DeliveredAt), + nameof(Order.Quantity), + nameof(Order.CustomerEmail), + } + ) + { + var property = orderType.FindProperty(propertyName)!; + Console.WriteLine($"{propertyName} column nullable -> {property.IsNullable}"); + } + } + + static Order NewOrder(string email, decimal total, OrderStatusKind status) => + new() + { + Id = OrderId.Hydrate(Guid.NewGuid()), + CustomerEmail = EmailAddress.Create(email), + Total = Money.Create(total, CurrencyCode.Create("USD")), + Status = OrderStatus.Hydrate(status), + }; +} diff --git a/src/src/Sample/OrderDto.cs b/src/src/Sample/OrderDto.cs new file mode 100644 index 0000000..e395fc8 --- /dev/null +++ b/src/src/Sample/OrderDto.cs @@ -0,0 +1,64 @@ +namespace Purview.ValueObjects.Sample; + +/// +/// The DTO shape: nullable scalars for the nullable domain values, while the entity keeps the +/// non-nullable persistence scalars behind nullable properties. +/// +sealed class OrderDto +{ + public OrderId Id { get; set; } + + public EmailAddress CustomerEmail { get; set; } + + public NullableNickname? Nickname { get; set; } + + public NullableNickname PreferredName { get; set; } + + public NullableTimestamp DeliveredAt { get; set; } + + public NullableQuantity Quantity { get; set; } +} + +/// +/// Boundary mapping between the entity and the DTO. Hydrate is used when reading a persisted +/// value (a replay path that must not re-validate); Create is used when writing input, so the +/// non-nullable scalar applies its trim/casing/validation. +/// +static class OrderMappings +{ + public static OrderDto ToDto(this Order order) => + new() + { + Id = order.Id, + CustomerEmail = order.CustomerEmail, + Nickname = order.Nickname, + PreferredName = order.PreferredName.HasValue + ? NullableNickname.Hydrate(order.PreferredName.Value.Value) + : NullableNickname.Null(), + DeliveredAt = order.DeliveredAt.HasValue + ? NullableTimestamp.Hydrate(order.DeliveredAt.Value.Value) + : NullableTimestamp.Null(), + Quantity = order.Quantity.HasValue + ? NullableQuantity.Hydrate(order.Quantity.Value.Value) + : NullableQuantity.Null(), + }; + + public static Order ToEntity(this OrderDto dto) => + new() + { + Id = dto.Id, + CustomerEmail = dto.CustomerEmail, + Nickname = dto.Nickname, + // The explicit cast is required: a bare `null` would convert through the implicit + // creation operator into a populated scalar whose Value is null. + PreferredName = dto.PreferredName.HasValue + ? NonNullableNickname.Create(dto.PreferredName.Value!) + : (NonNullableNickname?)null, + DeliveredAt = dto.DeliveredAt.HasValue + ? NonNullableTimestamp.Create(dto.DeliveredAt.Value.GetValueOrDefault()) + : null, + Quantity = dto.Quantity.HasValue + ? NonNullableQuantity.Create(dto.Quantity.Value.GetValueOrDefault()) + : null, + }; +} diff --git a/src/src/Sample/Program.cs b/src/src/Sample/Program.cs index aa7ca6c..9e5d5da 100644 --- a/src/src/Sample/Program.cs +++ b/src/src/Sample/Program.cs @@ -85,3 +85,5 @@ var emailToMatch = EmailAddress.Create("demo@example.com"); var matched = await db.Orders.Where(o => o.CustomerEmail == emailToMatch).ToListAsync(); Console.WriteLine($"Orders for {emailToMatch.Value} -> {matched.Count}"); + +await NullableScalarDemo.RunAsync(db); diff --git a/src/src/Sample/README.md b/src/src/Sample/README.md index 1941e4c..d38b333 100644 --- a/src/src/Sample/README.md +++ b/src/src/Sample/README.md @@ -11,8 +11,9 @@ dotnet run --project src/src/Sample ## What it shows -- `[Scalar]` value objects: `Create`/`Hydrate`/`TryCreate`, normalization, validation, implicit conversions, - equality with the primitive. +- `[Scalar]` value objects: `Create`/`Hydrate`/`TryCreate`, normalization, validation, implicit + conversions (explicit extraction for nullable scalars), equality with the primitive, and + `value == null` comparisons for nullable scalars. - `[ValueObject]` complex value objects with cross-field validation. - JSON round-trip of an entity whose members are value objects, using a `JsonSerializerOptions` configured with `ScalarJsonConverterFactory` (the shape EF stores in a JSON column). @@ -22,4 +23,9 @@ dotnet run --project src/src/Sample complex types (EF Core 8+). - Queries use the value object type directly — no `.Value` required — including nested members of complex value objects (`o.Total.Amount > 20`). + - Nullable scalars: a `T?` entity property maps to a nullable column using the generated `T` + converter, and `Null()` is stored as SQL `NULL`. The sample also shows the two-scalar pattern + (`NonNullableNickname?` on the entity, `NullableNickname` on the DTO) and the boundary mapping, + including the explicit-operator form. A nullable scalar on a non-nullable entity property is + reported as `VO1031`. - See `SampleDbContext` for the `OnModelCreating` wiring. \ No newline at end of file diff --git a/src/src/SourceGenerator.Refactorings/ScalarNullabilityCodeFixProvider.cs b/src/src/SourceGenerator.Refactorings/ScalarNullabilityCodeFixProvider.cs new file mode 100644 index 0000000..4e7b99a --- /dev/null +++ b/src/src/SourceGenerator.Refactorings/ScalarNullabilityCodeFixProvider.cs @@ -0,0 +1,210 @@ +using System.Collections.Immutable; +using System.Composition; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Formatting; +using Purview.ValueObjects.SourceGenerator.Common; + +namespace Purview.ValueObjects.SourceGenerator.Refactorings; + +/// +/// Resolves the duplicate nullability declaration reported as VO1028: the automatic +/// [Scalar<T>(Nullable = true)] form already declares the underlying Value property, +/// so a second declaration is a conflicting source of nullability. Three repairs are offered because +/// the right one depends on which source the author wants to keep. +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ScalarNullabilityCodeFixProvider)), Shared] +public sealed class ScalarNullabilityCodeFixProvider : CodeFixProvider +{ + const string RemoveDeclaredPropertyKey = "ScalarNullability.RemoveDeclaredProperty"; + + const string UseManualFormKey = "ScalarNullability.UseManualForm"; + + const string RemoveNullableOptionKey = "ScalarNullability.RemoveNullableOption"; + + /// + public override ImmutableArray FixableDiagnosticIds => + [DiagnosticLibrary.ScalarNullabilityDeclaredTwice.Id]; + + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + return; + + foreach (var diagnostic in context.Diagnostics) + { + var property = root.FindToken(diagnostic.Location.SourceSpan.Start) + .Parent?.AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + if (property is null || !IsNamedValue(property)) + continue; + + context.RegisterCodeFix( + CodeAction.Create( + title: "Remove the declared 'Value' property", + createChangedDocument: cancellationToken => + RemovePropertyAsync(context.Document, property, cancellationToken), + equivalenceKey: RemoveDeclaredPropertyKey + ), + diagnostic + ); + + var attribute = await FindScalarAttributeAsync(root, property, context.CancellationToken) + .ConfigureAwait(false); + if (attribute is null) + continue; + + context.RegisterCodeFix( + CodeAction.Create( + title: "Use the manual [Scalar] form (remove the type argument and 'Nullable = true')", + createChangedDocument: cancellationToken => + RewriteAttributeAsync(context.Document, attribute, useManualForm: true, cancellationToken), + equivalenceKey: UseManualFormKey + ), + diagnostic + ); + + context.RegisterCodeFix( + CodeAction.Create( + title: "Remove only 'Nullable = true'", + createChangedDocument: cancellationToken => + RewriteAttributeAsync(context.Document, attribute, useManualForm: false, cancellationToken), + equivalenceKey: RemoveNullableOptionKey + ), + diagnostic + ); + } + } + + static bool IsNamedValue(PropertyDeclarationSyntax property) => + string.Equals(property.Identifier.ValueText, "Value", StringComparison.Ordinal); + + /// + /// The [Scalar]/[Scalar<T>] attribute applied to the property's containing type. + /// The lookup is syntactic: a code-fix document in a bare workspace has no metadata references, so + /// the attribute class cannot be resolved through the semantic model. A partial declaration that + /// carries the attribute in a different document is not found, in which case only the + /// remove-the-property repair is offered. + /// + static Task FindScalarAttributeAsync( + SyntaxNode root, + PropertyDeclarationSyntax property, + CancellationToken cancellationToken + ) + { + cancellationToken.ThrowIfCancellationRequested(); + + var typeDeclaration = property.AncestorsAndSelf().OfType().FirstOrDefault(); + if (typeDeclaration is null) + return Task.FromResult(null); + + var typeName = typeDeclaration.Identifier.ValueText; + var attribute = root.DescendantNodes() + .OfType() + .Where(candidate => string.Equals(candidate.Identifier.ValueText, typeName, StringComparison.Ordinal)) + .SelectMany(candidate => candidate.AttributeLists) + .SelectMany(list => list.Attributes) + .FirstOrDefault(candidate => + string.Equals(GetSimpleName(candidate.Name).Identifier.ValueText, "Scalar", StringComparison.Ordinal) + ); + + return Task.FromResult(attribute); + } + + static SimpleNameSyntax GetSimpleName(NameSyntax name) => + name switch + { + GenericNameSyntax generic => generic, + QualifiedNameSyntax qualified => GetSimpleName(qualified.Right), + AliasQualifiedNameSyntax alias => GetSimpleName(alias.Name), + SimpleNameSyntax simple => simple, + _ => SyntaxFactory.IdentifierName(name.ToString()), + }; + + static async Task RemovePropertyAsync( + Document document, + PropertyDeclarationSyntax property, + CancellationToken cancellationToken + ) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + return document; + + var updated = root.RemoveNode(property, SyntaxRemoveOptions.KeepNoTrivia); + return updated is null + ? document + : await FormatAsync(document.WithSyntaxRoot(updated), cancellationToken).ConfigureAwait(false); + } + + /// + /// Rewrites the scalar attribute. also drops the underlying-type + /// argument (and any positional property-name argument), turning [Scalar<T>(...)] or + /// [Scalar(typeof(T), ...)] into the manual [Scalar(...)] form, so the declared property + /// becomes valid instead of merely swapping one error for VO1022. + /// + static async Task RewriteAttributeAsync( + Document document, + AttributeSyntax attribute, + bool useManualForm, + CancellationToken cancellationToken + ) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + return document; + + var updatedName = useManualForm ? RemoveGenericArity(attribute.Name) : attribute.Name; + var arguments = (attribute.ArgumentList?.Arguments ?? default) + .Where(argument => !IsNullableOption(argument) && !(useManualForm && IsPositional(argument))) + .ToImmutableArray(); + + var argumentList = arguments.IsDefaultOrEmpty + ? null + : SyntaxFactory.AttributeArgumentList(SyntaxFactory.SeparatedList(arguments)); + var updated = attribute + .WithName(updatedName) + .WithArgumentList(argumentList) + .WithAdditionalAnnotations(Formatter.Annotation); + + var newRoot = root.ReplaceNode(attribute, updated); + return await FormatAsync(document.WithSyntaxRoot(newRoot), cancellationToken).ConfigureAwait(false); + } + + static bool IsNullableOption(AttributeArgumentSyntax argument) => + string.Equals(argument.NameEquals?.Name.Identifier.ValueText, "Nullable", StringComparison.Ordinal); + + static bool IsPositional(AttributeArgumentSyntax argument) => + argument.NameEquals is null && argument.NameColon is null; + + /// + /// Removes the generic type argument from an attribute name, preserving any namespace qualification: + /// Scalar<string> becomes Scalar, and Json.Scalar<string> becomes + /// Json.Scalar. + /// + static NameSyntax RemoveGenericArity(NameSyntax name) => + name switch + { + GenericNameSyntax generic => SyntaxFactory.IdentifierName(generic.Identifier), + QualifiedNameSyntax qualified => qualified.WithRight((SimpleNameSyntax)RemoveGenericArity(qualified.Right)), + AliasQualifiedNameSyntax alias => alias.WithName((SimpleNameSyntax)RemoveGenericArity(alias.Name)), + _ => name, + }; + + static async Task FormatAsync(Document document, CancellationToken cancellationToken) + { + var formatted = await Formatter + .FormatAsync(document, cancellationToken: cancellationToken) + .ConfigureAwait(false); + return formatted; + } +} diff --git a/src/src/SourceGenerator/AnalyzerReleases.Unshipped.md b/src/src/SourceGenerator/AnalyzerReleases.Unshipped.md index 6d4b1dc..cbaa31d 100644 --- a/src/src/SourceGenerator/AnalyzerReleases.Unshipped.md +++ b/src/src/SourceGenerator/AnalyzerReleases.Unshipped.md @@ -11,3 +11,7 @@ Rule ID | Category | Severity | Notes VO1025 | ValueObjects | Error | [StringNormalize] is only supported on a string member of a complex [ValueObject]; on a [Scalar] or any other type it has no effect VO1026 | ValueObjects | Error | [ValueObjectDefaults] repeats a default already supplied by the project's MSBuild configuration VO1027 | ValueObjects | Error | A [Scalar]/[ValueObject] option repeats the effective assembly default +VO1028 | ValueObjects | Error | An automatic [Scalar(Nullable = true)] also declares the property, so nullability has two sources; remove one of them +VO1029 | ValueObjects | Error | An automatic [Scalar(Nullable = false)] declares a nullable Value property, contradicting the option +VO1030 | ValueObjects | Error | An automatic [Scalar] declares a Value property whose type does not match the underlying type +VO1031 | ValueObjects | Warning | A nullable scalar is used as a non-nullable entity/complex property, which Entity Framework Core maps to a required column diff --git a/src/src/SourceGenerator/Analyzers/ValueObjectDiagnosticAnalyzer.cs b/src/src/SourceGenerator/Analyzers/ValueObjectDiagnosticAnalyzer.cs index 32fb2b7..a38e817 100644 --- a/src/src/SourceGenerator/Analyzers/ValueObjectDiagnosticAnalyzer.cs +++ b/src/src/SourceGenerator/Analyzers/ValueObjectDiagnosticAnalyzer.cs @@ -30,6 +30,10 @@ public sealed class ValueObjectDiagnosticAnalyzer : DiagnosticAnalyzer DiagnosticLibrary.StringNormalizeRequiresValueObject, DiagnosticLibrary.AssemblyDefaultIsRedundant, DiagnosticLibrary.ValueObjectOptionIsRedundant, + DiagnosticLibrary.ScalarNullabilityDeclaredTwice, + DiagnosticLibrary.ScalarNullabilityContradictsProperty, + DiagnosticLibrary.ScalarPropertyTypeMismatch, + DiagnosticLibrary.NullableScalarPropertyShouldBeNullable, ]; public override void Initialize(AnalysisContext context) @@ -65,6 +69,10 @@ public override void Initialize(AnalysisContext context) ); } + compilationContext.RegisterCompilationEndAction(context => + ValidateNullableScalarProperties(context, scalarAttribute, genericScalarAttribute, valueObjectAttribute) + ); + compilationContext.RegisterSymbolAction( context => ValidateValueObject(context, scalarAttribute, genericScalarAttribute, valueObjectAttribute), SymbolKind.NamedType @@ -139,6 +147,205 @@ is not TypeDeclarationSyntax syntax ValidateTypeDefaults(context, typeSymbol, scalarAttribute, genericScalarAttribute, valueObjectAttribute); } + /// + /// Reports a nullable scalar used as a non-nullable property on a type Entity Framework Core maps. + /// Entity Framework Core derives a column's nullability from the CLR property type, and it refuses to + /// mark a non-nullable struct property optional (CannotBeNullable), so a nullable scalar + /// declared without ? is a required column that cannot persist its own Null() value. + /// + /// + /// Scoped to types the compilation can see being mapped — entity types discovered from + /// DbSet<T> properties and IEntityTypeConfiguration<T> implementations, plus + /// complex-type-mapped [ValueObject] types (whose members become columns). Plain DTOs are + /// deliberately excluded: a DTO property of a nullable scalar is the supported way to represent the + /// scalar's Null() value. Entity types configured only in a referenced assembly, or built + /// entirely at runtime, are not discovered; the rule stays a warning for that reason. + /// + static void ValidateNullableScalarProperties( + CompilationAnalysisContext context, + INamedTypeSymbol? scalarAttribute, + INamedTypeSymbol? genericScalarAttribute, + INamedTypeSymbol? valueObjectAttribute + ) + { + if (scalarAttribute is null && genericScalarAttribute is null) + return; + + var compilation = context.Compilation; + if (!ValueObjectSymbolInspector.IsEFReferenced(compilation)) + return; + + var notMappedAttribute = compilation.GetTypeByMetadataName( + "System.ComponentModel.DataAnnotations.Schema.NotMappedAttribute" + ); + var keyAttribute = compilation.GetTypeByMetadataName("System.ComponentModel.DataAnnotations.KeyAttribute"); + + var mappedTypes = CollectMappedTypes(context, valueObjectAttribute); + + HashSet reported = new(SymbolEqualityComparer.Default); + foreach (var type in mappedTypes) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + for (var current = type; current is not null; current = current.BaseType) + { + foreach (var property in current.GetMembers().OfType()) + { + if (!reported.Add(property)) + continue; + + if (property.IsStatic || property.IsIndexer) + continue; + + if (notMappedAttribute is not null && HasAttribute(property.GetAttributes(), notMappedAttribute)) + continue; + + if (IsKeyProperty(property, type, keyAttribute)) + continue; + + if (property.Type is not INamedTypeSymbol { TypeKind: TypeKind.Struct } propertyType) + continue; + + if (propertyType.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T) + continue; + + if ( + !ValueObjectSymbolInspector.TryGetScalarIsNullable( + propertyType, + compilation, + out var isNullable + ) || !isNullable + ) + continue; + + var location = property.Locations.FirstOrDefault(static location => location.IsInSource); + if (location is null) + continue; + + context.ReportDiagnostic( + Diagnostic.Create( + DiagnosticLibrary.NullableScalarPropertyShouldBeNullable, + location, + property.ContainingType.Name, + property.Name, + ValueObjectSymbolInspector.ToTypeName(property.Type) + ) + ); + } + } + } + } + + /// + /// True when the property is a key: explicitly with [Key], or by the Id / + /// {EntityTypeName}Id convention Entity Framework Core uses. A key must be non-nullable, so the + /// "declare the property as nullable" advice would be wrong for it. + /// + static bool IsKeyProperty(IPropertySymbol property, INamedTypeSymbol entityType, INamedTypeSymbol? keyAttribute) => + (keyAttribute is not null && HasAttribute(property.GetAttributes(), keyAttribute)) + || string.Equals(property.Name, "Id", StringComparison.Ordinal) + || string.Equals(property.Name, entityType.Name + "Id", StringComparison.Ordinal); + + static bool DerivesFrom(INamedTypeSymbol type, INamedTypeSymbol baseType) + { + for (var current = type.BaseType; current is not null; current = current.BaseType) + { + if (SymbolEqualityComparer.Default.Equals(current, baseType)) + return true; + } + + return false; + } + + /// + /// The types whose members Entity Framework Core maps as columns in this compilation: entities + /// discovered from DbSet<T> properties and IEntityTypeConfiguration<T> + /// implementations, plus [ValueObject] types mapped as complex types. A JSON-mapped complex + /// type is a single column, so its members' nullability does not affect the schema; complex-type + /// mapping needs Entity Framework Core 8 and later. The two scans are combined so the compilation's + /// source types are walked once. + /// + static HashSet CollectMappedTypes( + CompilationAnalysisContext context, + INamedTypeSymbol? valueObjectAttribute + ) + { + var compilation = context.Compilation; + var cancellationToken = context.CancellationToken; + HashSet mappedTypes = new(SymbolEqualityComparer.Default); + + var dbContext = compilation.GetTypeByMetadataName("Microsoft.EntityFrameworkCore.DbContext"); + var dbSet = compilation.GetTypeByMetadataName("Microsoft.EntityFrameworkCore.DbSet`1"); + var entityConfiguration = compilation.GetTypeByMetadataName( + "Microsoft.EntityFrameworkCore.IEntityTypeConfiguration`1" + ); + var complexMappingEnabled = + valueObjectAttribute is not null && ValueObjectSymbolInspector.IsEF8Referenced(compilation); + + foreach ( + var type in compilation + .GetSymbolsWithName(static _ => true, SymbolFilter.Type, cancellationToken) + .OfType() + ) + { + cancellationToken.ThrowIfCancellationRequested(); + + // Entity discovered from the context's DbSet properties. + if (dbContext is not null && dbSet is not null && DerivesFrom(type, dbContext)) + { + foreach (var property in type.GetMembers().OfType()) + { + if ( + property.Type is INamedTypeSymbol { IsGenericType: true } propertyType + && SymbolEqualityComparer.Default.Equals(propertyType.OriginalDefinition, dbSet) + && propertyType.TypeArguments.Length == 1 + && propertyType.TypeArguments[0] is INamedTypeSymbol entityType + ) + mappedTypes.Add(entityType); + } + } + + // Entity discovered from an IEntityTypeConfiguration implementation. + if (entityConfiguration is not null) + { + foreach (var @interface in type.AllInterfaces) + { + if ( + @interface is INamedTypeSymbol { IsGenericType: true } configured + && SymbolEqualityComparer.Default.Equals(configured.OriginalDefinition, entityConfiguration) + && configured.TypeArguments.Length == 1 + && configured.TypeArguments[0] is INamedTypeSymbol entityType + ) + mappedTypes.Add(entityType); + } + } + + if (!complexMappingEnabled) + continue; + + // A complex-type-mapped [ValueObject]: its members become columns. + var attributes = type.GetAttributes(); + if (!HasAttribute(attributes, valueObjectAttribute!)) + continue; + + var options = ValueObjectDefaultsHelper.Apply( + ValueObjectAttributeData.FromAttributeData(attributes), + ValueObjectDefaultsHelper.ResolveAssemblyDefaults(type.ContainingAssembly.GetAttributes()), + attributes + ); + + if (options.EFMapping is not null && ValueObjectSymbolInspector.IsEFMappingNone(options.EFMapping)) + continue; + + if (options.EFMapping is not null && ValueObjectSymbolInspector.IsEFMappingJson(options.EFMapping)) + continue; + + mappedTypes.Add(type); + } + + return mappedTypes; + } + /// /// Reports [StringNormalize] on a member of a type that is neither a [Scalar] nor a /// [ValueObject]. Those two shapes are validated by the model builders, which report the same @@ -462,4 +669,10 @@ static bool HasAttribute(INamedTypeSymbol typeSymbol, INamedTypeSymbol attribute // definition resolved from metadata is the open generic (ScalarAttribute). || SymbolEqualityComparer.Default.Equals(attribute.AttributeClass?.OriginalDefinition, attributeType) ); + + static bool HasAttribute(ImmutableArray attributes, INamedTypeSymbol attributeType) => + attributes.Any(attribute => + SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, attributeType) + || SymbolEqualityComparer.Default.Equals(attribute.AttributeClass?.OriginalDefinition, attributeType) + ); } diff --git a/src/src/SourceGenerator/Common/DiagnosticLibrary.cs b/src/src/SourceGenerator/Common/DiagnosticLibrary.cs index 69386ee..c808b44 100644 --- a/src/src/SourceGenerator/Common/DiagnosticLibrary.cs +++ b/src/src/SourceGenerator/Common/DiagnosticLibrary.cs @@ -242,4 +242,45 @@ public static class DiagnosticLibrary defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true ); + + /// VO1028: Nullability is declared by both the attribute and a declared property + public static readonly DiagnosticDescriptor ScalarNullabilityDeclaredTwice = new( + id: "VO1028", + title: "Scalar nullability is declared twice", + messageFormat: "Value object '{0}' uses [Scalar(Nullable = true)], which declares property '{1}', but '{1}' is also declared explicitly; remove the '{1}' declaration, or remove 'Nullable = true'", + category: ValueObjectCategory, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + /// VO1029: Nullable = false contradicts a nullable declared property + public static readonly DiagnosticDescriptor ScalarNullabilityContradictsProperty = new( + id: "VO1029", + title: "Scalar nullability contradicts the declared property", + messageFormat: "Value object '{0}' sets 'Nullable = false' on [Scalar] but declares nullable property '{1}'; remove the '{1}' declaration, or use [Scalar] (without a type argument) to own the property yourself", + category: ValueObjectCategory, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + /// VO1030: A declared property does not match the automatic form's scalar type + public static readonly DiagnosticDescriptor ScalarPropertyTypeMismatch = new( + id: "VO1030", + title: "Declared scalar property has the wrong type", + messageFormat: "Value object '{0}' declares property '{1}' of type '{2}', which does not match the [Scalar] underlying type '{3}'; remove the declaration, or use [Scalar] (without a type argument) to own the property yourself", + category: ValueObjectCategory, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true + ); + + /// VO1031: A nullable scalar is used as a non-nullable mapped property + public static readonly DiagnosticDescriptor NullableScalarPropertyShouldBeNullable = new( + id: "VO1031", + title: "Nullable scalar should be declared as a nullable property", + messageFormat: "Property '{0}.{1}' is declared as non-nullable '{2}', but '{2}' can hold null; declare it as '{2}?' when null is a valid value, or map a non-nullable scalar through a nullable property. Entity Framework Core maps a non-nullable struct property to a required column and cannot persist '{2}.Null()'.", + category: ValueObjectCategory, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + customTags: [WellKnownDiagnosticTags.CompilationEnd] + ); } diff --git a/src/src/SourceGenerator/Generators/ValueObjectSourceGenerator.cs b/src/src/SourceGenerator/Generators/ValueObjectSourceGenerator.cs index 2f5faed..a985078 100644 --- a/src/src/SourceGenerator/Generators/ValueObjectSourceGenerator.cs +++ b/src/src/SourceGenerator/Generators/ValueObjectSourceGenerator.cs @@ -280,7 +280,8 @@ IncrementalValueProvider generationContext EFProviderTypeName: null, EFHydrateCastTypeName: null, HasEFMembers: true, - GenerateEFValueGenerator: model.EFValueGeneratorEnabled + GenerateEFValueGenerator: model.EFValueGeneratorEnabled, + IsNullable: model.IsNullable ); } diff --git a/src/src/SourceGenerator/ValueObject/Models/EFValueObjectDescriptors.cs b/src/src/SourceGenerator/ValueObject/Models/EFValueObjectDescriptors.cs index bb231ce..3e09d3b 100644 --- a/src/src/SourceGenerator/ValueObject/Models/EFValueObjectDescriptors.cs +++ b/src/src/SourceGenerator/ValueObject/Models/EFValueObjectDescriptors.cs @@ -25,7 +25,10 @@ readonly record struct EFScalarDescriptor( // True when the scalar's underlying property is a nullable reference type. The provider type is the // non-nullable CLR type (a nullable annotation is not representable in typeof), so the inline // conversion reads the property with the null-forgiving operator. - bool ProviderTypeIsNullableReference = false + bool ProviderTypeIsNullableReference = false, + // True when the scalar itself can represent null (`Nullable = true` or a `T?` underlying value). + // The registry uses it to make a mapped property optional where Entity Framework Core permits it. + bool IsNullable = false ); /// diff --git a/src/src/SourceGenerator/ValueObject/Models/ScalarValueObjectModel.cs b/src/src/SourceGenerator/ValueObject/Models/ScalarValueObjectModel.cs index 11239d1..424b74d 100644 --- a/src/src/SourceGenerator/ValueObject/Models/ScalarValueObjectModel.cs +++ b/src/src/SourceGenerator/ValueObject/Models/ScalarValueObjectModel.cs @@ -11,7 +11,7 @@ sealed record class ScalarValueObjectModel( bool GenerateScalarProperty, bool ScalarCanBeNull, bool ScalarTypeIsNullableReference, - bool ScalarValueIsNullable, + bool IsNullable, bool ScalarIsReferenceType, bool IsReferenceType, bool IsRecord, @@ -55,6 +55,7 @@ sealed record class ScalarValueObjectModel( bool ImplementsSelfEquatable, bool EmptyExists, bool NullFactoryExists, + bool HasValueExists, string EmptyValueExpression, bool HasPrimitiveToValueObjectConversion, bool HasValueObjectToPrimitiveConversion, diff --git a/src/src/SourceGenerator/ValueObject/ReferencedEFValueObjectDiscovery.cs b/src/src/SourceGenerator/ValueObject/ReferencedEFValueObjectDiscovery.cs index 26aeae2..6375496 100644 --- a/src/src/SourceGenerator/ValueObject/ReferencedEFValueObjectDiscovery.cs +++ b/src/src/SourceGenerator/ValueObject/ReferencedEFValueObjectDiscovery.cs @@ -118,7 +118,8 @@ CancellationToken cancellationToken // The declaring assembly owns the generator; the consumer only references it, so the // option is read from that assembly's attribute and the emitted member is verified. GenerateEFValueGenerator: HasEFMember(type, TypeLibrary.EFValueGeneratorFactoryMemberName) - && IsValueGenerationEnabled(type, scalarMarker.TypeArguments[1]) + && IsValueGenerationEnabled(type, scalarMarker.TypeArguments[1]), + IsNullable: ValueObjectSymbolInspector.IsNullableScalarType(scalarMarker.TypeArguments[1]) ) ); return; @@ -183,7 +184,8 @@ CancellationToken cancellationToken && options.GenerateEFConverter && ValueObjectSymbolInspector.IsGuidProviderType(scalarProperty.Type), ProviderTypeIsNullableReference: scalarProperty.Type.IsReferenceType - && scalarProperty.Type.NullableAnnotation == NullableAnnotation.Annotated + && scalarProperty.Type.NullableAnnotation == NullableAnnotation.Annotated, + IsNullable: ValueObjectSymbolInspector.IsNullableScalarType(scalarProperty.Type) ) ); return; diff --git a/src/src/SourceGenerator/ValueObject/ScalarAttributeParser.cs b/src/src/SourceGenerator/ValueObject/ScalarAttributeParser.cs index 3200cfa..0a1209e 100644 --- a/src/src/SourceGenerator/ValueObject/ScalarAttributeParser.cs +++ b/src/src/SourceGenerator/ValueObject/ScalarAttributeParser.cs @@ -48,6 +48,26 @@ public static bool IsGeneric(AttributeData attribute) => /// Whether the automatic form declares the property as a nullable reference type. public static bool GetNullable(AttributeData attribute) => attribute.GetNamedArgument("Nullable", false); + /// + /// Whether Nullable was set explicitly on the attribute, and its value. The automatic form + /// derives its nullability from this option; an explicit Nullable = false is what makes a + /// declared nullable property a contradiction (VO1029) rather than a duplicate (VO1022). + /// + public static bool TryGetNullable(AttributeData attribute, out bool nullable) + { + foreach (var argument in attribute.NamedArguments) + { + if (string.Equals(argument.Key, "Nullable", StringComparison.Ordinal)) + { + nullable = argument.Value.Value is true; + return true; + } + } + + nullable = false; + return false; + } + /// /// Reads the shared option set from either attribute form into the same model. /// diff --git a/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Equality.cs b/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Equality.cs index a9ed901..9d1f54f 100644 --- a/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Equality.cs +++ b/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Equality.cs @@ -61,7 +61,7 @@ static void EmitEquality(CodeWriter writer, ScalarValueObjectModel model) // (string? or int?) still hashes null as 0 at runtime, so the annotation is suppressed // rather than branched. ExpressionBody = - $"global::System.Collections.Generic.EqualityComparer<{model.ScalarTypeName}>.Default.GetHashCode({model.ScalarPropertyName}{(model.ScalarValueIsNullable ? "!" : string.Empty)})", + $"global::System.Collections.Generic.EqualityComparer<{model.ScalarTypeName}>.Default.GetHashCode({model.ScalarPropertyName}{(model.IsNullable ? "!" : string.Empty)})", } ); } @@ -70,6 +70,11 @@ static void EmitEquality(CodeWriter writer, ScalarValueObjectModel model) static void EmitOperators(CodeWriter writer, ScalarValueObjectModel model) { var valueObjectType = ValueObjectType(model); + // A reference scalar's comparison parameter is annotated nullable so `value == null` does not + // warn under nullable reference types (CS8625) and so null is a legal operand; the parameter is + // annotation-only, so the CLR signature is unchanged. A nullable value type already names its + // own `T?` parameter. + var primitiveOperand = ScalarComparableParameter(model, writer); if (!model.SameTypeEqualityOperatorExists) { @@ -98,7 +103,7 @@ static void EmitOperators(CodeWriter writer, ScalarValueObjectModel model) ValueObjectEmitterHelpers.EmitBinaryOperator( writer, valueObjectType, - model.ScalarTypeReference, + primitiveOperand, "==", model.IsReferenceType ? "left is null ? false : left.Equals(right)" : "left.Equals(right)" ); @@ -109,7 +114,7 @@ static void EmitOperators(CodeWriter writer, ScalarValueObjectModel model) ValueObjectEmitterHelpers.EmitBinaryOperator( writer, valueObjectType, - model.ScalarTypeReference, + primitiveOperand, "!=", "!(left == right)" ); @@ -119,7 +124,7 @@ static void EmitOperators(CodeWriter writer, ScalarValueObjectModel model) { ValueObjectEmitterHelpers.EmitBinaryOperator( writer, - model.ScalarTypeReference, + primitiveOperand, valueObjectType, "==", model.IsReferenceType ? "right is not null && right.Equals(left)" : "right.Equals(left)" @@ -130,7 +135,7 @@ static void EmitOperators(CodeWriter writer, ScalarValueObjectModel model) { ValueObjectEmitterHelpers.EmitBinaryOperator( writer, - model.ScalarTypeReference, + primitiveOperand, valueObjectType, "!=", "!(left == right)" @@ -153,7 +158,14 @@ static void EmitConversions(CodeWriter writer, ScalarValueObjectModel model) ) { Accessibility = TypeDeclarationAccessibility.Public, - Kind = OperatorDeclarationKind.ImplicitConversion, + // A nullable scalar's extraction is explicit: `value == null` must resolve to the + // operators against the underlying value instead of being ambiguous with the + // record's (T, T) equality, and it must not run Create. A non-nullable scalar keeps + // the ergonomic implicit conversion, where comparing to null is not a valid + // operation in the first place. + Kind = model.IsNullable + ? OperatorDeclarationKind.ExplicitConversion + : OperatorDeclarationKind.ImplicitConversion, ExpressionBody = $"valueObject.{model.ScalarPropertyName}", } ) @@ -219,7 +231,7 @@ static void EmitToString(CodeWriter writer, ScalarValueObjectModel model) static void EmitFormatToString(CodeWriter writer, ScalarValueObjectModel model) { var value = model.ScalarPropertyName; - var expressionBody = model.ScalarValueIsNullable + var expressionBody = model.IsNullable ? model.ScalarIsReferenceType ? $"{value}?.ToString(format) ?? string.Empty" : $"{value}.HasValue ? {value}.GetValueOrDefault().ToString(format) : string.Empty" @@ -250,7 +262,7 @@ static void EmitFormattedToString(CodeWriter writer, ScalarValueObjectModel mode var formattableValue = $"((global::System.IFormattable){value})"; var formatCall = $"{formattableValue}.ToString(format, formatProvider)"; - var expressionBody = model.ScalarValueIsNullable + var expressionBody = model.IsNullable ? model.ScalarIsReferenceType ? $"{value} is null ? string.Empty : {formatCall}" : $"{value}.HasValue ? {formatCall} : string.Empty" @@ -314,7 +326,7 @@ [new MethodCallArgumentOptions("reader", ParameterModifier.Ref), "options"], // A nullable scalar (`string?`, `int?`) round-trips JSON null; only a non-nullable // reference scalar rejects it. - if (model.ScalarCanBeNull && !model.ScalarValueIsNullable) + if (model.ScalarCanBeNull && !model.IsNullable) { methodBody.IfBlock( "value is null", diff --git a/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Mirroring.cs b/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Mirroring.cs index 2b44624..2f6d22d 100644 --- a/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Mirroring.cs +++ b/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.Mirroring.cs @@ -73,7 +73,7 @@ static void EmitTryFormat(CodeWriter writer, ScalarValueObjectModel model, bool { // A null underlying value writes nothing and reports success, matching the parameterless // ToString override returning the empty string. - if (model.ScalarValueIsNullable) + if (model.IsNullable) { body.IfBlock( $"{model.ScalarPropertyName} is null", diff --git a/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.cs b/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.cs index dbdf920..9bbcec5 100644 --- a/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.cs +++ b/src/src/SourceGenerator/ValueObject/ScalarValueObjectEmitter.cs @@ -27,6 +27,7 @@ static void EmitBody(CodeWriter writer, ScalarValueObjectModel model, bool emitE EmitScalarProperty(writer, model); EmitHookDeclarations(writer, model); EmitScalarInterfaceValue(writer, model); + EmitHasValue(writer, model); EmitFactories(writer, model); EmitEmpty(writer, model); EmitNullFactory(writer, model); @@ -238,9 +239,7 @@ model.TypeModel.Namespace is null /// cast to the scalar type to select the primitive overload unambiguously. /// internal static string ScalarPatternArgument(ScalarValueObjectModel model) => - model.ScalarValueIsNullable && !model.ScalarIsReferenceType - ? $"({model.ScalarTypeName})primitive" - : "primitive"; + model.IsNullable && !model.ScalarIsReferenceType ? $"({model.ScalarTypeName})primitive" : "primitive"; /// /// The parameter type for the primitive comparison overload. The BCL comparison interfaces annotate the @@ -305,11 +304,36 @@ static void EmitEmpty(CodeWriter writer, ScalarValueObjectModel model) ); } + /// + /// Emits HasValue for a scalar whose underlying value can be null: a nullable scalar + /// (Nullable = true, a T? value type) or any reference-backed scalar, whose + /// Empty/Hydrate(null!) value is null even when the domain contract says non-null. + /// It never validates or normalizes, so it is the cheap way to ask "is there a value?" without + /// constructing one through Create. + /// + static void EmitHasValue(CodeWriter writer, ScalarValueObjectModel model) + { + if ((!model.IsNullable && !model.ScalarIsReferenceType) || model.HasValueExists) + return; + + writer + .XmlSummary( + "Gets whether the underlying value is not null. Never runs validation or normalization;", + "a null value is the unknown value (see Null() and Hydrate(null))." + ) + .Property( + new("HasValue", PurviewTypeLibrary.System.Boolean, TypeDeclarationAccessibility.Public) + { + ExpressionBody = $"{model.ScalarPropertyName} is not null", + } + ); + } + static void EmitNullFactory(CodeWriter writer, ScalarValueObjectModel model) { // Only a nullable scalar has a "null" domain value; a non-nullable value type's default // (DateTimeOffset.MinValue, 0, ...) is a real value, not an unknown one. - if (!model.ScalarValueIsNullable || model.NullFactoryExists) + if (!model.IsNullable || model.NullFactoryExists) return; writer diff --git a/src/src/SourceGenerator/ValueObject/ScalarValueObjectModelBuilder.cs b/src/src/SourceGenerator/ValueObject/ScalarValueObjectModelBuilder.cs index a3909d2..00b9a04 100644 --- a/src/src/SourceGenerator/ValueObject/ScalarValueObjectModelBuilder.cs +++ b/src/src/SourceGenerator/ValueObject/ScalarValueObjectModelBuilder.cs @@ -106,10 +106,9 @@ out var scalarProperty var scalarTypeIsNullableReference = scalarType.IsReferenceType && scalarType.NullableAnnotation == NullableAnnotation.Annotated; // Whether null is a valid domain value: an annotated reference type (`string?`) or a nullable value - // type (`int?`). Distinct from ScalarCanBeNull, which is also true for a non-nullable reference type. - var scalarValueIsNullable = - scalarType.NullableAnnotation == NullableAnnotation.Annotated - || scalarType is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T }; + // type (`int?`). This is the single nullability flag every generated member, the Entity Framework + // mapping and the diagnostics use. + var isNullable = ValueObjectSymbolInspector.IsNullableScalarType(scalarType); var scalarIsReferenceType = scalarType.IsReferenceType; var isReferenceType = typeSymbol.TypeKind == TypeKind.Class; var createExists = ValueObjectSymbolInspector.HasStaticFactory(typeSymbol, "Create", [scalarType]); @@ -273,7 +272,7 @@ formatProviderType is not null generateScalarProperty, scalarCanBeNull, scalarTypeIsNullableReference, - scalarValueIsNullable, + isNullable, scalarIsReferenceType, isReferenceType, typeSymbol.IsRecord, @@ -320,6 +319,7 @@ formatProviderType is not null ValueObjectSymbolInspector.ImplementsSelfEquatable(typeSymbol), ValueObjectSymbolInspector.HasMemberWithName(typeSymbol, "Empty"), ValueObjectSymbolInspector.HasMemberWithName(typeSymbol, "Null"), + ValueObjectSymbolInspector.HasMemberWithName(typeSymbol, "HasValue"), ValueObjectSymbolInspector.GetEmptyValueExpression(scalarType), ValueObjectSymbolInspector.HasConversionOperator(typeSymbol, scalarType, fromPrimitive: true), ValueObjectSymbolInspector.HasConversionOperator(typeSymbol, scalarType, fromPrimitive: false), @@ -512,9 +512,13 @@ List diagnostics /// /// The manual [Scalar] form requires the author to declare the named property (VO1004); /// the automatic [Scalar<T>] form derives the type from the attribute argument and forbids - /// a declared member with that name (VO1022). Nullable = true only affects the automatic - /// form: it annotates a reference type or wraps a non-nullable value type in , - /// so [Scalar<DateTimeOffset>(Nullable = true)] emits the same property as the manual form + /// a declared member with that name. A declared Value is classified specifically when it also + /// asserts nullability — VO1028 for Nullable = true, VO1029 for an explicit + /// Nullable = false against a nullable declaration, and VO1030 for a mismatched type — + /// and is VO1022 otherwise. Every case is blocking, so no partial is emitted for a type that + /// would produce CS0102. Nullable = true only affects the automatic form: it annotates a + /// reference type or wraps a non-nullable value type in , so + /// [Scalar<DateTimeOffset>(Nullable = true)] emits the same property as the manual form /// declaring a DateTimeOffset? member. /// static bool TryResolveScalarTarget( @@ -537,11 +541,8 @@ out IPropertySymbol? scalarProperty if (ScalarAttributeParser.GetValueType(scalarAttribute) is { } declaredType) { - scalarType = ApplyNullableOption( - declaredType, - ScalarAttributeParser.GetNullable(scalarAttribute), - compilation - ); + var nullableOptionSpecified = ScalarAttributeParser.TryGetNullable(scalarAttribute, out var nullableOption); + scalarType = ValueObjectSymbolInspector.ApplyNullableOption(declaredType, nullableOption, compilation); generateScalarProperty = true; // The generator owns the named property, so a declared member with that name (or with the @@ -555,11 +556,71 @@ out IPropertySymbol? scalarProperty if (conflictingName is not null) { + var conflictLocation = GetDeclaredMemberLocation(typeSymbol, conflictingName) ?? location; + + // A declared `Value` on the automatic form is a second source of nullability, so it is + // classified specifically: the attribute's explicit option, a contradictory nullable + // declaration, or a type that does not match the underlying type. Everything else stays + // the generic duplicate-declaration error (VO1022). + if ( + string.Equals(conflictingName, "Value", StringComparison.Ordinal) + && FindDeclaredProperty(typeSymbol, conflictingName) is { } declaredProperty + ) + { + if (nullableOptionSpecified && nullableOption) + { + diagnostics.Add( + ReportableDiagnostic.Create( + DiagnosticLibrary.ScalarNullabilityDeclaredTwice, + isBlocking: true, + conflictLocation, + typeSymbol.Name, + conflictingName + ) + ); + return false; + } + + if ( + nullableOptionSpecified + && !nullableOption + && ValueObjectSymbolInspector.IsNullableScalarType(declaredProperty.Type) + ) + { + diagnostics.Add( + ReportableDiagnostic.Create( + DiagnosticLibrary.ScalarNullabilityContradictsProperty, + isBlocking: true, + conflictLocation, + typeSymbol.Name, + conflictingName + ) + ); + return false; + } + + if (!SymbolEqualityComparer.Default.Equals(scalarType, declaredProperty.Type)) + { + diagnostics.Add( + ReportableDiagnostic.Create( + DiagnosticLibrary.ScalarPropertyTypeMismatch, + isBlocking: true, + conflictLocation, + typeSymbol.Name, + conflictingName, + ValueObjectSymbolInspector.ToTypeName(declaredProperty.Type), + ValueObjectSymbolInspector.ToTypeName(scalarType) + ) + ); + return false; + } + } + diagnostics.Add( ReportableDiagnostic.Create( DiagnosticLibrary.ScalarPropertyIsGenerated, isBlocking: true, - GetDeclaredMemberLocation(typeSymbol, conflictingName) ?? location, + conflictLocation, typeSymbol.Name, conflictingName ) @@ -594,34 +655,6 @@ out IPropertySymbol? scalarProperty return true; } - /// - /// Applies Nullable = true to the automatic form's declared value type. A reference type gains - /// the nullable annotation; a non-nullable value type is wrapped in , which is - /// how nullability is represented for a value type. An already-nullable value type is left alone so - /// [Scalar<int?>(Nullable = true)] does not double-wrap, and a ref-like struct is left alone - /// because cannot be constructed over a ref struct. - /// - static ITypeSymbol ApplyNullableOption(ITypeSymbol declaredType, bool nullable, Compilation compilation) - { - if (!nullable) - return declaredType; - - if (declaredType.IsReferenceType) - return declaredType.WithNullableAnnotation(NullableAnnotation.Annotated); - - if ( - !declaredType.IsValueType - || declaredType is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } - || declaredType is INamedTypeSymbol { IsRefLikeType: true } - ) - { - return declaredType; - } - - // The type is a non-nullable value type, so wrap it in Nullable to represent nullability. - return compilation.GetSpecialType(SpecialType.System_Nullable_T).Construct(declaredType); - } - /// /// True when the type already declares, in user-authored source, a property or field with the given /// name. Used by the automatic form, where the generator owns the named property: a declared member @@ -637,6 +670,19 @@ static bool HasDeclaredMember(INamedTypeSymbol typeSymbol, string name) => member.DeclaringSyntaxReferences.Any(static reference => !IsGeneratedSyntaxTree(reference.SyntaxTree)) ); + /// + /// The user-authored property with the given name, or when the member is a + /// field, lives in generated code, or does not exist. Used to classify a declared Value on the + /// automatic form (VO1028-VO1030). + /// + static IPropertySymbol? FindDeclaredProperty(INamedTypeSymbol typeSymbol, string name) => + typeSymbol + .GetMembers(name) + .OfType() + .FirstOrDefault(static property => + property.DeclaringSyntaxReferences.Any(static reference => !IsGeneratedSyntaxTree(reference.SyntaxTree)) + ); + static Location? GetDeclaredMemberLocation(INamedTypeSymbol typeSymbol, string name) => typeSymbol .GetMembers(name) diff --git a/src/src/SourceGenerator/ValueObject/ValueObjectEFRegistryEmitter.cs b/src/src/SourceGenerator/ValueObject/ValueObjectEFRegistryEmitter.cs index b79ef2e..24d3655 100644 --- a/src/src/SourceGenerator/ValueObject/ValueObjectEFRegistryEmitter.cs +++ b/src/src/SourceGenerator/ValueObject/ValueObjectEFRegistryEmitter.cs @@ -63,8 +63,8 @@ bool isEF8Referenced EmitUseValueObjectKeyGenerators(body); EmitInlineConverterFields(body, scalarDefinition); EmitInlineConverterFields(body, inlineJsonDefinitions); - EmitHelperClass(body, "ScalarMapping", hasProviderMappable: true); - EmitHelperClass(body, "JsonMapping", hasProviderMappable: false); + EmitHelperClass(body, "ScalarMapping", hasProviderMappable: true, hasIsNullable: true); + EmitHelperClass(body, "JsonMapping", hasProviderMappable: false, hasIsNullable: false); EmitInlineConverters(body, scalarDefinition, isEF8Referenced); EmitInlineConverters(body, inlineJsonDefinitions, isEF8Referenced); EmitInlineKeyValueGenerators(body, keyValueGenerators, isEF8Referenced); @@ -213,7 +213,7 @@ bool isEF8Referenced var scalarEntries = string.Join( ", ", scalars.Select(descriptor => - $"[typeof({descriptor.TypeName})] = new ScalarMapping({ConverterExpression(descriptor, inlineScalarConverters)}, {ComparerExpression(descriptor)}, {Bool(descriptor.ProviderMappable)})" + $"[typeof({descriptor.TypeName})] = new ScalarMapping({ConverterExpression(descriptor, inlineScalarConverters)}, {ComparerExpression(descriptor)}, {Bool(descriptor.ProviderMappable)}, {Bool(descriptor.IsNullable)})" ) ); var jsonEntries = string.Join( @@ -352,15 +352,35 @@ static void EmitScalarConversionLoop(CodeWriter writer) entityType.Block( "foreach (var property in entityType.ClrType.GetProperties().Where(p => IsMappableValueObjectProperty(entityType, p)))", property => + { + // A nullable value object property is Nullable; match the underlying type so the + // same T converter maps both T and T? properties. + property.Comment( + "A T? property arrives as Nullable; match the underlying type so the same", + "converter maps both T and T? properties." + ); + property.Assignment( + "var", + "propertyType", + "global::System.Nullable.GetUnderlyingType(property.PropertyType) ?? property.PropertyType" + ); property.IfElse( - "scalarMappings.TryGetValue(property.PropertyType, out var scalar) && scalar.Converter is not null && scalar.ProviderMappable", - scalar => ApplyConverterAndComparer(writer, "scalar", "property", "entityType.ClrType!"), + "scalarMappings.TryGetValue(propertyType, out var scalar) && scalar.Converter is not null && scalar.ProviderMappable", + scalar => + ApplyConverterAndComparer( + writer, + "scalar", + "property", + "entityType.ClrType!", + nullabilityExpression: "scalar.IsNullable" + ), elseBody => elseBody.IfBlock( - "jsonMappings.TryGetValue(property.PropertyType, out var json) && json.Converter is not null", + "jsonMappings.TryGetValue(propertyType, out var json) && json.Converter is not null", json => ApplyConverterAndComparer(writer, "json", "property", "entityType.ClrType!") ) - ) + ); + } ) ); } @@ -422,39 +442,74 @@ static void EmitComplexMappingBlock(CodeWriter writer) /// inherited property is declared by a base type that is usually not an entity type at all, and asking the /// model builder for it would add a new entity type while the registry is walking the model. /// + /// + /// The expression telling whether the mapping's scalar can represent null (for example + /// scalar.IsNullable), or when the mapping carries no such information. + /// static void ApplyConverterAndComparer( CodeWriter writer, string mappingName, string propertyName, - string entityTypeExpression + string entityTypeExpression, + string? nullabilityExpression = null ) { // The builder API is used so the property is configured regardless of convention discovery order. + var builderName = $"{mappingName}Builder"; var builderExpression = $"modelBuilder.Entity({entityTypeExpression}).Property({propertyName}.PropertyType, {propertyName}.Name)"; + writer.Assignment("var", builderName, builderExpression); writer.IfElse( $"{mappingName}.Comparer is not null", withComparer => withComparer.MethodCall( "HasConversion", [$"{mappingName}.Converter", $"{mappingName}.Comparer"], - receiver: builderExpression + receiver: builderName ), withoutComparer => - withoutComparer.MethodCall("HasConversion", [$"{mappingName}.Converter"], receiver: builderExpression) + withoutComparer.MethodCall("HasConversion", [$"{mappingName}.Converter"], receiver: builderName) ); + + if (nullabilityExpression is not null) + { + writer.IfBlock( + EmittedNullabilityCondition(nullabilityExpression, propertyName, builderName), + body => body.MethodCall("IsRequired", ["false"], receiver: builderName) + ); + } } + /// + /// The emitted guard for making a mapped property optional. A nullable scalar says the column must be + /// nullable, but Entity Framework Core only accepts IsRequired(false) for a nullable CLR type: + /// a non-nullable struct property throws CannotBeNullable. The guard therefore skips those + /// (the scalar's Null() cannot be persisted through a non-nullable property either way), never + /// marks a key nullable, and leaves an explicit or data-annotated nullability configuration in place. + /// + static string EmittedNullabilityCondition(string nullabilityExpression, string propertyName, string builderName) => + $"({nullabilityExpression} || global::System.Nullable.GetUnderlyingType({propertyName}.PropertyType) is not null)" + + $" && (!{propertyName}.PropertyType.IsValueType || global::System.Nullable.GetUnderlyingType({propertyName}.PropertyType) is not null)" + + $" && !{builderName}.Metadata.IsKey()" + + $" && ((global::Microsoft.EntityFrameworkCore.Metadata.IConventionProperty){builderName}.Metadata)" + + ".GetIsNullableConfigurationSource() is null or global::Microsoft.EntityFrameworkCore.Metadata.ConfigurationSource.Convention"; + static void EmitComplexMember(CodeWriter writer) { // Every complex type member is declared explicitly so it maps to a column; nested scalar value // objects additionally receive a value conversion. var memberBuilder = "modelBuilder.Entity(entityType.ClrType!).ComplexProperty(complexProperty.ComplexType.ClrType, complexProperty.Name).Property(member.PropertyType, member.Name)"; + writer.Assignment( + "var", + "memberType", + "global::System.Nullable.GetUnderlyingType(member.PropertyType) ?? member.PropertyType" + ); writer.Assignment("var", "memberBuilder", memberBuilder); writer.IfElse( - "scalarMappings.TryGetValue(member.PropertyType, out var scalar) && scalar.Converter is not null && scalar.ProviderMappable", + "scalarMappings.TryGetValue(memberType, out var scalar) && scalar.Converter is not null && scalar.ProviderMappable", scalar => + { scalar.IfElse( "scalar.Comparer is not null", withComparer => @@ -465,10 +520,15 @@ static void EmitComplexMember(CodeWriter writer) ), withoutComparer => withoutComparer.MethodCall("HasConversion", ["scalar.Converter"], receiver: "memberBuilder") - ), + ); + scalar.IfBlock( + EmittedNullabilityCondition("scalar.IsNullable", "member", "memberBuilder"), + body => body.MethodCall("IsRequired", ["false"], receiver: "memberBuilder") + ); + }, elseBody => elseBody.IfBlock( - "jsonMappings.TryGetValue(member.PropertyType, out var json) && json.Converter is not null", + "jsonMappings.TryGetValue(memberType, out var json) && json.Converter is not null", json => json.IfElse( "json.Comparer is not null", @@ -489,7 +549,7 @@ static void EmitComplexMember(CodeWriter writer) ); } - static void EmitHelperClass(CodeWriter writer, string className, bool hasProviderMappable) + static void EmitHelperClass(CodeWriter writer, string className, bool hasProviderMappable, bool hasIsNullable) { var valueConverterType = TypeLibrary.Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter; var valueComparerType = TypeLibrary.Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer; @@ -508,6 +568,8 @@ static void EmitHelperClass(CodeWriter writer, string className, bool hasProvide parameterList.Add(new("comparer", valueComparerType.MakeNullable(writer))); if (hasProviderMappable) parameterList.Add(new("providerMappable", PurviewTypeLibrary.System.Boolean)); + if (hasIsNullable) + parameterList.Add(new("isNullable", PurviewTypeLibrary.System.Boolean)); body.Constructor( new ConstructorDeclarationOptions(className, TypeDeclarationAccessibility.Public) @@ -520,6 +582,8 @@ static void EmitHelperClass(CodeWriter writer, string className, bool hasProvide ctor.Assignment("Comparer", "comparer"); if (hasProviderMappable) ctor.Assignment("ProviderMappable", "providerMappable"); + if (hasIsNullable) + ctor.Assignment("IsNullable", "isNullable"); } ) .Property( @@ -556,6 +620,20 @@ static void EmitHelperClass(CodeWriter writer, string className, bool hasProvide } ); } + + if (hasIsNullable) + { + body.Property( + new PropertyDeclarationOptions( + "IsNullable", + PurviewTypeLibrary.System.Boolean, + TypeDeclarationAccessibility.Public + ) + { + HasGetter = true, + } + ); + } } ); } diff --git a/src/src/SourceGenerator/ValueObject/ValueObjectSymbolInspector.cs b/src/src/SourceGenerator/ValueObject/ValueObjectSymbolInspector.cs index af464dc..1e6ed8e 100644 --- a/src/src/SourceGenerator/ValueObject/ValueObjectSymbolInspector.cs +++ b/src/src/SourceGenerator/ValueObject/ValueObjectSymbolInspector.cs @@ -240,7 +240,10 @@ public static bool HasConversionOperator(INamedTypeSymbol typeSymbol, ITypeSymbo .OfType() .Any(method => method.MethodKind == MethodKind.Conversion - && method.Name == "op_Implicit" + // Either direction counts: declaring an `explicit` conversion where the generator would + // otherwise emit an `implicit` one (or the reverse) is still the same source/target pair, + // so emitting another would be CS0557. + && method.Name is "op_Implicit" or "op_Explicit" && ( fromPrimitive ? method.Parameters.Length == 1 @@ -625,6 +628,79 @@ public static ITypeSymbol UnwrapNullable(ITypeSymbol type) => ? nullable.TypeArguments[0] : type; + /// + /// True when represents the absence of a value: a nullable reference type + /// (string?) or a nullable value type (Nullable<T>). This is the single + /// nullability definition every generated member, the Entity Framework mapping and the diagnostics + /// use. A non-nullable reference type is not "nullable" here even though it can hold null at runtime: + /// the domain contract says it must not. + /// + public static bool IsNullableScalarType(ITypeSymbol type) => + type.NullableAnnotation == NullableAnnotation.Annotated + || type is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T }; + + /// + /// Applies Nullable = true to the automatic form's declared value type. A reference type gains + /// the nullable annotation; a non-nullable value type is wrapped in , which is + /// how nullability is represented for a value type. An already-nullable value type is left alone so + /// [Scalar<int?>(Nullable = true)] does not double-wrap, and a ref-like struct is left alone + /// because cannot be constructed over a ref struct. + /// + public static ITypeSymbol ApplyNullableOption(ITypeSymbol declaredType, bool nullable, Compilation compilation) + { + if (!nullable) + return declaredType; + + if (declaredType.IsReferenceType) + return declaredType.WithNullableAnnotation(NullableAnnotation.Annotated); + + if ( + !declaredType.IsValueType + || declaredType is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } + || declaredType is INamedTypeSymbol { IsRefLikeType: true } + ) + { + return declaredType; + } + + // The type is a non-nullable value type, so wrap it in Nullable to represent nullability. + return compilation.GetSpecialType(SpecialType.System_Nullable_T).Construct(declaredType); + } + + /// + /// Resolves whether a scalar value object can represent null, from whichever form declares it: + /// the automatic form's Nullable option (or an already-nullable type argument), or the + /// manual form's declared scalar property. Returns false for a type that is not a [Scalar] + /// or whose declaration is incomplete; the model builder reports those separately. + /// + public static bool TryGetScalarIsNullable(INamedTypeSymbol scalarType, Compilation compilation, out bool isNullable) + { + isNullable = false; + + var attributes = scalarType.GetAttributes(); + var scalarAttribute = ScalarAttributeParser.Find(attributes); + if (scalarAttribute is null) + return false; + + if (ScalarAttributeParser.GetValueType(scalarAttribute) is { } declaredType) + { + isNullable = IsNullableScalarType( + ApplyNullableOption(declaredType, ScalarAttributeParser.GetNullable(scalarAttribute), compilation) + ); + return true; + } + + var scalarProperty = scalarType + .GetMembers(ScalarAttributeParser.Parse(scalarAttribute).PropertyName) + .OfType() + .FirstOrDefault(property => !property.IsStatic && property.GetMethod is not null); + if (scalarProperty is null) + return false; + + isNullable = IsNullableScalarType(scalarProperty.Type); + return true; + } + /// /// True when , or a base type, declares an instance ToString overload whose /// parameters match exactly. The base chain is walked so an enum's diff --git a/src/src/ValueObjects/Sdk/README.md b/src/src/ValueObjects/Sdk/README.md index 11ee88b..ae68742 100644 --- a/src/src/ValueObjects/Sdk/README.md +++ b/src/src/ValueObjects/Sdk/README.md @@ -8,7 +8,7 @@ incremental source generator produces: - `Create` / `Hydrate` / `TryCreate` factories with `OnNormalize` normalization (or built-in string trim/casing) and `OnValidate` validation - `Empty` instances, equality, comparison, `CompareTo`, `ToString` (including the underlying value's `IFormattable` - and format overloads), and implicit conversions + and format overloads), and implicit conversions (with explicit extraction for nullable scalars) - The underlying value's standard interfaces — `IEquatable`, `ISpanFormattable`, `IUtf8SpanFormattable`, `IParsable`, `ISpanParsable`, and `IUtf8SpanParsable` — mirrored per type - JSON converters (scalar value objects serialize as their underlying value) diff --git a/src/src/ValueObjects/Serialization/ScalarAttribute.cs b/src/src/ValueObjects/Serialization/ScalarAttribute.cs index f6edf23..f1c7c8a 100644 --- a/src/src/ValueObjects/Serialization/ScalarAttribute.cs +++ b/src/src/ValueObjects/Serialization/ScalarAttribute.cs @@ -9,7 +9,8 @@ namespace Purview.ValueObjects.Serialization; /// /// A scalar value object wraps a single underlying primitive value. When the generator processes a type /// decorated with this attribute, it can emit a JSON converter, support, -/// comparison operators, implicit conversions to and from the primitive, and an Empty instance. +/// comparison operators, an implicit conversion from the primitive, an explicit conversion to it, and an +/// Empty instance. /// /// /// The identifies the member holding the underlying value. diff --git a/src/src/ValueObjects/Serialization/ScalarAttributeOfT.cs b/src/src/ValueObjects/Serialization/ScalarAttributeOfT.cs index 76a1657..426da4a 100644 --- a/src/src/ValueObjects/Serialization/ScalarAttributeOfT.cs +++ b/src/src/ValueObjects/Serialization/ScalarAttributeOfT.cs @@ -22,7 +22,8 @@ namespace Purview.ValueObjects.Serialization; /// [Scalar<DateTimeOffset>(Nullable = true)] produces /// public DateTimeOffset? Value { get; init; }. Nullable value types can also be written directly, /// for example [Scalar<int?>]. A nullable scalar exposes an additional -/// Null() factory, equivalent to Hydrate(null). +/// Null() factory, equivalent to Hydrate(null), and a HasValue property that +/// reports whether the underlying value is null without validating it. /// /// /// Readability. The automatic form is concise, but the underlying type and nullability are @@ -40,7 +41,15 @@ namespace Purview.ValueObjects.Serialization; /// public string? Value { get; init; } /// } /// -/// Both are supported; neither is reported as a diagnostic. +/// Each form is supported on its own. Combining them is reported as an error: VO1022 for a +/// member with the generator-owned name, and VO1028/VO1029/VO1030 when the +/// declaration also asserts nullability or a mismatched type. +/// +/// +/// Comparisons. A nullable scalar can be compared with null and with the underlying value +/// in either order. Its conversion to the underlying value is explicit ((string)value or +/// value.Value), which keeps value == null unambiguous against the synthesized +/// (TSelf, TSelf) equality. A non-nullable scalar keeps the implicit conversion. /// /// [AttributeUsage(AttributeTargets.Struct | AttributeTargets.Class)] diff --git a/src/src/ValueObjects/Serialization/ScalarOptionsAttribute.cs b/src/src/ValueObjects/Serialization/ScalarOptionsAttribute.cs index afb83eb..2c84a2d 100644 --- a/src/src/ValueObjects/Serialization/ScalarOptionsAttribute.cs +++ b/src/src/ValueObjects/Serialization/ScalarOptionsAttribute.cs @@ -74,9 +74,16 @@ public abstract class ScalarOptionsAttribute(string propertyName) : Attribute public bool GenerateImplicitFromPrimitive { get; init; } = true; /// - /// Gets or sets whether an implicit conversion to the primitive value should be generated. + /// Gets or sets whether a conversion to the primitive value should be generated. /// /// Defaults to . + /// + /// The conversion is implicit for a non-nullable scalar, and explicit for a nullable one. + /// Keeping the nullable scalar's extraction explicit is what makes value == null resolve to + /// the operators against the underlying value instead of being ambiguous with the synthesized + /// (TSelf, TSelf) equality. The conversion from the primitive remains implicit for both and + /// goes through . + /// public bool GenerateImplicitToPrimitive { get; init; } = true; /// diff --git a/src/tests/SourceGenerator.IntegrationTests/Generators/NullableScalarEntityFrameworkTests.cs b/src/tests/SourceGenerator.IntegrationTests/Generators/NullableScalarEntityFrameworkTests.cs new file mode 100644 index 0000000..522b140 --- /dev/null +++ b/src/tests/SourceGenerator.IntegrationTests/Generators/NullableScalarEntityFrameworkTests.cs @@ -0,0 +1,306 @@ +namespace Purview.ValueObjects.SourceGenerator.Generators; + +/// +/// Entity Framework Core registry and diagnostic coverage for nullable scalars: a T? property +/// matches the T converter, a nullable scalar maps to an optional column where Entity Framework +/// Core permits it, and using the scalar on a non-nullable mapped property is reported as VO1031. +/// +public sealed class NullableScalarEntityFrameworkTests : ValueObjectEFSourceGeneratorTestBase +{ + const string RegistrySource = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct RequiredText { } + + public sealed class Widget + { + public int Id { get; set; } + + public NullableText DataJson { get; set; } + + public NullableText? Alias { get; set; } + + public RequiredText Required { get; set; } + } + } + """; + + static string Normalize(string source) => string.Concat(source.Where(static c => !char.IsWhiteSpace(c))); + + [Test] + public async Task EFRegistry_GivenNullableScalar_MapsBothPropertyShapes(CancellationToken cancellationToken) + { + using var result = await GenerateAsync( + RegistrySource, + ValueObjectsEFGeneratorTestOptions.Default.Compile(), + cancellationToken + ); + + var registry = Normalize( + result.Generated().GetClass("ValueObjectEFExtensions", "Microsoft.EntityFrameworkCore").Node.ToString() + ); + + // The Nullable unwrap is what makes a T? property find the T converter. + await Assert + .That(registry) + .Contains("global::System.Nullable.GetUnderlyingType(property.PropertyType)??property.PropertyType"); + await Assert + .That(registry) + .Contains( + "[typeof(global::Testing.NullableText)]=newScalarMapping(global::Testing.NullableText.EF.Converter,global::Testing.NullableText.EF.Comparer,true,true)" + ); + await Assert + .That(registry) + .Contains( + "[typeof(global::Testing.RequiredText)]=newScalarMapping(global::Testing.RequiredText.EF.Converter,global::Testing.RequiredText.EF.Comparer,true,false)" + ); + + // Only EF-accepted optional configuration is emitted: no non-nullable struct, no key, and never + // over an explicit IsRequired. + await Assert.That(registry).Contains(".IsRequired(false)"); + await Assert.That(registry).Contains("!scalarBuilder.Metadata.IsKey()"); + await Assert.That(registry).Contains("GetIsNullableConfigurationSource()"); + await Assert + .That(registry) + .Contains("scalar.IsNullable||global::System.Nullable.GetUnderlyingType(property.PropertyType)isnotnull"); + } + + [Test] + public async Task EFRegistry_GivenComplexTypeMember_UnwrapsAndMarksNullableMembers( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + [Purview.ValueObjects.Serialization.ValueObject] + public readonly partial record struct Basket + { + public NullableText Note { get; } + + public NullableText? OptionalNote { get; } + } + + public sealed class Widget + { + public int Id { get; set; } + + public Basket Basket { get; set; } + } + } + """; + + using var result = await GenerateAsync( + source, + ValueObjectsEFGeneratorTestOptions.Default.Compile(), + cancellationToken + ); + + var registry = Normalize( + result.Generated().GetClass("ValueObjectEFExtensions", "Microsoft.EntityFrameworkCore").Node.ToString() + ); + + await Assert + .That(registry) + .Contains("global::System.Nullable.GetUnderlyingType(member.PropertyType)??member.PropertyType"); + await Assert.That(registry).Contains("scalarMappings.TryGetValue(memberType,outvarscalar)"); + await Assert.That(registry).Contains("memberBuilder.Metadata.IsKey()"); + await Assert.That(registry).Contains("memberBuilder.IsRequired(false)"); + } + + [Test] + public async Task VO1031_GivenEntityPropertyOfNullableScalar_ReportsWarning(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + public sealed class Widget + { + public int Id { get; set; } + + public NullableText DataJson { get; set; } + } + + public sealed class ShopContext : global::Microsoft.EntityFrameworkCore.DbContext + { + public global::Microsoft.EntityFrameworkCore.DbSet Widgets { get; set; } = null!; + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } + + [Test] + public async Task VO1031_GivenEntityConfiguredByEntityTypeConfiguration_ReportsWarning( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct OptionalTimestamp { } + + public sealed class Widget + { + public int Id { get; set; } + + public OptionalTimestamp ObservedAt { get; set; } + } + + public sealed class WidgetConfiguration + : global::Microsoft.EntityFrameworkCore.IEntityTypeConfiguration + { + public void Configure( + global::Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder builder + ) + { + } + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } + + [Test] + public async Task VO1031_GivenPlainDtoProperty_DoesNotReport(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + public sealed class WidgetDto + { + public NullableText DataJson { get; set; } + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } + + [Test] + public async Task VO1031_GivenNullablePropertyOrNonNullableScalar_DoesNotReport(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct RequiredText { } + + public sealed class Widget + { + public int Id { get; set; } + + public NullableText? Alias { get; set; } + + public RequiredText Required { get; set; } + } + + public sealed class ShopContext : global::Microsoft.EntityFrameworkCore.DbContext + { + public global::Microsoft.EntityFrameworkCore.DbSet Widgets { get; set; } = null!; + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } + + [Test] + public async Task VO1031_GivenKeyProperty_DoesNotReport(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + public sealed class Widget + { + public NullableText Id { get; set; } + } + + public sealed class ShopContext : global::Microsoft.EntityFrameworkCore.DbContext + { + public global::Microsoft.EntityFrameworkCore.DbSet Widgets { get; set; } = null!; + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } + + [Test] + public async Task VO1031_GivenComplexTypeMember_ReportsWarning(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + [Purview.ValueObjects.Serialization.ValueObject] + public readonly partial record struct Basket + { + public NullableText Note { get; } + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } + + [Test] + public async Task VO1031_GivenJsonMappedComplexTypeMember_DoesNotReport(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + [Purview.ValueObjects.Serialization.ValueObject( + EFMapping = Purview.ValueObjects.Serialization.EntityFrameworkMapping.Json + )] + public readonly partial record struct Basket + { + public NullableText Note { get; } + } + } + """; + + using var result = await GenerateAsync(source, ValueObjectsEFGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } +} diff --git a/src/tests/SourceGenerator.UnitTests/Analyzers/ScalarNullabilityDiagnosticTests.cs b/src/tests/SourceGenerator.UnitTests/Analyzers/ScalarNullabilityDiagnosticTests.cs new file mode 100644 index 0000000..c051037 --- /dev/null +++ b/src/tests/SourceGenerator.UnitTests/Analyzers/ScalarNullabilityDiagnosticTests.cs @@ -0,0 +1,165 @@ +namespace Purview.ValueObjects.SourceGenerator.Analyzers; + +/// +/// Analyzer coverage for the two nullability declarations of an automatic scalar: the attribute's +/// Nullable option and a user-declared Value property. The manual form is the supported +/// way to own the property, so it is never diagnosed. +/// +public sealed class ScalarNullabilityDiagnosticTests : AnalyzerTestBase +{ + [Test] + public async Task Generate_GivenAutomaticNullableWithDeclaredValue_ReportsVO1028( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct Handle + { + public string? Value { get; init; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.ScalarNullabilityDeclaredTwice); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarPropertyIsGenerated); + } + + [Test] + public async Task Generate_GivenTypeofAutomaticNullableWithDeclaredValue_ReportsVO1028( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(typeof(System.DateTimeOffset), Nullable = true)] + public readonly partial record struct ObservedAt + { + public System.DateTimeOffset? Value { get; init; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.ScalarNullabilityDeclaredTwice); + } + + [Test] + public async Task Generate_GivenExplicitNonNullableWithNullableDeclaredValue_ReportsVO1029( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = false)] + public readonly partial record struct ObservedAt + { + public System.DateTimeOffset? Value { get; init; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.ScalarNullabilityContradictsProperty); + } + + [Test] + public async Task Generate_GivenDeclaredValueOfTheWrongType_ReportsVO1030(CancellationToken cancellationToken) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct Handle + { + public int Value { get; init; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.ScalarPropertyTypeMismatch); + } + + [Test] + public async Task Generate_GivenDeclaredValueMatchingTheUnderlyingType_ReportsVO1022( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct InstallationId + { + public System.Guid Value { get; init; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).HasDiagnostic(DiagnosticLibrary.ScalarPropertyIsGenerated); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarNullabilityDeclaredTwice); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarNullabilityContradictsProperty); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarPropertyTypeMismatch); + } + + [Test] + public async Task Generate_GivenManualNullableScalar_ReportsNoNullabilityDiagnostic( + CancellationToken cancellationToken + ) + { + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct Handle + { + public string? Value { get; init; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarNullabilityDeclaredTwice); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarNullabilityContradictsProperty); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarPropertyTypeMismatch); + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.ScalarPropertyIsGenerated); + } + + [Test] + public async Task Generate_GivenNullableScalarPropertyWithoutEntityFramework_DoesNotReportVO1031( + CancellationToken cancellationToken + ) + { + // VO1031 explains an Entity Framework Core mapping consequence, so it only runs where the + // project references Entity Framework Core. This compilation does not. + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + public sealed class Widget + { + public NullableText DataJson { get; set; } + } + } + """; + + var result = await AnalyzeAsync(source, cancellationToken); + + await Assert.That(result).DoesNotHaveDiagnostic(DiagnosticLibrary.NullableScalarPropertyShouldBeNullable); + } +} diff --git a/src/tests/SourceGenerator.UnitTests/Generators/ScalarBoundaryMappingTests.cs b/src/tests/SourceGenerator.UnitTests/Generators/ScalarBoundaryMappingTests.cs new file mode 100644 index 0000000..7c3f202 --- /dev/null +++ b/src/tests/SourceGenerator.UnitTests/Generators/ScalarBoundaryMappingTests.cs @@ -0,0 +1,199 @@ +namespace Purview.ValueObjects.SourceGenerator.Generators; + +/// +/// The documented boundary mapping between a nullable domain scalar and a non-nullable persistence +/// scalar (NonNullableNickname? on the entity, NullableNickname on the DTO). Both the +/// extension-method form and the explicit-operator form must compile and agree. +/// +public sealed class ScalarBoundaryMappingTests : ValueObjectSourceGeneratorTestBase +{ + const string Source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableNickname + { + public static explicit operator NullableNickname(NonNullableNickname? value) => + value is null ? Null() : Hydrate(value.Value.Value); + + public static explicit operator NonNullableNickname?(NullableNickname value) => + value.Value is null ? (NonNullableNickname?)null : NonNullableNickname.Create(value.Value); + } + + [Purview.ValueObjects.Serialization.Scalar(Trim = true, Casing = Purview.ValueObjects.Serialization.StringCasing.LowerInvariant)] + public readonly partial record struct NonNullableNickname { } + + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableTimestamp { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct NonNullableTimestamp { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct NullableQuantity { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct NonNullableQuantity { } + + public sealed class Order + { + public NonNullableNickname? Nickname { get; set; } + + public NonNullableTimestamp? DeliveredAt { get; set; } + + public NonNullableQuantity? Quantity { get; set; } + } + + public sealed class OrderDto + { + public NullableNickname Nickname { get; set; } + + public NullableTimestamp DeliveredAt { get; set; } + + public NullableQuantity Quantity { get; set; } + } + + public static class OrderMappings + { + public static OrderDto ToDto(Order order) => new() + { + Nickname = order.Nickname is null + ? NullableNickname.Null() + : NullableNickname.Hydrate(order.Nickname.Value.Value), + DeliveredAt = order.DeliveredAt is null + ? NullableTimestamp.Null() + : NullableTimestamp.Hydrate(order.DeliveredAt.Value.Value), + Quantity = order.Quantity is null + ? NullableQuantity.Null() + : NullableQuantity.Hydrate(order.Quantity.Value.Value), + }; + + public static Order ToEntity(OrderDto dto) => new() + { + Nickname = dto.Nickname.Value is null + ? (NonNullableNickname?)null + : NonNullableNickname.Create(dto.Nickname.Value), + DeliveredAt = dto.DeliveredAt.Value is null + ? null + : NonNullableTimestamp.Create(dto.DeliveredAt.Value.Value), + Quantity = dto.Quantity.Value is null + ? null + : NonNullableQuantity.Create(dto.Quantity.Value.Value), + }; + } + + public static class Harness + { + public static bool[] RoundTripPreservesValues() + { + var deliveredAt = new System.DateTimeOffset(2026, 3, 4, 5, 6, 7, System.TimeSpan.Zero); + var entity = new Order + { + Nickname = NonNullableNickname.Create(" Mixed "), + DeliveredAt = NonNullableTimestamp.Create(deliveredAt), + Quantity = NonNullableQuantity.Create(3), + }; + + var dto = OrderMappings.ToDto(entity); + var back = OrderMappings.ToEntity(dto); + + return + [ + dto.Nickname.Value == "mixed", + dto.DeliveredAt.Value == deliveredAt, + dto.Quantity.Value == 3, + back.Nickname!.Value.Value == "mixed", + back.DeliveredAt!.Value.Value == deliveredAt, + back.Quantity!.Value.Value == 3, + ]; + } + + public static bool[] NullMapsToNullValueAndBack() + { + var entity = new Order(); + var dto = OrderMappings.ToDto(entity); + var back = OrderMappings.ToEntity(dto); + + return + [ + dto.Nickname.Value is null, + dto.DeliveredAt.Value is null, + dto.Quantity.Value is null, + back.Nickname is null, + back.DeliveredAt is null, + back.Quantity is null, + ]; + } + + public static bool[] CastsAgreeWithTheMappingMethods() + { + var deliveredAt = new System.DateTimeOffset(2026, 3, 4, 5, 6, 7, System.TimeSpan.Zero); + var entity = new Order + { + Nickname = NonNullableNickname.Create(" Mixed "), + DeliveredAt = NonNullableTimestamp.Create(deliveredAt), + Quantity = NonNullableQuantity.Create(7), + }; + + var cast = new NullableNickname[] { (NullableNickname)entity.Nickname }; + var nullCast = new NullableNickname[] { (NullableNickname)new Order().Nickname }; + var back = (NonNullableNickname?)cast[0]; + + return + [ + cast[0].Value == "mixed", + nullCast[0].Value is null, + back!.Value.Value == "mixed", + ((NonNullableNickname?)NullableNickname.Null()) is null, + ]; + } + } + } + """; + + async Task GenerateAssemblyAsync(CancellationToken cancellationToken) + { + using var result = await GenerateAsync( + Source, + ValueObjectsGeneratorTestOptions.Default.Compile(), + cancellationToken + ); + return await Assert.That(result.CompilationResult.Assembly).IsNotNull(); + } + + static async Task AssertResultsAsync(System.Reflection.Assembly assembly, string methodName, bool[] expected) + { + var harness = assembly.GetType("Testing.Harness")!; + var actual = (bool[])harness.GetMethod(methodName)!.Invoke(null, null)!; + + await Assert.That(string.Join(",", actual)).IsEqualTo(string.Join(",", expected)); + } + + [Test] + public async Task BoundaryMapping_GivenValues_RoundTripsAndNormalizesOnTheEntityPath( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync(assembly, "RoundTripPreservesValues", [true, true, true, true, true, true]); + } + + [Test] + public async Task BoundaryMapping_GivenNullValues_MapsToNullAndBack(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync(assembly, "NullMapsToNullValueAndBack", [true, true, true, true, true, true]); + } + + [Test] + public async Task BoundaryMapping_GivenExplicitOperators_AgreesWithTheMappingMethods( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync(assembly, "CastsAgreeWithTheMappingMethods", [true, true, true, true]); + } +} diff --git a/src/tests/SourceGenerator.UnitTests/Generators/ScalarComparisonOperatorTests.cs b/src/tests/SourceGenerator.UnitTests/Generators/ScalarComparisonOperatorTests.cs new file mode 100644 index 0000000..328298d --- /dev/null +++ b/src/tests/SourceGenerator.UnitTests/Generators/ScalarComparisonOperatorTests.cs @@ -0,0 +1,227 @@ +namespace Purview.ValueObjects.SourceGenerator.Generators; + +/// +/// Regression tests for null comparison operators. Before the extraction conversion became explicit, +/// value == null was ambiguous (CS9342) between the operator against the underlying value and the +/// record's synthesized (T, T) equality. The compile itself is the regression test; the runtime +/// assertions pin the results. +/// +public sealed class ScalarComparisonOperatorTests : ValueObjectSourceGeneratorTestBase +{ + const string Source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableString { } + + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct OptionalDateTimeOffset { } + + [Purview.ValueObjects.Serialization.Scalar(Trim = true, Casing = Purview.ValueObjects.Serialization.StringCasing.LowerInvariant)] + public readonly partial record struct NormalizedText { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct NonNullableText { } + + public static class Harness + { + public static bool[] NullComparisons() + { + NullableString DataJson = NullableString.Null(); + return + [ + DataJson == null, + DataJson != null, + null == DataJson, + DataJson == "value", + "value" == DataJson, + ]; + } + + public static bool[] ValueComparisons() + { + NullableString name = NullableString.Create("value"); + return + [ + name == null, + name != null, + name == "value", + "value" == name, + name != "other", + "other" != name, + ]; + } + + public static bool[] NullishValues() + { + return + [ + default(NullableString) == null, + NullableString.Empty == null, + NullableString.Null() == null, + NullableString.Create(null) == null, + null == NullableString.Create(null), + ]; + } + + public static bool[] ValueTypeComparisons() + { + OptionalDateTimeOffset observed = OptionalDateTimeOffset.Null(); + var epoch = System.DateTimeOffset.UnixEpoch; + return + [ + observed == null, + observed != null, + null == observed, + observed == epoch, + epoch == observed, + observed != epoch, + OptionalDateTimeOffset.Create(epoch) == epoch, + epoch == OptionalDateTimeOffset.Create(epoch), + default(OptionalDateTimeOffset) == null, + OptionalDateTimeOffset.Empty == null, + OptionalDateTimeOffset.Create(null) == null, + ]; + } + + public static bool ExtractionIsExplicit() + { + string value = (string)NullableString.Create("value"); + return value == "value"; + } + + public static bool CreationConversionRemainsImplicit() + { + NullableString value = "value"; + return value.Value == "value"; + } + + public static bool ExtractionIsImplicitForNonNullable() + { + string value = NonNullableText.Create("value"); + return value == "value"; + } + + public static bool OperatorsUseTheUnderlyingValueWithoutNormalizing() + { + NormalizedText normalized = NormalizedText.Create(" VALUE "); + return normalized.Value == "value" + && normalized == "value" + && normalized != " VALUE "; + } + + public static bool ExtractionOperatorIsExplicit() + { + var methods = typeof(NullableString).GetMethods(); + return System.Linq.Enumerable.Any( + methods, + method => method.Name == "op_Explicit" + && method.ReturnType == typeof(string) + && method.GetParameters()[0].ParameterType == typeof(NullableString)) + && !System.Linq.Enumerable.Any( + methods, + method => method.Name == "op_Implicit" + && method.ReturnType == typeof(string)); + } + public static bool NonNullableExtractionIsImplicit() + { + var methods = typeof(NonNullableText).GetMethods(); + return System.Linq.Enumerable.Any( + methods, + method => method.Name == "op_Implicit" + && method.ReturnType == typeof(string) + && method.GetParameters()[0].ParameterType == typeof(NonNullableText)) + && !System.Linq.Enumerable.Any( + methods, + method => method.Name == "op_Explicit" + && method.ReturnType == typeof(string)); + } + } + } + """; + + async Task GenerateAssemblyAsync(CancellationToken cancellationToken) + { + using var result = await GenerateAsync( + Source, + ValueObjectsGeneratorTestOptions.Default.Compile(), + cancellationToken + ); + return await Assert.That(result.CompilationResult.Assembly).IsNotNull(); + } + + static async Task AssertResultsAsync(System.Reflection.Assembly assembly, string methodName, bool[] expected) + { + var harness = assembly.GetType("Testing.Harness")!; + var actual = (bool[])harness.GetMethod(methodName)!.Invoke(null, null)!; + + await Assert.That(actual.SequenceEqual(expected)).IsTrue(); + } + + [Test] + public async Task Comparison_GivenNullableReferenceScalar_CompilesAndReturnsExpected( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync(assembly, "NullComparisons", [true, false, true, false, false]); + await AssertResultsAsync(assembly, "ValueComparisons", [false, true, true, true, true, true]); + await AssertResultsAsync(assembly, "NullishValues", [true, true, true, true, true]); + } + + [Test] + public async Task Comparison_GivenNullableValueTypeScalar_CompilesAndReturnsExpected( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync( + assembly, + "ValueTypeComparisons", + [true, false, true, false, false, true, true, true, true, true, true] + ); + } + + [Test] + public async Task Comparison_GivenNullableScalar_UsesExplicitExtractionAndImplicitCreation( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var extraction = (bool)harness.GetMethod("ExtractionIsExplicit")!.Invoke(null, null)!; + var creation = (bool)harness.GetMethod("CreationConversionRemainsImplicit")!.Invoke(null, null)!; + var operatorShape = (bool)harness.GetMethod("ExtractionOperatorIsExplicit")!.Invoke(null, null)!; + + await Assert.That(extraction).IsTrue(); + await Assert.That(creation).IsTrue(); + await Assert.That(operatorShape).IsTrue(); + } + + [Test] + public async Task Comparison_GivenNonNullableScalar_KeepsImplicitExtraction(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var extraction = (bool)harness.GetMethod("ExtractionIsImplicitForNonNullable")!.Invoke(null, null)!; + var operatorShape = (bool)harness.GetMethod("NonNullableExtractionIsImplicit")!.Invoke(null, null)!; + + await Assert.That(extraction).IsTrue(); + await Assert.That(operatorShape).IsTrue(); + } + + [Test] + public async Task Comparison_GivenNormalizingScalar_ComparesWithoutNormalizing(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var raw = (bool)harness.GetMethod("OperatorsUseTheUnderlyingValueWithoutNormalizing")!.Invoke(null, null)!; + + await Assert.That(raw).IsTrue(); + } +} diff --git a/src/tests/SourceGenerator.UnitTests/Generators/ScalarHasValueTests.cs b/src/tests/SourceGenerator.UnitTests/Generators/ScalarHasValueTests.cs new file mode 100644 index 0000000..0754d49 --- /dev/null +++ b/src/tests/SourceGenerator.UnitTests/Generators/ScalarHasValueTests.cs @@ -0,0 +1,128 @@ +namespace Purview.ValueObjects.SourceGenerator.Generators; + +/// +/// Tests the generated HasValue member. It is emitted for a scalar whose underlying value can be +/// null: a nullable scalar, or any reference-backed scalar (whose Empty/Hydrate(null!) +/// value is null even when the domain contract says non-null). It never validates or normalizes. +/// +public sealed class ScalarHasValueTests : ValueObjectSourceGeneratorTestBase +{ + const string Source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct NullableText { } + + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct OptionalTimestamp { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct RequiredText { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct RequiredId { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct ManualNullable + { + public string? Value { get; init; } + } + + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct DeclaredHasValue + { + public bool HasValue => true; + } + + public static class Harness + { + public static bool[] NullableShapes() + { + return + [ + NullableText.Null().HasValue == false, + NullableText.Create("value").HasValue, + default(NullableText).HasValue == false, + NullableText.Empty.HasValue == false, + OptionalTimestamp.Null().HasValue == false, + OptionalTimestamp.Create(System.DateTimeOffset.UnixEpoch).HasValue, + ManualNullable.Null().HasValue == false, + ManualNullable.Create("value").HasValue, + ]; + } + + public static bool[] ReferenceBackedShapes() + { + return + [ + RequiredText.Empty.HasValue == false, + RequiredText.Create("value").HasValue, + RequiredText.Hydrate(null!).HasValue == false, + ]; + } + + public static bool DeclaredHasValueIsNotDuplicated() => + typeof(DeclaredHasValue) + .GetProperties() + .Count(property => property.Name == nameof(DeclaredHasValue.HasValue)) == 1 + && DeclaredHasValue.Null().HasValue; + } + } + """; + + async Task GenerateAssemblyAsync(CancellationToken cancellationToken) + { + using var result = await GenerateAsync( + Source, + ValueObjectsGeneratorTestOptions.Default.Compile(), + cancellationToken + ); + return await Assert.That(result.CompilationResult.Assembly).IsNotNull(); + } + + static async Task AssertResultsAsync(System.Reflection.Assembly assembly, string methodName, bool[] expected) + { + var harness = assembly.GetType("Testing.Harness")!; + var actual = (bool[])harness.GetMethod(methodName)!.Invoke(null, null)!; + + await Assert.That(string.Join(",", actual)).IsEqualTo(string.Join(",", expected)); + } + + [Test] + public async Task HasValue_GivenNullableScalars_ReportsWhetherTheValueIsNull(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync(assembly, "NullableShapes", [true, true, true, true, true, true, true, true]); + } + + [Test] + public async Task HasValue_GivenReferenceBackedScalar_ReportsWhetherTheValueIsNull( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + await AssertResultsAsync(assembly, "ReferenceBackedShapes", [true, true, true]); + } + + [Test] + public async Task HasValue_GivenNonNullableValueTypeScalar_IsNotGenerated(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + var requiredId = assembly.GetType("Testing.RequiredId")!; + await Assert.That(requiredId.GetProperty("HasValue")).IsNull(); + } + + [Test] + public async Task HasValue_GivenAuthorDeclaredMember_IsNotDuplicated(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var notDuplicated = (bool)harness.GetMethod("DeclaredHasValueIsNotDuplicated")!.Invoke(null, null)!; + + await Assert.That(notDuplicated).IsTrue(); + } +} diff --git a/src/tests/SourceGenerator.UnitTests/Generators/ScalarNullabilityTests.cs b/src/tests/SourceGenerator.UnitTests/Generators/ScalarNullabilityTests.cs new file mode 100644 index 0000000..d709389 --- /dev/null +++ b/src/tests/SourceGenerator.UnitTests/Generators/ScalarNullabilityTests.cs @@ -0,0 +1,239 @@ +using System.Reflection; + +namespace Purview.ValueObjects.SourceGenerator.Generators; + +/// +/// Tests the single nullability flag every scalar derives. The automatic form takes it from +/// Nullable = true (or an already-nullable type argument); the manual form takes it from the +/// declared property. Both feed Null(), the JSON contract and the Entity Framework mapping. +/// +public sealed class ScalarNullabilityTests : ValueObjectSourceGeneratorTestBase +{ + const string Source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct AutoNullable { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct AutoRequired { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct ManualNullable + { + public string? Value { get; init; } + } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct ManualRequired + { + public string Value { get; init; } + } + + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct AutoNullableValueType { } + + [Purview.ValueObjects.Serialization.Scalar] + public readonly partial record struct ManualNullableValueType + { + public System.DateTimeOffset? Value { get; init; } + } + + public static class Harness + { + public static bool AutoNullableIsNull() => AutoNullable.Null().Value is null; + + public static bool ManualNullableIsNull() => ManualNullable.Null().Value is null; + + public static bool AutoNullableValueTypeIsNull() => + AutoNullableValueType.Null().Value is null; + + public static bool ManualNullableValueTypeIsNull() => + ManualNullableValueType.Null().Value is null; + + public static string? DeserializeAutoNullable(string json) => + System.Text.Json.JsonSerializer.Deserialize(json)!.Value; + + public static string? DeserializeManualNullable(string json) => + System.Text.Json.JsonSerializer.Deserialize(json)!.Value; + + public static System.DateTimeOffset? DeserializeAutoNullableValueType(string json) => + System.Text.Json.JsonSerializer.Deserialize(json)!.Value; + + public static System.DateTimeOffset? DeserializeManualNullableValueType(string json) => + System.Text.Json.JsonSerializer.Deserialize(json)!.Value; + + public static bool AutoRequiredRejectsNull() + { + try + { + System.Text.Json.JsonSerializer.Deserialize("null"); + return false; + } + catch (System.Text.Json.JsonException) + { + return true; + } + } + + public static bool ManualRequiredRejectsNull() + { + try + { + System.Text.Json.JsonSerializer.Deserialize("null"); + return false; + } + catch (System.Text.Json.JsonException) + { + return true; + } + } + } + } + """; + + async Task GenerateAssemblyAsync(CancellationToken cancellationToken) + { + using var result = await GenerateAsync( + Source, + ValueObjectsGeneratorTestOptions.Default.Compile(), + cancellationToken + ); + return await Assert.That(result.CompilationResult.Assembly).IsNotNull(); + } + + [Test] + public async Task Nullability_GivenAutomaticNullable_InfersNullable(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var isNull = (bool)harness.GetMethod("AutoNullableIsNull")!.Invoke(null, null)!; + var deserialized = harness.GetMethod("DeserializeAutoNullable")!.Invoke(null, ["null"]); + + await Assert.That(isNull).IsTrue(); + await Assert.That(deserialized).IsNull(); + } + + [Test] + public async Task Nullability_GivenManualNullable_InfersNullable(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var isNull = (bool)harness.GetMethod("ManualNullableIsNull")!.Invoke(null, null)!; + var deserialized = harness.GetMethod("DeserializeManualNullable")!.Invoke(null, ["null"]); + + await Assert.That(isNull).IsTrue(); + await Assert.That(deserialized).IsNull(); + } + + [Test] + public async Task Nullability_GivenAutomaticNullableValueType_InfersNullable(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var isNull = (bool)harness.GetMethod("AutoNullableValueTypeIsNull")!.Invoke(null, null)!; + var deserialized = harness.GetMethod("DeserializeAutoNullableValueType")!.Invoke(null, ["null"]); + + await Assert.That(isNull).IsTrue(); + await Assert.That(deserialized).IsNull(); + } + + [Test] + public async Task Nullability_GivenManualNullableValueType_InfersNullable(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var isNull = (bool)harness.GetMethod("ManualNullableValueTypeIsNull")!.Invoke(null, null)!; + var deserialized = harness.GetMethod("DeserializeManualNullableValueType")!.Invoke(null, ["null"]); + + await Assert.That(isNull).IsTrue(); + await Assert.That(deserialized).IsNull(); + } + + [Test] + public async Task Nullability_GivenNonNullableReference_RejectsNull(CancellationToken cancellationToken) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + var harness = assembly.GetType("Testing.Harness")!; + + var automatic = (bool)harness.GetMethod("AutoRequiredRejectsNull")!.Invoke(null, null)!; + var manual = (bool)harness.GetMethod("ManualRequiredRejectsNull")!.Invoke(null, null)!; + + await Assert.That(automatic).IsTrue(); + await Assert.That(manual).IsTrue(); + } + + [Test] + public async Task Nullability_GivenNonNullableScalar_DoesNotEmitNullFactory(CancellationToken cancellationToken) + { + using var result = await GenerateAsync(Source, ValueObjectsGeneratorTestOptions.Default, cancellationToken); + + await Assert.That(result.Generated().GetRecord("AutoRequired", "Testing").HasMethod("Null")).IsFalse(); + await Assert.That(result.Generated().GetRecord("ManualRequired", "Testing").HasMethod("Null")).IsFalse(); + } + + [Test] + public async Task Nullability_GivenDuplicateDeclaration_DoesNotEmitTheType(CancellationToken cancellationToken) + { + // The analyzer reports VO1028, but the generator itself must also refuse to emit: a partial with + // a second Value property would be CS0102 in code the consumer cannot edit. This run disables + // the analyzer, so only the generator's blocking decision is observable. + const string source = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct Handle + { + public string? Value { get; init; } + } + } + """; + + using var result = await GenerateAsync( + source, + ValueObjectsGeneratorTestOptions.Default.Compile() with + { + AnalyzerTypes = [], + }, + cancellationToken + ); + var assembly = await Assert.That(result.CompilationResult.Assembly).IsNotNull(); + + var handle = assembly.GetType("Testing.Handle")!; + await Assert.That(handle.GetMethod("Create", BindingFlags.Public | BindingFlags.Static)).IsNull(); + + var duplicateErrors = result + .CompilationResult.Compilation.GetDiagnostics(cancellationToken) + .Where(diagnostic => + diagnostic.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error + && diagnostic.Id is "CS0102" or "CS0111" + ) + .ToArray(); + await Assert.That(duplicateErrors).IsEmpty(); + } + + [Test] + public async Task Nullability_GivenNullableScalar_DeclaresNullableUnderlyingProperty( + CancellationToken cancellationToken + ) + { + var assembly = await GenerateAssemblyAsync(cancellationToken); + + var automatic = assembly.GetType("Testing.AutoNullable")!.GetProperty("Value")!; + var manual = assembly.GetType("Testing.ManualNullable")!.GetProperty("Value")!; + var automaticValueType = assembly.GetType("Testing.AutoNullableValueType")!.GetProperty("Value")!; + var manualValueType = assembly.GetType("Testing.ManualNullableValueType")!.GetProperty("Value")!; + + NullabilityInfoContext context = new(); + ; + + await Assert.That(context.Create(automatic).ReadState).IsEqualTo(NullabilityState.Nullable); + await Assert.That(context.Create(manual).ReadState).IsEqualTo(NullabilityState.Nullable); + await Assert.That(automaticValueType.PropertyType).IsEqualTo(typeof(DateTimeOffset?)); + await Assert.That(manualValueType.PropertyType).IsEqualTo(typeof(DateTimeOffset?)); + } +} diff --git a/src/tests/SourceGenerator.UnitTests/Refactorings/CodeFixTestHarness.cs b/src/tests/SourceGenerator.UnitTests/Refactorings/CodeFixTestHarness.cs index 91896f7..0b2c927 100644 --- a/src/tests/SourceGenerator.UnitTests/Refactorings/CodeFixTestHarness.cs +++ b/src/tests/SourceGenerator.UnitTests/Refactorings/CodeFixTestHarness.cs @@ -22,6 +22,19 @@ public static class CodeFixTestHarness public static Task ApplyAsync( string source, CancellationToken cancellationToken + ) + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() => + ApplyAsync(source, actionIndex: 0, cancellationToken); + + /// + /// Applies the code fix at (in registration order), so a provider that + /// offers several repairs can be exercised one at a time. + /// + public static Task ApplyAsync( + string source, + int actionIndex, + CancellationToken cancellationToken ) where TAnalyzer : DiagnosticAnalyzer, new() where TCodeFix : CodeFixProvider, new() => @@ -31,6 +44,7 @@ CancellationToken cancellationToken new TAnalyzer(), new TCodeFix(), fixAll: false, + actionIndex, cancellationToken ); @@ -46,6 +60,7 @@ CancellationToken cancellationToken new TAnalyzer(), new TCodeFix(), fixAll: true, + actionIndex: 0, cancellationToken ); @@ -59,7 +74,7 @@ public static Task ApplyAsync( DiagnosticAnalyzer analyzer, CodeFixProvider provider, CancellationToken cancellationToken - ) => RunAsync(source, generator, analyzer, provider, fixAll: false, cancellationToken); + ) => RunAsync(source, generator, analyzer, provider, fixAll: false, actionIndex: 0, cancellationToken); /// Applies every registered code fix using components supplied by the caller. public static Task ApplyFixAllAsync( @@ -68,7 +83,7 @@ public static Task ApplyFixAllAsync( DiagnosticAnalyzer analyzer, CodeFixProvider provider, CancellationToken cancellationToken - ) => RunAsync(source, generator, analyzer, provider, fixAll: true, cancellationToken); + ) => RunAsync(source, generator, analyzer, provider, fixAll: true, actionIndex: 0, cancellationToken); static async Task RunAsync( string source, @@ -76,6 +91,7 @@ static async Task RunAsync( DiagnosticAnalyzer analyzer, CodeFixProvider provider, bool fixAll, + int actionIndex, CancellationToken cancellationToken ) { @@ -103,7 +119,8 @@ CancellationToken cancellationToken if (!fixAll) { - var fixedDocument = await ApplySingleActionAsync(document, codeActions[0], cancellationToken); + var selected = codeActions[Math.Min(actionIndex, codeActions.Count - 1)]; + var fixedDocument = await ApplySingleActionAsync(document, selected, cancellationToken); return new HarnessResult( (await fixedDocument.GetTextAsync(cancellationToken)).ToString(), [.. applicable.Select(static diagnostic => diagnostic.Id)] diff --git a/src/tests/SourceGenerator.UnitTests/Refactorings/PackagedAnalyzerComponents.cs b/src/tests/SourceGenerator.UnitTests/Refactorings/PackagedAnalyzerComponents.cs index 0e07a9a..2938fab 100644 --- a/src/tests/SourceGenerator.UnitTests/Refactorings/PackagedAnalyzerComponents.cs +++ b/src/tests/SourceGenerator.UnitTests/Refactorings/PackagedAnalyzerComponents.cs @@ -35,6 +35,9 @@ public sealed class PackagedAnalyzerComponents : IDisposable const string CodeFixTypeName = "Purview.ValueObjects.SourceGenerator.Refactorings.AddPartialModifierCodeFixProvider"; + const string ScalarNullabilityCodeFixTypeName = + "Purview.ValueObjects.SourceGenerator.Refactorings.ScalarNullabilityCodeFixProvider"; + const string MergedArtifactDirectoryName = "purview-merged"; readonly PackagedAnalyzerLoadContext _loadContext; @@ -43,13 +46,15 @@ public sealed class PackagedAnalyzerComponents : IDisposable PackagedAnalyzerLoadContext loadContext, IIncrementalGenerator generator, DiagnosticAnalyzer analyzer, - CodeFixProvider codeFix + CodeFixProvider codeFix, + CodeFixProvider scalarNullabilityCodeFix ) { _loadContext = loadContext; Generator = generator; Analyzer = analyzer; CodeFix = codeFix; + ScalarNullabilityCodeFix = scalarNullabilityCodeFix; } /// Gets the value-object source generator loaded from the packaged analyzer artifact. @@ -61,6 +66,9 @@ CodeFixProvider codeFix /// Gets the code-fix provider loaded from the packaged code-fix component. public CodeFixProvider CodeFix { get; } + /// Gets the scalar-nullability code-fix provider loaded from the packaged component. + public CodeFixProvider ScalarNullabilityCodeFix { get; } + /// Loads the packaged analyzer and code-fix components for the current build configuration. public static PackagedAnalyzerComponents Load() { @@ -77,7 +85,8 @@ public static PackagedAnalyzerComponents Load() loadContext, (IIncrementalGenerator)CreateComponent(analyzerAssembly, GeneratorTypeName), (DiagnosticAnalyzer)CreateComponent(analyzerAssembly, AnalyzerTypeName), - (CodeFixProvider)CreateComponent(codeFixAssembly, CodeFixTypeName) + (CodeFixProvider)CreateComponent(codeFixAssembly, CodeFixTypeName), + (CodeFixProvider)CreateComponent(codeFixAssembly, ScalarNullabilityCodeFixTypeName) ); } catch diff --git a/src/tests/SourceGenerator.UnitTests/Refactorings/ScalarNullabilityCodeFixTests.cs b/src/tests/SourceGenerator.UnitTests/Refactorings/ScalarNullabilityCodeFixTests.cs new file mode 100644 index 0000000..6236711 --- /dev/null +++ b/src/tests/SourceGenerator.UnitTests/Refactorings/ScalarNullabilityCodeFixTests.cs @@ -0,0 +1,111 @@ +using Purview.ValueObjects.SourceGenerator.Analyzers; + +namespace Purview.ValueObjects.SourceGenerator.Refactorings; + +/// +/// The three repairs for VO1028: nullability is declared by both the automatic attribute and a +/// declared Value property, so either source (or the automatic form itself) has to go. +/// +public sealed class ScalarNullabilityCodeFixTests +{ + const string DuplicateNullability = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(Nullable = true)] + public readonly partial record struct Handle + { + public string? Value { get; init; } + } + } + """; + + const string TypeofDuplicateNullability = """ + namespace Testing + { + [Purview.ValueObjects.Serialization.Scalar(typeof(string), Nullable = true)] + public readonly partial record struct TypeofHandle + { + public string? Value { get; init; } + } + } + """; + + [Test] + public async Task GivenDuplicateNullability_RemovesTheDeclaredProperty(CancellationToken cancellationToken) + { + var result = await CodeFixTestHarness.ApplyAsync< + ValueObjectDiagnosticAnalyzer, + ScalarNullabilityCodeFixProvider + >(DuplicateNullability, cancellationToken); + + await Assert.That(result.FixedCode).DoesNotContain("public string? Value"); + await Assert.That(result.FixedCode).Contains("Scalar(Nullable = true)"); + await Assert.That(result.FixedDiagnosticIds).Contains(DiagnosticLibrary.ScalarNullabilityDeclaredTwice.Id); + } + + [Test] + public async Task GivenDuplicateNullability_ConvertsToTheManualScalarForm(CancellationToken cancellationToken) + { + var result = await CodeFixTestHarness.ApplyAsync< + ValueObjectDiagnosticAnalyzer, + ScalarNullabilityCodeFixProvider + >(DuplicateNullability, actionIndex: 1, cancellationToken); + + await Assert.That(result.FixedCode).Contains("Scalar]"); + await Assert.That(result.FixedCode).DoesNotContain("Scalar"); + await Assert.That(result.FixedCode).Contains("public string? Value"); + } + + [Test] + public async Task GivenDuplicateNullability_RemovesOnlyTheNullableOption(CancellationToken cancellationToken) + { + var result = await CodeFixTestHarness.ApplyAsync< + ValueObjectDiagnosticAnalyzer, + ScalarNullabilityCodeFixProvider + >(DuplicateNullability, actionIndex: 2, cancellationToken); + + await Assert.That(result.FixedCode).Contains("Scalar"); + await Assert.That(result.FixedCode).DoesNotContain("Nullable = true"); + await Assert.That(result.FixedCode).Contains("public string? Value"); + } + + [Test] + public async Task GivenTypeofDuplicateNullability_ConvertsToTheManualScalarForm(CancellationToken cancellationToken) + { + var result = await CodeFixTestHarness.ApplyAsync< + ValueObjectDiagnosticAnalyzer, + ScalarNullabilityCodeFixProvider + >(TypeofDuplicateNullability, actionIndex: 1, cancellationToken); + + await Assert.That(result.FixedCode).Contains("Scalar]"); + await Assert.That(result.FixedCode).DoesNotContain("typeof"); + await Assert.That(result.FixedCode).Contains("public string? Value"); + } + + [Test] + public async Task GivenPackagedAnalyzerShape_FixableDiagnosticIds_ExposeVO1028() + { + using var components = PackagedAnalyzerComponents.Load(); + + await Assert + .That(components.ScalarNullabilityCodeFix.FixableDiagnosticIds) + .Contains(DiagnosticLibrary.ScalarNullabilityDeclaredTwice.Id); + } + + [Test] + public async Task GivenPackagedAnalyzerShape_RemovesTheDeclaredProperty(CancellationToken cancellationToken) + { + using var components = PackagedAnalyzerComponents.Load(); + + var result = await CodeFixTestHarness.ApplyAsync( + DuplicateNullability, + components.Generator, + components.Analyzer, + components.ScalarNullabilityCodeFix, + cancellationToken + ); + + await Assert.That(result.FixedCode).DoesNotContain("public string? Value"); + await Assert.That(result.FixedDiagnosticIds).Contains(DiagnosticLibrary.ScalarNullabilityDeclaredTwice.Id); + } +} diff --git a/src/tests/ValueObjects.EFCompatibility.IntegrationTests/CompatibilityModels.cs b/src/tests/ValueObjects.EFCompatibility.IntegrationTests/CompatibilityModels.cs index 7973a27..0012c2b 100644 --- a/src/tests/ValueObjects.EFCompatibility.IntegrationTests/CompatibilityModels.cs +++ b/src/tests/ValueObjects.EFCompatibility.IntegrationTests/CompatibilityModels.cs @@ -42,6 +42,34 @@ static partial void OnValidate(string value) [Scalar] public readonly partial record struct CompatibilityScore { } +/// A nullable string scalar: null is a valid domain value and it round-trips JSON null. +[Scalar(Nullable = true)] +public readonly partial record struct CompatibilityNickname { } + +/// +/// The non-nullable counterpart of . Mapping an entity property as +/// CompatibilityName? gives a nullable column whose NULL means "absent", while the DTO +/// keeps the nullable scalar and its Null() value. +/// +[Scalar(Trim = true)] +public readonly partial record struct CompatibilityName { } + +/// A nullable timestamp scalar, exercising the nullable value-type provider type. +[Scalar(Nullable = true)] +public readonly partial record struct CompatibilityObservedAt { } + +/// The non-nullable counterpart of . +[Scalar] +public readonly partial record struct CompatibilityRecordedAt { } + +/// A nullable integer scalar. +[Scalar] +public readonly partial record struct CompatibilityQuantity { } + +/// The non-nullable counterpart of . +[Scalar] +public readonly partial record struct CompatibilityCount { } + /// /// A complex value object mapped to a JSON column. This mapping is a value converter, so it works on every /// Entity Framework Core version the package supports. @@ -83,7 +111,40 @@ sealed class CompatibilityEntity public CompatibilityCode Code { get; set; } /// Gets or sets the nullable value-type scalar column. - public CompatibilityScore Score { get; set; } + public CompatibilityScore? Score { get; set; } + + /// Gets or sets the direct nullable-scalar shape: a T? property. + public CompatibilityNickname? Nickname { get; set; } + + /// + /// Gets or sets a nullable scalar on a non-nullable property. Entity Framework Core cannot mark a + /// non-nullable struct property optional, so this stays a required column; VO1031 reports it, + /// which this deliberately exercises. + /// +#pragma warning disable VO1031 + public CompatibilityNickname RequiredNickname { get; set; } +#pragma warning restore VO1031 + + /// Gets or sets the two-scalar shape: a non-nullable scalar behind a nullable property. + public CompatibilityName? PreferredName { get; set; } + + /// Gets or sets a nullable timestamp scalar through a nullable property. + public CompatibilityObservedAt? ObservedAt { get; set; } + + /// Gets or sets a required timestamp scalar. + public CompatibilityRecordedAt RecordedAt { get; set; } + + /// Gets or sets a nullable integer scalar through a nullable property. + public CompatibilityQuantity? OptionalQuantity { get; set; } + + /// Gets or sets a required integer scalar. + public CompatibilityCount Count { get; set; } + + /// + /// Gets or sets a nullable scalar whose optionality is configured explicitly. The generated registry + /// must leave that explicit configuration alone. + /// + public CompatibilityNickname? MandatoryNickname { get; set; } /// Gets or sets the JSON column. public CompatibilityStamp Stamp { get; set; } @@ -133,6 +194,9 @@ protected override void ConfigureConventions(ModelConfigurationBuilder configura protected override void OnModelCreating(ModelBuilder modelBuilder) { + // Configured before the registry, so the explicit requirement is the one that must survive. + modelBuilder.Entity().Property(entity => entity.MandatoryNickname).IsRequired(); + modelBuilder.ConfigureValueObjects(); } } diff --git a/src/tests/ValueObjects.EFCompatibility.IntegrationTests/EntityFrameworkCompatibilityTests.cs b/src/tests/ValueObjects.EFCompatibility.IntegrationTests/EntityFrameworkCompatibilityTests.cs index 738f56d..3025686 100644 --- a/src/tests/ValueObjects.EFCompatibility.IntegrationTests/EntityFrameworkCompatibilityTests.cs +++ b/src/tests/ValueObjects.EFCompatibility.IntegrationTests/EntityFrameworkCompatibilityTests.cs @@ -88,7 +88,142 @@ public async Task NullableScalarValueObject_ConverterHandlesNullAndColumnRoundTr context.ChangeTracker.Clear(); var found = await context.Entities.SingleAsync(row => row.Id == generated); - await Assert.That(found.Score.Value).IsEqualTo(42); + await Assert.That(found.Score.GetValueOrDefault().Value).IsEqualTo(42); + } + + [Test] + public async Task NullableScalar_GivenNullableProperty_IsAnOptionalColumnWithTheGeneratedConverter() + { + await using var connection = await OpenConnectionAsync(); + await using var context = CreateContext(connection); + + var property = context + .Model.FindEntityType(typeof(CompatibilityEntity))! + .FindProperty(nameof(CompatibilityEntity.Nickname))!; + + // T? unwraps to T for the converter lookup; the column follows the nullable CLR type. + await Assert.That(property.IsNullable).IsTrue(); + await Assert + .That(property.GetValueConverter()?.GetType()) + .IsEqualTo(CompatibilityNickname.EF.Converter.GetType()); + await Assert.That(property.GetRelationalTypeMapping().StoreType).IsEqualTo("TEXT"); + } + + [Test] + public async Task NullableScalar_GivenNonNullableProperty_RemainsRequired() + { + await using var connection = await OpenConnectionAsync(); + await using var context = CreateContext(connection); + + var entityType = context.Model.FindEntityType(typeof(CompatibilityEntity))!; + var required = entityType.FindProperty(nameof(CompatibilityEntity.RequiredNickname))!; + var requiredTimestamp = entityType.FindProperty(nameof(CompatibilityEntity.RecordedAt))!; + var requiredCount = entityType.FindProperty(nameof(CompatibilityEntity.Count))!; + + // Entity Framework Core refuses to mark a non-nullable struct property optional, so a nullable + // scalar on the non-nullable shape stays required (VO1031 reports it at compile time). + await Assert.That(required.IsNullable).IsFalse(); + await Assert + .That(required.GetValueConverter()?.GetType()) + .IsEqualTo(CompatibilityNickname.EF.Converter.GetType()); + await Assert.That(requiredTimestamp.IsNullable).IsFalse(); + await Assert.That(requiredCount.IsNullable).IsFalse(); + } + + [Test] + public async Task NullableScalar_GivenTwoScalarShape_MapsNullableColumnsWithTheNonNullableConverter() + { + await using var connection = await OpenConnectionAsync(); + await using var context = CreateContext(connection); + + var entityType = context.Model.FindEntityType(typeof(CompatibilityEntity))!; + var preferredName = entityType.FindProperty(nameof(CompatibilityEntity.PreferredName))!; + var observedAt = entityType.FindProperty(nameof(CompatibilityEntity.ObservedAt))!; + var optionalQuantity = entityType.FindProperty(nameof(CompatibilityEntity.OptionalQuantity))!; + + await Assert.That(preferredName.IsNullable).IsTrue(); + await Assert + .That(preferredName.GetValueConverter()?.GetType()) + .IsEqualTo(CompatibilityName.EF.Converter.GetType()); + await Assert.That(observedAt.IsNullable).IsTrue(); + await Assert + .That(observedAt.GetValueConverter()?.GetType()) + .IsEqualTo(CompatibilityObservedAt.EF.Converter.GetType()); + await Assert.That(optionalQuantity.IsNullable).IsTrue(); + await Assert + .That(optionalQuantity.GetValueConverter()?.GetType()) + .IsEqualTo(CompatibilityQuantity.EF.Converter.GetType()); + } + + [Test] + public async Task NullableScalar_GivenNullableValueTypeScalar_IsAnOptionalColumn() + { + await using var connection = await OpenConnectionAsync(); + await using var context = CreateContext(connection); + + var score = context + .Model.FindEntityType(typeof(CompatibilityEntity))! + .FindProperty(nameof(CompatibilityEntity.Score))!; + + await Assert.That(score.IsNullable).IsTrue(); + await Assert.That(score.GetValueConverter()?.ProviderClrType).IsEqualTo(typeof(int?)); + } + + [Test] + public async Task NullableScalar_GivenExplicitIsRequired_LeavesTheExplicitConfigurationAlone() + { + await using var connection = await OpenConnectionAsync(); + await using var context = CreateContext(connection); + + var configured = context + .Model.FindEntityType(typeof(CompatibilityEntity))! + .FindProperty(nameof(CompatibilityEntity.MandatoryNickname))!; + + // The property is a nullable scalar on a T? CLR type, so the registry would normally mark it + // optional. The explicit IsRequired configured in OnModelCreating must win instead. + await Assert.That(configured.IsNullable).IsFalse(); + } + + [Test] + public async Task NullableScalar_GivenNullableProperty_RoundTripsNullAndNullValueAsDatabaseNull() + { + await using var connection = await OpenConnectionAsync(); + await using var context = CreateContext(connection); + await context.Database.EnsureCreatedAsync(); + + var withoutValue = CreateEntity("NULL"); + withoutValue.Nickname = null; + withoutValue.PreferredName = null; + withoutValue.ObservedAt = null; + withoutValue.OptionalQuantity = null; + + var explicitNull = CreateEntity("NULLVALUE"); + explicitNull.Nickname = CompatibilityNickname.Null(); + explicitNull.PreferredName = CompatibilityName.Create(" Mixed "); + explicitNull.ObservedAt = CompatibilityObservedAt.Null(); + explicitNull.OptionalQuantity = CompatibilityQuantity.Null(); + + context.Entities.AddRange(withoutValue, explicitNull); + await context.SaveChangesAsync(); + var withoutValueId = withoutValue.Id; + var explicitNullId = explicitNull.Id; + context.ChangeTracker.Clear(); + + var nullRow = await context.Entities.SingleAsync(row => row.Id == withoutValueId); + var nullValueRow = await context.Entities.SingleAsync(row => row.Id == explicitNullId); + + await Assert.That(nullRow.Nickname).IsNull(); + await Assert.That(nullRow.PreferredName).IsNull(); + await Assert.That(nullRow.ObservedAt).IsNull(); + await Assert.That(nullRow.OptionalQuantity).IsNull(); + + // A nullable scalar's Null() is a null provider value, so a T? property reads it back as null. + await Assert.That(nullValueRow.Nickname).IsNull(); + await Assert.That(nullValueRow.ObservedAt).IsNull(); + await Assert.That(nullValueRow.OptionalQuantity).IsNull(); + + // The non-nullable scalar normalises through Create and the nullable property preserves absence. + await Assert.That(nullValueRow.PreferredName!.Value.Value).IsEqualTo("Mixed"); } [Test] @@ -239,6 +374,10 @@ static CompatibilityEntity CreateEntity(string code) => { Code = CompatibilityCode.Create(code), Score = CompatibilityScore.Create(0), + RequiredNickname = CompatibilityNickname.Create("required"), + MandatoryNickname = CompatibilityNickname.Create("mandatory"), + RecordedAt = CompatibilityRecordedAt.Create(new DateTimeOffset(2026, 3, 4, 5, 6, 7, TimeSpan.Zero)), + Count = CompatibilityCount.Create(0), Stamp = CreateStamp(), #if !NET8_0 Total = CompatibilityMoney.Create(1250.75m, CompatibilityCode.Create("GBP")),