fix(output-options): correctly reference `x-go-type-skip-optional-pointer` in `allOf` #2042

magnetikonline · 2025-07-23T02:06:42Z

I've noted a regression as part of #2021.

Previously, since Schema{}.SkipOptionalPointer was never set, it always defaulted to false, which suited our use case.

With the change as part of #2021 and in the new https://github.com/oapi-codegen/oapi-codegen/releases/tag/v2.5.0 release - this now uses the global output option setting of prefer-skip-optional-pointer: in all cases - and can't be over-ridden locally within a type.

This change fixes that behaviour, with x-go-type-skip-optional-pointer checked and evaluated up front for the type/referenced type - meaning a schema blob such as below will now respect the x-go-type-skip-optional-pointer: false value where referenced against apples|oranges and of course fallback to the global output option if unset:

Example:

MyAction:
  type: object
  additionalProperties: false
  required:
    - description
  properties:
    description:
      type: string
      description: Your reasons for doing this.
      example: We need more fruit!
    apples:
      description: Hello apples.
      allOf:
        - $ref: '#/components/schemas/Referenced'
    oranges:
      description: Hello oranges.
      allOf:
        - $ref: '#/components/schemas/Referenced'

Referenced:
  type: object
  description: Please reference me.
  additionalProperties: false
  x-go-type-skip-optional-pointer: false

kusari-inspector · 2025-07-23T02:07:11Z

Kusari Analysis Results

Analysis for commit: bd7226e, performed at: 2025-07-25T17:16:20Z

• @kusari-inspector rerun - Trigger a re-analysis of this PR

• @kusari-inspector feedback [your message] - Send feedback to our AI and team

Recommendation

✅ PROCEED with this Pull Request

Summary

✅ No Flagged Issues Detected

All values appear to be within acceptable risk parameters.

No pinned version dependency changes, code issues or exposed secrets detected!

Found this helpful? Give it a 👍 or 👎 reaction!

