diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f0d2f5d..692d7dd 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -36,6 +36,7 @@ When releasing a new version: ### Bug fixes: - genqlient now explicitly rejects argument, operation, and type names which are Go keywords, rather than failing with an opaque error. +- genqlient now gives better error messages if it thinks your schema is invalid. ## v0.4.0 diff --git a/generate/parse.go b/generate/parse.go index 2f62392..4569f25 100644 --- a/generate/parse.go +++ b/generate/parse.go @@ -10,11 +10,10 @@ import ( "strconv" "strings" - "github.com/vektah/gqlparser/v2" "github.com/vektah/gqlparser/v2/ast" - "github.com/vektah/gqlparser/v2/gqlerror" "github.com/vektah/gqlparser/v2/parser" "github.com/vektah/gqlparser/v2/validator" + _ "github.com/vektah/gqlparser/v2/validator/rules" ) func getSchema(globs StringList) (*ast.Schema, error) { @@ -32,21 +31,41 @@ func getSchema(globs StringList) (*ast.Schema, error) { sources[i] = &ast.Source{Name: filename, Input: string(text)} } - // Multi step schema validation - // Step 1 assume schema implicitly declares types that are required by the graphql spec - // Step 2 assume schema explicitly declares types that are required by the graphql spec - var ( - schema *ast.Schema - graphqlError *gqlerror.Error - ) - schema, graphqlError = gqlparser.LoadSchema(sources...) + // Ideally here we'd just call gqlparser.LoadSchema. But the schema we are + // given may or may not contain the builtin types String, Int, etc. (The + // spec says it shouldn't, but introspection will return those types, and + // some introspection-to-SDL tools aren't smart enough to remove them.) So + // we inline LoadSchema and insert some checks. + document, graphqlError := parser.ParseSchemas(sources...) if graphqlError != nil { - schema, graphqlError = validator.LoadSchema(sources...) - if graphqlError != nil { - return nil, errorf(nil, "invalid schema: %v", graphqlError) + // Schema doesn't even parse. + return nil, errorf(nil, "invalid schema: %v", graphqlError) + } + + // Check if we have a builtin type. (String is an arbitrary choice.) + hasBuiltins := false + for _, def := range document.Definitions { + if def.Name == "String" { + hasBuiltins = true + break } } + if !hasBuiltins { + // modified from parser.ParseSchemas + var preludeAST *ast.SchemaDocument + preludeAST, graphqlError = parser.ParseSchema(validator.Prelude) + if graphqlError != nil { + return nil, errorf(nil, "invalid prelude (probably a gqlparser bug): %v", graphqlError) + } + document.Merge(preludeAST) + } + + schema, graphqlError := validator.ValidateSchemaDocument(document) + if graphqlError != nil { + return nil, errorf(nil, "invalid schema: %v", graphqlError) + } + return schema, nil } diff --git a/generate/testdata/errors/InvalidSchema.go b/generate/testdata/errors/InvalidSchemaSyntax.go similarity index 100% rename from generate/testdata/errors/InvalidSchema.go rename to generate/testdata/errors/InvalidSchemaSyntax.go diff --git a/generate/testdata/errors/InvalidSchema.graphql b/generate/testdata/errors/InvalidSchemaSyntax.graphql similarity index 100% rename from generate/testdata/errors/InvalidSchema.graphql rename to generate/testdata/errors/InvalidSchemaSyntax.graphql diff --git a/generate/testdata/errors/InvalidSchema.schema.graphql b/generate/testdata/errors/InvalidSchemaSyntax.schema.graphql similarity index 100% rename from generate/testdata/errors/InvalidSchema.schema.graphql rename to generate/testdata/errors/InvalidSchemaSyntax.schema.graphql diff --git a/generate/testdata/errors/InvalidSchemaWithBuiltins.graphql b/generate/testdata/errors/InvalidSchemaWithBuiltins.graphql new file mode 100644 index 0000000..36cf9f0 --- /dev/null +++ b/generate/testdata/errors/InvalidSchemaWithBuiltins.graphql @@ -0,0 +1 @@ +query MyQuery { f g } diff --git a/generate/testdata/errors/InvalidSchemaWithBuiltins.schema.graphql b/generate/testdata/errors/InvalidSchemaWithBuiltins.schema.graphql new file mode 100644 index 0000000..297c238 --- /dev/null +++ b/generate/testdata/errors/InvalidSchemaWithBuiltins.schema.graphql @@ -0,0 +1,101 @@ +type Query { + f: String + g: Bogus +} + +scalar Int +scalar Float +scalar String +scalar Boolean +scalar ID + +directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +directive @deprecated(reason: String = "No longer supported") on FIELD_DEFINITION | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | ENUM_VALUE +directive @specifiedBy(url: String!) on SCALAR + +type __Schema { + description: String + types: [__Type!]! + queryType: __Type! + mutationType: __Type + subscriptionType: __Type + directives: [__Directive!]! +} + +type __Type { + kind: __TypeKind! + name: String + description: String + fields(includeDeprecated: Boolean = false): [__Field!] + interfaces: [__Type!] + possibleTypes: [__Type!] + enumValues(includeDeprecated: Boolean = false): [__EnumValue!] + inputFields: [__InputValue!] + ofType: __Type + specifiedByURL: String +} + +type __Field { + name: String! + description: String + args: [__InputValue!]! + type: __Type! + isDeprecated: Boolean! + deprecationReason: String +} + +type __InputValue { + name: String! + description: String + type: __Type! + defaultValue: String +} + +type __EnumValue { + name: String! + description: String + isDeprecated: Boolean! + deprecationReason: String +} + +enum __TypeKind { + SCALAR + OBJECT + INTERFACE + UNION + ENUM + INPUT_OBJECT + LIST + NON_NULL +} + +type __Directive { + name: String! + description: String + locations: [__DirectiveLocation!]! + args: [__InputValue!]! + isRepeatable: Boolean! +} + +enum __DirectiveLocation { + QUERY + MUTATION + SUBSCRIPTION + FIELD + FRAGMENT_DEFINITION + FRAGMENT_SPREAD + INLINE_FRAGMENT + VARIABLE_DEFINITION + SCHEMA + SCALAR + OBJECT + FIELD_DEFINITION + ARGUMENT_DEFINITION + INTERFACE + UNION + ENUM + ENUM_VALUE + INPUT_OBJECT + INPUT_FIELD_DEFINITION +} diff --git a/generate/testdata/errors/InvalidSchemaWithoutBuiltins.graphql b/generate/testdata/errors/InvalidSchemaWithoutBuiltins.graphql new file mode 100644 index 0000000..36cf9f0 --- /dev/null +++ b/generate/testdata/errors/InvalidSchemaWithoutBuiltins.graphql @@ -0,0 +1 @@ +query MyQuery { f g } diff --git a/generate/testdata/errors/InvalidSchemaWithoutBuiltins.schema.graphql b/generate/testdata/errors/InvalidSchemaWithoutBuiltins.schema.graphql new file mode 100644 index 0000000..e81082f --- /dev/null +++ b/generate/testdata/errors/InvalidSchemaWithoutBuiltins.schema.graphql @@ -0,0 +1,4 @@ +type Query { + f: String + g: Bogus +} diff --git a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchema-go b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchema-go deleted file mode 100644 index 6c9eca4..0000000 --- a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchema-go +++ /dev/null @@ -1 +0,0 @@ -testdata/errors/InvalidSchema.schema.graphql:4: invalid schema: Expected :, found } diff --git a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchema-graphql b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchema-graphql deleted file mode 100644 index 6c9eca4..0000000 --- a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchema-graphql +++ /dev/null @@ -1 +0,0 @@ -testdata/errors/InvalidSchema.schema.graphql:4: invalid schema: Expected :, found } diff --git a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaSyntax-go b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaSyntax-go new file mode 100644 index 0000000..0120cee --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaSyntax-go @@ -0,0 +1 @@ +testdata/errors/InvalidSchemaSyntax.schema.graphql:4: invalid schema: Expected :, found } diff --git a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaSyntax-graphql b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaSyntax-graphql new file mode 100644 index 0000000..0120cee --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaSyntax-graphql @@ -0,0 +1 @@ +testdata/errors/InvalidSchemaSyntax.schema.graphql:4: invalid schema: Expected :, found } diff --git a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaWithBuiltins-graphql b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaWithBuiltins-graphql new file mode 100644 index 0000000..1c33e00 --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaWithBuiltins-graphql @@ -0,0 +1 @@ +testdata/errors/InvalidSchemaWithBuiltins.schema.graphql:3: invalid schema: Undefined type Bogus. diff --git a/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaWithoutBuiltins-graphql b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaWithoutBuiltins-graphql new file mode 100644 index 0000000..3c07e62 --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateErrors-InvalidSchemaWithoutBuiltins-graphql @@ -0,0 +1 @@ +testdata/errors/InvalidSchemaWithoutBuiltins.schema.graphql:3: invalid schema: Undefined type Bogus.