From c2e7dc4e5b12c834054df106de0527055b204b38 Mon Sep 17 00:00:00 2001
From: Ben Kraft <benkraft@khanacademy.org>
Date: Tue, 1 Jun 2021 14:37:33 -0700
Subject: [PATCH] Handle omitempty correctly for slices

We were generating broken code; fixes #43.  Also fixes a bug where
applying omitempty to the entire query was broken by e597cac74c3.
---
 generate/comments.go                          |  4 +-
 generate/generate.go                          |  4 +-
 generate/operation.go.tmpl                    |  4 ++
 generate/testdata/queries/Omitempty.graphql   |  6 +--
 .../testdata/queries/Omitempty.graphql.go     | 46 ++++++++++++++++---
 .../testdata/queries/Omitempty.graphql.json   |  2 +-
 generate/testdata/queries/schema.graphql      |  2 +
 7 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/generate/comments.go b/generate/comments.go
index f64071f..330926a 100644
--- a/generate/comments.go
+++ b/generate/comments.go
@@ -43,7 +43,9 @@ type GenqlientDirective struct {
 	pos *ast.Position
 
 	// If set, this argument will be omitted if it's equal to its Go zero
-	// value.  For example, given the following query:
+	// value, or is an empty slice.
+	//
+	// For example, given the following query:
 	//	# @genqlient(omitempty: true)
 	//	query MyQuery(arg: String) { ... }
 	// genqlient will generate a function
diff --git a/generate/generate.go b/generate/generate.go
index 1d20dba..e265ef2 100644
--- a/generate/generate.go
+++ b/generate/generate.go
@@ -60,6 +60,7 @@ type argument struct {
 	GoName      string
 	GoType      string
 	GraphQLName string
+	IsSlice     bool
 	Options     *GenqlientDirective
 }
 
@@ -127,7 +128,8 @@ func (g *generator) getArgument(
 		GraphQLName: graphQLName,
 		GoName:      lowerFirst(graphQLName),
 		GoType:      goType,
-		Options:     directive,
+		IsSlice:     arg.Type.Elem != nil,
+		Options:     operationDirective.merge(directive),
 	}, nil
 }
 
diff --git a/generate/operation.go.tmpl b/generate/operation.go.tmpl
index 82899b6..301e59a 100644
--- a/generate/operation.go.tmpl
+++ b/generate/operation.go.tmpl
@@ -31,10 +31,14 @@ func {{.Name}}(
     }
     {{range .Args -}}
     {{if .Options.GetOmitempty -}}
+    {{if .IsSlice -}}
+    if len({{.GoName}}) > 0 {
+    {{else -}}
     {{/* zero_{{.GoType}} would be a better name, but {{.GoType}} would require
          munging since it might be, say, `time.Time`. */}}
     var zero_{{.GoName}} {{.GoType}}
     if {{.GoName}} != zero_{{.GoName}} {
+    {{end -}}
         variables["{{.GraphQLName}}"] = {{.GoName}}
     }
     {{end}}
diff --git a/generate/testdata/queries/Omitempty.graphql b/generate/testdata/queries/Omitempty.graphql
index fdf80bf..ddae07d 100644
--- a/generate/testdata/queries/Omitempty.graphql
+++ b/generate/testdata/queries/Omitempty.graphql
@@ -1,14 +1,14 @@
 # @genqlient(omitempty: true)
 query OmitEmptyQuery(
   $query: UserQueryInput,
+  $queries: [UserQueryInput],
   $dt: DateTime,
   $tz: String,
   # @genqlient(omitempty: false)
   $tzNoOmitEmpty: String,
 ) {
-  user(query: $query) {
-    id
-  }
+  user(query: $query) { id }
+  users(query: $queries) { id }
   maybeConvert(dt: $dt, tz: $tz)
   convert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)
 }
diff --git a/generate/testdata/queries/Omitempty.graphql.go b/generate/testdata/queries/Omitempty.graphql.go
index d957428..fe3b86e 100644
--- a/generate/testdata/queries/Omitempty.graphql.go
+++ b/generate/testdata/queries/Omitempty.graphql.go
@@ -15,9 +15,10 @@ type OmitEmptyQueryResponse struct {
 	//
 	// See UserQueryInput for what stuff is supported.
 	// If query is null, returns the current user.
-	User         OmitEmptyQueryUser `json:"user"`
-	MaybeConvert time.Time          `json:"maybeConvert"`
-	Convert2     time.Time          `json:"convert2"`
+	User         OmitEmptyQueryUser      `json:"user"`
+	Users        OmitEmptyQueryUsersUser `json:"users"`
+	MaybeConvert time.Time               `json:"maybeConvert"`
+	Convert2     time.Time               `json:"convert2"`
 }
 
 // OmitEmptyQueryUser includes the requested fields of the GraphQL type User.