…r` defined on referenced types

kusari-inspector · 2025-07-23T02:19:35Z

Kusari PR Analysis rerun based on - 42d87c7 performed at: 2025-07-23T02:19:13Z - link to updated analysis

jamietanna · 2025-07-23T07:43:58Z

Hey @magnetikonline thanks for raising this!

Taking a look at the example you've shared in the body of the PR, I can't seem to make this fail on main, before these changes are applied.

On `main`

If I make the changes:

diff --git examples/output-options/preferskipoptionalpointer/api.yaml examples/output-options/preferskipoptionalpointer/api.yaml
index 0395e89..684432d 100644
--- examples/output-options/preferskipoptionalpointer/api.yaml
+++ examples/output-options/preferskipoptionalpointer/api.yaml
@@ -37,3 +37,26 @@ components:
         client:
           description: This field is optional, but the `prefer-skip-optional-pointer` Output Option ensures that this should not have an optional pointer.
           $ref: '#/components/schemas/Client'
+
+    MyAction:
+      type: object
+      additionalProperties: false
+      required:
+        - description
+      properties:
+        description:
+          type: string
+          description: Your reasons for doing this.
+          example: We need more fruit!
+        apples:
+          description: Hello apples.
+          $ref: '#/components/schemas/Referenced'
+        oranges:
+          description: Hello oranges.
+          $ref: '#/components/schemas/Referenced'
+
+    Referenced:
+      type: object
+      description: Please reference me.
+      additionalProperties: false
+      x-go-type-skip-optional-pointer: false

This then generates the code:

diff --git examples/output-options/preferskipoptionalpointer/gen.go examples/output-options/preferskipoptionalpointer/gen.go
index 0d6a9fd..9c3c270 100644
--- examples/output-options/preferskipoptionalpointer/gen.go
+++ examples/output-options/preferskipoptionalpointer/gen.go
@@ -24,7 +24,22 @@ type ClientWithExtension struct {
        PointerId *float32 `json:"pointer_id,omitempty"`
 }
 
+// MyAction defines model for MyAction.
+type MyAction struct {
+       // Apples Please reference me.
+       Apples *Referenced `json:"apples,omitempty"`
+
+       // Description Your reasons for doing this.
+       Description string `json:"description"`
+
+       // Oranges Please reference me.
+       Oranges *Referenced `json:"oranges,omitempty"`
+}
+
 // NestedType defines model for NestedType.
 type NestedType struct {
        Client Client `json:"client,omitempty"`
 }
+
+// Referenced Please reference me.
+type Referenced = map[string]interface{}

With this PR

If I then use the changes in this PR, this doesn't end up changing anything 🤔 Mind taking a look?

magnetikonline · 2025-07-23T07:50:31Z

Thanks @jamietanna will do.

Things are certainly working as I'd expect with our internal schemas (which of course I can't post here) - but I'll get the example into a state which trips the change I'm trying to make. 👍

Cheers.

kusari-inspector · 2025-07-23T07:54:38Z

Kusari PR Analysis rerun based on - 201bd1e performed at: 2025-07-23T07:54:13Z - link to updated analysis

jamietanna · 2025-07-23T07:55:04Z

NP, thanks! I've pushed a (failing) change for the example above, which flags that the example you'd shared doesn't seem to work right now

(I'm very happy to collaborate on this, or if you'd prefer I move the fix for the issue I've just pushed to a separate PR)

magnetikonline · 2025-07-24T02:19:37Z

Hey @jamietanna - was having a brain fart yesterday trying to work this out 😄 - with fresh eyes today and reviewing our internal schemas - I've been able to produce an example that shows drift between the prior v2.5.0 release, v2.5.0 and what this PR will remediate.

It's when the referenced type is a child of an allOf / anyOf / etc.

As a diff to your update:

diff --git examples/output-options/preferskipoptionalpointer/api.yaml examples/output-options/preferskipoptionalpointer/api.yaml
index 369d281..8bb64c8 100644
--- examples/output-options/preferskipoptionalpointer/api.yaml
+++ examples/output-options/preferskipoptionalpointer/api.yaml
@@ -51,7 +51,8 @@ components:
           # TODO
           $ref: '#/components/schemas/ReferencedWithoutExtensionMap'
         withExtension:
-          $ref: '#/components/schemas/ReferencedWithExtension'
+          allOf:
+            - $ref: '#/components/schemas/ReferencedWithExtension'

     ReferencedWithExtension:
       type: object

This will give different behaviour between this PR and the v2.5.0 release - which I've tested.

To expand as to why?

Pre v2.5.0

oapi-codegen/pkg/codegen/schema.go

Lines 272 to 278 in e93501d

    
           	return Schema{ 
        
           		GoType:         refType, 
        
           		Description:    schema.Description, 
        
           		DefineViaAlias: true, 
        
           		OAPISchema:     schema, 
        
           	}, nil 
        
           }

Since Schema{} was never returned with property SkipOptionalPointer set, it was always false - which wasn't quite right, but suited our usage patterns.

Moving to fad490f - now SkipOptionalPointer is set to the global skip pointer setting - totally changing the behavior for these type definitions and the Golang client(s) generated.

This PR will repair both these above cases. I still have to make changes to our schemas with an additional sprinkingling of x-go-type-skip-optional-pointer: false properties - but it is now more "correct" and I've got total control over how the Golang clients are generated.

I've not committed to my branch - I didn't want to clobber your commits - but I hope this should give you a good working example of what this PR fixes. I've also updated the PR description to suit to match the test example. 👍

(I'm very happy to collaborate on this, or if you'd prefer I move the fix for the issue I've just pushed to a separate PR)

nah more than happy to pair on this PR here 👍

jamietanna · 2025-07-24T08:19:44Z

Hey @magnetikonline happy for you to push up to this branch (you could do it as separate commits on top of what's pushed here, as we generally squash-merge external contributions)

Thanks for digging into that, that makes sense

…lpointer` OpenAPI YAML

kusari-inspector · 2025-07-25T02:38:20Z

Kusari PR Analysis rerun based on - 02f8b04 performed at: 2025-07-25T02:37:55Z - link to updated analysis

…mposition

kusari-inspector · 2025-07-25T03:10:11Z

Kusari PR Analysis rerun based on - 6ce0234 performed at: 2025-07-25T03:09:47Z - link to updated analysis

