diff --git a/configuration-schema.json b/configuration-schema.json index 9047914479..acb348d131 100644 --- a/configuration-schema.json +++ b/configuration-schema.json @@ -110,7 +110,11 @@ }, "preserve-original-operation-id-casing-in-embedded-spec": { "type": "boolean", - "description": "When `oapi-codegen` parses the original OpenAPI specification, it will apply the configured `output-options.name-normalizer` to each operation's `operationId` before that is used to generate code from.\nHowever, this is also applied to the copy of the `operationId`s in the `embedded-spec` generation, which means that the embedded OpenAPI specification is then out-of-sync with the input specificiation.\nTo ensure that the `operationId` in the embedded spec is preserved as-is from the input specification, set this. NOTE that this will not impact generated code.\nNOTE that if you're using `include-operation-ids` or `exclude-operation-ids` you may want to ensure that the `operationId`s used are correct." + "description": "When `oapi-codegen` parses the original OpenAPI specification, it will apply the configured `output-options.name-normalizer` to each operation's `operationId` before that is used to generate code from.\nHowever, this is also applied to the copy of the `operationId`s in the `embedded-spec` generation, which means that the embedded OpenAPI specification is then out-of-sync with the input specification.\nTo ensure that the `operationId` in the embedded spec is preserved as-is from the input specification, set this. NOTE that this will not impact generated code.\nNOTE that if you're using `include-operation-ids` or `exclude-operation-ids` you may want to ensure that the `operationId`s used are correct." + }, + "enum-server-variables-conflict": { + "type": "boolean", + "description": "Enum server variable values can generate conflicting typenames when `default` is used as a value and there's a default value. Set to `true` to avoid such conflicts. Caveat, it will result in some enum types being renamed in existing code. Please see: Please see https://github.com/oapi-codegen/oapi-codegen/issues/2003." } } }, diff --git a/examples/generate/serverurls/api.yaml b/examples/generate/serverurls/api.yaml index 48695165ba..3620944d14 100644 --- a/examples/generate/serverurls/api.yaml +++ b/examples/generate/serverurls/api.yaml @@ -28,12 +28,11 @@ servers: default: v2 # an example of a type that's defined, but doesn't have a default noDefault: {} - # # TODO this conflict will cause broken generated code https://github.com/oapi-codegen/oapi-codegen/issues/2003 - # conflicting: - # enum: - # - 'default' - # - '443' - # default: 'default' + conflicting: + enum: + - 'default' + - '443' + default: 'default' # clash with the previous definition of `Development server` to trigger a new name - url: http://localhost:80 description: Development server diff --git a/examples/generate/serverurls/cfg.yaml b/examples/generate/serverurls/cfg.yaml index 8815cb6a68..22f052c266 100644 --- a/examples/generate/serverurls/cfg.yaml +++ b/examples/generate/serverurls/cfg.yaml @@ -1,6 +1,8 @@ # yaml-language-server: $schema=../../../configuration-schema.json package: serverurls output: gen.go +compatibility: + enum-server-variables-conflict: true generate: server-urls: true output-options: diff --git a/examples/generate/serverurls/gen.go b/examples/generate/serverurls/gen.go index 5f9e069d3e..e40d6e0f4f 100644 --- a/examples/generate/serverurls/gen.go +++ b/examples/generate/serverurls/gen.go @@ -35,20 +35,32 @@ type ServerUrlTheProductionAPIServerBasePathVariable string // ServerUrlTheProductionAPIServerBasePathVariableDefault is the default value for the `basePath` variable for ServerUrlTheProductionAPIServer const ServerUrlTheProductionAPIServerBasePathVariableDefault = "v2" +// ServerUrlTheProductionAPIServerConflictingVariable is the `conflicting` variable for ServerUrlTheProductionAPIServer +type ServerUrlTheProductionAPIServerConflictingVariable string + +// ServerUrlTheProductionAPIServerConflictingVariableEnumDefault is one of the accepted values for the `conflicting` variable for ServerUrlTheProductionAPIServer +const ServerUrlTheProductionAPIServerConflictingVariableEnumDefault ServerUrlTheProductionAPIServerConflictingVariable = "default" + +// ServerUrlTheProductionAPIServerConflictingVariableEnum443 is one of the accepted values for the `conflicting` variable for ServerUrlTheProductionAPIServer +const ServerUrlTheProductionAPIServerConflictingVariableEnum443 ServerUrlTheProductionAPIServerConflictingVariable = "443" + +// ServerUrlTheProductionAPIServerConflictingVariableDefault is the default choice, for the accepted values for the `conflicting` variable for ServerUrlTheProductionAPIServer +const ServerUrlTheProductionAPIServerConflictingVariableDefault ServerUrlTheProductionAPIServerConflictingVariable = ServerUrlTheProductionAPIServerConflictingVariableEnumDefault + // ServerUrlTheProductionAPIServerNoDefaultVariable is the `noDefault` variable for ServerUrlTheProductionAPIServer type ServerUrlTheProductionAPIServerNoDefaultVariable string // ServerUrlTheProductionAPIServerPortVariable is the `port` variable for ServerUrlTheProductionAPIServer type ServerUrlTheProductionAPIServerPortVariable string -// ServerUrlTheProductionAPIServerPortVariable8443 is one of the accepted values for the `port` variable for ServerUrlTheProductionAPIServer -const ServerUrlTheProductionAPIServerPortVariable8443 ServerUrlTheProductionAPIServerPortVariable = "8443" +// ServerUrlTheProductionAPIServerPortVariableEnum8443 is one of the accepted values for the `port` variable for ServerUrlTheProductionAPIServer +const ServerUrlTheProductionAPIServerPortVariableEnum8443 ServerUrlTheProductionAPIServerPortVariable = "8443" -// ServerUrlTheProductionAPIServerPortVariable443 is one of the accepted values for the `port` variable for ServerUrlTheProductionAPIServer -const ServerUrlTheProductionAPIServerPortVariable443 ServerUrlTheProductionAPIServerPortVariable = "443" +// ServerUrlTheProductionAPIServerPortVariableEnum443 is one of the accepted values for the `port` variable for ServerUrlTheProductionAPIServer +const ServerUrlTheProductionAPIServerPortVariableEnum443 ServerUrlTheProductionAPIServerPortVariable = "443" // ServerUrlTheProductionAPIServerPortVariableDefault is the default choice, for the accepted values for the `port` variable for ServerUrlTheProductionAPIServer -const ServerUrlTheProductionAPIServerPortVariableDefault ServerUrlTheProductionAPIServerPortVariable = ServerUrlTheProductionAPIServerPortVariable8443 +const ServerUrlTheProductionAPIServerPortVariableDefault ServerUrlTheProductionAPIServerPortVariable = ServerUrlTheProductionAPIServerPortVariableEnum8443 // ServerUrlTheProductionAPIServerUsernameVariable is the `username` variable for ServerUrlTheProductionAPIServer type ServerUrlTheProductionAPIServerUsernameVariable string @@ -57,10 +69,12 @@ type ServerUrlTheProductionAPIServerUsernameVariable string const ServerUrlTheProductionAPIServerUsernameVariableDefault = "demo" // NewServerUrlTheProductionAPIServer constructs the Server URL for The production API server, with the provided variables. -func NewServerUrlTheProductionAPIServer(basePath ServerUrlTheProductionAPIServerBasePathVariable, noDefault ServerUrlTheProductionAPIServerNoDefaultVariable, port ServerUrlTheProductionAPIServerPortVariable, username ServerUrlTheProductionAPIServerUsernameVariable) (string, error) { +func NewServerUrlTheProductionAPIServer(basePath ServerUrlTheProductionAPIServerBasePathVariable, conflicting ServerUrlTheProductionAPIServerConflictingVariable, noDefault ServerUrlTheProductionAPIServerNoDefaultVariable, port ServerUrlTheProductionAPIServerPortVariable, username ServerUrlTheProductionAPIServerUsernameVariable) (string, error) { u := "https://{username}.gigantic-server.com:{port}/{basePath}" u = strings.ReplaceAll(u, "{basePath}", string(basePath)) + // TODO in the future, this will validate that the value is part of the ServerUrlTheProductionAPIServerConflictingVariable enum + u = strings.ReplaceAll(u, "{conflicting}", string(conflicting)) u = strings.ReplaceAll(u, "{noDefault}", string(noDefault)) // TODO in the future, this will validate that the value is part of the ServerUrlTheProductionAPIServerPortVariable enum u = strings.ReplaceAll(u, "{port}", string(port)) diff --git a/examples/generate/serverurls/gen_test.go b/examples/generate/serverurls/gen_test.go index 2a2ecfb41a..010bb1bb3e 100644 --- a/examples/generate/serverurls/gen_test.go +++ b/examples/generate/serverurls/gen_test.go @@ -10,7 +10,7 @@ import ( func TestServerUrlTheProductionAPIServer(t *testing.T) { t.Run("when no values are provided, it does not error", func(t *testing.T) { - serverUrl, err := NewServerUrlTheProductionAPIServer("", "", "", "") + serverUrl, err := NewServerUrlTheProductionAPIServer("", "", "", "", "") require.NoError(t, err) assert.Equal(t, "https://.gigantic-server.com:/", serverUrl) @@ -25,6 +25,7 @@ func TestServerUrlTheProductionAPIServer(t *testing.T) { invalidPort := ServerUrlTheProductionAPIServerPortVariable("12345") serverUrl, err := NewServerUrlTheProductionAPIServer( ServerUrlTheProductionAPIServerBasePathVariableDefault, + ServerUrlTheProductionAPIServerConflictingVariableDefault, ServerUrlTheProductionAPIServerNoDefaultVariable(""), invalidPort, ServerUrlTheProductionAPIServerUsernameVariableDefault, @@ -37,6 +38,7 @@ func TestServerUrlTheProductionAPIServer(t *testing.T) { t.Run("when default values are provided, it does not error", func(t *testing.T) { serverUrl, err := NewServerUrlTheProductionAPIServer( ServerUrlTheProductionAPIServerBasePathVariableDefault, + ServerUrlTheProductionAPIServerConflictingVariableDefault, ServerUrlTheProductionAPIServerNoDefaultVariable(""), ServerUrlTheProductionAPIServerPortVariableDefault, ServerUrlTheProductionAPIServerUsernameVariableDefault, diff --git a/pkg/codegen/codegen.go b/pkg/codegen/codegen.go index 6e602ea05a..986f4e1f2b 100644 --- a/pkg/codegen/codegen.go +++ b/pkg/codegen/codegen.go @@ -111,9 +111,14 @@ func constructImportMapping(importMapping map[string]string) importMap { return result } -// Generate uses the Go templating engine to generate all of our server wrappers from -// the descriptions we've built up above from the schema objects. -// opts defines +// Generate creates all server wrappers and related code using the Go templating engine. +// It processes the provided OpenAPI schema (spec) and generation options (opts), producing code for +// clients, servers, models, and other components as specified in opts. +// +// Parameters: +// +// spec - the OpenAPI specification describing the API schema +// opts - options controlling what code to generate (client, server, etc.) func Generate(spec *openapi3.T, opts Configuration) (string, error) { // This is global state globalState.options = opts @@ -204,7 +209,7 @@ func Generate(spec *openapi3.T, opts Configuration) (string, error) { var serverURLsDefinitions string if opts.Generate.ServerURLs { - serverURLsDefinitions, err = GenerateServerURLs(t, spec) + serverURLsDefinitions, err = GenerateServerURLs(t, spec, globalState.options.Compatibility.EnumServerVariablesConflict) if err != nil { return "", fmt.Errorf("error generating Server URLs: %w", err) } diff --git a/pkg/codegen/configuration.go b/pkg/codegen/configuration.go index 1d9ff3eaea..8148667244 100644 --- a/pkg/codegen/configuration.go +++ b/pkg/codegen/configuration.go @@ -234,6 +234,14 @@ type CompatibilityOptions struct { // NOTE that this will not impact generated code. // NOTE that if you're using `include-operation-ids` or `exclude-operation-ids` you may want to ensure that the `operationId`s used are correct. PreserveOriginalOperationIdCasingInEmbeddedSpec bool `yaml:"preserve-original-operation-id-casing-in-embedded-spec"` + + // EnumServerVariablesConflict controls whether the code generator should handle conflicts + // between enum values and server variable names. When set to true, the generator will + // apply logic to avoid naming collisions between enum types and server variables in the + // generated code. + // + // Corresponds to the `enum-server-variables-conflict` property in the configuration schema. + EnumServerVariablesConflict bool `yaml:"enum-server-variables-conflict,omitempty"` } func (co CompatibilityOptions) Validate() map[string]string { diff --git a/pkg/codegen/server_urls.go b/pkg/codegen/server_urls.go index d10c2d9f60..71a4fea74a 100644 --- a/pkg/codegen/server_urls.go +++ b/pkg/codegen/server_urls.go @@ -18,9 +18,12 @@ type ServerObjectDefinition struct { // OAPISchema is the underlying OpenAPI representation of the Server OAPISchema *openapi3.Server + + // EnumServerVariablesConflict indicates whether the server variables conflict should be avoided + EnumServerVariablesConflict bool } -func GenerateServerURLs(t *template.Template, spec *openapi3.T) (string, error) { +func GenerateServerURLs(t *template.Template, spec *openapi3.T, enumServerVariablesConflict bool) (string, error) { names := make(map[string]*openapi3.Server) for _, server := range spec.Servers { @@ -71,8 +74,9 @@ func GenerateServerURLs(t *template.Template, spec *openapi3.T) (string, error) i := 0 for _, k := range keys { servers[i] = ServerObjectDefinition{ - GoName: k, - OAPISchema: names[k], + GoName: k, + OAPISchema: names[k], + EnumServerVariablesConflict: enumServerVariablesConflict, } i++ } diff --git a/pkg/codegen/templates/server-urls.tmpl b/pkg/codegen/templates/server-urls.tmpl index f3599e5fa6..03db31809e 100644 --- a/pkg/codegen/templates/server-urls.tmpl +++ b/pkg/codegen/templates/server-urls.tmpl @@ -4,37 +4,45 @@ // {{ .GoName }} defines the Server URL for {{ .OAPISchema.Description }} const {{ .GoName}} = "{{ .OAPISchema.URL }}" {{ else }} -{{/* URLs with variables are not straightforward, as we may need multiple types, and so will model them as a function */}} - -{{/* first, we'll start by generating requisite types */}} - +{{/* + URLs with variables are not straightforward, as we may need multiple types, and so will model them as a function + first, we'll start by generating requisite types +*/}} {{ $goName := .GoName }} +{{ $enumServerVariablesConflict := .EnumServerVariablesConflict }} + {{ range $k, $v := .OAPISchema.Variables }} - {{ $prefix := printf "%s%sVariable" $goName ($k | ucFirst) }} - // {{ $prefix }} is the `{{ $k }}` variable for {{ $goName }} - type {{ $prefix }} string - {{ range $v.Enum }} - {{/* TODO this may result in broken generated code if any of the `enum` values are the literal value `default` https://github.com/oapi-codegen/oapi-codegen/issues/2003 */}} - // {{ $prefix }}{{ . | ucFirst }} is one of the accepted values for the `{{ $k }}` variable for {{ $goName }} - const {{ $prefix }}{{ . | ucFirst }} {{ $prefix }} = "{{ . }}" - {{ end }} - - {{/* TODO we should introduce a `Valid() error` method to enums https://github.com/oapi-codegen/oapi-codegen/issues/2006 */}} - - {{ if $v.Default }} - {{ if gt (len $v.Enum) 0 }} - {{/* if we have an enum, we should use the type defined for it for its default value - and reference the constant we've already defined for the value */}} - {{/* TODO this may result in broken generated code if any of the `enum` values are the literal value `default` https://github.com/oapi-codegen/oapi-codegen/issues/2003 */}} - {{/* TODO this may result in broken generated code if the `default` isn't found in `enum` (which is an issue with the spec) https://github.com/oapi-codegen/oapi-codegen/issues/2007 */}} - // {{ $prefix }}Default is the default choice, for the accepted values for the `{{ $k }}` variable for {{ $goName }} - const {{ $prefix }}Default {{ $prefix }} = {{ $prefix }}{{ $v.Default | ucFirst }} - {{ else }} - // {{ $prefix }}Default is the default value for the `{{ $k }}` variable for {{ $goName }} - const {{ $prefix }}Default = "{{ $v.Default }}" - {{ end }} - {{ end }} -{{ end }} +{{ $prefix := printf "%s%sVariable" $goName ($k | ucFirst) }} +{{ $enumVarPrefix := $prefix }} +{{ if $enumServerVariablesConflict }} +{{ $enumVarPrefix = printf "%sEnum" $prefix }} +{{ end }} {{/* if $enumServerVariablesConflict */}} + +// {{ $prefix }} is the `{{ $k }}` variable for {{ $goName }} +type {{ $prefix }} string + +{{ range $v.Enum }} +// {{ $enumVarPrefix }}{{ . | ucFirst }} is one of the accepted values for the `{{ $k }}` variable for {{ $goName }} +const {{ $enumVarPrefix }}{{ . | ucFirst }} {{ $prefix }} = "{{ . }}" +{{ end }} {{/* range $v.Enum */}} + +{{/* TODO we should introduce a `Valid() error` method to enums https://github.com/oapi-codegen/oapi-codegen/issues/2006 */}} + +{{ if $v.Default }} +{{ if gt (len $v.Enum) 0 }} +{{/* + if we have an enum, we should use the type defined for it for its default value + and reference the constant we've already defined for the value +*/}} +{{/* TODO this may result in broken generated code if the `default` isn't found in `enum` (which is an issue with the spec) https://github.com/oapi-codegen/oapi-codegen/issues/2007 */}} +// {{ $prefix }}Default is the default choice, for the accepted values for the `{{ $k }}` variable for {{ $goName }} +const {{ $prefix }}Default {{ $prefix }} = {{ $enumVarPrefix }}{{ $v.Default | ucFirst }} +{{ else }} +// {{ $prefix }}Default is the default value for the `{{ $k }}` variable for {{ $goName }} +const {{ $prefix }}Default = "{{ $v.Default }}" +{{ end }} {{/* if gt (len $v.Enum) 0 */}} +{{ end }} {{/* if $v.Default */}} +{{ end }} {{/* range $k, $v := .OAPISchema.Variables */}} // New{{ .GoName }} constructs the Server URL for {{ .OAPISchema.Description }}, with the provided variables. @@ -42,20 +50,20 @@ func New{{ .GoName }}({{ genServerURLWithVariablesFunctionParams .GoName .OAPISc u := "{{ .OAPISchema.URL }}" {{ range $k, $v := .OAPISchema.Variables }} - {{- $placeholder := printf "{%s}" $k -}} - {{- if gt (len $v.Enum) 0 -}} - {{/* TODO https://github.com/oapi-codegen/oapi-codegen/issues/2006 */}} - // TODO in the future, this will validate that the value is part of the {{ printf "%s%sVariable" $goName ($k | ucFirst) }} enum - {{ end -}} - u = strings.ReplaceAll(u, "{{ $placeholder }}", string({{ $k }})) - {{ end }} + {{- $placeholder := printf "{%s}" $k }} + {{- if gt (len $v.Enum) 0 -}} + {{/* TODO https://github.com/oapi-codegen/oapi-codegen/issues/2006 */}} + // TODO in the future, this will validate that the value is part of the {{ printf "%s%sVariable" $goName ($k | ucFirst) }} enum + {{- end }} {{/* if gt (len $v.Enum) 0 */}} + u = strings.ReplaceAll(u, "{{ $placeholder }}", string({{ $k }})) + {{- end }} {{/* range $k, $v := .OAPISchema.Variables */}} if strings.Contains(u, "{") || strings.Contains(u, "}") { - return "", fmt.Errorf("after mapping variables, there were still `{` or `}` characters in the string: %#v", u) + return "", fmt.Errorf("after mapping variables, there were still `{` or `}` characters in the string: %#v", u) } return u, nil } -{{ end }} -{{ end }} +{{ end }} {{/* if eq 0 (len .OAPISchema.Variables) */}} +{{ end }} {{/* range . */}}