Skip to content

fix: allow x-go-type and x-go-type-skip-optional-pointer for allOf#1610

Merged
mromaszewicz merged 7 commits into
oapi-codegen:mainfrom
turip:fix-allof-type-override
Apr 29, 2026
Merged

fix: allow x-go-type and x-go-type-skip-optional-pointer for allOf#1610
mromaszewicz merged 7 commits into
oapi-codegen:mainfrom
turip:fix-allof-type-override

Conversation

@turip

@turip turip commented May 14, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Right now if a schema contains x-go-type, for example:

components:
  schemas:
    Pet:
      x-go-type: pet.Pet
      x-go-type-import:
        path: github.com/somepetproject/pkg/pet
      type: object
      required:
        - pet_type
      properties:
        pet_type:
          type: string
      discriminator:
        propertyName: pet_type
    Dog:     # "Dog" is a value for the pet_type property (the discriminator value)
      x-go-type: dog.Dog
      x-go-type-import:
        path: github.com/somepetproject/pkg/dog
      allOf: # Combines the main `Pet` schema with `Dog`-specific properties 
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Dog`
          properties:
            bark:
              type: boolean
            breed:
              type: string
              enum: [Dingo, Husky, Retriever, Shepherd]
    Cat:     # "Cat" is a value for the pet_type property (the discriminator value)
      x-go-type: cat.Cat
      x-go-type-import:
        path: github.com/somepetproject/pkg/cat
      allOf: # Combines the main `Pet` schema with `Cat`-specific properties 
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Cat`
          properties:
            hunts:
              type: boolean
            age:
              type: integer

old behavior

The generated code will still take the pet as the x-go-type:


	"github.com/getkin/kin-openapi/openapi3"
	"github.com/labstack/echo/v4"
	"github.com/somepetproject/pkg/pet"
)

// Cat defines model for Cat.
type Cat = pet.Pet

// Dog defines model for Dog.
type Dog = pet.Pet

// Pet defines model for Pet.
type Pet = pet.Pet

new behavior

This patch makes sure that we are generating the expected output:

	"github.com/somepetproject/pkg/cat"
	"github.com/somepetproject/pkg/dog"
	"github.com/somepetproject/pkg/pet"
)

// Cat defines model for Cat.
type Cat = cat.Cat

// Dog defines model for Dog.
type Dog = dog.Dog

// Pet defines model for Pet.
type Pet = pet.Pet

This is a corner case, as in most cases the same type is used (e.g. splitting the ID from the CreateRequest but using the same underlying object), but in case we would want to compose the objects it would make sense to make this overridable on the parent level.

This patch doesn't introduce a breaking change, as if the x-go-* values are not set, the same inheritance will hapen.

turip added 2 commits May 14, 2024 21:09
@turip
fix: allow x-go-type and x-go-type-skip-optional-pointer for allOf
de857df 
This patch ensures that if a schema is defined using allOf the main
schema entry's x-go-type and x-go-type-skip-optional-pointer values
are considered instead of a random allOf member's.
@turip
Add unit tests
1cb4516
@turip turip mentioned this pull request May 15, 2024
Merged
7 tasks
@jamietanna jamietanna added this to the v2.3.0 milestone Jun 4, 2024
@jamietanna jamietanna modified the milestones: v2.4.0, v2.5.0 Sep 20, 2024
@jamietanna jamietanna requested a review from a team as a code owner November 28, 2024 11:35
@jamietanna jamietanna mentioned this pull request May 9, 2025
Closed
@jamietanna jamietanna modified the milestones: v2.5.0, v2.6.0 Jul 15, 2025
@mromaszewicz

mromaszewicz commented Mar 5, 2026

Copy link
Copy Markdown
Member

@greptileai

@greptile-apps

greptile-apps Bot commented Mar 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes x-go-type and x-go-type-skip-optional-pointer resolution for schemas that combine an outer x-go-type annotation with allOf composition (e.g. discriminator sub-types). The x-go-type case is handled by the existing early-return that already precedes the allOf block; the only net code change is propagating SkipOptionalPointer to the merged schema when the outer schema explicitly sets x-go-type-skip-optional-pointer.

  • The global PreferSkipOptionalPointer default is not applied to allOf-merged schemas unless the outer schema has an explicit extension, unlike every other code path — this inconsistency may silently ignore the global option for allOf schemas.

Confidence Score: 3/5

Merge with caution — a P1 inconsistency means the global PreferSkipOptionalPointer option is silently ignored for allOf schemas unless the author confirms this is intentional.

One P1 finding (global default not propagated to allOf-merged path) brings the ceiling to 4; the missing test coverage for the actual schema.go change further reduces confidence to 3.

pkg/codegen/schema.go — the SkipOptionalPointer propagation block around line 403.

Important Files Changed
Filename Overview
pkg/codegen/schema.go Adds SkipOptionalPointer propagation to the allOf-merge return path; the global PreferSkipOptionalPointer default is not applied when the outer schema lacks an explicit x-go-type-skip-optional-pointer, inconsistent with all other code paths.
pkg/codegen/codegen_test.go New unit test verifies x-go-type override for allOf schemas (Cat/Dog/Pet); does not cover the x-go-type-skip-optional-pointer change that is the actual diff in schema.go.
pkg/codegen/test_specs/x-go-type-pet-allof.yaml New test fixture covering the discriminator/allOf + x-go-type scenario; no x-go-type-skip-optional-pointer usage included.

Reviews (2): Last reviewed commit: "update PR for upstream changes" | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Mar 5, 2026
View reviewed changes
Comment thread pkg/codegen/schema.go Outdated
@mromaszewicz
Merge remote-tracking branch 'upstream/main' into pr/1610
cc6e460
@mromaszewicz
update PR for upstream changes
a479ef0 
Address greptile review feedback and adapt to upstream main:

- pkg/codegen/schema.go: simplify the allOf branch. Upstream main now
  catches x-go-type at the schema level via the combined extensions
  early return, so the PR's allOf-specific x-go-type override block is
  unreachable; remove it. Keep x-go-type-skip-optional-pointer
  propagation as a conditional override (only when the parent sets it
  explicitly), which preserves the decorator-idiom behavior MergeSchemas
  relies on for issue oapi-codegen#1957. Read from combined extensions for parity
  with the rest of GenerateGoSchema.

- pkg/codegen/test_specs/x-go-type-pet-allof.yaml: add trailing newline.
@mromaszewicz

mromaszewicz commented Apr 29, 2026

Copy link
Copy Markdown
Member

@greptileai please review again after latest commits.

@mromaszewicz mromaszewicz added the bug Something isn't working label Apr 29, 2026
greptile-apps[bot]
greptile-apps Bot reviewed Apr 29, 2026
View reviewed changes
Comment thread pkg/codegen/schema.go
@mromaszewicz mromaszewicz merged commit eff4a2b into oapi-codegen:main Apr 29, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mromaszewicz mromaszewicz mromaszewicz approved these changes

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

v2.7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@turip @mromaszewicz @jamietanna