Repository: Khan/genqlient
Pull request #155 (2021-11-08 16:02:02)

Add "struct_references" configuration
Discussion around this: https://github.com/Khan/genqlient/issues/149  

Adds a "use_weak_references" configuration that will:

  * Use a pointer type for struct fields that are a complex type
    * The behaviour can be overridden by setting "pointer: false" in the
      proceeding comment blocks
  * Sets the "omitempty: true" flag on fields matching the criteria.
    This can also be overriden by setting "omitempty: false"

Comments & suggestions are very welcome, I'm not at all sure this is the best way to implement this.

Although we'd debated not setting the pointer for array elements, it did turn out to be simpler to set them everywhere and also made the documentation cleaner to have a single rule to explain.


nathanstitt: @benjaminjkraft ready for another look whenever you're able.  Sorry for the delay, I reworked my project to use this to double check it's all ok.   It worked great, I still had to add approx 10 "omitempty" statements but that's quite manageable.
arjunyel: @nathanstitt hey I just wanted to thank you so much for doing this PR, you're helping a lot of people :D
nathanstitt: Thanks @benjaminjkraft!   

I think output types should be affected by the rule because:
  1) It's a bit better (IMO) to check for missing association by testing for nil vs making sure the pkey field is included and then checking it for it's zero value.     However, this is a bit unsafer and runs a risk of panics, so it really could go either way.  Personally I'd rather the code blow up if there's a misconception about a missing value vs continuing to run with garbage data.  I could probably be convinced either way though so 🤷 
  2) It's better to use a single rule everywhere if possible.  That's both easier to document and also to keep straight mentally.
nathanstitt: Oh and I also signed CLA, so should be good to merge whenever you're good with it

Patch:
diff --git a/docs/genqlient.yaml b/docs/genqlient.yaml
index 25aef2e..f68ff65 100644
--- a/docs/genqlient.yaml
+++ b/docs/genqlient.yaml
@@ -77,6 +77,17 @@ context_type: context.Context
 # without making a query.
 client_getter: "github.com/you/yourpkg.GetClient"
 
+
+# If set, fields with a struct type will default to having
+# the "pointer: true, omitempty: true" flag.
+#
+# This can be useful for struct schema where it would be burdensome
+# to manually set the flags on a large number of fields.
+#
+# Defaults to false.
+use_struct_references: boolean
+
+
 # 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 dc88eb5..60fb2c3 100644
--- a/generate/config.go
+++ b/generate/config.go
@@ -27,6 +27,7 @@ type Config struct {
 	ContextType      string                  `yaml:"context_type"`
 	ClientGetter     string                  `yaml:"client_getter"`
 	Bindings         map[string]*TypeBinding `yaml:"bindings"`
+	StructReferences bool                    `yaml:"use_struct_references"`
 
 	// Set to true to use features that aren't fully ready to use.
 	//
diff --git a/generate/convert.go b/generate/convert.go
index 3841e28..d4a21f1 100644
--- a/generate/convert.go
+++ b/generate/convert.go
@@ -244,7 +244,15 @@ func (g *generator) convertType(
 	goTyp, err := g.convertDefinition(
 		namePrefix, def, typ.Position, selectionSet, options, queryOptions)
 
-	if options.GetPointer() {
+	if g.getStructReference(def) {
+		if options.Pointer == nil || *options.Pointer {
+			goTyp = &goPointerType{goTyp}
+		}
+		if options.Omitempty == nil || *options.Omitempty {
+			oe := true
+			options.Omitempty = &oe
+		}
+	} else if options.GetPointer() {
 		// Whatever we get, wrap it in a pointer.  (Because of the way the
 		// options work, recursing here isn't as connvenient.)
 		// Note this does []*T or [][]*T, not e.g. *[][]T.  See #16.
@@ -253,6 +261,14 @@ func (g *generator) convertType(
 	return goTyp, err
 }
 
+// getStructReference decides if a field should be of pointer type and have the omitempty flag set.
+func (g *generator) getStructReference(
+	def *ast.Definition,
+) bool {
+	return g.Config.StructReferences &&
+		(def.Kind == ast.Object || def.Kind == ast.InputObject)
+}
+
 // convertDefinition decides the Go type we will generate corresponding to a
 // particular GraphQL named type.
 //


Test patch:
diff --git a/generate/generate_test.go b/generate/generate_test.go
index 8cce225..f12c31c 100644
--- a/generate/generate_test.go
+++ b/generate/generate_test.go
@@ -170,6 +170,10 @@ func TestGenerateWithConfig(t *testing.T) {
 			Generated:   "generated.go",
 			ContextType: "github.com/Khan/genqlient/internal/testutil.MyContext",
 		}},
+		{"StructReferences", "", &Config{
+			StructReferences: true,
+			Generated:        "generated-structrefs.go",
+		}},
 		{"NoContext", "", &Config{
 			Generated:   "generated.go",
 			ContextType: "-",
diff --git a/generate/testdata/snapshots/TestGenerateWithConfig-StructReferences-testdata-queries-generated-structrefs.go b/generate/testdata/snapshots/TestGenerateWithConfig-StructReferences-testdata-queries-generated-structrefs.go
new file mode 100644
index 0000000..e22ee66
--- /dev/null
+++ b/generate/testdata/snapshots/TestGenerateWithConfig-StructReferences-testdata-queries-generated-structrefs.go
@@ -0,0 +1,59 @@
+package queries
+
+// Code generated by github.com/Khan/genqlient, DO NOT EDIT.
+
+import (
+	"context"
+
+	"github.com/Khan/genqlient/graphql"
+)
+
+// SimpleQueryResponse is returned by SimpleQuery on success.
+type SimpleQueryResponse 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 *SimpleQueryUser `json:"user"`
+}
+
+// GetUser returns SimpleQueryResponse.User, and is useful for accessing the field via an interface.
+func (v *SimpleQueryResponse) GetUser() *SimpleQueryUser { return v.User }
+
+// SimpleQueryUser includes the requested fields of the GraphQL type User.
+// The GraphQL type's documentation follows.
+//
+// A User is a user!
+type SimpleQueryUser struct {
+	// id is the user's ID.
+	//
+	// It is stable, unique, and opaque, like all good IDs.
+	Id string `json:"id"`
+}
+
+// GetId returns SimpleQueryUser.Id, and is useful for accessing the field via an interface.
+func (v *SimpleQueryUser) GetId() string { return v.Id }
+
+func SimpleQuery(
+	ctx context.Context,
+	client graphql.Client,
+) (*SimpleQueryResponse, error) {
+	var err error
+
+	var retval SimpleQueryResponse
+	err = client.MakeRequest(
+		ctx,
+		"SimpleQuery",
+		`
+query SimpleQuery {
+	user {
+		id
+	}
+}
+`,
+		&retval,
+		nil,
+	)
+	return &retval, err
+}
+