Go SDK: NullableString awkwardness

[nickstenning]

The way that the Go SDK handles optional strings is somewhat awkward, and while this ship may have sailed, @tasn asked me to provide some feedback here.

First of all, I totally understand that there's a need for a NullableString type of some kind. While idiomatic Go loves to use "zero values" including the empty string, other languages and APIs don't always work well with that, and so a string type that can also represent unset/JSON null is necessary.

But the implementation and use of NullableString in the Go SDK is awkward. Perhaps the most awkward aspect is that the helper function svix.NullableString a) takes a *string rather than a string, and b) returns not an openapi.NullableString but a *openapi.NullableString.

That means that the only way to create an openapi.NullableString from a static string literal in a single line, is the rather unwieldy *svix.NullableString(svix.String("mystring")).

The problem is compounded by the fact that openapi.NullableString is a private type, and as a result it is impossible to write 3rd-party code that manipulates it directly:

// This will not compile because the openapi package is internal.
func nullableString(s string) openapi.NullableString {
	return *svix.NullableString(&s)
}

What's particularly odd about the choice to return a pointer from svix.NullableString is that the data types that need nullable strings only ever want an openapi.NullableString, not a pointer to one, which leads to code like:

appIn := svix.ApplicationIn{
	...
	Uid: *svix.NullableString(svix.String("myuid"))
}

Suggested alternative

I think a much more Go-friendly implementation here would be to make the type something like database/sql's NullString (https://pkg.go.dev/database/sql#NullString).

package types

type NullableString {
	String string
	Valid bool
}

and then define your helper function as

package svix

func NullableString(s string) types.NullableString {
	return types.NullableString{
		String: s,
		Valid: true,
	}
}

That creates a situation where the zero-valued types.NullableString is null, because the zero value of Valid is false:

type ApplicationIn struct {
	Uid types.NullableString
}

app := ApplicationIn{}

fmt.Printf("%v\n", app.Uid.Valid) // Prints false

And constructing one to pass into a type is equally straightforward:

app := ApplicationIn{
	Uid: NullableString("my-app-id"),
}

fmt.Printf("%v\n", app.Uid.Valid) // Prints true

Here's another example to play around with: https://go.dev/play/p/jYGq7qCCC11

[ccqpein]

How is this issue progressing? It is in working stage or still in proposal?

And I am playing around find this solution can also pass the compiler (but we need to change version in go.mod to > =1.18)

type Nullable[T any] struct {
	V     T
	Valid bool
}

type NullableString Nullable[*string]

func NewNullableString(s *string) NullableString {
	if s == nil {
		return NullableString{V: s, Valid: false}
	}
	return NullableString{V: s, Valid: true}
}

[tasn]

Sorry, we didn't really get to it yet, but @svix-jplatte added a label a couple of weeks ago to make sure we don't miss it and fix it soon!

[tasn]

I think @svix-onelson is working on upgrading the OpenAPI generator which will also fix this.

[tasn]

@svix-onelson, is this fixed?

[svix-onelson]

This doesn't look so different with the latest generator.

I think what I'd like to do for this is to expose/re-export openapi.NullableString and making some changes to the public API to make construction more convenient as suggested by the OP.

I'd like to avoid adding a new type (though generics will now be on the table since we're targeting go 1.20) as described in #1198 (comment). The rationale being I think we'd need to template around set/unset checks in the rest of the codegen output to account for the new type.
It could be possible to do this cleanly, but the former would definitely be more straightforward.