@@ -31,6 +32,17 @@ type OmitEmptyQueryUser struct {
 	Id mypkg.ID `json:"id"`
 }
 
+// OmitEmptyQueryUsersUser includes the requested fields of the GraphQL type User.
+// The GraphQL type's documentation follows.
+//
+// A User is a user!
+type OmitEmptyQueryUsersUser struct {
+	// id is the user's ID.
+	//
+	// It is stable, unique, and opaque, like all good IDs.
+	Id mypkg.ID `json:"id"`
+}
+
 // Role is a type a user may have.
 type Role string
 
@@ -62,26 +74,46 @@ type UserQueryInput struct {
 func OmitEmptyQuery(
 	client graphql.Client,
 	query UserQueryInput,
+	queries []UserQueryInput,
 	dt time.Time,
 	tz string,
 	tzNoOmitEmpty string,
 ) (*OmitEmptyQueryResponse, error) {
 	variables := map[string]interface{}{
-		"query":         query,
-		"dt":            dt,
-		"tz":            tz,
 		"tzNoOmitEmpty": tzNoOmitEmpty,
 	}
 
+	var zero_query UserQueryInput
+	if query != zero_query {
+		variables["query"] = query
+	}
+
+	if len(queries) > 0 {
+		variables["queries"] = queries
+	}
+
+	var zero_dt time.Time
+	if dt != zero_dt {
+		variables["dt"] = dt
+	}
+
+	var zero_tz string
+	if tz != zero_tz {
+		variables["tz"] = tz
+	}
+
 	var retval OmitEmptyQueryResponse
 	err := client.MakeRequest(
 		nil,
 		"OmitEmptyQuery",
 		`
-query OmitEmptyQuery ($query: UserQueryInput, $dt: DateTime, $tz: String, $tzNoOmitEmpty: String) {
+query OmitEmptyQuery ($query: UserQueryInput, $queries: [UserQueryInput], $dt: DateTime, $tz: String, $tzNoOmitEmpty: String) {
 	user(query: $query) {
 		id
 	}
+	users(query: $queries) {
+		id
+	}
 	maybeConvert(dt: $dt, tz: $tz)
 	convert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)
 }
diff --git a/generate/testdata/queries/Omitempty.graphql.json b/generate/testdata/queries/Omitempty.graphql.json
index 95a4920..8fcedc6 100644
--- a/generate/testdata/queries/Omitempty.graphql.json
+++ b/generate/testdata/queries/Omitempty.graphql.json
@@ -2,7 +2,7 @@
   "operations": [
     {
       "operationName": "OmitEmptyQuery",
-      "query": "\nquery OmitEmptyQuery ($query: UserQueryInput, $dt: DateTime, $tz: String, $tzNoOmitEmpty: String) {\n\tuser(query: $query) {\n\t\tid\n\t}\n\tmaybeConvert(dt: $dt, tz: $tz)\n\tconvert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)\n}\n",
+      "query": "\nquery OmitEmptyQuery ($query: UserQueryInput, $queries: [UserQueryInput], $dt: DateTime, $tz: String, $tzNoOmitEmpty: String) {\n\tuser(query: $query) {\n\t\tid\n\t}\n\tusers(query: $queries) {\n\t\tid\n\t}\n\tmaybeConvert(dt: $dt, tz: $tz)\n\tconvert2: maybeConvert(dt: $dt, tz: $tzNoOmitEmpty)\n}\n",
       "sourceLocation": "testdata/queries/Omitempty.graphql"
     }
   ]
diff --git a/generate/testdata/queries/schema.graphql b/generate/testdata/queries/schema.graphql
index 9d93abc..377bed6 100644
--- a/generate/testdata/queries/schema.graphql
+++ b/generate/testdata/queries/schema.graphql
@@ -99,6 +99,8 @@ type Query {
   """
   user(query: UserQueryInput): User
 
+  users(query: [UserQueryInput]): User
+
   """usersWithRole looks a user up by role."""
   usersWithRole(role: Role!): [User!]!
   root: Topic!