If requested, validate binding-types get the right fields

generate/binding_validation.go

@@ -0,0 +1,111 @@
+package generate
+
+// This file is responsible for doing the validation for type-bindings, if they
+// are so configured (see TypeBinding).
+
+import (
+	"fmt"
+
+	"github.com/vektah/gqlparser/v2/ast"
+	"github.com/vektah/gqlparser/v2/parser"
+)
+
+// selectionsMatch recursively compares the two selection-sets, and returns an
+// error if they differ.
+//
+// It does not check arguments and directives, only field names, aliases,
+// order, and fragment-structure.  It does not recurse into named fragments, it
+// only checks that their names match.
+//
+// TODO(benkraft): Should we check arguments/directives?
+func selectionsMatch(
+	pos *ast.Position,
+	expectedSelectionSet, actualSelectionSet ast.SelectionSet,
+) error {
+	if len(expectedSelectionSet) != len(actualSelectionSet) {
+		return errorf(
+			pos, "expected %d fields, got %d",
+			len(expectedSelectionSet), len(actualSelectionSet))
+	}
+
+	for i, expected := range expectedSelectionSet {
+		switch expected := expected.(type) {
+		case *ast.Field:
+			actual, ok := actualSelectionSet[i].(*ast.Field)
+			switch {
+			case !ok:
+				return errorf(actual.Position,
+					"expected selection #%d to be field, got %T",
+					i, actualSelectionSet[i])
+			case actual.Name != expected.Name:
+				return errorf(actual.Position,
+					"expected field %d to be %s, got %s",
+					i, expected.Name, actual.Name)
+			case actual.Alias != expected.Alias:
+				return errorf(actual.Position,
+					"expected field %d's alias to be %s, got %s",
+					i, expected.Alias, actual.Alias)
+			}
+			err := selectionsMatch(actual.Position, expected.SelectionSet, actual.SelectionSet)
+			if err != nil {
+				return fmt.Errorf("in %s sub-selection: %w", actual.Alias, err)
+			}
+		case *ast.InlineFragment:
+			actual, ok := actualSelectionSet[i].(*ast.InlineFragment)
+			switch {
+			case !ok:
+				return errorf(actual.Position,
+					"expected selection %d to be inline fragment, got %T",
+					i, actualSelectionSet[i])
+			case actual.TypeCondition != expected.TypeCondition:
+				return errorf(actual.Position,
+					"expected fragment %d to be on type %s, got %s",
+					i, expected.TypeCondition, actual.TypeCondition)
+			}
+			err := selectionsMatch(actual.Position, expected.SelectionSet, actual.SelectionSet)
+			if err != nil {
+				return fmt.Errorf("in inline fragment on %s: %w", actual.TypeCondition, err)
+			}
+		case *ast.FragmentSpread:
+			actual, ok := actualSelectionSet[i].(*ast.FragmentSpread)
+			switch {
+			case !ok:
+				return errorf(actual.Position,
+					"expected selection %d to be fragment spread, got %T",
+					i, actualSelectionSet[i])
+			case actual.Name != expected.Name:
+				return errorf(actual.Position,
+					"expected fragment %d to be ...%s, got ...%s",
+					i, expected.Name, actual.Name)
+			}
+		}
+	}
+	return nil
+}
+
+// validateBindingSelection checks that if you requested in your type-binding
+// that this type must always request certain fields, then in fact it does.
+func (g *generator) validateBindingSelection(
+	typeName string,
+	binding *TypeBinding,
+	pos *ast.Position,
+	selectionSet ast.SelectionSet,
+) error {
+	if binding.ExpectExactFields == "" {
+		return nil // no validation requested
+	}
+
+	// HACK: we parse the selection as if it were a query, which is basically
+	// the same (for syntax purposes; it of course wouldn't validate)
+	doc, gqlErr := parser.ParseQuery(&ast.Source{Input: binding.ExpectExactFields})
+	if gqlErr != nil {
+		return errorf(
+			nil, "invalid type-binding %s.expect_exact_fields: %w", typeName, gqlErr)
+	}
+
+	err := selectionsMatch(pos, doc.Operations[0].SelectionSet, selectionSet)
+	if err != nil {
+		return fmt.Errorf("invalid selection for type-binding %s: %w", typeName, err)
+	}
+	return nil
+}

generate/config.go

@@ -104,6 +104,26 @@ type TypeBinding struct {
 	//  map[string]interface{}
 	//	github.com/you/yourpkg/subpkg.MyType
 	Type string `yaml:"type"`
+	// If set, a GraphQL selection which must exactly match the fields
+	// requested whenever this type is used.  Only applies if the GraphQL type
+	// is a composite output type (object, interface, or union).
+	//
+	// This is useful if Type is a struct whose UnmarshalJSON or other methods
+	// expect that you requested certain fields.  You can specify those fields
+	// like
+	//  MyType:
+	//		type: path/to/my.GoType
+	//		expect_exact_fields: "{ id name }"
+	// and then genqlient will reject if you make a query
+	//	{ fieldOfMytype { id title } }
+	// The fields must match exactly, including the ordering: "{ name id }"
+	// will be rejected.  But the arguments and directives, if any, need not
+	// match.
+	//
+	// TODO(benkraft): Also add ExpectIncludesFields and ExpectSubsetOfFields,
+	// or something, if you want to say, for example, that you have to request
+	// certain fields but others are optional.
+	ExpectExactFields string `yaml:"expect_exact_fields"`
 }
 
 // baseDir returns the directory of the config-file (relative to which

