diff --git a/pkg/codegen/operations.go b/pkg/codegen/operations.go index cbe75b780..c8a5a88cd 100644 --- a/pkg/codegen/operations.go +++ b/pkg/codegen/operations.go @@ -2031,7 +2031,7 @@ func GenerateBodyDefinitions(operationID string, bodyOrRef *openapi3.RequestBody bodyTypeName := operationID + tag + "Body" bodySchema, err := GenerateGoSchema(content.Schema, []string{bodyTypeName}) if err != nil { - return nil, nil, fmt.Errorf("error generating request body definition: %w", err) + return nil, nil, fmt.Errorf("error generating request body definition for %s (%s): %w", operationID, contentType, err) } // If the body is a pre-defined type @@ -2113,6 +2113,16 @@ func GenerateResponseDefinitions(operationID string, responses map[string]*opena } response := responseOrRef.Value + // Reusable components.responses are generated with no operation + // (see the GenerateResponseDefinitions("", ...) call in Generate), + // and the map key is then a response component name rather than a + // status code. Name it alone instead of qualifying it with an empty + // operation, which would read as a leading ".". + responseLabel := statusCode + if operationID != "" { + responseLabel = operationID + "." + statusCode + } + var responseContentDefinitions []ResponseContentDefinition for _, contentType := range SortedMapKeys(response.Content) { @@ -2149,7 +2159,7 @@ func GenerateResponseDefinitions(operationID string, responses map[string]*opena responseBodyTypeName := responseTypeName + "Body" contentSchema, err := GenerateGoSchema(content.Schema, []string{responseBodyTypeName}) if err != nil { - return nil, fmt.Errorf("error generating request body definition: %w", err) + return nil, fmt.Errorf("error generating response body definition for %s (%s): %w", responseLabel, contentType, err) } // Hoist inline response-root schemas that need method-emitting @@ -2193,7 +2203,7 @@ func GenerateResponseDefinitions(operationID string, responses map[string]*opena header := response.Headers[headerName] contentSchema, err := GenerateGoSchema(header.Value.Schema, []string{}) if err != nil { - return nil, fmt.Errorf("error generating response header definition: %w", err) + return nil, fmt.Errorf("error generating response header definition for %s header %q: %w", responseLabel, headerName, err) } // When the header component itself is an external `$ref` (it lives // in another file) and its schema references a named type, that diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index a8bcf076a..802e45f6e 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -956,15 +956,105 @@ func isMultiTypeUnion(t *openapi3.Types) bool { return false } for _, name := range s { - switch name { - case "array", "boolean", "integer", "number", "object", "string": - default: + if name == "null" || !isJSONSchemaType(name) { return false } } return true } +// jsonSchemaTypes are the values a schema's `type` may name, per JSON Schema +// (and so per OpenAPI). The primitive-type dispatch handles every one of +// them, which is what lets unhandledSchemaTypeError treat an unrecognized +// name as a misspelling. +var jsonSchemaTypes = []string{"array", "boolean", "integer", "null", "number", "object", "string"} + +func isJSONSchemaType(name string) bool { + return slices.Contains(jsonSchemaTypes, name) +} + +// quoteTypeNames renders a `type` value the way the spec author wrote it: a +// bare quoted name for a single type, a bracketed list for the 3.1 list form. +func quoteTypeNames(names []string) string { + quoted := make([]string, len(names)) + for i, name := range names { + quoted[i] = strconv.Quote(name) + } + if len(names) == 1 { + return quoted[0] + } + return "[" + strings.Join(quoted, ", ") + "]" +} + +// unhandledSchemaTypeError explains why a schema's `type` fell off the end of +// the primitive-type dispatch. The dispatch covers every JSON Schema type, so +// reaching it means the `type` is not one this document may carry: a name +// that is not a JSON Schema type at all, or the 3.1-only list form in an +// earlier document. The message names the declared value verbatim and the +// reason, so a typo is visible at a glance. +// See https://github.com/oapi-codegen/oapi-codegen/issues/1977. +// +// declared is the schema's `type` as written, not the "null"-stripped value +// the dispatch runs on, so the reader sees what is in their spec. +// +// Precondition: globalState.is31 must be set (see schemaIsNullable for +// context). An unset is31 reads a 3.1 document as an earlier one and picks +// the version branch below. +func unhandledSchemaTypeError(declared *openapi3.Types) error { + names := declared.Slice() + + var unknown []string + for _, name := range names { + if !isJSONSchemaType(name) { + unknown = append(unknown, strconv.Quote(name)) + } + } + + expected := fmt.Sprintf("expected one of %s", strings.Join(jsonSchemaTypes, ", ")) + + switch { + case len(unknown) > 0: + return fmt.Errorf("unhandled Schema type %s: %s (%s)", + quoteTypeNames(names), invalidTypeNamesPhrase(names, unknown), expected) + case len(names) > 1 && !globalState.is31: + // Every entry names a real type, so the list form itself is what is + // unusable here: a 3.1 document would have mapped it to `any`. + return fmt.Errorf("unhandled Schema type %s: a list of types requires OpenAPI 3.1 or later%s", + quoteTypeNames(names), declaredVersionSuffix()) + default: + // An empty `type`, or a 3.1 list naming nothing but the "null" + // nullability marker, which leaves no type to map. + return fmt.Errorf("unhandled Schema type %s: type must name a JSON Schema type (%s)", + quoteTypeNames(names), expected) + } +} + +// invalidTypeNamesPhrase names the entries that are not JSON Schema types. +// A single-entry `type` is its own offender, so the phrase does not repeat +// the name the message already printed. +func invalidTypeNamesPhrase(names, unknown []string) string { + switch { + case len(names) == 1: + return "not a valid JSON Schema type" + case len(unknown) == 1: + return unknown[0] + " is not a valid JSON Schema type" + default: + return strings.Join(unknown, ", ") + " are not valid JSON Schema types" + } +} + +// declaredVersionSuffix names the document's version when that explains the +// failure, and says nothing when it would not. globalState.spec is unset +// when codegen internals are exercised directly, and SetGlobalStateSpec sets +// the spec without is31, so a spec already declaring 3.1 would contradict a +// message about needing 3.1. Silence beats a self-contradicting hint. +func declaredVersionSuffix() string { + if globalState.spec == nil || globalState.spec.OpenAPI == "" || globalState.spec.IsOpenAPI31OrLater() { + return "" + } + return fmt.Sprintf(", but this document declares OpenAPI %s", globalState.spec.OpenAPI) +} + // schemaUnionTypes returns t's member types as a string slice when t is an // OpenAPI 3.1 multi-type union after the "null" nullability marker is // stripped, or nil otherwise. Generated code passes the list to the @@ -1622,7 +1712,7 @@ func oapiSchemaToGoType(schema *openapi3.Schema, path []string, outSchema *Schem outSchema.SkipOptionalPointer = true outSchema.DefineViaAlias = true } else { - return fmt.Errorf("unhandled Schema type: %v", t) + return unhandledSchemaTypeError(schema.Type) } return nil } @@ -1893,7 +1983,10 @@ func generateUnion(outSchema *Schema, elements openapi3.SchemaRefs, discriminato elementPath := append(path, fmt.Sprint(i)) elementSchema, err := GenerateGoSchema(element, elementPath) if err != nil { - return err + // The caller names the keyword (anyOf/oneOf); the index is the + // only thing that says which inline branch failed, and it is + // otherwise discarded with elementPath. + return fmt.Errorf("branch %d: %w", i, err) } if element.Ref == "" { diff --git a/pkg/codegen/schema_test.go b/pkg/codegen/schema_test.go index 88ee2a072..3ed716681 100644 --- a/pkg/codegen/schema_test.go +++ b/pkg/codegen/schema_test.go @@ -792,7 +792,7 @@ func TestOapiSchemaToGoType_MultiTypeUnion(t *testing.T) { for _, tc := range []struct { name string types openapi3.Types - wantErr bool + wantErr string wantType string wantSkip bool }{ @@ -823,7 +823,7 @@ func TestOapiSchemaToGoType_MultiTypeUnion(t *testing.T) { { name: "misspelled type name is still an error", types: openapi3.Types{"strng", "number"}, - wantErr: true, + wantErr: `unhandled Schema type ["strng", "number"]: "strng" is not a valid JSON Schema type`, }, } { t.Run(tc.name, func(t *testing.T) { @@ -834,8 +834,8 @@ func TestOapiSchemaToGoType_MultiTypeUnion(t *testing.T) { var out Schema err := oapiSchemaToGoType(&openapi3.Schema{Type: &tc.types}, []string{"Value"}, &out) - if tc.wantErr { - assert.ErrorContains(t, err, "unhandled Schema type") + if tc.wantErr != "" { + assert.ErrorContains(t, err, tc.wantErr) return } require.NoError(t, err) @@ -997,7 +997,167 @@ func TestOapiSchemaToGoType_MultiTypeUnionRequires31(t *testing.T) { var out Schema err := oapiSchemaToGoType(&openapi3.Schema{Type: &openapi3.Types{"string", "number"}}, []string{"Value"}, &out) - assert.ErrorContains(t, err, "unhandled Schema type") + assert.ErrorContains(t, err, + `unhandled Schema type ["string", "number"]: a list of types requires OpenAPI 3.1 or later`) +} + +// The dispatch handles every JSON Schema type, so falling off the end of it +// means the `type` is not one the document may carry. The error has to say +// which value that was and why, or the user is left with #1976's +// "unhandled Schema type: &[int]" and nothing to act on. +func TestOapiSchemaToGoType_UnhandledTypeError(t *testing.T) { + const expected = "expected one of array, boolean, integer, null, number, object, string" + + for _, tc := range []struct { + name string + is31 bool + spec *openapi3.T + types openapi3.Types + want []string + }{ + { + name: "unknown single type name", + types: openapi3.Types{"int"}, + want: []string{`unhandled Schema type "int": not a valid JSON Schema type`, expected}, + }, + { + name: "unknown type name in a 3.1 list", + is31: true, + types: openapi3.Types{"strng", "number"}, + want: []string{ + `unhandled Schema type ["strng", "number"]`, + `"strng" is not a valid JSON Schema type`, + expected, + }, + }, + { + name: "several unknown type names in a 3.1 list", + is31: true, + types: openapi3.Types{"strng", "numbr"}, + want: []string{ + `unhandled Schema type ["strng", "numbr"]`, + `"strng", "numbr" are not valid JSON Schema types`, + }, + }, + { + // The list form itself is what is unusable here: the entries are valid + // types, so the hint names the version. + name: "list of valid types under a 3.0 document", + spec: &openapi3.T{OpenAPI: "3.0.3"}, + types: openapi3.Types{"string", "number"}, + want: []string{ + `unhandled Schema type ["string", "number"]`, + "a list of types requires OpenAPI 3.1 or later, but this document declares OpenAPI 3.0.3", + }, + }, + { + // The declared `type` is reported verbatim, "null" marker and + // all, so it matches what the reader has in front of them. + name: "unknown type name alongside the null marker", + is31: true, + types: openapi3.Types{"strng", "null"}, + want: []string{`unhandled Schema type ["strng", "null"]`, `"strng" is not a valid JSON Schema type`}, + }, + { + name: "a type list naming nothing but null", + is31: true, + types: openapi3.Types{"null", "null"}, + want: []string{`unhandled Schema type ["null", "null"]`, "type must name a JSON Schema type"}, + }, + { + name: "an empty type list", + types: openapi3.Types{}, + want: []string{"unhandled Schema type []", "type must name a JSON Schema type"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + prev := globalState + t.Cleanup(func() { globalState = prev }) + globalState.is31 = tc.is31 + globalState.spec = tc.spec + globalState.typeMapping = DefaultTypeMapping + + var out Schema + err := oapiSchemaToGoType(&openapi3.Schema{Type: &tc.types}, []string{"Value"}, &out) + require.Error(t, err) + for _, want := range tc.want { + assert.ErrorContains(t, err, want) + } + }) + } +} + +// The version hint is a suffix, not the message: a caller that never ran +// Generate() has no spec to read, and SetGlobalStateSpec sets the spec +// without is31, so a 3.1 spec would otherwise be told it needs 3.1. Both +// drop the suffix and keep the requirement. +func TestUnhandledSchemaTypeErrorVersionSuffix(t *testing.T) { + for _, tc := range []struct { + name string + spec *openapi3.T + }{ + {name: "no spec at all", spec: nil}, + {name: "spec with no declared version", spec: &openapi3.T{}}, + { + // SetGlobalStateSpec sets the spec without is31, so the two can + // disagree. Citing 3.1 here would contradict the requirement. + name: "spec already declares 3.1", + spec: &openapi3.T{OpenAPI: "3.1.0"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + prev := globalState + t.Cleanup(func() { globalState = prev }) + globalState.is31 = false + globalState.spec = tc.spec + + err := unhandledSchemaTypeError(&openapi3.Types{"string", "number"}) + assert.ErrorContains(t, err, "a list of types requires OpenAPI 3.1 or later") + assert.NotContains(t, err.Error(), "but this document declares", + "an unusable or contradictory version must not be cited") + }) + } +} + +// components.responses generate with no operation, so the response label is +// the component name alone rather than an empty operation and a stray dot. +func TestGenerateResponseDefinitionsNamesComponentResponses(t *testing.T) { + prev := globalState + t.Cleanup(func() { globalState = prev }) + globalState.typeMapping = DefaultTypeMapping + + responses := map[string]*openapi3.ResponseRef{ + "NotFound": {Value: &openapi3.Response{ + Content: openapi3.Content{ + "application/json": &openapi3.MediaType{ + Schema: &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"int"}}}, + }, + }, + }}, + } + + _, err := GenerateResponseDefinitions("", responses, "") + require.Error(t, err) + assert.ErrorContains(t, err, "for NotFound (application/json)") + assert.NotContains(t, err.Error(), "for .NotFound", "no empty operation prefix") +} + +// An inline oneOf/anyOf branch is only identifiable by its index, which +// generateUnion computes for naming and used to discard on the error path. +func TestGenerateUnionNamesFailingBranch(t *testing.T) { + prev := globalState + t.Cleanup(func() { globalState = prev }) + globalState.typeMapping = DefaultTypeMapping + + _, err := GenerateGoSchema(&openapi3.SchemaRef{Value: &openapi3.Schema{ + OneOf: openapi3.SchemaRefs{ + {Value: &openapi3.Schema{Type: &openapi3.Types{"string"}}}, + {Value: &openapi3.Schema{Type: &openapi3.Types{"int"}}}, + }, + }}, []string{"Choice"}) + require.Error(t, err) + assert.ErrorContains(t, err, "branch 1") + assert.ErrorContains(t, err, `unhandled Schema type "int"`) } // schemaUnionTypes feeds the Types bind option emitted for union