modelina icon indicating copy to clipboard operation
modelina copied to clipboard

Nullable types in golang

Open wiegell opened this issue 1 year ago • 9 comments

Reason/Context

Please try answering few of those questions

  • Why we need this improvement? Nullable strings are vital in many languages, also in go! In our current project we have put a lot of effort to support nullability between frontend and bff (graphql) - if it cannot be kept between services (asyncapi), it's a dealbreaker! Currently type: ["integer", "null"] is translated to interface{} which is not typesafe. Other generators, e.g. https://github.com/99designs/gqlgen solves the problem by converting optional primitives to pointers, which are nullable. E.g. a type string becomes type *string.

https://github.com/asyncapi/modelina/commit/53edd32fcbc2dde8f9f36a0d344e2abc45b1fec2 This change addresses nullable types in typescript - could it be expanded to go?

Description

Please try answering few of those questions

  • Will this be a breaking change? Yes

wiegell avatar Jun 22 '23 14:06 wiegell

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

github-actions[bot] avatar Jun 22 '23 14:06 github-actions[bot]

@wiegell thanks for opening the issue!

As I am not that well versed in Go, hope you don't mind answering some questions 😅 Is it ALL types that just need *type for it to be nullable?

v2 actually can handle this, we just need to alter a few lines of code for the type generation 🙂

jonaslagoni avatar Jun 24 '23 21:06 jonaslagoni

Yes all pointers (*) are nullable, but some value types can also be nil.

See this proposal: https://github.com/golang/go/issues/30177

Motivation -> 2nd bullet explains using a pointer to simulate nullable. Proposal -> first bullet list is a list of types that cannot be nil.

So it would be needed to add * to the golang type corresponding to a nullable version of the following asyncapi types: string number integer boolean object

I presume asyncapi array type is translated to golang slice (which is nullable in contrast to a golang array of fixed length), so in that case a value type without * will suffice.

I'm not with my computer the next couple of days, but i would like to check if my reasoning here checks out with the graphql generator mentioned above, and i'll get back to you!

wiegell avatar Jun 25 '23 11:06 wiegell

As a really good example you can check this project for struct generation: https://github.com/deepmap/oapi-codegen

But we also need to take care of non required with the omitempty struct tag: https://pkg.go.dev/encoding/json#Marshal

zekth avatar Jul 04 '23 21:07 zekth

So i just checked with openapi - they use string pointers for non-required fields as well and then value types when the string is required.

wiegell avatar Jul 17 '23 11:07 wiegell

I started using AsyncAPI generation for Go and found this current limitation, which is a deal breaker for me as well.

arthurspa avatar Nov 13 '23 18:11 arthurspa

@arthurspa @wiegell @zekth

In Modelina v2, the constraint logic now have access to nullable and non-required fields, so all we have to change is: https://github.com/asyncapi/modelina/blob/master/src/generators/go/GoConstrainer.ts

You can use the Java constrainer as reference: https://github.com/asyncapi/modelina/blob/e53c2af1095e020667ee2a14d4f0aa09f5546796/src/generators/java/JavaConstrainer.ts#L106

Happy to help guide as much as needed 🙂

jonaslagoni avatar Nov 13 '23 18:11 jonaslagoni

Thanks @jonaslagoni for pointing out the file. I'll take a look at that some time this week.

arthurspa avatar Nov 14 '23 08:11 arthurspa

Just an update: I ended up writing my own template and made a quick fix for the nullable types I needed, which is not worth sharing since it's more a hack than a proper solution. So I won't spend time with this ticket, unfortunately.

arthurspa avatar Jan 04 '24 08:01 arthurspa

:tada: This issue has been resolved in version 4.0.0-next.31 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

asyncapi-bot avatar Apr 25 '24 10:04 asyncapi-bot