Repository: Khan/genqlient
Pull request #252 (2023-02-10 02:31:17)

Add "generic" option to the "optional" configuration for handling nullable types
This is an implementation for #251, it adds a new `"generic"` option for the `"optional"` configuration, and a companion type `"optional_generic_type"` which is a fully qualified type with a placeholder `%` for the generic parameter.

I have tested this locally with a custom `optional.Value[A]` implementation but I'm unsure how to add tests for this without bumping the Go version from 1.16 to 1.18. I would appreciate any input on how best to achieve this. Interested in any feedback on the design before I fill out the details on documentation and changelog.

I have:
- [x] Written a clear PR title and description (above)
- [x] Signed the [Khan Academy CLA](https://www.khanacademy.org/r/cla)
- [x] Added tests covering my changes, if applicable
- [x] Included a link to the issue fixed, if applicable
- [x] Included documentation, for new features
- [ ] Added an entry to the changelog


DylanRJohnston: Here's an example of it in use https://github.com/DylanRJohnston/genqlient/compare/main...DylanRJohnston:genqlient:example
benjaminjkraft: I love the concept, thanks very much for implementing it! I don't have time for a proper review today but one thing I see is we'll want tests for this -- there are snapshot tests already set up and you'll just have to make some way to skip these on versions below 1.18. I'm a bit busy right now but I'll try to get back with a full review within a week or so.
benjaminjkraft: Actually, one more thought on integration tests: I'm ok just running all the integration tests only against 1.18+, I don't think there's much we really need to test against older Go versions there. Not sure if that will be enough to make it easy though, so what I said before still applies.
DylanRJohnston: @benjaminjkraft I've added a snapshot test and some more documentation. Do you want the the entire integration test to be updated to use the `testutil.Option` or to just leave it for now?
benjaminjkraft: Ok, I pulled the version upgrade into #257, so once that merges you should be able to just merge and avoid dealing with that!
benjaminjkraft: I went ahead and merged main and added the changelog. Thanks again for writing this -- I'm planning on using it in a project I'm working on!

Patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 783e67d..221f87a 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -24,6 +24,8 @@ When releasing a new version:
 
 ### New features:
 
+- The new `optional: generic` allows using a generic type to represent optionality. See the [documentation](genqlient.yaml) for details.
+
 ### Bug fixes:
 
 ## v0.6.0
diff --git a/docs/genqlient.yaml b/docs/genqlient.yaml
index 3ffa2e3..5e5601b 100644
--- a/docs/genqlient.yaml
+++ b/docs/genqlient.yaml
@@ -109,8 +109,19 @@ use_extensions: boolean
 #   pointers-to-slices, so the GraphQL type `[String]` will map to the Go
 #   type `[]*string`, not `*[]*string`; GraphQL null and empty list simply
 #   map to Go nil- and empty-slice.
+# - generic: optional fields are generated as type parameters to a generic type
+#   specified by `optional_generic_type`. E.g. fields with GraphQL type `String`
+#   will map to the Go type `generic.Type[string]`. This is useful if you have a
+#   type that mimics the behavior of Option<A> or Maybe<A> in other languages like
+#   Rust, Java, or Haskell.
 optional: value
 
+# Only used when `optional: generic` is set. `example.Type` must be a fully qualified
+# generic type with only one generic parameter e.g. atomic.Value[string]. 
+# It must also implement the `encoding/json.Marshaler` and `encoding/json.Unmarshaler`
+# interface if you want it to serialize / deserialize properly.
+optional_generic_type: github.com/organisation/repository/example.Type
+
 # A map from GraphQL type name to Go fully-qualified type name to override
 # the Go type genqlient will use for this GraphQL type.
 #
diff --git a/generate/config.go b/generate/config.go
index 37cbe7e..31db2d6 100644
--- a/generate/config.go
+++ b/generate/config.go
@@ -22,18 +22,19 @@ type Config struct {
 	// The following fields are documented in the [genqlient.yaml docs].
 	//
 	// [genqlient.yaml docs]: https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
-	Schema           StringList              `yaml:"schema"`
-	Operations       StringList              `yaml:"operations"`
-	Generated        string                  `yaml:"generated"`
-	Package          string                  `yaml:"package"`
-	ExportOperations string                  `yaml:"export_operations"`
-	ContextType      string                  `yaml:"context_type"`
-	ClientGetter     string                  `yaml:"client_getter"`
-	Bindings         map[string]*TypeBinding `yaml:"bindings"`
-	PackageBindings  []*PackageBinding       `yaml:"package_bindings"`
-	Optional         string                  `yaml:"optional"`
-	StructReferences bool                    `yaml:"use_struct_references"`
-	Extensions       bool                    `yaml:"use_extensions"`
+	Schema              StringList              `yaml:"schema"`
+	Operations          StringList              `yaml:"operations"`
+	Generated           string                  `yaml:"generated"`
+	Package             string                  `yaml:"package"`
+	ExportOperations    string                  `yaml:"export_operations"`
+	ContextType         string                  `yaml:"context_type"`
+	ClientGetter        string                  `yaml:"client_getter"`
+	Bindings            map[string]*TypeBinding `yaml:"bindings"`
+	PackageBindings     []*PackageBinding       `yaml:"package_bindings"`
+	Optional            string                  `yaml:"optional"`
+	OptionalGenericType string                  `yaml:"optional_generic_type"`
+	StructReferences    bool                    `yaml:"use_struct_references"`
+	Extensions          bool                    `yaml:"use_extensions"`
 
 	// Set to true to use features that aren't fully ready to use.
 	//
@@ -99,6 +100,16 @@ func (c *Config) ValidateAndFillDefaults(baseDir string) error {
 		c.ContextType = "context.Context"
 	}
 
+	if c.Optional != "" && c.Optional != "value" && c.Optional != "pointer" && c.Optional != "generic" {
+		return errorf(nil, "optional must be one of: 'value' (default), 'pointer', or 'generic'")
+	}
+
+	if c.Optional == "generic" && c.OptionalGenericType == "" {
+		return errorf(nil, "if optional is set to 'generic', optional_generic_type must be set to the fully"+
+			"qualified name of a type with a single generic parameter"+
+			"\nExample: \"github.com/Org/Repo/optional.Value\"")
+	}
+
 	if c.Package == "" {
 		abs, err := filepath.Abs(c.Generated)
 		if err != nil {
diff --git a/generate/convert.go b/generate/convert.go
index caf3754..41eb123 100644
--- a/generate/convert.go
+++ b/generate/convert.go
@@ -262,6 +262,17 @@ func (g *generator) convertType(
 		// options work, recursing here isn't as connvenient.)
 		// Note this does []*T or [][]*T, not e.g. *[][]T.  See #16.
 		goTyp = &goPointerType{goTyp}
+	} else if !typ.NonNull && g.Config.Optional == "generic" {
+		var genericRef string
+		genericRef, err = g.ref(g.Config.OptionalGenericType)
+		if err != nil {
+			return nil, err
+		}
+
+		goTyp = &goGenericType{
+			GoGenericRef: genericRef,
+			Elem:         goTyp,
+		}
 	}
 	return goTyp, err
 }
diff --git a/generate/types.go b/generate/types.go
index f50df7a..07c661c 100644
--- a/generate/types.go
+++ b/generate/types.go
@@ -55,6 +55,7 @@ var (
 	_ goType = (*goEnumType)(nil)
 	_ goType = (*goStructType)(nil)
 	_ goType = (*goInterfaceType)(nil)
+	_ goType = (*goGenericType)(nil)
 )
 
 type (
@@ -80,6 +81,12 @@ type (
 	// user (perhaps to handle nulls explicitly, or to avoid copying large
 	// structures).
 	goPointerType struct{ Elem goType }
+	// goGenericType represent the Go type GoGenericRef[Elem], used when requested by the
+	// user to box nullable data without using pointers or sentinel values
+	goGenericType struct {
+		GoGenericRef string
+		Elem         goType
+	}
 )
 
 // Opaque types are defined by the user; pointers and slices need no definition
@@ -91,21 +98,27 @@ func (typ *goTypenameForBuiltinType) WriteDefinition(w io.Writer, g *generator)
 }
 func (typ *goSliceType) WriteDefinition(io.Writer, *generator) error   { return nil }
 func (typ *goPointerType) WriteDefinition(io.Writer, *generator) error { return nil }
+func (typ *goGenericType) WriteDefinition(io.Writer, *generator) error { return nil }
 
 func (typ *goOpaqueType) Reference() string             { return typ.GoRef }
 func (typ *goTypenameForBuiltinType) Reference() string { return typ.GoTypeName }
 func (typ *goSliceType) Reference() string              { return "[]" + typ.Elem.Reference() }
 func (typ *goPointerType) Reference() string            { return "*" + typ.Elem.Reference() }
+func (typ *goGenericType) Reference() string {
+	return fmt.Sprintf("%s[%s]", typ.GoGenericRef, typ.Elem.Reference())
+}
 
 func (typ *goOpaqueType) SelectionSet() ast.SelectionSet             { return nil }
 func (typ *goTypenameForBuiltinType) SelectionSet() ast.SelectionSet { return nil }
 func (typ *goSliceType) SelectionSet() ast.SelectionSet              { return typ.Elem.SelectionSet() }
 func (typ *goPointerType) SelectionSet() ast.SelectionSet            { return typ.Elem.SelectionSet() }
+func (typ *goGenericType) SelectionSet() ast.SelectionSet            { return typ.Elem.SelectionSet() }
 
 func (typ *goOpaqueType) GraphQLTypeName() string             { return typ.GraphQLName }
 func (typ *goTypenameForBuiltinType) GraphQLTypeName() string { return typ.GraphQLName }
 func (typ *goSliceType) GraphQLTypeName() string              { return typ.Elem.GraphQLTypeName() }
 func (typ *goPointerType) GraphQLTypeName() string            { return typ.Elem.GraphQLTypeName() }
+func (typ *goGenericType) GraphQLTypeName() string            { return typ.Elem.GraphQLTypeName() }
 
 // goEnumType represents a Go named-string type used to represent a GraphQL
 // enum.  In this case, we generate both the type (`type T string`) and also a
@@ -529,6 +542,7 @@ func (typ *goOpaqueType) Unwrap() goType             { return typ }
 func (typ *goTypenameForBuiltinType) Unwrap() goType { return typ }
 func (typ *goSliceType) Unwrap() goType              { return typ.Elem.Unwrap() }
 func (typ *goPointerType) Unwrap() goType            { return typ.Elem.Unwrap() }
+func (typ *goGenericType) Unwrap() goType            { return typ.Elem.Unwrap() }
 func (typ *goEnumType) Unwrap() goType               { return typ }
 func (typ *goStructType) Unwrap() goType             { return typ }
 func (typ *goInterfaceType) Unwrap() goType          { return typ }
@@ -537,6 +551,7 @@ func (typ *goOpaqueType) SliceDepth() int             { return 0 }
 func (typ *goTypenameForBuiltinType) SliceDepth() int { return 0 }
 func (typ *goSliceType) SliceDepth() int              { return typ.Elem.SliceDepth() + 1 }
 func (typ *goPointerType) SliceDepth() int            { return 0 }
+func (typ *goGenericType) SliceDepth() int            { return 0 }
 func (typ *goEnumType) SliceDepth() int               { return 0 }
 func (typ *goStructType) SliceDepth() int             { return 0 }
 func (typ *goInterfaceType) SliceDepth() int          { return 0 }
@@ -545,6 +560,7 @@ func (typ *goOpaqueType) IsPointer() bool             { return false }
 func (typ *goTypenameForBuiltinType) IsPointer() bool { return false }
 func (typ *goSliceType) IsPointer() bool              { return typ.Elem.IsPointer() }
 func (typ *goPointerType) IsPointer() bool            { return true }
+func (typ *goGenericType) IsPointer() bool            { return false }
 func (typ *goEnumType) IsPointer() bool               { return false }
 func (typ *goStructType) IsPointer() bool             { return false }
 func (typ *goInterfaceType) IsPointer() bool          { return false }


Test patch:
diff --git a/generate/generate_test.go b/generate/generate_test.go
index c976244..6043391 100644
--- a/generate/generate_test.go
+++ b/generate/generate_test.go
@@ -214,6 +214,11 @@ func TestGenerateWithConfig(t *testing.T) {
 			Generated: "generated.go",
 			Optional:  "pointer",
 		}},
+		{"OptionalGeneric", "", []string{"ListInput.graphql", "QueryWithSlices.graphql"}, &Config{
+			Generated:           "generated.go",
+			Optional:            "generic",
+			OptionalGenericType: "github.com/Khan/genqlient/internal/testutil.Option",
+		}},
 	}
 
 	sourceFilename := "SimpleQuery.graphql"
diff --git a/generate/testdata/snapshots/TestGenerateWithConfig-OptionalGeneric-testdata-queries-generated.go b/generate/testdata/snapshots/TestGenerateWithConfig-OptionalGeneric-testdata-queries-generated.go
new file mode 100644
index 0000000..24cbd22
--- /dev/null
+++ b/generate/testdata/snapshots/TestGenerateWithConfig-OptionalGeneric-testdata-queries-generated.go
@@ -0,0 +1,153 @@
+// Code generated by github.com/Khan/genqlient, DO NOT EDIT.
+
+package queries
+
+import (
+	"context"
+
+	"github.com/Khan/genqlient/graphql"
+	"github.com/Khan/genqlient/internal/testutil"
+)
+
+// ListInputQueryResponse is returned by ListInputQuery on success.
+type ListInputQueryResponse struct {
+	// user looks up a user by some stuff.
+	//
+	// See UserQueryInput for what stuff is supported.
+	// If query is null, returns the current user.
+	User testutil.Option[ListInputQueryUser] `json:"user"`
+}
+
+// GetUser returns ListInputQueryResponse.User, and is useful for accessing the field via an interface.
+func (v *ListInputQueryResponse) GetUser() testutil.Option[ListInputQueryUser] { return v.User }
+
+// ListInputQueryUser includes the requested fields of the GraphQL type User.
+// The GraphQL type's documentation follows.
+//
+// A User is a user!
+type ListInputQueryUser struct {
+	// id is the user's ID.
+	//
+	// It is stable, unique, and opaque, like all good IDs.
+	Id string `json:"id"`
+}
+
+// GetId returns ListInputQueryUser.Id, and is useful for accessing the field via an interface.
+func (v *ListInputQueryUser) GetId() string { return v.Id }
+
+// QueryWithSlicesResponse is returned by QueryWithSlices on success.
+type QueryWithSlicesResponse struct {
+	// user looks up a user by some stuff.
+	//
+	// See UserQueryInput for what stuff is supported.
+	// If query is null, returns the current user.
+	User testutil.Option[QueryWithSlicesUser] `json:"user"`
+}
+
+// GetUser returns QueryWithSlicesResponse.User, and is useful for accessing the field via an interface.
+func (v *QueryWithSlicesResponse) GetUser() testutil.Option[QueryWithSlicesUser] { return v.User }
+
+// QueryWithSlicesUser includes the requested fields of the GraphQL type User.
+// The GraphQL type's documentation follows.
+//
+// A User is a user!
+type QueryWithSlicesUser struct {
+	Emails                []string                  `json:"emails"`
+	EmailsOrNull          []string                  `json:"emailsOrNull"`
+	EmailsWithNulls       []testutil.Option[string] `json:"emailsWithNulls"`
+	EmailsWithNullsOrNull []testutil.Option[string] `json:"emailsWithNullsOrNull"`
+}
+
+// GetEmails returns QueryWithSlicesUser.Emails, and is useful for accessing the field via an interface.
+func (v *QueryWithSlicesUser) GetEmails() []string { return v.Emails }
+
+// GetEmailsOrNull returns QueryWithSlicesUser.EmailsOrNull, and is useful for accessing the field via an interface.
+func (v *QueryWithSlicesUser) GetEmailsOrNull() []string { return v.EmailsOrNull }
+
+// GetEmailsWithNulls returns QueryWithSlicesUser.EmailsWithNulls, and is useful for accessing the field via an interface.
+func (v *QueryWithSlicesUser) GetEmailsWithNulls() []testutil.Option[string] {
+	return v.EmailsWithNulls
+}
+
+// GetEmailsWithNullsOrNull returns QueryWithSlicesUser.EmailsWithNullsOrNull, and is useful for accessing the field via an interface.
+func (v *QueryWithSlicesUser) GetEmailsWithNullsOrNull() []testutil.Option[string] {
+	return v.EmailsWithNullsOrNull
+}
+
+// __ListInputQueryInput is used internally by genqlient
+type __ListInputQueryInput struct {
+	Names []testutil.Option[string] `json:"names"`
+}
+
+// GetNames returns __ListInputQueryInput.Names, and is useful for accessing the field via an interface.
+func (v *__ListInputQueryInput) GetNames() []testutil.Option[string] { return v.Names }
+
+// The query or mutation executed by ListInputQuery.
+const ListInputQuery_Operation = `
+query ListInputQuery ($names: [String]) {
+	user(query: {names:$names}) {
+		id
+	}
+}
+`
+
+func ListInputQuery(
+	ctx context.Context,
+	client graphql.Client,
+	names []testutil.Option[string],
+) (*ListInputQueryResponse, error) {
+	req := &graphql.Request{
+		OpName: "ListInputQuery",
+		Query:  ListInputQuery_Operation,
+		Variables: &__ListInputQueryInput{
+			Names: names,
+		},
+	}
+	var err error
+
+	var data ListInputQueryResponse
+	resp := &graphql.Response{Data: &data}
+
+	err = client.MakeRequest(
+		ctx,
+		req,
+		resp,
+	)
+
+	return &data, err
+}
+
+// The query or mutation executed by QueryWithSlices.
+const QueryWithSlices_Operation = `
+query QueryWithSlices {
+	user {
+		emails
+		emailsOrNull
+		emailsWithNulls
+		emailsWithNullsOrNull
+	}
+}
+`
+
+func QueryWithSlices(
+	ctx context.Context,
+	client graphql.Client,
+) (*QueryWithSlicesResponse, error) {
+	req := &graphql.Request{
+		OpName: "QueryWithSlices",
+		Query:  QueryWithSlices_Operation,
+	}
+	var err error
+
+	var data QueryWithSlicesResponse
+	resp := &graphql.Response{Data: &data}
+
+	err = client.MakeRequest(
+		ctx,
+		req,
+		resp,
+	)
+
+	return &data, err
+}
+
diff --git a/internal/testutil/types.go b/internal/testutil/types.go
index c202859..0fc4330 100644
--- a/internal/testutil/types.go
+++ b/internal/testutil/types.go
@@ -2,6 +2,7 @@ package testutil
 
 import (
 	"context"
+	"encoding/json"
 	"time"
 
 	"github.com/Khan/genqlient/graphql"
@@ -49,3 +50,60 @@ func UnmarshalDate(b []byte, t *time.Time) error {
 	*t, err = time.Parse(`"`+dateFormat+`"`, string(b))
 	return err
 }
+
+type Option[V any] struct {
+	value V
+	ok    bool
+}
+
+func Some[V any](value V) Option[V] {
+	return Option[V]{value: value, ok: true}
+}
+
+func None[V any]() Option[V] {
+	return Option[V]{ok: false}
+}
+
+func (v Option[V]) Unpack() (V, bool) {
+	return v.value, v.ok
+}
+
+func (v Option[V]) Get(fallback V) V {
+	if v.ok {
+		return v.value
+	}
+
+	return fallback
+}
+
+func FromPtr[V any](ptr *V) Option[V] {
+	if ptr == nil {
+		return None[V]()
+	}
+
+	return Some(*ptr)
+}
+
+func (value Option[V]) MarshalJSON() ([]byte, error) {
+	if value.ok {
+		return json.Marshal(value.value)
+	} else {
+		return json.Marshal((*V)(nil))
+	}
+}
+
+func (value *Option[V]) UnmarshalJSON(data []byte) error {
+	v := (*V)(nil)
+
+	err := json.Unmarshal(data, &v)
+	if err != nil {
+		return err
+	}
+
+	if v != nil {
+		value.value = *v
+		value.ok = true
+	}
+
+	return nil
+}