diff --git a/configuration-schema.json b/configuration-schema.json index 1cf2d8599c..fefbc19c86 100644 --- a/configuration-schema.json +++ b/configuration-schema.json @@ -238,6 +238,11 @@ "type": "boolean", "description": "Allows defining at a global level whether to omit the pointer for a type to indicate that the field/type is optional. This is the same as adding `x-go-type-skip-optional-pointer` to each field (manually, or using an OpenAPI Overlay). A field can set `x-go-type-skip-optional-pointer: false` to still require the optional pointer.", "default": false + }, + "prefer-skip-optional-pointer-on-container-types": { + "type": "boolean", + "description": "Allows disabling the generation of an 'optional pointer' for an optional field that is a container type (such as a slice or a map), which ends up requiring an additional, unnecessary, `... != nil` check. A field can set `x-go-type-skip-optional-pointer: false` to still require the optional pointer.", + "default": false } } }, diff --git a/internal/test/issues/issue1561/config.yaml b/internal/test/issues/issue1561/config.yaml new file mode 100644 index 0000000000..e1df8f3b7e --- /dev/null +++ b/internal/test/issues/issue1561/config.yaml @@ -0,0 +1,8 @@ +# yaml-language-server: $schema=../../../../configuration-schema.json +package: issue1561 +generate: + models: true +output: issue1561.gen.go +output-options: + skip-prune: true + prefer-skip-optional-pointer-on-container-types: true diff --git a/internal/test/issues/issue1561/generate.go b/internal/test/issues/issue1561/generate.go new file mode 100644 index 0000000000..21cf1d305d --- /dev/null +++ b/internal/test/issues/issue1561/generate.go @@ -0,0 +1,3 @@ +package issue1561 + +//go:generate go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen --config=config.yaml openapi.yaml diff --git a/internal/test/issues/issue1561/issue1561.gen.go b/internal/test/issues/issue1561/issue1561.gen.go new file mode 100644 index 0000000000..f011e318c1 --- /dev/null +++ b/internal/test/issues/issue1561/issue1561.gen.go @@ -0,0 +1,21 @@ +// Package issue1561 provides primitives to interact with the openapi HTTP API. +// +// Code generated by github.com/oapi-codegen/oapi-codegen/v2 version v2.0.0-00010101000000-000000000000 DO NOT EDIT. +package issue1561 + +// Pong defines model for Pong. +type Pong struct { + Ping string `json:"ping"` +} + +// ResponseBody defines model for ResponseBody. +type ResponseBody struct { + AMap map[string]Pong `json:"a_map,omitempty"` + ASlice []Pong `json:"a_slice,omitempty"` + ASliceWithAdditionalProps []map[string]Pong `json:"a_slice_with_additional_props,omitempty"` + AdditionalProps map[string]Pong `json:"additional_props,omitempty"` + Bytes []byte `json:"bytes,omitempty"` + BytesWithOverride *[]byte `json:"bytes_with_override,omitempty"` + RequiredSlice []Pong `json:"required_slice"` + UnknownObject map[string]interface{} `json:"unknown_object,omitempty"` +} diff --git a/internal/test/issues/issue1561/issue1561_test.go b/internal/test/issues/issue1561/issue1561_test.go new file mode 100644 index 0000000000..0873793feb --- /dev/null +++ b/internal/test/issues/issue1561/issue1561_test.go @@ -0,0 +1,64 @@ +package issue1561 + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestResponseBody_DoesNotHaveOptionalPointerToContainerTypes(t *testing.T) { + pong0 := Pong{ + Ping: "0th", + } + + pong1 := Pong{ + Ping: "1th", + } + + slice := []Pong{ + pong0, + pong1, + } + + m := map[string]Pong{ + "0": pong0, + "1": pong1, + } + + byteData := []byte("some bytes") + + body := ResponseBody{ + RequiredSlice: slice, + ASlice: slice, + AMap: m, + UnknownObject: map[string]any{}, + AdditionalProps: m, + ASliceWithAdditionalProps: []map[string]Pong{m}, + Bytes: byteData, + BytesWithOverride: &byteData, + } + + assert.NotNil(t, body.RequiredSlice) + assert.NotZero(t, body.RequiredSlice) + + assert.NotNil(t, body.ASlice) + assert.NotZero(t, body.ASlice) + + assert.NotNil(t, body.AMap) + assert.NotZero(t, body.AMap) + + assert.NotNil(t, body.UnknownObject) + assert.Empty(t, body.UnknownObject) + + assert.NotNil(t, body.AdditionalProps) + assert.NotZero(t, body.AdditionalProps) + + assert.NotNil(t, body.ASliceWithAdditionalProps) + assert.NotZero(t, body.ASliceWithAdditionalProps) + + assert.NotNil(t, body.Bytes) + assert.NotZero(t, body.Bytes) + + assert.NotNil(t, body.BytesWithOverride) + assert.NotZero(t, body.BytesWithOverride) +} diff --git a/internal/test/issues/issue1561/openapi.yaml b/internal/test/issues/issue1561/openapi.yaml new file mode 100644 index 0000000000..4e5cf771c0 --- /dev/null +++ b/internal/test/issues/issue1561/openapi.yaml @@ -0,0 +1,49 @@ +openapi: "3.0.0" +info: + version: 1.0.0 + title: "When using `prefer-skip-optional-pointer-on-container-types`, container types do not have an 'optional pointer'" +paths: +components: + schemas: + ResponseBody: + type: object + required: + - required_slice + properties: + required_slice: + type: array + items: + $ref: '#/components/schemas/Pong' + a_slice: + type: array + items: + $ref: '#/components/schemas/Pong' + a_map: + additionalProperties: + $ref: '#/components/schemas/Pong' + unknown_object: + type: object + additional_props: + type: object + additionalProperties: + $ref: '#/components/schemas/Pong' + a_slice_with_additional_props: + type: array + items: + additionalProperties: + $ref: '#/components/schemas/Pong' + bytes: + type: string + format: byte + bytes_with_override: + type: string + format: byte + x-go-type-skip-optional-pointer: false + Pong: + type: object + required: + - ping + properties: + ping: + type: string + example: pong diff --git a/pkg/codegen/configuration.go b/pkg/codegen/configuration.go index bcafb476fa..5608e227cd 100644 --- a/pkg/codegen/configuration.go +++ b/pkg/codegen/configuration.go @@ -289,6 +289,9 @@ type OutputOptions struct { // PreferSkipOptionalPointer allows defining at a global level whether to omit the pointer for a type to indicate that the field/type is optional. // This is the same as adding `x-go-type-skip-optional-pointer` to each field (manually, or using an OpenAPI Overlay) PreferSkipOptionalPointer bool `yaml:"prefer-skip-optional-pointer,omitempty"` + + // PreferSkipOptionalPointerOnContainerTypes allows disabling the generation of an "optional pointer" for an optional field that is a container type (such as a slice or a map), which ends up requiring an additional, unnecessary, `... != nil` check + PreferSkipOptionalPointerOnContainerTypes bool `yaml:"prefer-skip-optional-pointer-on-container-types,omitempty"` } func (oo OutputOptions) Validate() map[string]string { diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index dbe22f8055..7d061bc7d8 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -327,6 +327,7 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // We have an object with no properties. This is a generic object // expressed as a map. outType = "map[string]interface{}" + setSkipOptionalPointerForContainerType(&outSchema) } else { // t == "" // If we don't even have the object designator, we're a completely // generic type. @@ -389,6 +390,7 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // since we don't need them for a simple map. outSchema.HasAdditionalProperties = false outSchema.GoType = fmt.Sprintf("map[string]%s", additionalPropertiesType(outSchema)) + setSkipOptionalPointerForContainerType(&outSchema) return outSchema, nil } @@ -584,6 +586,7 @@ func oapiSchemaToGoType(schema *openapi3.Schema, path []string, outSchema *Schem if sliceContains(globalState.options.OutputOptions.DisableTypeAliasesForType, "array") { outSchema.DefineViaAlias = false } + setSkipOptionalPointerForContainerType(outSchema) } else if t.Is("integer") { // We default to int if format doesn't ask for something else. @@ -625,6 +628,7 @@ func oapiSchemaToGoType(schema *openapi3.Schema, path []string, outSchema *Schem switch f { case "byte": outSchema.GoType = "[]byte" + setSkipOptionalPointerForContainerType(outSchema) case "email": outSchema.GoType = "openapi_types.Email" case "date": @@ -892,3 +896,14 @@ func generateUnion(outSchema *Schema, elements openapi3.SchemaRefs, discriminato return nil } + +// setSkipOptionalPointerForContainerType ensures that the "optional pointer" is skipped on container types (such as a slice or a map). +// This is controlled using the `prefer-skip-optional-pointer-on-container-types` Output Option +// NOTE that it is still possible to override this on a per-field basis with `x-go-type-skip-optional-pointer` +func setSkipOptionalPointerForContainerType(outSchema *Schema) { + if !globalState.options.OutputOptions.PreferSkipOptionalPointerOnContainerTypes { + return + } + + outSchema.SkipOptionalPointer = true +}