diff --git a/generate/binding_validation.go b/generate/binding_validation.go new file mode 100644 index 0000000..9f5f8e2 --- /dev/null +++ b/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 +} diff --git a/generate/config.go b/generate/config.go index 7ea3fcf..08c2a0c 100644 --- a/generate/config.go +++ b/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 diff --git a/generate/convert.go b/generate/convert.go index 8d47117..d1531b8 100644 --- a/generate/convert.go +++ b/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 } diff --git a/generate/generate_test.go b/generate/generate_test.go index a909127..9c21915 100644 --- a/generate/generate_test.go +++ b/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, }) diff --git a/generate/testdata/errors/BindingWithIncorrectSelection.go b/generate/testdata/errors/BindingWithIncorrectSelection.go new file mode 100644 index 0000000..9b5ae1a --- /dev/null +++ b/generate/testdata/errors/BindingWithIncorrectSelection.go @@ -0,0 +1,7 @@ +package errors + +const _ = `# @genqlient +query GetPokemonWrongFields { + pokemon { species } +} +` diff --git a/generate/testdata/errors/BindingWithIncorrectSelection.graphql b/generate/testdata/errors/BindingWithIncorrectSelection.graphql new file mode 100644 index 0000000..38eba3d --- /dev/null +++ b/generate/testdata/errors/BindingWithIncorrectSelection.graphql @@ -0,0 +1,3 @@ +query GetPokemonWrongFields { + pokemon { species species } +} diff --git a/generate/testdata/errors/BindingWithIncorrectSelection.schema.graphql b/generate/testdata/errors/BindingWithIncorrectSelection.schema.graphql new file mode 100644 index 0000000..ca01037 --- /dev/null +++ b/generate/testdata/errors/BindingWithIncorrectSelection.schema.graphql @@ -0,0 +1,8 @@ +type Query { + pokemon: Pokemon +} + +type Pokemon { + species: String! + level: Int! +} diff --git a/generate/testdata/queries/Pokemon.graphql b/generate/testdata/queries/Pokemon.graphql index 5b40414..f41db71 100644 --- a/generate/testdata/queries/Pokemon.graphql +++ b/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 } diff --git a/generate/testdata/snapshots/TestGenerateErrors-BindingWithIncorrectSelection.go b/generate/testdata/snapshots/TestGenerateErrors-BindingWithIncorrectSelection.go new file mode 100644 index 0000000..6768bbf --- /dev/null +++ b/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 diff --git a/generate/testdata/snapshots/TestGenerateErrors-BindingWithIncorrectSelection.graphql b/generate/testdata/snapshots/TestGenerateErrors-BindingWithIncorrectSelection.graphql new file mode 100644 index 0000000..fbaaea2 --- /dev/null +++ b/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