Repository: Khan/genqlient
Pull request #344 (2024-07-03 11:42:40)

disable input validation when `use_struct_references` is enabled
<!--
Thanks for your contribution! Check out the
[contributing docs](https://github.com/Khan/genqlient/blob/main/docs/CONTRIBUTING.md)
for more on contributing to genqlient.
-->

Fix for https://github.com/Khan/genqlient/issues/342. As `use_struct_references` is incompatible with `omitempty` and `pointer` validation, the most simple change is to disable the validation when the `use_struct_references` is enabled. Note this implementation disables the validation even for types that are not influenced by `use_struct_references`, but still better than disabling the validation globally.

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
- [ ] Included documentation, for new features
- [x] Added an entry to the changelog



Patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index bb73a75..b15e61f 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -23,7 +23,7 @@ When releasing a new version:
 ### Breaking changes:
 
 - omitempty validation:
-  - forbid `omitempty: false` (including implicit behaviour) when using pointer on non-null, no-default input field
+  - forbid `omitempty: false` (including implicit behaviour) when using pointer on non-null input field
 
 ### New features:
 
@@ -34,6 +34,7 @@ When releasing a new version:
 - omitempty validation:
   - allow `omitempty` on non-nullable input field, if the field has a default
   - allow `omitempty: false` on an input field, even when it is non-nullable
+- don't do `omitempty` and `pointer` input types validation when `use_struct_reference` is used, as the generated type is often not compatible with validation logic.
 
 ## v0.7.0
 
diff --git a/generate/convert.go b/generate/convert.go
index b61f227..44db36d 100644
--- a/generate/convert.go
+++ b/generate/convert.go
@@ -248,6 +248,9 @@ func (g *generator) convertType(
 	def := g.schema.Types[typ.Name()]
 	goTyp, err := g.convertDefinition(
 		namePrefix, def, typ.Position, selectionSet, options, queryOptions)
+	if err != nil {
+		return nil, err
+	}
 
 	if g.getStructReference(def) {
 		if options.Pointer == nil || *options.Pointer {
@@ -274,7 +277,8 @@ func (g *generator) convertType(
 			Elem:         goTyp,
 		}
 	}
-	return goTyp, err
+
+	return goTyp, nil
 }
 
 // getStructReference decides if a field should be of pointer type and have the omitempty flag set.
@@ -450,17 +454,22 @@ func (g *generator) convertDefinition(
 				return nil, err
 			}
 
-			// Try to protect against generating field type that has possibility to send `null` to non-nullable graphQL
-			// type. This does not protect against lists/slices, as Go zero-slices are already serialized as `null`
-			// (which can therefore currently send invalid graphQL value - e.g. `null` for [String!]!)
-			// And does not protect against custom MarshalJSON.
-			_, isPointer := fieldGoType.(*goPointerType)
-			if field.Type.NonNull && isPointer && !fieldOptions.GetOmitempty() {
-				return nil, errorf(pos, "pointer on non-null input field can only be used together with omitempty: %s.%s", name, field.Name)
-			}
-
-			if fieldOptions.GetOmitempty() && field.Type.NonNull && field.DefaultValue == nil {
-				return nil, errorf(pos, "omitempty may only be used on optional arguments: %s.%s", name, field.Name)
+			if !g.Config.StructReferences {
+				// Only do these validation when StructReferences are not used, as that can generate types that would not
+				// pass these validations. See https://github.com/Khan/genqlient/issues/342
+
+				// Try to protect against generating field type that has possibility to send `null` to non-nullable graphQL
+				// type. This does not protect against lists/slices, as Go zero-slices are already serialized as `null`
+				// (which can therefore currently send invalid graphQL value - e.g. `null` for [String!]!).
+				// And does not protect against custom MarshalJSON.
+				_, isPointer := fieldGoType.(*goPointerType)
+				if field.Type.NonNull && isPointer && !fieldOptions.GetOmitempty() {
+					return nil, errorf(pos, "pointer on non-null input field can only be used together with omitempty: %s.%s", name, field.Name)
+				}
+
+				if fieldOptions.GetOmitempty() && field.Type.NonNull && field.DefaultValue == nil {
+					return nil, errorf(pos, "omitempty may only be used on optional arguments: %s.%s", name, field.Name)
+				}
 			}
 
 			goType.Fields[i] = &goStructField{


Test patch:
diff --git a/generate/generate_test.go b/generate/generate_test.go
index 21dcaae..01c4645 100644
--- a/generate/generate_test.go
+++ b/generate/generate_test.go
@@ -240,6 +240,11 @@ func TestGenerateWithConfig(t *testing.T) {
 				Enums: map[string]CasingAlgorithm{"Role": CasingRaw},
 			},
 		}},
+		{
+			"UseStructReference", "", []string{"UseStructReference.graphql"}, &Config{
+				StructReferences: true,
+			},
+		},
 	}
 
 	sourceFilename := "SimpleQuery.graphql"
diff --git a/generate/testdata/queries/UseStructReference.graphql b/generate/testdata/queries/UseStructReference.graphql
new file mode 100644
index 0000000..dc682a6
--- /dev/null
+++ b/generate/testdata/queries/UseStructReference.graphql
@@ -0,0 +1,6 @@
+#https://github.com/Khan/genqlient/issues/342
+query UseStructReference(
+  $input: UseStructReferencesInput!
+) {
+  useStructReferencesInput(input: $input)
+}
diff --git a/generate/testdata/queries/schema.graphql b/generate/testdata/queries/schema.graphql
index 5255982..528b21e 100644
--- a/generate/testdata/queries/schema.graphql
+++ b/generate/testdata/queries/schema.graphql
@@ -189,6 +189,7 @@ type Query {
   # But it is here for completeness - maybe it will be used in future or cause some other unexpected issues.
   default(input: InputWithDefaults! = {field: "input omitted"}): Boolean
   omitempty(input: OmitemptyInput): Boolean
+  useStructReferencesInput(input: UseStructReferencesInput!): Boolean
 }
 
 type Mutation {
@@ -226,3 +227,15 @@ input OmitemptyInput {
   field: String!
   nullableField: String
 }
+
+input StructInput {
+  field: String
+}
+
+input UseStructReferencesInput {
+  struct: StructInput!
+  nullableStruct: StructInput
+  list: [StructInput!]!
+  listOfNullable: [StructInput]!
+  nullableList: [StructInput!]
+}
diff --git a/generate/testdata/snapshots/TestGenerate-UseStructReference.graphql-UseStructReference.graphql.go b/generate/testdata/snapshots/TestGenerate-UseStructReference.graphql-UseStructReference.graphql.go
new file mode 100644
index 0000000..09c5c6c
--- /dev/null
+++ b/generate/testdata/snapshots/TestGenerate-UseStructReference.graphql-UseStructReference.graphql.go
@@ -0,0 +1,89 @@
+// Code generated by github.com/Khan/genqlient, DO NOT EDIT.
+
+package test
+
+import (
+	"github.com/Khan/genqlient/graphql"
+)
+
+type StructInput struct {
+	Field string `json:"field"`
+}
+
+// GetField returns StructInput.Field, and is useful for accessing the field via an interface.
+func (v *StructInput) GetField() string { return v.Field }
+
+// UseStructReferenceResponse is returned by UseStructReference on success.
+type UseStructReferenceResponse struct {
+	UseStructReferencesInput bool `json:"useStructReferencesInput"`
+}
+
+// GetUseStructReferencesInput returns UseStructReferenceResponse.UseStructReferencesInput, and is useful for accessing the field via an interface.
+func (v *UseStructReferenceResponse) GetUseStructReferencesInput() bool {
+	return v.UseStructReferencesInput
+}
+
+type UseStructReferencesInput struct {
+	Struct         StructInput   `json:"struct"`
+	NullableStruct StructInput   `json:"nullableStruct"`
+	List           []StructInput `json:"list"`
+	ListOfNullable []StructInput `json:"listOfNullable"`
+	NullableList   []StructInput `json:"nullableList"`
+}
+
+// GetStruct returns UseStructReferencesInput.Struct, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetStruct() StructInput { return v.Struct }
+
+// GetNullableStruct returns UseStructReferencesInput.NullableStruct, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetNullableStruct() StructInput { return v.NullableStruct }
+
+// GetList returns UseStructReferencesInput.List, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetList() []StructInput { return v.List }
+
+// GetListOfNullable returns UseStructReferencesInput.ListOfNullable, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetListOfNullable() []StructInput { return v.ListOfNullable }
+
+// GetNullableList returns UseStructReferencesInput.NullableList, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetNullableList() []StructInput { return v.NullableList }
+
+// __UseStructReferenceInput is used internally by genqlient
+type __UseStructReferenceInput struct {
+	Input UseStructReferencesInput `json:"input"`
+}
+
+// GetInput returns __UseStructReferenceInput.Input, and is useful for accessing the field via an interface.
+func (v *__UseStructReferenceInput) GetInput() UseStructReferencesInput { return v.Input }
+
+// The query or mutation executed by UseStructReference.
+const UseStructReference_Operation = `
+query UseStructReference ($input: UseStructReferencesInput!) {
+	useStructReferencesInput(input: $input)
+}
+`
+
+// https://github.com/Khan/genqlient/issues/342
+func UseStructReference(
+	client_ graphql.Client,
+	input UseStructReferencesInput,
+) (*UseStructReferenceResponse, error) {
+	req_ := &graphql.Request{
+		OpName: "UseStructReference",
+		Query:  UseStructReference_Operation,
+		Variables: &__UseStructReferenceInput{
+			Input: input,
+		},
+	}
+	var err_ error
+
+	var data_ UseStructReferenceResponse
+	resp_ := &graphql.Response{Data: &data_}
+
+	err_ = client_.MakeRequest(
+		nil,
+		req_,
+		resp_,
+	)
+
+	return &data_, err_
+}
+
diff --git a/generate/testdata/snapshots/TestGenerate-UseStructReference.graphql-UseStructReference.graphql.json b/generate/testdata/snapshots/TestGenerate-UseStructReference.graphql-UseStructReference.graphql.json
new file mode 100644
index 0000000..2f3bf39
--- /dev/null
+++ b/generate/testdata/snapshots/TestGenerate-UseStructReference.graphql-UseStructReference.graphql.json
@@ -0,0 +1,9 @@
+{
+  "operations": [
+    {
+      "operationName": "UseStructReference",
+      "query": "\nquery UseStructReference ($input: UseStructReferencesInput!) {\n\tuseStructReferencesInput(input: $input)\n}\n",
+      "sourceLocation": "testdata/queries/UseStructReference.graphql"
+    }
+  ]
+}
diff --git a/generate/testdata/snapshots/TestGenerateWithConfig-UseStructReference-testdata-queries-generated.go b/generate/testdata/snapshots/TestGenerateWithConfig-UseStructReference-testdata-queries-generated.go
new file mode 100644
index 0000000..c2c3723
--- /dev/null
+++ b/generate/testdata/snapshots/TestGenerateWithConfig-UseStructReference-testdata-queries-generated.go
@@ -0,0 +1,92 @@
+// Code generated by github.com/Khan/genqlient, DO NOT EDIT.
+
+package queries
+
+import (
+	"context"
+
+	"github.com/Khan/genqlient/graphql"
+)
+
+type StructInput struct {
+	Field string `json:"field"`
+}
+
+// GetField returns StructInput.Field, and is useful for accessing the field via an interface.
+func (v *StructInput) GetField() string { return v.Field }
+
+// UseStructReferenceResponse is returned by UseStructReference on success.
+type UseStructReferenceResponse struct {
+	UseStructReferencesInput bool `json:"useStructReferencesInput"`
+}
+
+// GetUseStructReferencesInput returns UseStructReferenceResponse.UseStructReferencesInput, and is useful for accessing the field via an interface.
+func (v *UseStructReferenceResponse) GetUseStructReferencesInput() bool {
+	return v.UseStructReferencesInput
+}
+
+type UseStructReferencesInput struct {
+	Struct         *StructInput   `json:"struct,omitempty"`
+	NullableStruct *StructInput   `json:"nullableStruct,omitempty"`
+	List           []*StructInput `json:"list,omitempty"`
+	ListOfNullable []*StructInput `json:"listOfNullable,omitempty"`
+	NullableList   []*StructInput `json:"nullableList,omitempty"`
+}
+
+// GetStruct returns UseStructReferencesInput.Struct, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetStruct() *StructInput { return v.Struct }
+
+// GetNullableStruct returns UseStructReferencesInput.NullableStruct, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetNullableStruct() *StructInput { return v.NullableStruct }
+
+// GetList returns UseStructReferencesInput.List, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetList() []*StructInput { return v.List }
+
+// GetListOfNullable returns UseStructReferencesInput.ListOfNullable, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetListOfNullable() []*StructInput { return v.ListOfNullable }
+
+// GetNullableList returns UseStructReferencesInput.NullableList, and is useful for accessing the field via an interface.
+func (v *UseStructReferencesInput) GetNullableList() []*StructInput { return v.NullableList }
+
+// __UseStructReferenceInput is used internally by genqlient
+type __UseStructReferenceInput struct {
+	Input *UseStructReferencesInput `json:"input,omitempty"`
+}
+
+// GetInput returns __UseStructReferenceInput.Input, and is useful for accessing the field via an interface.
+func (v *__UseStructReferenceInput) GetInput() *UseStructReferencesInput { return v.Input }
+
+// The query or mutation executed by UseStructReference.
+const UseStructReference_Operation = `
+query UseStructReference ($input: UseStructReferencesInput!) {
+	useStructReferencesInput(input: $input)
+}
+`
+
+// https://github.com/Khan/genqlient/issues/342
+func UseStructReference(
+	ctx_ context.Context,
+	client_ graphql.Client,
+	input *UseStructReferencesInput,
+) (*UseStructReferenceResponse, error) {
+	req_ := &graphql.Request{
+		OpName: "UseStructReference",
+		Query:  UseStructReference_Operation,
+		Variables: &__UseStructReferenceInput{
+			Input: input,
+		},
+	}
+	var err_ error
+
+	var data_ UseStructReferenceResponse
+	resp_ := &graphql.Response{Data: &data_}
+
+	err_ = client_.MakeRequest(
+		ctx_,
+		req_,
+		resp_,
+	)
+
+	return &data_, err_
+}
+