Skip to content

Commit

Permalink
Update validation rules
Browse files Browse the repository at this point in the history 
Ensure consistent behaviour with graphql-js, matching expected
test outputs. In many cases this has been a change to quoting and
formatting of identifiers, however other changes to application of
rules has also applied
  • Loading branch information
@dackroyd
dackroyd committed Apr 1, 2024
1 parent 8425242 commit 8e0039d
Showing 1 changed file with 94 additions and 26 deletions.
120 changes: 94 additions & 26 deletions internal/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type varSet map[*ast.InputValueDefinition]struct{}

type selectionPair struct{ a, b ast.Selection }

type nameSet map[string]errors.Location
type nameSet map[string][]errors.Location

type fieldInfo struct {
sf *ast.FieldDefinition
Expand Down Expand Up @@ -83,15 +83,18 @@ func Validate(s *ast.Schema, doc *ast.ExecutableDefinition, variables map[string
if op.Name.Name == "" && len(doc.Operations) != 1 {
c.addErr(op.Loc, "LoneAnonymousOperation", "This anonymous operation must be the only defined operation.")
}
if op.Name.Name != "" {
validateName(c, opNames, op.Name, "UniqueOperationNames", "operation")

if n := op.Name.Name; n != "" {
opNames[n] = append(opNames[n], op.Name.Loc)
}

validateDirectives(opc, string(op.Type), op.Directives)

varNames := make(nameSet)
for _, v := range op.Vars {
validateName(c, varNames, v.Name, "UniqueVariableNames", "variable")
varNames[v.Name.Name] = append(varNames[v.Name.Name], v.Name.Loc)

validateDirectives(opc, "VARIABLE_DEFINITION", v.Directives)

t := resolveType(c, v.Type)
if !canBeInput(t) {
Expand All @@ -114,6 +117,10 @@ func Validate(s *ast.Schema, doc *ast.ExecutableDefinition, variables map[string
}
}

for n, locs := range varNames {
validateName(c, locs, n, "UniqueVariableNames", "variable")
}

var entryPoint ast.NamedType
switch op.Type {
case query.Query:
Expand All @@ -135,12 +142,17 @@ func Validate(s *ast.Schema, doc *ast.ExecutableDefinition, variables map[string
}
}

fragNames := make(nameSet)
for n, locs := range opNames {
validateName(c, locs, n, "UniqueOperationNames", "operation")
}

fragNames := make(nameSet, len(doc.Fragments))
fragVisited := make(map[*ast.FragmentDefinition]struct{})
for _, frag := range doc.Fragments {
opc := &opContext{c, fragUsedBy[frag]}

validateName(c, fragNames, frag.Name, "UniqueFragmentNames", "fragment")
fragNames[frag.Name.Name] = append(fragNames[frag.Name.Name], frag.Name.Loc)

validateDirectives(opc, "FRAGMENT_DEFINITION", frag.Directives)

t := unwrapType(resolveType(c, &frag.On))
Expand All @@ -157,6 +169,10 @@ func Validate(s *ast.Schema, doc *ast.ExecutableDefinition, variables map[string
}
}

for n, locs := range fragNames {
validateName(c, locs, n, "UniqueFragmentNames", "fragment")
}

for _, frag := range doc.Fragments {
if len(fragUsedBy[frag]) == 0 {
c.addErr(frag.Loc, "NoUnusedFragments", "Fragment %q is never used.", frag.Name.Name)
Expand Down Expand Up @@ -475,7 +491,7 @@ func detectFragmentCycleSel(c *context, sel ast.Selection, fragVisited map[*ast.
if len(cyclePath) > 1 {
names := make([]string, len(cyclePath)-1)
for i, frag := range cyclePath[:len(cyclePath)-1] {
names[i] = frag.Name.Name
names[i] = fmt.Sprintf("%q", frag.Name.Name)
}
via = " via " + strings.Join(names, ", ")
}
Expand Down Expand Up @@ -573,7 +589,7 @@ func (c *context) validateFieldOverlap(a, b *ast.Field) ([]string, []errors.Loca
if asf := c.fieldMap[a].sf; asf != nil {
if bsf := c.fieldMap[b].sf; bsf != nil {
if !typesCompatible(asf.Type, bsf.Type) {
return []string{fmt.Sprintf("they return conflicting types %s and %s", asf.Type, bsf.Type)}, nil
return []string{fmt.Sprintf("they return conflicting types %q and %q", asf.Type, bsf.Type)}, nil
}
}
}
Expand All @@ -582,7 +598,7 @@ func (c *context) validateFieldOverlap(a, b *ast.Field) ([]string, []errors.Loca
bt := c.fieldMap[b].parent
if at == nil || bt == nil || at == bt {
if a.Name.Name != b.Name.Name {
return []string{fmt.Sprintf("%s and %s are different fields", a.Name.Name, b.Name.Name)}, nil
return []string{fmt.Sprintf("%q and %q are different fields", a.Name.Name, b.Name.Name)}, nil
}

if argumentsConflict(a.Arguments, b.Arguments) {
Expand Down Expand Up @@ -651,18 +667,17 @@ func resolveType(c *context, t ast.Type) ast.Type {
}

func validateDirectives(c *opContext, loc string, directives ast.DirectiveList) {
directiveNames := make(nameSet)
directiveNames := make(nameSet, len(directives))
for _, d := range directives {
dirName := d.Name.Name
validateNameCustomMsg(c.context, directiveNames, d.Name, "UniqueDirectivesPerLocation", func() string {
return fmt.Sprintf("The directive %q can only be used once at this location.", dirName)
})

directiveNames[dirName] = append(directiveNames[dirName], d.Name.Loc)

validateArgumentLiterals(c, d.Arguments)

dd, ok := c.schema.Directives[dirName]
if !ok {
c.addErr(d.Name.Loc, "KnownDirectives", "Unknown directive %q.", dirName)
c.addErr(d.Name.Loc, "KnownDirectives", "Unknown directive %q.", "@"+dirName)
continue
}

Expand All @@ -674,28 +689,57 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
}
}
if !locOK {
c.addErr(d.Name.Loc, "KnownDirectives", "Directive %q may not be used on %s.", dirName, loc)
c.addErr(d.Name.Loc, "KnownDirectives", "Directive %q may not be used on %s.", "@"+dirName, loc)
}

validateArgumentTypes(c, d.Arguments, dd.Arguments, d.Name.Loc,
func() string { return fmt.Sprintf("directive %q", "@"+dirName) },
func() string { return fmt.Sprintf("Directive %q", "@"+dirName) },
)
}

for n := range directiveNames {
dd, ok := c.schema.Directives[n]
if !ok {
// Invalid directive will have been flagged already
continue
}

if dd.Repeatable {
continue
}

ds := directiveNames[n]
if len(ds) <= 1 {
continue
}

for _, loc := range ds[1:] {
// Duplicate directive errors are inconsistent with the behaviour for other types in graphql-js
// Instead of reporting a single error with all locations, errors are reported for each duplicate after the first declaration
// with the original location, and the duplicate. Behaviour is replicated here, as we use those tests to validate the implementation
validateNameCustomMsg(c.context, []errors.Location{ds[0], loc}, "UniqueDirectivesPerLocation", func() string {
return fmt.Sprintf("The directive %q can only be used once at this location.", "@"+n)
})
}
}
}

func validateName(c *context, set nameSet, name ast.Ident, rule string, kind string) {
validateNameCustomMsg(c, set, name, rule, func() string {
return fmt.Sprintf("There can be only one %s named %q.", kind, name.Name)
func validateName(c *context, locs []errors.Location, name string, rule string, kind string) {
validateNameCustomMsg(c, locs, rule, func() string {
if kind == "variable" {
return fmt.Sprintf("There can be only one %s named %q.", kind, "$"+name)
}

return fmt.Sprintf("There can be only one %s named %q.", kind, name)
})
}

func validateNameCustomMsg(c *context, set nameSet, name ast.Ident, rule string, msg func() string) {
if loc, ok := set[name.Name]; ok {
c.addErrMultiLoc([]errors.Location{loc, name.Loc}, rule, msg())
func validateNameCustomMsg(c *context, locs []errors.Location, rule string, msg func() string) {
if len(locs) > 1 {
c.addErrMultiLoc(locs, rule, msg())
return
}
set[name.Name] = name.Loc
}

func validateArgumentTypes(c *opContext, args ast.ArgumentList, argDecls ast.ArgumentsDefinition, loc errors.Location, owner1, owner2 func() string) {
Expand All @@ -713,7 +757,11 @@ func validateArgumentTypes(c *opContext, args ast.ArgumentList, argDecls ast.Arg
for _, decl := range argDecls {
if _, ok := decl.Type.(*ast.NonNull); ok {
if _, ok := args.Get(decl.Name.Name); !ok {
c.addErr(loc, "ProvidedNonNullArguments", "%s argument %q of type %q is required but not provided.", owner2(), decl.Name.Name, decl.Type)
if decl.Default != nil {
continue
}

c.addErr(loc, "ProvidedRequiredArguments", "%s argument %q of type %q is required, but it was not provided.", owner2(), decl.Name.Name, decl.Type)
}
}
}
Expand All @@ -722,8 +770,13 @@ func validateArgumentTypes(c *opContext, args ast.ArgumentList, argDecls ast.Arg
func validateArgumentLiterals(c *opContext, args ast.ArgumentList) {
argNames := make(nameSet)
for _, arg := range args {
validateName(c.context, argNames, arg.Name, "UniqueArgumentNames", "argument")
validateLiteral(c, arg.Value)

argNames[arg.Name.Name] = append(argNames[arg.Name.Name], arg.Name.Loc)
}

for n, locs := range argNames {
validateName(c.context, locs, n, "UniqueArgumentNames", "argument")
}
}

Expand All @@ -732,9 +785,20 @@ func validateLiteral(c *opContext, l ast.Value) {
case *ast.ObjectValue:
fieldNames := make(nameSet)
for _, f := range l.Fields {
validateName(c.context, fieldNames, f.Name, "UniqueInputFieldNames", "input field")
fieldNames[f.Name.Name] = append(fieldNames[f.Name.Name], f.Name.Loc)
validateLiteral(c, f.Value)
}

for n, locs := range fieldNames {
if len(locs) <= 1 {
continue
}

// Again, inconsistent reporting of locations...
for _, loc := range locs[1:] {
validateName(c.context, []errors.Location{locs[0], loc}, n, "UniqueInputFieldNames", "input field")
}
}
case *ast.ListValue:
for _, entry := range l.Values {
validateLiteral(c, entry)
Expand Down Expand Up @@ -766,7 +830,9 @@ func validateValueType(c *opContext, v ast.Value, t ast.Type) (bool, string) {
if v2 := op.Vars.Get(v.Name); v2 != nil {
t2, err := common.ResolveType(v2.Type, c.schema.Resolve)
if _, ok := t2.(*ast.NonNull); !ok && v2.Default != nil {
t2 = &ast.NonNull{OfType: t2}
if _, ok := v2.Default.(*ast.NullValue); !ok {
t2 = &ast.NonNull{OfType: t2}
}
}
if err == nil && !typeCanBeUsedAs(t2, t) {
c.addErrMultiLoc([]errors.Location{v2.Loc, v.Loc}, "VariablesInAllowedPosition", "Variable %q of type %q used in position expecting type %q.", "$"+v.Name, t2, t)
Expand Down Expand Up @@ -918,6 +984,8 @@ func canBeInput(t ast.Type) bool {
return canBeInput(t.OfType)
case *ast.NonNull:
return canBeInput(t.OfType)
case nil:
return true
default:
return false
}
Expand Down

0 comments on commit 8e0039d

Please sign in to comment.