OpenAPI examples for non constant boundaries - #68
Merged
Conversation
Temporal, Guid, and Uri validation boundaries can never be C# constant expressions, so GetConstantValue always fails for them and the generated error example loses both its message and its metadata, silently. Plan a syntax-directed reconstruction over a closed, enumerated whitelist of constructors, factories, and well-known statics, with recursion so that a DateTimeOffset can carry a TimeSpan offset that is itself not a constant. The design constraints that shaped it, recorded so they are not rediscovered during implementation: - The generator targets netstandard2.0, where DateOnly and TimeOnly do not exist, so reconstructed values are structural payloads rather than boxed CLR objects. That also centralizes literal and canonical-message rendering, which is what keeps a message and its metadata entry from stating different values. - Evaluation runs real constructors, so expressions that compile but throw must degrade to a diagnostic; only cancellation propagates. - DateTimeKind.Local resolves against the executing process's time zone, which would make generated output depend on the build machine. - static readonly is not a promise that the initializer is the final value, and that is only decidable within a single syntax tree, so field resolution stays in the validator's own file for now. Refs #57 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184eh1G3XxvjRB11cNKyMRT
Three defects found in review of the reconstruction plan: DateTime has no year/month/day overload taking a DateTimeKind, so the table's "each also with a trailing DateTimeKind" named a constructor that does not exist. Enumerate the two Kind overloads that do. The static readonly criterion promised reconstruction for a field in the validator's file, while Technical Details also rejected any field whose declaring type is declared in more than one file. State the second condition in the criterion as well. Target-typed object creation is now explicitly supported rather than sitting ambiguously in the omissions table: new (2026, 7, 26, ...) is how this repository already writes such values, and it resolves to the same constructor symbol. DiagnosticsEqual compares id, severity, and message but not position, so an analysis whose diagnostics moved compares equal and the driver serves a cached result with stale locations. Since the new diagnostics point at a specific argument, equality and hashing must include source path and span, verified by an incremental test that reuses one driver. Also corrected the incremental-generation claim that no stage takes a dependency on the compilation; the analyzer already receives one. The property that matters is that a validator's output stays a function of its own syntax tree. Refs #57 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184eh1G3XxvjRB11cNKyMRT
The factory whitelist accepted only the double overloads, which rejected TimeSpan.FromHours(2) — the plan's own worked example for a nested DateTimeOffset offset. Verified against the installed SDK: .NET 10 exposes FromHours(Int32), FromHours(Int32, Int64...) and FromHours(Double), and the int literal binds to the exact match. Which family binds is not a property of the source text: the same expression resolves to the double overload on netstandard2.0 or net6.0 and to the integral one on net8.0 and later, so both must be accepted or reconstruction would depend on the consumer's target framework. Accept the single-argument overloads of FromDays, FromHours, FromMinutes, FromSeconds, FromMilliseconds and FromMicroseconds in both forms, the last of these being the explicit decision the review asked for. Exclude the multi-argument component overloads, and require matching on the full resolved signature, because their optional parameters make them candidates for a name-based match. Record that the integral overloads cannot be called by the generator at all, since netstandard2.0 lacks them: compute those from ticks in checked arithmetic rather than substituting the double overload, which rounds to milliseconds and has a different range. Refs #57 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184eh1G3XxvjRB11cNKyMRT
Reconstruct the whitelisted temporal, Guid, and Uri validation boundary shapes, including recursively nested values and eligible static readonly fields. Keep generated messages and metadata aligned through a deterministic structural representation. Report unreconstructable values and unsupported multi-file field resolution at the offending argument, and include diagnostic locations in incremental equality. Cover accepted and rejected shapes, incremental movement, published examples, and document the behavior. Closes #57
new Uri(string) implies UriKind.Absolute, but reconstruction evaluated the shape with UriKind.RelativeOrAbsolute. A relative boundary such as new Uri("items/42") was reconstructed into an example even though the same call throws UriFormatException at runtime. Evaluate with UriKind.Absolute so the shape degrades with the diagnostic, matching runtime behavior. The emitted literal keeps RelativeOrAbsolute, which round-trips OriginalString either way.
… arms MetadataValueModel.Value can only hold a folded constant, an attribute constant, a parameter's explicit default, or a ReconstructedMetadataValue, so the DateTime, DateTimeOffset, TimeSpan, Guid, and Uri arms of ValidatorOpenApiEmitter.ToLiteral and the reflection-based TryCreateDateOnlyOrTimeOnlyLiteral were dead code kept green only by hand-built emitter test models. Removing them lands the plan's centralized rendering design: ReconstructedMetadataValue.ToCSharpLiteral is the single renderer for reconstructed boundaries. The identical ToStringLiteral/EscapeChar copies in the emitter and ReconstructedMetadataValue move into one shared CSharpLiterals helper, and the emitter test rows that fed the deleted arms are removed; temporal literal rendering stays covered end-to-end by the generator-driven reconstruction tests.
…dMetadataValue Reconstructed values are never compared by the incremental pipeline: ValidatorOpenApiAnalysis equality covers only hint name, source text, and diagnostics, and example de-duplication keys on GetDeduplicationValue strings. IEquatable<ReconstructedMetadataValue>, Equals(object), and GetHashCode had no production or test caller, so they are removed instead of covered; the members can be re-added non-breakingly if a real consumer appears.
…lizers TryReconstruct returned Unsupported whenever GetConstantValue succeeded. That arm cannot fire at the top level because the analyzer folds constants first, but it also governs the recursion into a static readonly field's initializer, so a field such as private static readonly int Hours = 2 used as new TimeSpan(Hours, 0, 0) degraded with LPRSG0015 while the equivalent const field worked. The reconstructor's value channel is widened from ReconstructedMetadataValue to object so a folding initializer yields its constant, making a static readonly field behave exactly like the same expression written inline, as the plan requires.
Exercise cancellation, nested diagnostic propagation, and invalid DateOnly, TimeOnly, and TimeSpan values. Use exception filters so cancellation retains its propagation contract without uncovered rethrow arms.
Cache framework type symbols across metadata arguments, expose the reconstruction API publicly, and name TimeSpan constructor cases by arity.
Signed-off-by: Kenny Pflug <kenny.pflug@live.de>
Roslyn intercepts a pre-cancelled token before metadata reconstruction, so the removed test did not constrain the reconstructor's exception filters. Record that the cancellation contract is currently a structural guarantee.
7 tasks
Minimum allowed line rate is |
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.
Closes #57