fix(openapi): let WithParameter reach the document

internal/router/openapi_generator.go

@@ -365,6 +365,21 @@ func (g *openAPIGenerator) processRoute(spec *OpenAPISpec, route RouteInfo) erro
 		}
 	}
 
+	// Parameters declared on the route with WithParameter fill in what the
+	// schema-driven sources above did not describe. Both branches converge here
+	// so a declaration behaves the same whether the handler takes a request
+	// struct or not.
+	//
+	// The already-collected parameters go in first, so a name described by both
+	// a Go type and a declaration keeps the type-derived definition. That is the
+	// same precedence the three sources already follow among themselves, and it
+	// is the right way round: the type knows the real shape, while a declaration
+	// only knows what someone typed into the call.
+	operation.Parameters = mergeParameters(
+		operation.Parameters,
+		g.extractRouteParameters(route.Metadata),
+	)
+
 	// Process response schemas
 	if err := g.extractResponseSchemas(spec, operation, route); err != nil {
 		return err
@@ -1106,6 +1121,71 @@ func (g *openAPIGenerator) extractHeaderParameters(metadata map[string]any) []Pa
 	return generateHeaderParamsFromStruct(g.schemas, headerSchema)
 }
 
+// extractRouteParameters converts the parameters declared on the route itself
+// with WithParameter.
+//
+// The three sources above all read a Go type: the path template, the query
+// schema, the header schema. This one reads a hand-written declaration, which is
+// what a route reaches for when the value never passes through the request
+// struct at all -- a repeatable parameter the binder cannot decode, a header
+// some middleware consumes, anything the handler pulls off the raw request.
+// Without this the declaration went into route metadata and stopped there.
+func (g *openAPIGenerator) extractRouteParameters(metadata map[string]any) []Parameter {
+	if metadata == nil {
+		return nil
+	}
+
+	defs, ok := metadata["parameters"].([]ParameterDef)
+	if !ok {
+		return nil
+	}
+
+	params := make([]Parameter, 0, len(defs))
+
+	for _, def := range defs {
+		params = append(params, Parameter{
+			Name:        def.Name,
+			In:          def.In,
+			Description: def.Description,
+			// A path parameter is required by definition, whatever the
+			// declaration says, and OpenAPI rejects one that claims otherwise.
+			Required: def.Required || def.In == "path",
+			Schema:   schemaFromExample(def.Example),
+			Example:  def.Example,
+		})
+	}
+
+	return params
+}
+
+// schemaFromExample types a declared parameter from its example value.
+//
+// ParameterDef carries a name, a location, a description, a required flag and an
+// example. The example is the only one of those that says anything about what
+// the parameter holds, so it is what the schema is built from: an int example
+// makes an integer, a []string example makes an array of string, which is how a
+// repeatable parameter gets described.
+//
+// With no example there is nothing to read and the parameter falls back to
+// string. That is a guess, but it is the useful guess -- most parameters are
+// strings, and a parameter with no schema at all is one no client can type.
+func schemaFromExample(example any) *Schema {
+	stringSchema := &Schema{Type: "string"}
+
+	if example == nil {
+		return stringSchema
+	}
+
+	// A throwaway generator with no component registry, so typing a parameter
+	// cannot register components or emit a $ref into them.
+	schema, err := newSchemaGenerator(nil, nil).GenerateSchema(example)
+	if err != nil || schema == nil {
+		return stringSchema
+	}
+
+	return schema
+}
+
 // processSecurityRequirements adds security requirements to operation.
 func (g *openAPIGenerator) processSecurityRequirements(operation *Operation, metadata map[string]any) {
 	if metadata == nil {

internal/router/openapi_route_params_test.go

@@ -0,0 +1,169 @@
+package router
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/xraph/vessel"
+)
+
+// paramNamed returns the named parameter from an operation, or nil.
+func paramNamed(op *Operation, name, in string) *Parameter {
+	if op == nil {
+		return nil
+	}
+
+	for i := range op.Parameters {
+		if op.Parameters[i].Name == name && op.Parameters[i].In == in {
+			return &op.Parameters[i]
+		}
+	}
+
+	return nil
+}
+
+// routeParamSpec registers one route carrying the given options and returns the
+// generated operation for it.
+func routeParamSpec(t *testing.T, path string, opts ...RouteOption) *Operation {
+	t.Helper()
+
+	router := NewRouter(WithContainer(vessel.New()))
+
+	require.NoError(t, router.GET(path, func(ctx Context) error { return nil }, opts...))
+
+	gen := newOpenAPIGenerator(OpenAPIConfig{Title: "Test", Version: "1"}, router, nil, "")
+
+	spec, err := gen.Generate()
+	require.NoError(t, err)
+
+	// Look the operation up rather than keying on the registered path: the
+	// generator rewrites :id into {id} on its way into the document.
+	require.Len(t, spec.Paths, 1, "one route was registered")
+
+	for _, item := range spec.Paths {
+		require.NotNil(t, item.Get, "the registered GET should be in the document")
+
+		return item.Get
+	}
+
+	return nil
+}
+
+// WithParameter used to write route metadata that nothing read. It compiled, it
+// ran, it returned no error, and the parameter reached no document -- so every
+// client generated off that document was missing a parameter the server honours.
+//
+// The declaration is the whole point of the option. If it does not arrive here,
+// the option is a comment with a function call around it.
+func TestWithParameter_ReachesTheDocument(t *testing.T) {
+	op := routeParamSpec(t, "/things",
+		WithOperationID("listThings"),
+		WithParameter("tenant", "query", "Tenant to scope the listing to", true, "acme"),
+	)
+
+	param := paramNamed(op, "tenant", "query")
+	require.NotNil(t, param, "the declared parameter should be in the document")
+
+	assert.Equal(t, "Tenant to scope the listing to", param.Description)
+	assert.True(t, param.Required)
+	assert.Equal(t, "acme", param.Example)
+
+	require.NotNil(t, param.Schema, "a parameter needs a schema for a client to type it")
+	assert.Equal(t, "string", param.Schema.Type)
+}
+
+// The option carries no type, so the example is the only thing that can say what
+// the parameter holds. A parameter typed off an integer example as a string is
+// how a client ends up quoting a number.
+func TestWithParameter_TypesTheSchemaFromTheExample(t *testing.T) {
+	cases := []struct {
+		name    string
+		example any
+		want    string
+	}{
+		{name: "string", example: "acme", want: "string"},
+		{name: "integer", example: 25, want: "integer"},
+		{name: "number", example: 1.5, want: "number"},
+		{name: "boolean", example: true, want: "boolean"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			op := routeParamSpec(t, "/things",
+				WithOperationID("listThings"),
+				WithParameter("limit", "query", "", false, tc.example),
+			)
+
+			param := paramNamed(op, "limit", "query")
+			require.NotNil(t, param)
+			require.NotNil(t, param.Schema)
+			assert.Equal(t, tc.want, param.Schema.Type)
+		})
+	}
+}
+
+// A repeatable parameter is an array, and a slice example is the only way this
+// option can say so. Query parameters default to style form with explode true,
+// so an array is already "send it once per value".
+func TestWithParameter_TypesASliceExampleAsAnArray(t *testing.T) {
+	op := routeParamSpec(t, "/things",
+		WithOperationID("listThings"),
+		WithParameter("resource", "query", "Repeatable", false, []string{"https://api.example.com"}),
+	)
+
+	param := paramNamed(op, "resource", "query")
+	require.NotNil(t, param)
+	require.NotNil(t, param.Schema)
+
+	assert.Equal(t, "array", param.Schema.Type)
+	require.NotNil(t, param.Schema.Items)
+	assert.Equal(t, "string", param.Schema.Items.Type)
+}
+
+// With no example there is nothing to infer from, and a parameter with no schema
+// is one a generator cannot type at all. String is the least surprising floor.
+func TestWithParameter_FallsBackToStringWithoutAnExample(t *testing.T) {
+	op := routeParamSpec(t, "/things",
+		WithOperationID("listThings"),
+		WithParameter("cursor", "query", "", false, nil),
+	)
+
+	param := paramNamed(op, "cursor", "query")
+	require.NotNil(t, param)
+	require.NotNil(t, param.Schema)
+	assert.Equal(t, "string", param.Schema.Type)
+	assert.Nil(t, param.Example, "no example was given, so none should be published")
+}
+
+// A path parameter named in the template is already described from the template.
+// The declared one must not double it up, and the richer of the two wins, which
+// is the same precedence the struct-derived sources already follow.
+func TestWithParameter_DoesNotDuplicateAParameterAlreadyDescribed(t *testing.T) {
+	op := routeParamSpec(t, "/things/:id",
+		WithOperationID("getThing"),
+		WithParameter("id", "path", "Thing ID", true, "123"),
+	)
+
+	count := 0
+
+	for _, p := range op.Parameters {
+		if p.Name == "id" && p.In == "path" {
+			count++
+		}
+	}
+
+	assert.Equal(t, 1, count, "the parameter should appear exactly once")
+}
+
+// Several declarations on one route all have to arrive, in any location.
+func TestWithParameter_CarriesEveryDeclaration(t *testing.T) {
+	op := routeParamSpec(t, "/things",
+		WithOperationID("listThings"),
+		WithParameter("tenant", "query", "", false, "acme"),
+		WithParameter("X-Request-Id", "header", "", false, "abc"),
+	)
+
+	assert.NotNil(t, paramNamed(op, "tenant", "query"))
+	assert.NotNil(t, paramNamed(op, "X-Request-Id", "header"))
+}