Skip to content

Commit

Permalink
Validate against a case gqlparser doesn't catch (#197)
Browse files Browse the repository at this point in the history
While writing tests at some point I came across an invalid query that
gqlparser doesn't catch, and which causes a panic for us.  Now we
validate for it and return a nice error instead of panicing.

Fixes #176.

Test plan: make check
  • Loading branch information
benjaminjkraft authored May 18, 2022
1 parent d4ec64f commit 3685f3f
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 4 deletions.
32 changes: 28 additions & 4 deletions generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,16 +224,40 @@ func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
validator.Walk(g.schema, doc, &observers)
}

// validateOperation checks for a few classes of operations that gqlparser
// considers valid but we don't allow, and returns an error if this operation
// is invalid for genqlient's purposes.
func (g *generator) validateOperation(op *ast.OperationDefinition) error {
opType, err := g.baseTypeForOperation(op.Operation)
switch {
case err != nil:
// (e.g. operation has subscriptions, which we don't support)
return err
case opType == nil:
// gqlparser should err here, but doesn't [1], so we err to prevent
// panics later.
// TODO(benkraft): Remove once gqlparser is fixed.
// [1] https://github.com/vektah/gqlparser/issues/221
return errorf(op.Position, "schema has no %v type", op.Operation)
}

if op.Name == "" {
return errorf(op.Position, "operations must have operation-names")
} else if goKeywords[op.Name] {
return errorf(op.Position, "operation name must not be a go keyword")
}

return nil
}

// addOperation adds to g.Operations the information needed to generate a
// genqlient entrypoint function for the given operation. It also adds to
// g.typeMap any types referenced by the operation, except for types belonging
// to named fragments, which are added separately by Generate via
// convertFragment.
func (g *generator) addOperation(op *ast.OperationDefinition) error {
if op.Name == "" {
return errorf(op.Position, "operations must have operation-names")
} else if goKeywords[op.Name] {
return errorf(op.Position, "operation name must not be a go keyword")
if err := g.validateOperation(op); err != nil {
return err
}

queryDoc := &ast.QueryDocument{
Expand Down
1 change: 1 addition & 0 deletions generate/testdata/errors/NoMutationType.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mutation M { g }
1 change: 1 addition & 0 deletions generate/testdata/errors/NoMutationType.schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type Query { f: String }
1 change: 1 addition & 0 deletions generate/testdata/errors/NoQueryType.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
query Q { f }
1 change: 1 addition & 0 deletions generate/testdata/errors/NoQueryType.schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
type T { f: String }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testdata/errors/NoMutationType.graphql:1: schema has no mutation type
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
testdata/errors/NoQueryType.graphql:1: schema has no query type

0 comments on commit 3685f3f

Please sign in to comment.