Names must not start with two underscores `__`
qwerdenkerXD opened this issue · 6 comments
Description
The Specification declares the following:
# Reserved Names
Any Name within a GraphQL type system must not start with two underscores "__"
unless it is part of the introspection system as defined by this specification.
Currently, this is not satisfied by apollo-compiler@1.0.0-beta.6
and has been forgotten in PR #713
Steps to reproduce
Validate the following schema which has some of such invalid names defined:
type Query {
__field(__arg: __MyScalar!): __MyType!
}
type __MyType {}
interface __MyInterface {}
scalar __MyScalar
enum __MyEnum {
__EnumValue
}
union __MyUnion = __MyType | __Schema
input __MyInput {
__field: __MyScalar!
}
Here a code sample.
Expected result
The validation should fail and have some respective diagnostics.
Actual result
The validation succeeds.
Environment
- Operating system and version: Ubuntu 20.04.6 LTS
- Shell (bash/zsh/powershell): zsh
apollo-rs
crate: apollo-compiler- Crate version: 1.0.0-beta.6
Name
also represents introspection names, so it's intentional that it supports underscores. We should handle this in validation, though!
I think we could check this either:
In various.validate()
methods, underif file_id != BUILT_IN
for introspection names. I believe so far we don’t have validation rules that need to find every singleName
, so we’ll need to be careful not to miss some of them in various parts of data structures.When converting from CST to AST, underif file_id != BUILT_IN
. This requires adding the ability for this conversion to emit parse errors. (Currently it relies on apollo-parser having emitted all relevant errors.)In apollo-parser’s lexer, with a newallow_builtin_names
configuration
I have a mild preference for the lexer, since that’s where we check other aspects of Name
syntax
I wrote the above with the incorrect assumption that every single Name
anywhere in a document would need checking. The spec phrasing isn’t great but I think the spirit of it is about definitions that introduce a new named thing in a type system. So yes, checking in Schema::validation
makes more sense.
The reserved name bit of spec does say "Any Name within a GraphQL type system" though. Type system being a schema. 🤔
oh, my bad, you're right.