Names must not start with two underscores __

[qwerdenkerXD]

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 [email protected] 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

[goto-bus-stop]

Name also represents introspection names, so it's intentional that it supports underscores. We should handle this in validation, though!

[SimonSapin]

I think we could check this either:

• In various .validate() methods, under if file_id != BUILT_IN for introspection names. I believe so far we don’t have validation rules that need to find every single Name, 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, under if 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 new allow_builtin_names configuration

I have a mild preference for the lexer, since that’s where we check other aspects of Name syntax

[SimonSapin]

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.

[qwerdenkerXD]

sounds like everything to me. So including it in executables is also necessary.
But I agree to check only for definitions, as you aren't able to refer to such definitions anyway if you can't define them.

[SimonSapin]

The reserved name bit of spec does say "Any Name within a GraphQL type system" though. Type system being a schema. 🤔

[qwerdenkerXD]

oh, my bad, you're right.