generate/convert.go

@@ -190,6 +190,13 @@ func (g *generator) convertDefinition(
 	// unless the binding is "-" which means "ignore the global binding".
 	globalBinding, ok := g.Config.Bindings[def.Name]
 	if ok && options.Bind != "-" {
+		if def.Kind == ast.Object || def.Kind == ast.Interface || def.Kind == ast.Union {
+			err := g.validateBindingSelection(
+				name, globalBinding, pos, selectionSet)
+			if err != nil {
+				return nil, err
+			}
+		}
 		goRef, err := g.addRef(globalBinding.Type)
 		return &goOpaqueType{goRef}, err
 	}

generate/generate_test.go

@@ -52,11 +52,14 @@ func TestGenerate(t *testing.T) {
 				Generated:        goFilename,
 				ExportOperations: queriesFilename,
 				Bindings: map[string]*TypeBinding{
-					"ID":           {Type: "github.com/Khan/genqlient/internal/testutil.ID"},
-					"DateTime":     {Type: "time.Time"},
-					"Junk":         {Type: "interface{}"},
-					"ComplexJunk":  {Type: "[]map[string]*[]*map[string]interface{}"},
-					"Pokemon":      {Type: "github.com/Khan/genqlient/internal/testutil.Pokemon"},
+					"ID":          {Type: "github.com/Khan/genqlient/internal/testutil.ID"},
+					"DateTime":    {Type: "time.Time"},
+					"Junk":        {Type: "interface{}"},
+					"ComplexJunk": {Type: "[]map[string]*[]*map[string]interface{}"},
+					"Pokemon": {
+						Type:              "github.com/Khan/genqlient/internal/testutil.Pokemon",
+						ExpectExactFields: "{ species level }",
+					},
 					"PokemonInput": {Type: "github.com/Khan/genqlient/internal/testutil.Pokemon"},
 				},
 				AllowBrokenFeatures: true,
@@ -117,6 +120,14 @@ func TestGenerate(t *testing.T) {
 	}
 }
 
+// TestGenerate is a snapshot-based test of error text.
+//
+// For each .go or .graphql file in testdata/errors, and corresponding
+// .schema.graphql file, it asserts that the given query returns an error, and
+// that that error's string-text matches the snapshot.  The snapshotting is
+// useful to ensure we don't accidentally make the text less readable, drop the
+// line numbers, etc.  We include both .go and .graphql tests, to make sure the
+// line numbers work in both cases.
 func TestGenerateErrors(t *testing.T) {
 	files, err := ioutil.ReadDir(errorsDir)
 	if err != nil {
@@ -143,6 +154,10 @@ func TestGenerateErrors(t *testing.T) {
 				Bindings: map[string]*TypeBinding{
 					"ValidScalar":   {Type: "string"},
 					"InvalidScalar": {Type: "bogus"},
+					"Pokemon": {
+						Type:              "github.com/Khan/genqlient/internal/testutil.Pokemon",
+						ExpectExactFields: "{ species level }",
+					},
 				},
 				AllowBrokenFeatures: true,
 			})

generate/testdata/errors/BindingWithIncorrectSelection.go

@@ -0,0 +1,7 @@
+package errors
+
+const _ = `# @genqlient
+query GetPokemonWrongFields {
+  pokemon { species }
+}
+`

generate/testdata/errors/BindingWithIncorrectSelection.graphql

@@ -0,0 +1,3 @@
+query GetPokemonWrongFields {
+  pokemon { species species }
+}

generate/testdata/errors/BindingWithIncorrectSelection.schema.graphql

@@ -0,0 +1,8 @@
+type Query {
+  pokemon: Pokemon
+}
+
+type Pokemon {
+  species: String!
+  level: Int!
+}

generate/testdata/queries/Pokemon.graphql

@@ -8,7 +8,8 @@ query GetPokemonSiblings($input: PokemonInput!) {
     roles
     name
     # this is mapped globally to internal/testutil.Pokemon:
-    pokemon { species level }
+    # note field ordering matters, but whitespace shouldn't.
+    pokemon {    species level }
     # this overrides said mapping:
     # @genqlient(bind: "-")
     genqlientPokemon: pokemon { species level }

generate/testdata/snapshots/TestGenerateErrors-BindingWithIncorrectSelection.go

@@ -0,0 +1 @@
+invalid selection for type-binding GetPokemonWrongFieldsPokemon: testdata/errors/BindingWithIncorrectSelection.schema.graphql:2: expected 2 fields, got 1

generate/testdata/snapshots/TestGenerateErrors-BindingWithIncorrectSelection.graphql

@@ -0,0 +1 @@
+invalid selection for type-binding GetPokemonWrongFieldsPokemon: testdata/errors/BindingWithIncorrectSelection.graphql:2: expected field 1 to be level, got species