Reject schema properties marked both readOnly and writeOnly#72
Merged
Conversation
A property carrying both flags has no well-defined serialization direction. The dictionary converter previously generated code that skipped such properties in *both* directions, silently dropping required values during deserialization. Detect the combination during schema build and emit the OAW004 diagnostic instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request prevents silent data loss in generated dictionary converters by rejecting OpenAPI schema properties that are simultaneously marked readOnly: true and writeOnly: true, reporting the existing unsupported-feature diagnostic (OAW004) instead of generating code that skips the property in both directions.
Changes:
- Add validation in schema transformation to throw
UnsupportedGenerationExceptionwhen a property is bothreadOnlyandwriteOnly. - Add a diagnostics test asserting OAW004 is produced and no client source is generated for such schemas.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/OpenApiWeaver.Tests/ClientGeneratorTests.Diagnostics.cs | Adds a regression test ensuring the invalid RO+WO combination reports OAW004 and blocks client generation. |
| src/OpenApiWeaver/ClientGenerator.Transformer.Schemas.cs | Adds early validation during SchemaDefinition construction to reject properties marked both readOnly and writeOnly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
OpenAPI allows declaring a property as
readOnly: trueandwriteOnly: true, but the combination has no well-defined serialization direction. The dictionary converter generated code that skipped such properties in both directions (IsRequestSerializerOptions(options) || IsResponseSerializerOptions(options)→reader.Skip()), so arequiredproperty in that state silently dropped on every read — data loss without diagnostic.Detect the combination while building each
SchemaDefinitionand throwUnsupportedGenerationException, which surfaces as OAW004. Consumers that hit this either fix the schema (the spec-correct outcome) or get a clear failure instead of silent data loss.Related issue
None.
Type of change
Validation
dotnet build OpenApiWeaver.slnxdotnet test OpenApiWeaver.slnxImpact checklist
README.mdor documentation when user-facing behavior, configuration, or limitations changedAnalyzerReleases.Unshipped.mdwhen diagnostics were introduced or changedNotes for reviewers
AnalyzerReleases.Unshipped.mdis unchanged because no new rule is introduced.{ ReadOnly: true, WriteOnly: true }branch inBuildRequiredDictionaryConverterPropertyExpressionand the skip block — those paths are now unreachable from any valid input, but removing them is a separate clean-up.🤖 Generated with Claude Code