fix: report a diagnostic for metadata shapes System.Text.Json cannot construct [patch] - #6
Merged
Merged
Conversation
…construct [patch] Deserialize<T> caught only JsonException. JsonSerializer reports a shape it cannot construct — an interface or abstract type, or a type with several parameterized constructors and no [JsonConstructor] — as NotSupportedException, and some converter-configuration failures as InvalidOperationException. Neither derives from JsonException, so both escaped to the Roslyn driver, which reported its own generic CS8785 naming neither the file nor the reason, and abandoned every other declared file in the same RegisterSourceOutput invocation. Catch all three and report MetadataParseFailed with the exception message, the way malformed JSON already was. Only the deserialize call is guarded, so a failure to report a diagnostic is not itself swallowed. Fixes #5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ra5um9FcK7GaeADvaxRABt
SonarCloud's quality gate failed the PR at 75% coverage on new code: of the three catches added to Deserialize<T>, only JsonException and NotSupportedException had a test driving them. Add ConflictingNamesMetadata — two properties claiming one JSON name — which System.Text.Json rejects while building the contract, with InvalidOperationException rather than JsonException or NotSupportedException. Confirmed by removing that catch alone: this test, and only this test, then fails on the collision. Line coverage of both shipping assemblies is now 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ra5um9FcK7GaeADvaxRABt
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #5
The problem
MetadataFile.Deserialize<T>wrappedJsonSerializer.Deserialize<T>in atrythat caught onlySystem.Text.Json.JsonException. But malformed input is not the only way deserialization fails:JsonSerializercannot construct — an interface or abstract type, or a type with several parameterized constructors and no[JsonConstructor]— throwsNotSupportedException.InvalidOperationException.Neither derives from
JsonException, so both escapedDeserialize<T>, escapedGeneratorBase<T>.Generate, and escaped theRegisterSourceOutputcallback uncaught. The Roslyn driver then reported its own genericCS8785, which names neither the offending file nor the reason — and, because the throw unwinds the whole callback, every other file declared by the same generator went unread.This contradicted the toolkit's own stated guarantee (
CLAUDE.md): a missing or malformed file always reports.The fix
Deserialize<T>now catchesNotSupportedExceptionandInvalidOperationExceptionalongsideJsonException, reportingMetadataParseFailedwith the exception message exactly as the existing branch did.Only the
JsonSerializer.Deserializecall sits inside the guarded region — the deserialized-to-null report was moved out, so a failure while reporting a diagnostic is not itself swallowed by the new catches.Tests
Four tests added to
GeneratorBaseTests, covering both acceptance criteria on the issue plus each new catch:AShapeTheSerializerCannotConstructIsReportedRatherThanCrashingTheGeneratorTST002error, not a crash (NotSupportedException)AmbiguousConstructorsAreReportedRatherThanCrashingTheGenerator[JsonConstructor](NotSupportedException)AConverterConfigurationFailureIsReportedRatherThanCrashingTheGeneratorInvalidOperationException)OneFileFailingOnAnUnsupportedShapeStillLeavesTheOthersProcessedEach asserts
result.Exception is null, which is what distinguishes a reported diagnostic from a generator that threw.Supporting fixtures in
TestGenerators.cs:IThingsMetadata,AmbiguousMetadata,ConflictingNamesMetadata, their three generators, andResilientPairGenerator— the last emits from whichever file parsed, so the test can tell one file failing apart from the whole invocation being abandoned before the second file was reached.Verification
Each new catch was verified by removing it alone and confirming that the matching test — and only that test — then fails on the expected exception type:
MetadataFile.csrestored, new tests in place: 3 failed, 39 passed, each failure an uncaughtNotSupportedExceptionreaching the driver viaGeneratorBase.Initialize.InvalidOperationExceptioncatch removed alone: 1 failed, 42 passed —The JSON property name ... collides with another property.dotnet build -c Release— 0 warnings, 0 errors.CustomAfterMicrosoftCommonProps=.sonarlint/sonar-local.props, perCLAUDE.md) — 0 warnings, 0 errors.Docs
CLAUDE.md's "A missing or malformed file always reports" decision note now says what "malformed" covers, and the testing section lists the new fixtures.CHANGELOG.md/VERSION.mdare pipeline-written and untouched; both commits carry[patch].🤖 Generated with Claude Code
https://claude.ai/code/session_01Ra5um9FcK7GaeADvaxRABt