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

Not possible to generate a struct with attributes in the same order as the API specs #105

Open
yeforriak opened this issue Apr 17, 2024 · 5 comments · May be fixed by #106
Open

Not possible to generate a struct with attributes in the same order as the API specs #105

yeforriak opened this issue Apr 17, 2024 · 5 comments · May be fixed by #106

Comments

@yeforriak
Copy link

I want to generate a struct with attributes ordered as defined in the OpenAPI specification doc.

Api specification:

    Settings:
      type: object
      required:
        - name
        - enabled
      properties:
        name:
          type: string
          enum: [ setting-a, setting-b, setting-c, setting-d ]
        enabled:
          type: boolean

The generated struct attributes are sorted alphabetically:

type Setting struct {
	Enabled bool        `json:"enabled"`
	Name    SettingName `json:"name"`
}

but I would like the struct to be defined as

type Setting struct {
        Name    SettingName `json:"name"`
	Enabled bool        `json:"enabled"`
}
@karitham
Copy link
Collaborator

Hi, can I ask why you would need that? I understand your feature request perfectly but it reads to me like a XY problem. Is it just about the order of the actual fields?

Because at the end of the day the JSON will still have roughly random order fields, and if it's about malign we can run that on the generated code.

When you use the struct, you can also use any order so I'm failing to see the value behind such a feature request.

@yeforriak
Copy link
Author

yeforriak commented Apr 19, 2024

Hi, thanks for your prompt response.

The intention was to use the generated model directly to generate an HTTP response (json API) from the server code and I wanted to have some control over the order of the fields.

In my particular case, it is not a big deal since I can take care of the ordering in the UI code or even map it to another struct on the server side if needed.

When I was investigating this I found it interesting that there could be optimisations done by sorting struct attributes, see this

But anyway, I'm happy to close this issue. Thanks 👍

@karitham
Copy link
Collaborator

When I was investigating this I found it interesting that there could be optimisations done by sorting struct attributes, see this

Go actually has a vet for this, https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/[email protected] which we could likely integrate as a pass after generation of the fields.

I forgot exactly if there is a provided auto-fix for it, but adding this code to the ouput pipeline is not something I'm against. We explicitly do not guarantee order anywhere, we just sort the fields as part of pre-passes to make the output deterministic.

Unfortunately the openAPI spec represents them as a map so we'd need to switch to ordered maps. I think the openapi library we use did the change on their end but it'd be quite a big refactoring on our end. I do not mind either (or even both options).

Open to contributions, in the best case scenario it's a small 10 line changes somewhere here https://github.com/discord-gophers/goapi-gen/blob/main/codegen/codegen.go#L227-L235

@gustavobretas
Copy link

Hey folks!

If you don't mind, I will express my support in favor to @yeforriak's suggestion. In my opinion, maintaining the order of properties as defined in the API specs would enhances readability and helps developers keep related data together. I couldn't think in a better example, but, like the example below, the ID would be "out of place" alphabeticaly ordered.

{
  "ID": "",
  "Address1": "",
  "Address2": "",
  "City": "",
  "State": "",
  "Zipcode": ""
}

Data grouping is valuable during development, especially when working on database modeling. Implementing this change would improve the developer experience keeping the code intuitive.

@diamondburned
Copy link
Contributor

diamondburned commented Aug 12, 2024

I'm currently working on implementing this feature. Here is my ongoing PR: #106.

diamondburned added a commit to diamondburned/goapi-gen that referenced this issue Aug 12, 2024
@diamondburned diamondburned linked a pull request Aug 12, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants