fix(compilers/openapi): report forbidden querystring keywords - #462
Open
fuad-daoud wants to merge 2 commits into
Open
fix(compilers/openapi): report forbidden querystring keywords#462fuad-daoud wants to merge 2 commits into
fuad-daoud wants to merge 2 commits into
Conversation
OpenAPI 3.2 forbids style, explode, and allowReserved at a parameter's in: querystring — the location binds the whole query string from the parameter's content, so its serialization is stated by the media type alone. The bundled parser enforces only the style half of that rule, so a document declaring explode or allowReserved there reached the IR with nothing said: the value still lowered as declared (dropping content the document states is an emitter's call, not a compiler's), but silently. Add diag.InvalidLocationKeyword, a new stable code this compiler reports at the keyword's own pointer with error severity — matching what the parser already gives style at the same location — whenever explode or allowReserved is declared at in: querystring. The value keeps lowering exactly as before; only the diagnostic is new. A conformance case pins both keywords together, and the existing param-querystring fixture's already-declared explode now carries the diagnostic it always should have. Both carry an error diagnostic, so both move to the harness's known-invalid list, with reasons matching the entries already there. Closes #408 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJZJ8HaugGZTjzVueafLAT
Review of the previous commit found that error severity for diag.InvalidLocationKeyword was the wrong call: unlike style's finding, which is the parser's own pre-compile refusal, this diagnostic is the compiler keeping a value it already lowered and saying so — the same shape as reserved-header-name and invalid-method-key, both warnings. Switch it to warning, and update its GoDoc to explain that split instead of claiming all three keywords share one severity. With the severity fixed, both conformance fixtures pass TestConformance's ordinary assertNoErrorDiags again, so the errorCarryingCases() exception this added is no longer needed; removed it along with its branch in TestConformance. Both fixtures also drop out of internal/harness/corpus_test.go's knownInvalid list, restoring that file to its pre-existing form, and go run ./cmd/morphic-harness now reports "ok" on both — back in the irverify/determinism/ order-invariance sweep rather than stopped at the first error. Regenerated both conformance goldens; the only change in either is the diagnostics' severity field. Split the ir-design OpenAPI row's querystring clause so it no longer reads as if style shared the new code, and trimmed the #408 rationale out of the test GoDocs that had started repeating it, leaving diag.go's GoDoc as the one place it lives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJZJ8HaugGZTjzVueafLAT
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.
Summary
OpenAPI 3.2 forbids
style,explodeandallowReservedatin: querystring: the location binds the whole query string from the parameter'scontent, so its serialization is stated by the media type and nothing else. The bundled parser enforces onlystyle, and the compiler reported what the parser gave it — soexplodeandallowReservedreached the IR with nothing said (#408). Keeping them is deliberate and stays: the document states them, and dropping declared content is an emitter's call. The gap was the silence.The compiler now reports each of the two under a new stable code,
openapi/invalid-location-keyword, at the keyword's own pointer, while still lowering the value as declared. It is a warning, not the errorstylegets:style's finding is the parser's refusal-class validation, whereas this is the compiler keeping a value it lowered and saying so — the same class asreserved-header-nameandinvalid-method-keybeside it — and--fail-on warninglets a user refuse the document. That choice is what keeps both fixtures inside the harness's full sweep (irverify, determinism, order-invariance), where an error would have stopped the oracle at the first diagnostic.ir-design.md's OpenAPI row records the rule beside thein: querystringclause.The emit site sits in
lowerParameter, whose single caller receives every mount — path-item and operation parameters, the 3.2querymethod, callbacks — with$refentries resolved to their target first, so a component parameter reports at/components/parameters/X/explode.Test plan
querystring-forbidden-keywordswith the issue's two operations: pins the binding still carriesexplode: false/allowReserved: trueand one warning at each keyword's pointer with the exact message. The pre-existingparam-querystring.yaml(from openapi: querystring parameters are stamped with a style the spec forbids #334) already declaredexplodeatquerystringin anticipation of this issue; its golden gains exactly the one new diagnostic.morphic-harnessreportsokon both fixtures; noknownInvalidentries needed.make gatepasses: lint 0 issues, coverage at 100%, fuzz, bench-smoke.Closes #408
🤖 Generated with Claude Code
https://claude.ai/code/session_01TJZJ8HaugGZTjzVueafLAT