Open qwerdenkerXD opened 10 months ago
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:~
.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.~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.)~allow_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.
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.
The reserved name bit of spec does say "Any Name within a GraphQL type system" though. Type system being a schema. :thinking:
oh, my bad, you're right.
Description
The Specification declares the following:
Currently, this is not satisfied by
apollo-compiler@1.0.0-beta.6
and has been forgotten in PR #713Steps to reproduce
Validate the following schema which has some of such invalid names defined:
Here a code sample.
Expected result
The validation should fail and have some respective diagnostics.
Actual result
The validation succeeds.
Environment
apollo-rs
crate: apollo-compiler