OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.91k stars 6.58k forks source link

[BUG][GO] Bug generating `Get<name>Ok` method when using `$ref` #11848

Open maros7 opened 2 years ago

maros7 commented 2 years ago

Bug Report Checklist

Description

Get<name>Ok() (<type>, bool) methods are generated incorrectly when using $ref. See example below.

https://github.com/OpenAPITools/openapi-generator/blob/v5.3.1/modules/openapi-generator/src/main/resources/go/model_simple.mustache#L124-L138

https://github.com/OpenAPITools/openapi-generator/blame/4a36be70025e9c0d3ff61731618b7fc2d942c4b6/modules/openapi-generator/src/main/resources/go/model_simple.mustache#L124-L138

openapi-generator version

v5.4.0 (works in e.g. v5.3.1)

OpenAPI declaration file content or url
{
  "openapi": "3.0.3",
  "info": {
    "title": "Bug",
    "version": "1.0"
  },
  "paths": {},
  "components": {
    "schemas": {
      "message": {
        "properties": {
          "header": {
            "$ref": "#/components/schemas/header",
            "description": "Some description",
            "type": "object"
          }
        },
        "required": [
          "header"
        ],
        "title": "Message",
        "type": "object"
      },
      "header": {
        "default": {},
        "description": "Some description",
        "properties": {
          "destination": {
            "default": "",
            "description": "Some description",
            "title": "destination",
            "type": "string"
          },
          "source": {
            "default": "",
            "description": "Some description",
            "title": "source",
            "type": "string"
          }
        },
        "required": [
          "source",
          "destination"
        ],
        "title": "header",
        "type": "object"
      }
    }
  }
}
Generation Details
$ cd /tmp; ls local # put provided example schema in /tmp/local
bug.json
$ for version in v5.3.1 v5.4.0 latest; do
  docker run --rm -v "/tmp/local:/local" openapitools/openapi-generator-cli:$version generate \
            -i /local/bug.json \
            -g go \
            -o /local/$version \
            --package-name bug \
            --global-property models,modelDocs=false \
            --additional-properties=disallowAdditionalPropertiesIfNotPresent=false \
            --skip-validate-spec
done
$ ls local
bug.json latest   v5.3.1   v5.4.0
$ cd local
$ go mod init bug
$ go build ./...
# bug/latest
latest/model_message.go:57:3: cannot use nil as type Header in return argument
# bug/v5.4.0
v5.4.0/model_message.go:57:3: cannot use nil as type Header in return argument
Steps to reproduce
for version in v5.3.1 v5.4.0 latest; do
  docker run --rm -v "/tmp/local:/local" openapitools/openapi-generator-cli:$version generate \
            -i /local/bug.json \
            -g go \
            -o /local/$version \
            --package-name bug \
            --global-property models,modelDocs=false \
            --additional-properties=disallowAdditionalPropertiesIfNotPresent=false \
            --skip-validate-spec
done
--- local/v5.3.1/model_message.go       2022-03-10 09:21:32.000000000 +0100
+++ local/v5.4.0/model_message.go       2022-03-10 09:21:35.000000000 +0100
@@ -52,11 +52,11 @@

 // GetHeaderOk returns a tuple with the Header field value
 // and a boolean to check if the value has been set.
-func (o *Message) GetHeaderOk() (*Header, bool) {
+func (o *Message) GetHeaderOk() (Header, bool) {
        if o == nil  {
                return nil, false
        }
-       return &o.Header, true
+       return o.Header, true
 }

 // SetHeader sets field value
--- local/v5.3.1/model_message.go       2022-03-10 09:21:32.000000000 +0100
+++ local/latest/model_message.go       2022-03-10 09:21:37.000000000 +0100
@@ -52,11 +52,11 @@

 // GetHeaderOk returns a tuple with the Header field value
 // and a boolean to check if the value has been set.
-func (o *Message) GetHeaderOk() (*Header, bool) {
-       if o == nil  {
+func (o *Message) GetHeaderOk() (Header, bool) {
+       if o == nil {
                return nil, false
        }
-       return &o.Header, true
+       return o.Header, true
 }

 // SetHeader sets field value
Related issues/PRs

Seems to be introduced in #8491.

Suggest a fix

Perhaps?

diff --git a/modules/openapi-generator/src/main/resources/go/model_simple.mustache b/modules/openapi-generator/src/main/resources/go/model_simple.mustache
index 7a9b2565444..f0c685a0e1f 100644
--- a/modules/openapi-generator/src/main/resources/go/model_simple.mustache
+++ b/modules/openapi-generator/src/main/resources/go/model_simple.mustache
@@ -123,7 +123,13 @@ func (o *{{classname}}) Get{{name}}() {{vendorExtensions.x-go-base-type}} {
 {{/deprecated}}
 func (o *{{classname}}) Get{{name}}Ok() ({{^isArray}}{{^isFreeFormObject}}*{{/isFreeFormObject}}{{/isArray}}{{vendorExtensions.x-go-base-type}}, bool) {
        if o == nil{{#isNullable}}{{#vendorExtensions.x-golang-is-container}} || o.{{name}} == nil{{/vendorExtensions.x-golang-is-container}}{{/isNullable}} {
+{{#isNullable}}
                return nil, false
+{{/isNullable}}
+{{^isNullable}}
+               var ret {{^isArray}}{{^isFreeFormObject}}*{{/isFreeFormObject}}{{/isArray}}{{vendorExtensions.x-go-base-type}}
+               return ret, false
+{{/isNullable}}
        }
 {{#isNullable}}
 {{#vendorExtensions.x-golang-is-container}}
maros7 commented 2 years ago

Albeit, I don't really understand why the Get<name>Ok needs to return a pointer at all when the field is a non-pointer. Would be more Go idiomatic just to return the actual/zero value.