Ben Kraft
e38a212de2
GraphQL schemas have some builtin types, like `String`. The spec says your SDL must not include those, but in practice some schemas do. (This is probably because introspection must include them, and some tools that create SDL from introspection don't know they're supposed to filter them out.) Anyway, we've since #145 had logic to handle this; we just parse with and without the prelude that defines them and see which works. The problem is that this makes for very confusing error messages if you have an invalid schema. (Or if you have a schema that you think is valid but gqlparser doesn't, which is the more common case in the wild; see for example #200.) Right now if both ways error we take the without-prelude error, which if you didn't define the builtins is just `undefined type String`; if we took the with-prelude error then if you did define the builtins you'd just get `type String defined twice`. So we actually have to be smart if we want good error messages for everyone. So in this commit we are smart: we check if your schema defines `String`, and include the prelude only if it does not. To do this I basically inlined `gqlparser.LoadSchema` (twice), so that in between parsing and validation we can check if you have `String` and if not add the prelude. This should in theory be both more efficient (we don't have to do everything twice) and give better error messages, although it's a bit more code. Fixes #175. Test plan: make check
2 lines
90 B
Plaintext
2 lines
90 B
Plaintext
testdata/errors/InvalidSchemaSyntax.schema.graphql:4: invalid schema: Expected :, found }
|