Feature/generator base - #206
Merged
Merged
Conversation
The generator only targets the System.Text.Json variant, but the name JsonSubTypes.Aot did not say so and could be read as applying to the Newtonsoft JsonSubTypes port as well. Renamed the project, package, generator namespace, the generated-code namespace (JsonSubTypes.Aot.Generated -> JsonSubTypes.Text.Json.Aot.Generated), the test/sample projects, the GeneratedCode marker, the diagnostic category and the golden master. Breaking, deliberate before the 1.0.0 stable release (the package is still rc): consumers must update their using directives (JsonSubTypes.Aot.Generated -> JsonSubTypes.Text.Json.Aot.Generated) and the PackageReference name. Verified: solution build, 22 generator tests, 77 Aot tests (4 new deep-chain and interface-overlap tests), 190 Text.Json tests, 147 Newtonsoft tests, all on net8.0 and net10.0, and the sample build.
…83 false positive
The generated TryGetProperty used a foreach that returns on its first
iteration, which static analysis flags (the loop never iterates more than
once). FirstOrDefault expresses the lookup directly and keeps the
case-insensitive fallback working.
Sonar's S2583 on GetTypeByPropertyPresence ('typesFound.Count > 1' always
false) is a false positive - the ambiguity throw is exercised by
DemoKnownSubTypeWithPropertyTests and DemoKnownSubTypeWithMultiplePropertiesTests -
but the analyzer cannot see it through HashSet growth. The ambiguity check
reverts to the pre-refactor loop form (result + ambiguous flag), which is
behavior-identical and not flagged. Verified by the 147 Newtonsoft tests.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #206 +/- ##
==========================================
+ Coverage 88.91% 93.20% +4.29%
==========================================
Files 19 19
Lines 1515 1560 +45
Branches 292 301 +9
==========================================
+ Hits 1347 1454 +107
+ Misses 101 50 -51
+ Partials 67 56 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ribute parsing The JsonIgnore-condition + JsonPropertyName parsing grew the method to cognitive complexity 22 (limit 15). The per-property attribute reading moves to TryReadPropertyMetadata (with an early true for always-ignored properties) and ReadIgnoreCondition. Emitted code is unchanged, verified by the 22 golden-master driver tests.
…r findings
Sonar's quality gate on the follow-up PR failed two conditions: reliability
rating (S8949) and 64.5% coverage on new code (< 80%).
Generator: CollectBaseProperties was already split (previous commit); the
remaining findings were S8949 (GetSyntax now receives the cancellation
token), S1192 ('Always' is a const) and S3776 on the rewritten attribute
parsing. JsonSubtypes.cs reverts to the Count > 1 form that matches master,
so the S2583/S4158 false positives (Sonar cannot model HashSet growth; the
ambiguity throw is covered by the DemoKnownSubTypeWith* tests) stop counting
as new code on the PR.
Coverage: the golden-master domain gains NullDiscriminatorAnimal (null
discriminator + base registered with a discriminator, exercising
SerializeBasePayload), DiscriminatorLast (AddDiscriminatorFirst=false),
DynamicShape (get-only property and JsonIgnore(WhenWritingNull)) and Fox (an
unregistered subtype for the dynamic-write path). CommittedGeneratedConverterTests
grows from 22 to 54 tests covering the shared skeleton's edge paths (null
serialize/deserialize, non-object throw, base-object write), the dynamic
subtype and custom resolver paths, the nested chains of Payload/Game, and the
conditional-ignore/get-only write rules. The test types are namespaced
(CA1050).
Measured locally from the cobertura reports: generator 86.3%, golden master
(weighted) 65.6%, aggregate 80.4% line coverage.
Verified: solution build, 54 generator tests, 77 Aot tests, 190 Text.Json
tests, 147 Newtonsoft tests, net8.0 and net10.0.
The deep-hierarchy and interface-overlap fixtures were appended in a second namespace block, which the single-namespace-per-file rule flags. They move into the file's existing namespace block instead; the pre-existing global-namespace domain types stay untouched.
…nore-condition branches The WriteBaseObject emitted '&& true' / '&& false' next to the naming-policy check (S1125). The policy block is now emitted only for properties without a custom name. A driver test runs the generator over a base exercising all four JsonIgnoreCondition values plus a get-only property, which covers the ReadIgnoreCondition switch cases and the conditional-write branches.
Adds tests that exercise each converter's SelectType fallbacks (missing or unknown discriminator, which also reach DeserializeBase and the ResolveStaticType null return), the string-form enum discriminators (RUN/WALK, GAME/COM, ElectronicCat), the unregistered-subtype write path in the shared base (TryWriteNestedObject/TryWriteDynamic fall-throughs) and the shared dynamic / custom-resolver paths. A separate unregistered Owl type keeps the unregistered test independent of the dynamic-registration test, which leaves a stale reverse entry in the private _dynamicReverse map.
… tests Covers the branches the golden-master domain cannot reach: JSTAOT001 for unsupported discriminators (double, bool, enum without a matching member), JSTAOT002 for duplicates, JSTAOT003 for value registrations ignored in presence mode, JsonPropertyName on a base property, an explicit JsonIgnore(Always) condition, and the abstract-base / parameterized-base DeserializeBase emissions. Generator line coverage goes to 98.4%.
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.
Description
Checklist
Please make sure the following points are addressed before the PR is reviewed:
[Unreleased]section ofCHANGELOG.mddescribing the change (bug fix, feature, breaking change), with the related issue number when applicable.