magnetikonline · 2025-07-25T03:12:11Z

Hey @magnetikonline happy for you to push up to this branch (you could do it as separate commits on top of what's pushed here, as we generally squash-merge external contributions)

Thanks @jamietanna - I've tried to keep the tests same/same to the others, I think that's enough to show it does what it proposes on the tin. 👍

Thanks for digging into that, that makes sense

not a problem - happy I was able to explain myself!

kusari-inspector · 2025-07-25T17:10:10Z

Kusari PR Analysis rerun based on - c362f22 performed at: 2025-07-25T17:09:45Z - link to updated analysis

kusari-inspector · 2025-07-25T17:16:42Z

Kusari PR Analysis rerun based on - bd7226e performed at: 2025-07-25T17:16:20Z - link to updated analysis

magnetikonline · 2025-07-26T04:24:53Z

Thanks @jamietanna for your time on this and getting it merged. Most appreciated. 👍

magnetikonline mentioned this pull request Jul 23, 2025

regression in v2.5.0: marshalling optional array property emits null instead of omitting the array #2031

Closed

magnetikonline marked this pull request as ready for review July 23, 2025 02:10

magnetikonline requested a review from a team as a code owner July 23, 2025 02:10

Respect setting of extension property `x-go-type-skip-optional-pointe…

42d87c7

…r` defined on referenced types

magnetikonline changed the title ~~Respect setting of x-go-type-skip-optional-pointeron referenced types~~ Respect setting of x-go-type-skip-optional-pointer on referenced types Jul 23, 2025

fixup! add test case

201bd1e

Add ReferencesATypeWithAnExtension: additions to `preferskipoptiona…

3912499

…lpointer` OpenAPI YAML

Added tests for ReferencesATypeWithAnExtension: generated client co…

6ce0234

…mposition

Jamie Tanna added 3 commits July 25, 2025 18:08

sq

b3aec6d

sq

2d8a348

sq

c362f22

sq

bd7226e

jamietanna changed the title ~~Respect setting of x-go-type-skip-optional-pointer on referenced types~~ fix(output-options): correctly reference x-go-type-skip-optional-pointer in allOf Jul 25, 2025

jamietanna added bug Something isn't working area:*of `oneOf`/`anyOf`/`allOf` labels Jul 25, 2025

jamietanna enabled auto-merge (squash) July 25, 2025 17:21

jamietanna approved these changes Jul 25, 2025

View reviewed changes

jamietanna merged commit 702cc91 into oapi-codegen:main Jul 25, 2025
21 checks passed

Uh oh!

fix(output-options): correctly reference x-go-type-skip-optional-pointer in allOf #2042

fix(output-options): correctly reference x-go-type-skip-optional-pointer in allOf #2042

Uh oh!

Conversation

magnetikonline commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kusari-inspector bot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Kusari Analysis Results

Recommendation

Summary

Uh oh!

kusari-inspector bot commented Jul 23, 2025

Uh oh!

jamietanna commented Jul 23, 2025

On main

With this PR

Uh oh!

magnetikonline commented Jul 23, 2025

Uh oh!

kusari-inspector bot commented Jul 23, 2025

Uh oh!

jamietanna commented Jul 23, 2025

Uh oh!

magnetikonline commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamietanna commented Jul 24, 2025

Uh oh!

kusari-inspector bot commented Jul 25, 2025

Uh oh!

kusari-inspector bot commented Jul 25, 2025

Uh oh!

magnetikonline commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kusari-inspector bot commented Jul 25, 2025

Uh oh!

kusari-inspector bot commented Jul 25, 2025

Uh oh!

Uh oh!

magnetikonline commented Jul 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(output-options): correctly reference `x-go-type-skip-optional-pointer` in `allOf` #2042

fix(output-options): correctly reference `x-go-type-skip-optional-pointer` in `allOf` #2042

magnetikonline commented Jul 23, 2025 •

edited

Loading

kusari-inspector bot commented Jul 23, 2025 •

edited

Loading

On `main`

magnetikonline commented Jul 24, 2025 •

edited

Loading

magnetikonline commented Jul 25, 2025 •

edited

Loading