fix(compilers/openapi): report forbidden querystring keywords

compilers/openapi/conformance_test.go

@@ -211,6 +211,7 @@ func conformanceCases() []conformanceCase {
 		{"param-styles", assertParamStyles, []string{"param-styles"}},
 		{"param-style-matrix", assertParamStyleMatrix, []string{"param-styles"}},
 		{"param-querystring", assertParamQuerystring, nil},
+		{"querystring-forbidden-keywords", assertQuerystringForbiddenKeywords, nil},
 		{"param-xml-residue", assertParamXMLResidue, nil},
 		{"param-ref-inheritance", assertParamRefInheritance, []string{"defaults", "deprecation", "docs-summary-description"}},
 		{"header-content-schema", assertHeaderContentSchema, nil},
@@ -2119,8 +2120,10 @@ func assertQuerystringParam(t *testing.T, doc *ir.Document) {
 // only the *default* is suppressed here, so an explode the document declares
 // survives at a location that takes no style. That case declares neither keyword
 // there, so reverting the early return that used to drop a declared explode
-// reddens this golden and leaves that one green.
-func assertParamQuerystring(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
+// reddens this golden and leaves that one green. It also pins the one
+// diag.InvalidLocationKeyword this fixture raises — see that code's GoDoc for
+// why it is a warning; querystring-forbidden-keywords pairs both keywords.
+func assertParamQuerystring(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
 	report, ok := opByName(doc, "runReport")
 	require.True(t, ok)
 	require.Len(t, report.Bindings.HTTP, 1)
@@ -2151,6 +2154,44 @@ func assertParamQuerystring(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	assert.Empty(t, declared.Style, "no style is invented beside a declared explode either")
 	require.NotNil(t, declared.Explode, "but the explode the document declares is not dropped")
 	assert.False(t, *declared.Explode)
+
+	assert.Equal(t,
+		"parameter field explode is not allowed for in=querystring; lowered as declared",
+		openapitest.DiagMessageAt(t, diags, diag.InvalidLocationKeyword, ir.SeverityWarning,
+			"/paths/~1reports~1raw/get/parameters/0/explode"))
+}
+
+// assertQuerystringForbiddenKeywords pins the construct assertParamQuerystring's
+// rawReport operation cannot see on its own: explode and allowReserved reported
+// at their own pointers rather than only one of the two. See
+// diag.InvalidLocationKeyword's GoDoc for why the parser catches neither and why
+// this compiler's report is a warning. Both values still lower as declared:
+// dropping content the document states is an emitter's call, not a compiler's.
+func assertQuerystringForbiddenKeywords(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
+	explodeOp, ok := opByName(doc, "explodeOnly")
+	require.True(t, ok)
+	require.Len(t, explodeOp.Bindings.HTTP, 1)
+	require.Len(t, explodeOp.Bindings.HTTP[0].ParamBindings, 1)
+	explodeBinding := explodeOp.Bindings.HTTP[0].ParamBindings[0]
+	assert.Equal(t, ir.HTTPLocationQuerystring, explodeBinding.Location)
+	require.NotNil(t, explodeBinding.Explode, "the declared explode is not dropped")
+	assert.False(t, *explodeBinding.Explode)
+	assert.Equal(t,
+		"parameter field explode is not allowed for in=querystring; lowered as declared",
+		openapitest.DiagMessageAt(t, diags, diag.InvalidLocationKeyword, ir.SeverityWarning,
+			"/paths/~1a/get/parameters/0/explode"))
+
+	reservedOp, ok := opByName(doc, "reservedOnly")
+	require.True(t, ok)
+	require.Len(t, reservedOp.Bindings.HTTP, 1)
+	require.Len(t, reservedOp.Bindings.HTTP[0].ParamBindings, 1)
+	reservedBinding := reservedOp.Bindings.HTTP[0].ParamBindings[0]
+	assert.Equal(t, ir.HTTPLocationQuerystring, reservedBinding.Location)
+	assert.True(t, reservedBinding.AllowReserved, "the declared allowReserved is not dropped")
+	assert.Equal(t,
+		"parameter field allowReserved is not allowed for in=querystring; lowered as declared",
+		openapitest.DiagMessageAt(t, diags, diag.InvalidLocationKeyword, ir.SeverityWarning,
+			"/paths/~1b/get/parameters/0/allowReserved"))
 }
 
 // assertParamRefInheritance pins ir-design §14 at a parameter whose schema is a

compilers/openapi/internal/diag/diag.go

@@ -315,6 +315,27 @@ const (
 	// than error because the document is otherwise lowered whole, and the entry
 	// that did survive is in it.
 	UnknownKeyEntryTaken = "openapi/unknown-key-entry-taken"
+	// InvalidLocationKeyword reports a serialization keyword OpenAPI 3.2 forbids
+	// at the parameter location that declares it: explode or allowReserved at
+	// in: querystring, where the location binds the whole query string from the
+	// parameter's content and states its serialization through the media type
+	// alone, leaving nothing for either keyword to qualify.
+	//
+	// The bundled parser enforces this rule for style at that location but not
+	// for its two neighbours (GitHub #408), so this compiler reports the gap
+	// itself rather than relying on a validation finding that never arrives. The
+	// value still lowers as declared: dropping content the document states is an
+	// emitter's call, not a compiler's (invariant 2), the same choice already
+	// made for style at this position.
+	//
+	// Warning, not the error style gets: style's finding is the parser's own
+	// refusal-class validation, raised before this compiler ever sees the
+	// document. This one is the opposite shape — the compiler already kept a
+	// value it lowered and is saying so — the same class as
+	// ReservedHeaderName and InvalidMethodKey beside it. All three keywords
+	// are reported; a caller who wants the document refused over it has
+	// --fail-on warning for that.
+	InvalidLocationKeyword = "openapi/invalid-location-keyword"
 )
 
 // Newf builds an ir.Diagnostic with a formatted message. It is the single

compilers/openapi/internal/diag/diag_test.go

@@ -144,6 +144,7 @@ func codes() []string {
 		diag.ReservedHeaderName, diag.UnpreservableConstruct,
 		diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget,
 		diag.UnknownKeyUnreachable, diag.UnknownKeyEntryTaken,
+		diag.InvalidLocationKeyword,
 	}
 }
 

compilers/openapi/internal/operation/params.go

@@ -64,9 +64,37 @@ func lowerParameter(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd
 	}
 	diags := fillParamType(c, ts, anchors, &param, &binding, p, pptr, name)
 	diags = append(diags, reservedHeaderParamDiag(c, name, in, pptr)...)
+	diags = append(diags, querystringKeywordDiags(c, in, p, pptr)...)
 	return param, binding, append(diags, fillParamDetail(c, &param, p, pptr)...)
 }
 
+// querystringKeywordDiags reports explode and allowReserved when they are
+// declared on a parameter at in: querystring. OpenAPI 3.2 forbids both there
+// alongside style — the location binds the whole query string from the
+// parameter's content, so its serialization is stated by the media type alone
+// and neither keyword has anything left to qualify. The bundled parser
+// enforces the rule for style but not for these two (GitHub #408), so this
+// compiler reports the gap itself; resolveStyleExplode already lowers each
+// value as declared regardless; see diag.InvalidLocationKeyword for why this
+// is a warning rather than the error style gets from the parser.
+func querystringKeywordDiags(c lowering.Ctx, in soa.ParameterIn, p *soa.Parameter, pptr string) []ir.Diagnostic {
+	if in != soa.ParameterInQueryString {
+		return nil
+	}
+	var diags []ir.Diagnostic
+	if p.Explode != nil {
+		diags = append(diags, c.DiagAt(ir.SeverityWarning, diag.InvalidLocationKeyword,
+			pptr+ids.Ptr("explode"),
+			"parameter field explode is not allowed for in=querystring; lowered as declared"))
+	}
+	if p.AllowReserved != nil {
+		diags = append(diags, c.DiagAt(ir.SeverityWarning, diag.InvalidLocationKeyword,
+			pptr+ids.Ptr("allowReserved"),
+			"parameter field allowReserved is not allowed for in=querystring; lowered as declared"))
+	}
+	return diags
+}
+
 // reservedHeaderParamDiag reports a header parameter OpenAPI §4.8.12 reserves —
 // one named Accept, Content-Type or Authorization, whose definition it says
 // SHALL be ignored. The comparison is case-insensitive because HTTP field names

compilers/openapi/internal/operation/params_test.go

@@ -248,15 +248,17 @@ func TestParams_QueryStringDeclaredStyleIsKeptAndReported(t *testing.T) {
 	assert.False(t, *qs.Explode, "and so does the explode qualifying it")
 }
 
-// TestParams_QueryStringDeclaredExplodeAloneIsKept covers the half of that rule
-// the case above cannot see, because it declares both keywords: an explode
-// written without a style beside it.
+// TestParams_QueryStringDeclaredExplodeAloneIsKeptAndReported covers the half
+// of that rule the style case above cannot see, because it declares both
+// keywords: an explode written without a style beside it.
 //
 // Suppressing the invented style must not take a declared explode with it. 3.2
-// forbids explode at this location as it forbids style, and the bundled parser
-// refuses neither — so this reaches the compiler, and erasing it would be the
-// same silent drop the invented style was, in the other direction.
-func TestParams_QueryStringDeclaredExplodeAloneIsKept(t *testing.T) {
+// forbids explode at this location as it forbids style, but the bundled parser
+// enforces only the style half — so this reaches the compiler unrefused, and
+// erasing it would be the same silent drop the invented style was, in the other
+// direction. See diag.InvalidLocationKeyword's GoDoc for why this compiler's
+// own report is the only diagnostic naming it, and why it is a warning.
+func TestParams_QueryStringDeclaredExplodeAloneIsKeptAndReported(t *testing.T) {
 	t.Parallel()
 	spec := openapitest.PathsSpecVer("3.2.0", `  /q:
     get:
@@ -283,6 +285,73 @@ func TestParams_QueryStringDeclaredExplodeAloneIsKept(t *testing.T) {
 	assert.Empty(t, qs.Style, "no style is invented at this location")
 	require.NotNil(t, qs.Explode, "but the declared explode is not dropped with it")
 	assert.False(t, *qs.Explode)
+
+	assert.Equal(t,
+		"parameter field explode is not allowed for in=querystring; lowered as declared",
+		openapitest.DiagMessageAt(t, diags, diag.InvalidLocationKeyword, ir.SeverityWarning,
+			"/paths/~1q/get/parameters/0/explode"))
+}
+
+// TestParams_QueryStringDeclaredAllowReservedIsKeptAndReported is the third
+// keyword 3.2 forbids at this location alongside style and explode; see
+// diag.InvalidLocationKeyword's GoDoc for why the parser enforces none of it
+// and why this compiler's report is a warning.
+func TestParams_QueryStringDeclaredAllowReservedIsKeptAndReported(t *testing.T) {
+	t.Parallel()
+	spec := openapitest.PathsSpecVer("3.2.0", `  /q:
+    get:
+      operationId: q
+      parameters:
+        - name: qs
+          in: querystring
+          allowReserved: true
+          content:
+            application/x-www-form-urlencoded:
+              schema: {type: object}
+      responses:
+        "200": {description: ok}
+`)
+	doc, diags := parseFull(t, spec)
+	openapitest.RequireNoErrorDiags(t, diags)
+	op := openapitest.FindOp(t, doc, "q")
+	require.Len(t, op.Bindings.HTTP, 1)
+	require.Len(t, op.Bindings.HTTP[0].ParamBindings, 1)
+
+	qs := op.Bindings.HTTP[0].ParamBindings[0]
+	require.Equal(t, ir.HTTPLocationQuerystring, qs.Location,
+		"the keyword below is only news at the location that forbids it")
+	assert.True(t, qs.AllowReserved, "the declared allowReserved lowers as declared")
+
+	assert.Equal(t,
+		"parameter field allowReserved is not allowed for in=querystring; lowered as declared",
+		openapitest.DiagMessageAt(t, diags, diag.InvalidLocationKeyword, ir.SeverityWarning,
+			"/paths/~1q/get/parameters/0/allowReserved"))
+}
+
+// TestParams_QueryStringUndeclaredKeywordsAreNotReported is the control for
+// both cases above: a querystring binding that declares neither keyword raises
+// diag.InvalidLocationKeyword at neither pointer, so the diagnostic tracks the
+// keyword's presence in the document rather than firing for every parameter at
+// this location.
+func TestParams_QueryStringUndeclaredKeywordsAreNotReported(t *testing.T) {
+	t.Parallel()
+	doc, diags := parseFull(t, openapitest.PathsSpecVer("3.2.0", `  /q:
+    get:
+      operationId: q
+      parameters:
+        - name: qs
+          in: querystring
+          content:
+            application/x-www-form-urlencoded:
+              schema: {type: object}
+      responses:
+        "200": {description: ok}
+`))
+	openapitest.RequireNoErrorDiags(t, diags)
+	op := openapitest.FindOp(t, doc, "q")
+	require.Len(t, op.Bindings.HTTP, 1)
+	require.Len(t, op.Bindings.HTTP[0].ParamBindings, 1)
+	assert.False(t, openapitest.HasDiag(diags, diag.InvalidLocationKeyword))
 }
 
 const componentParamRefSpec = `openapi: 3.1.0