Repository navigation
Rules updates - #14
Merged
Merged
Conversation
…ict deserialization
The Entity Framework registry mapped members the author had excluded, and broke the model outright
for a computed one. ConfigureValueObjects walked every readable property of every entity type and
configured it with modelBuilder.Entity(t).Property(type, name) - explicit configuration, which
outranks [NotMapped] and Ignore(...):
- a [NotMapped] property was pulled back into the model, producing a column and a migration nobody
asked for, against a production schema
- a property excluded with Ignore(...) was likewise re-added
- a computed, setter-less property made the entire DbContext fail to build, and with [NotMapped]
also being overridden there was no way to opt out
The registry now skips a property that is [NotMapped], ignored on the entity type, or computed and
setter-less. A get-only auto-property is still mapped: it has a compiler-generated backing field,
which EF maps, so the check is for that field rather than merely for a missing setter. [NotMapped]
and .Ignore( previously appeared nowhere in the test suite.
Also fixed:
- a string scalar's ordering disagreed with its own equality and varied by machine. Equality and
GetHashCode use EqualityComparer<string>.Default (ordinal) but CompareTo used
Comparer<string>.Default (current culture), so CompareTo == 0 did not imply Equals - breaking
SortedSet, SortedDictionary and List.BinarySearch, which use CompareTo for identity - and sort
order changed with the thread culture. String scalars now compare with StringComparer.Ordinal.
- [Scalar("CustomName")] did not compile. The generated partial always implements
IScalarValueObject<TSelf, TValue>, which declares a member named Value, but none was emitted for
a custom property name, so the build failed with CS0535 inside generated code. A forwarding Value
member is now emitted, keeping the author's chosen name as the primary accessor.
- TryCreate threw instead of returning false for a ZodSharp-validated value object: it caught only
ArgumentException, but a [ZodSchema] Create throws ZodException.
- a strict-mode deserialization failure returned a 500 instead of a 400. The validation exception
escaped JsonSerializer unwrapped; it is now wrapped in JsonException so System.Text.Json attaches
Path and LineNumber and hosts treat it as bad input.
- strict mode could silently fall back to Hydrate, skipping validation entirely. It now throws.
Trimming and Native AOT:
- Purview.ValueObjects is verified clean under both analyzers and marked IsAotCompatible.
- ScalarJsonConverterFactory declares its requirement on its constructor, so a host registering it
in a trimmed or AOT build is warned at the opt-in site. Generated scalars do not need it: they
carry a [JsonConverter] pointing at a reflection-free generated converter.
Also removed [assembly: InternalsVisibleTo] from the runtime assembly - it granted nothing, since
the assembly declares no internal members, and leaked a test-assembly name into shipped metadata.
Docs: a consolidated diagnostics reference for all 17 rules, the retired VO1011/1012/1014/1020 ids
recorded, a dead mkdocs edit_uri branch fixed, and a mojibake copyright corrected.
All nine remaining diagnostics moved into AnalyzerReleases.Shipped.md under Release 1.0.0.
…ported
VO1005 ("scalar constructor is missing") was recorded in AnalyzerReleases.Shipped.md as a shipped
Error and documented in docs/Diagnostics.md as requiring you to add a constructor taking the scalar's
underlying type. It was absent from the analyzer's SupportedDiagnostics and had no reporting site
anywhere: it was declared in the initial commit and never wired up, so it has never fired in any
version.
Implementing it would have been wrong. A missing constructor is not an error condition - it is the
normal case. When a [Scalar] type declares no constructor matching its scalar value,
ScalarValueObjectEmitter.EmitConstructor emits a private one, which is the documented and tested
behaviour. The rule's message told consumers to supply the very thing the generator exists to supply,
so reporting it would have failed every correctly written scalar value object.
The id now sits with VO1011, VO1012, VO1014 and VO1020 as retired and never reused, leaving 16 live
rules rather than 17. The docs call this one out specifically, because unlike the others it was
published in the catalogue as an Error and described a requirement that never existed.
DiagnosticLibrary.ScalarConstructorMissing is removed. It is a public member, but DiagnosticLibrary
lives in the source generator, which ships IL-merged inside the package, so no consumer can reference
it. A comment in its place records why the rule cannot be implemented, so it is not reintroduced.
Also removed the DoesNotHaveDiagnostic(ScalarConstructorMissing) assertion from
ScalarGeneration_GeneratesPrivateConstructorWhenMissing - it was vacuously true, asserting the
absence of a diagnostic nothing could report. The rest of that test is what actually covers this
behaviour: it proves the generated constructor is private and that Create and Hydrate work through
it.
Fixed two warnings introduced by the previous commit, so the build is clean again:
- IDE0270 in ScalarJsonConverterFactory.Read, simplified to `?? throw`.
- CA2000 in EFNotMappedTests.CreateContext, suppressed with justification. The SqliteConnection's
lifetime transfers to SelectiveMappingDbContext, which closes it in its own Dispose override;
disposing it locally would destroy the in-memory database the test has just populated.
…eratorFramework Purview.ValueObjects now targets net11.0 alongside net8.0, net9.0 and net10.0. The runtime was the only core package not building for the newest major, and the blocker was never the code: global.json set allowPrerelease to false, so the .NET 11 RC SDK installed on the machine was never selected and the build failed with NETSDK1045. It is true now, matching zodsharp and results. The Entity Framework compatibility matrix covers the new target. net10.0 and net11.0 share the centrally managed EF Core version: no EF Core release targets net11.0 yet, and a net11.0 project resolves the net10.0 assets. Give net11.0 its own pin when one ships. That project keeps an explicit TargetFrameworks list rather than a curated set, because each target is paired with its own EF reference set and a target arriving from a set would silently build with no EF pin - so the list is deliberately coupled to the matrix below it. The runtime project selects the SDK's 'All' set instead of listing net8.0;net9.0;net10.0;net11.0, so a runtime lifecycle change is an SDK bump rather than an edit here. Moves from Purview.BuildSdk 1.0.2.2 and Purview.SourceGeneratorFramework 1.0.0-prerelease.54 to the first stable releases of both. The mirrored .agents files are no longer tracked: the SDK generates .agents/.gitignore listing the files it mirrors from packages, so a package upgrade no longer shows up as a local modification. Tests: 206 passing, up from 198 - the eight new executions are the EF compatibility suite running on net11.0. Pack validation 2/2 with net11.0 included.
Adding net11.0 to Purview.ValueObjects left a gap in CI that does not show up locally: the shared purview-build/purview-release workflows default to the 10.0.x SDK, which cannot build a net11.0 target (NETSDK1045). It built here only because the .NET 11 RC SDK is installed on the development machine. global.json now pins the same SDK version the other net11.0 repositories pin, so the choice is deterministic rather than "whatever prerelease is newest", and both workflows pass it to the shared jobs. The exact RC version is used rather than 11.0.x because the shared workflows expose no dotnet-quality input, so a floating 11.0.x would not resolve a prerelease. Verified against the pinned SDK: build clean, 206 tests passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.