Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make arrays use concrete types, not aliases. #1246

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

jkj
Copy link
Contributor

@jkj jkj commented Sep 8, 2023

Currently when you create an array type using something like:

components:
  schemas:
    my_item:
      type: object
      properties:
        name:
          type: string
        age:
          type: integer
    example:
      type: array
      items:
        $ref: '#/components/schemas/my_item'

the generator produces a type alias for the array:

// Example defines model for example.
type Example = []MyItem

This causes problems, particularly for defining methods. This patch ensures that when arrays are generated, they use a concrete type and not a type alias. For example:

// Example defines model for example.
type Example []MyItem

The following example indicates why aliases are bad:

type item struct {
        name string
        age int
}

type Alias = []item
type Concrete []item

// func (*Alias) wont_compile() {
//     invalid receiver type *[]item
// }

func (*Concrete) will_compile() {
}

This is especially problematic if you need to create methods for things like a JSON Scan() method or an SQL Value() method for the type.

@jamietanna jamietanna added this to the v1.16.0 milestone Sep 17, 2023
@jamietanna jamietanna modified the milestones: v1.16.0, v2.1.0 Oct 23, 2023
@jamietanna
Copy link
Member

Thanks for raising this @jkj - this will be a breaking change, so we'd need to mark this behind an output-options feature flag, so folks can opt-in to this behaviour. Would you be OK to make the change?

@zsolt-p
Copy link

zsolt-p commented Jan 24, 2024

need to create methods for things like a JSON Scan() method or an SQL Value() method for the type.

I have this same exact use case, too.

my workaround is:

  • add the schema to my spec anyway (I do this so I can still $ref the type, etc)
  • -exclude-schemas=mySchema when I generate
  • implement the concrete type by hand outside the genned code within the same package, where I can implement its methods, etc

@jamietanna jamietanna added enhancement New feature or request and removed awaiting reply labels Jan 25, 2024
As noted in oapi-codegen#1247, in some cases it can be useful to have arrays defined
as a concrete type, not a type alias, so you are able to extend them
with methods.

This produces a new opt-in output-option,
`disable-type-aliases-for-type`, which can be used to control this
behaviour

Right now, we only want to add support for `array`s, but the
configuration is extensible in the future.

This also introduces a test case to demonstrate the capabilities.

We need to introduce a `sliceContains` method to simplify this lookup,
as we don't yet have `slices.Contains` due to the Go versions we're
targeting.
@jamietanna jamietanna merged commit 5cdb559 into oapi-codegen:master Jan 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants