fix(compilers/openapi): stop stamping querystring with a style

compilers/openapi/conformance_test.go

@@ -180,6 +180,7 @@ func conformanceCases() []conformanceCase {
 		{"tags-grouping", assertTagsGrouping},
 		{"http-binding", assertHTTPBinding},
 		{"param-styles", assertParamStyles},
+		{"param-querystring", assertParamQuerystring},
 		{"param-xml-residue", assertParamXMLResidue},
 		{"param-ref-inheritance", assertParamRefInheritance},
 		{"header-content-schema", assertHeaderContentSchema},
@@ -1452,6 +1453,35 @@ func assertAllowEmptyValueKept(t *testing.T, op ir.Operation) {
 	assert.JSONEq(t, `true`, string(entry.Value))
 }
 
+// assertParamQuerystring pins the 3.2 querystring location, where the whole
+// query string binds from the parameter's content: the binding carries that
+// media type and neither style nor explode, the two keywords the location
+// forbids (GitHub #334). The ordinary query parameter beside it keeps the
+// defaults its own location does admit, so what separates them is the location
+// rather than the presence of content.
+func assertParamQuerystring(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
+	report, ok := opByName(doc, "runReport")
+	require.True(t, ok)
+	require.Len(t, report.Bindings.HTTP, 1)
+	require.Len(t, report.Bindings.HTTP[0].ParamBindings, 1)
+	qs := report.Bindings.HTTP[0].ParamBindings[0]
+	assert.Equal(t, ir.HTTPLocationQuerystring, qs.Location)
+	assert.Equal(t, "application/x-www-form-urlencoded", qs.ContentType,
+		"the media type is the whole of a querystring binding's declared serialization")
+	assert.Empty(t, qs.Style, "style is not a legal keyword at in: querystring")
+	assert.Nil(t, qs.Explode, "and explode qualifies a style there is none of")
+
+	summary, ok := opByName(doc, "summarize")
+	require.True(t, ok)
+	require.Len(t, summary.Bindings.HTTP, 1)
+	require.Len(t, summary.Bindings.HTTP[0].ParamBindings, 1)
+	q := summary.Bindings.HTTP[0].ParamBindings[0]
+	assert.Equal(t, ir.HTTPLocationQuery, q.Location)
+	assert.Equal(t, "form", q.Style, "a query parameter still takes its own location's default")
+	require.NotNil(t, q.Explode)
+	assert.True(t, *q.Explode)
+}
+
 // assertParamRefInheritance pins ir-design §14 at a parameter whose schema is a
 // $ref: docs, deprecation and default come from the referent when the use site is
 // silent, and from the use site when it is not. Constraints inherit at neither

compilers/openapi/internal/operation/params.go

@@ -307,25 +307,40 @@ func preserveAllowEmptyValue(c lowering.Ctx, param *ir.Parameter, p *soa.Paramet
 
 // resolveStyleExplode materializes a parameter's resolved serialization style
 // and explode flag: an explicit value wins, else the OpenAPI per-location
-// default (query/cookie → form/true, path/header → simple/false). The result is
-// declared facts, not policy.
+// default (query/cookie → form/true, path/header → simple/false, querystring →
+// neither). The result is declared facts, not policy.
 func resolveStyleExplode(p *soa.Parameter, in soa.ParameterIn) (string, *bool) {
 	style := defaultParamStyle(in)
 	if p.Style != nil {
 		style = string(*p.Style)
 	}
-	explode := style == string(soa.SerializationStyleForm)
+	// A declared explode lowers as declared wherever it is written, for the same
+	// reason a declared style does: 3.2 forbids both at in: querystring, but
+	// dropping content the document states is an emitter's call, not a
+	// compiler's, and this position has an IR field to hold it.
 	if p.Explode != nil {
-		explode = *p.Explode
+		explode := *p.Explode
+		return style, &explode
+	}
+	// Absent one, explode qualifies a style, so a position resolving to no style
+	// gets no default either — a querystring binding's ContentType is the whole
+	// of its declared serialization (GitHub #334).
+	if style == "" {
+		return "", nil
 	}
+	explode := style == string(soa.SerializationStyleForm)
 	return style, &explode
 }
 
 // defaultParamStyle returns the OpenAPI default serialization style for a
-// parameter location.
+// parameter location, and "" for one that has none: 3.2 binds in: querystring
+// from the parameter's content and forbids style there, so that location has no
+// default to fall back on.
 func defaultParamStyle(in soa.ParameterIn) string {
 	switch in {
-	case soa.ParameterInQuery, soa.ParameterInCookie, soa.ParameterInQueryString:
+	case soa.ParameterInQueryString:
+		return ""
+	case soa.ParameterInQuery, soa.ParameterInCookie:
 		return string(soa.SerializationStyleForm)
 	default:
 		return string(soa.SerializationStyleSimple)

compilers/openapi/internal/operation/params_test.go

@@ -3,6 +3,7 @@ package operation_test
 import (
 	"testing"
 
+	"github.com/speakeasy-api/openapi/validation"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -180,19 +181,103 @@ func TestParams_AllLocationsAndStyles(t *testing.T) {
 	assert.True(t, hasDiag(diags, diag.NumericPrecision), "malformed param constraint warns")
 }
 
+// TestParams_QueryStringLocation pins the whole of a querystring binding's
+// declared serialization: the 3.2 location, and the media type from content
+// carrying it alone. Style and explode are not legal keywords there, so the
+// binding takes neither rather than the query defaults (GitHub #334).
 func TestParams_QueryStringLocation(t *testing.T) {
 	t.Parallel()
 	spec := pathsSpecVer("3.2.0", `  /q:
     get:
       operationId: q
       parameters:
-        - {name: qs, in: querystring, schema: {type: string}}
+        - name: qs
+          in: querystring
+          content:
+            application/x-www-form-urlencoded:
+              schema: {type: object, properties: {page: {type: string}}}
       responses:
         "200": {description: ok}
 `)
-	doc, _ := parseFull(t, spec)
+	doc, diags := parseFull(t, spec)
+	requireNoErrorDiags(t, diags)
 	op := findOp(t, doc, "q")
-	assert.Equal(t, ir.HTTPLocationQuerystring, op.Bindings.HTTP[0].ParamBindings[0].Location)
+	require.Len(t, op.Bindings.HTTP, 1)
+	require.Len(t, op.Bindings.HTTP[0].ParamBindings, 1)
+
+	qs := op.Bindings.HTTP[0].ParamBindings[0]
+	assert.Equal(t, ir.HTTPLocationQuerystring, qs.Location)
+	assert.Equal(t, "application/x-www-form-urlencoded", qs.ContentType)
+	assert.Empty(t, qs.Style, "3.2 binds the query string from content and forbids style there")
+	assert.Nil(t, qs.Explode, "and explode with it: it qualifies a style, and there is none")
+}
+
+// TestParams_QueryStringDeclaredStyleIsKeptAndReported pins the other half of
+// the same rule: the compiler stopped inventing a style at that location, it did
+// not start erasing one. A document declaring style there is invalid and the
+// parser says so; what it declared still lowers, because dropping declared
+// content is an emitter's call rather than a compiler's.
+func TestParams_QueryStringDeclaredStyleIsKeptAndReported(t *testing.T) {
+	t.Parallel()
+	spec := pathsSpecVer("3.2.0", `  /q:
+    get:
+      operationId: q
+      parameters:
+        - name: qs
+          in: querystring
+          style: form
+          explode: false
+          content:
+            application/x-www-form-urlencoded:
+              schema: {type: object}
+      responses:
+        "200": {description: ok}
+`)
+	doc, diags := parseFull(t, spec)
+	assertHasCode(t, diags, diag.Validation+"/"+string(validation.RuleValidationAllowedValues), ir.SeverityError)
+	op := 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]
+	assert.Equal(t, "form", qs.Style, "the declared style lowers as declared")
+	require.NotNil(t, qs.Explode)
+	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.
+//
+// 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) {
+	t.Parallel()
+	spec := pathsSpecVer("3.2.0", `  /q:
+    get:
+      operationId: q
+      parameters:
+        - name: qs
+          in: querystring
+          explode: false
+          content:
+            application/x-www-form-urlencoded:
+              schema: {type: object}
+      responses:
+        "200": {description: ok}
+`)
+	doc, diags := parseFull(t, spec)
+	requireNoErrorDiags(t, diags)
+	op := 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]
+	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)
 }
 
 const componentParamRefSpec = `openapi: 3.1.0