Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

3.13 Directive validation edits #1089

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

3.13 Directive validation edits #1089

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71a76ed
directive validation consistency edits
jbellenger Apr 2, 2024
cb28e5e
add missing period
jbellenger Apr 2, 2024
c8ca2c1
Update spec/Section 3 -- Type System.md
benjie May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions spec/Section 3 -- Type System.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,19 +2033,20 @@ directive @invalidExample(arg: String @invalidExample) on ARGUMENT_DEFINITION
Note: The order in which directives appear may be significant, including
repeatable directives.

**Validation**
**Type Validation**

1. A directive definition must not contain the use of a directive which
1. A Directive definition must include at least one DirectiveLocation.
Copy link
Author

@jbellenger jbellenger Apr 2, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this worded differently from the other "one or more" constraints?
If this were worded in the same style as Objects, it would read "A Directive must define one or more DirectiveLocations"

To be pedantic, while this sentence would attempt to use a plural form of the DirectiveLocation token, it could also be interpreted as a plural form of the DirectiveLocations token, which is given a distinct definition in Appendix B.4 and does not match the Directive grammar.

I thought that a phrasing that avoided pluralizing DirectiveLocation would be less ambiguous, though recognize there's also value in using consistent language. I'm open to feedback.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're also generally okay with must include one or more DirectiveLocation.

2. A Directive definition must not contain the use of a Directive which
references itself directly.
2. A directive definition must not contain the use of a directive which
3. A Directive definition must not contain the use of a Directive which
references itself indirectly by referencing a Type or Directive which
transitively includes a reference to this directive.
benjie marked this conversation as resolved.
Show resolved Hide resolved
3. The directive must not have a name which begins with the characters {"\_\_"}
4. The Directive must not have a name which begins with the characters {"\_\_"}
(two underscores).
4. For each argument of the directive:
5. For each argument of the Directive:
1. The argument must not have a name which begins with the characters
{"\_\_"} (two underscores).
2. The argument must have a unique name within that directive; no two
2. The argument must have a unique name within that Directive; no two
arguments may share the same name.
3. The argument must accept a type where {IsInputType(argumentType)} returns
{true}.
Expand Down