Commit:

9046c14d135e7e0785b6a43cd0e0ceef7e8773b4 [9046c14]
Parents:
657fbbba26
Author:
Lee Byron <[email protected]>
Date:
24 September 2015 at 7:49:45 AM SGT

[RFC] Move input field uniqueness validator

This proposes moving input field uniqueness assertion from the parser to the validator. This simplifies the parser and allows these errors to be reported as part of the collection of validation errors which is actually more valuable.

A follow-up RFC against the spec will be added

Author: sogko

language/parser/parser.go

@@ -706,18 +706,16 @@ func parseObject(parser *Parser, isConst bool) (*ast.ObjectValue, error) {
 		return nil, err
 	}
 	fields := []*ast.ObjectField{}
-	fieldNames := map[string]bool{}
 	for {
 		if skp, err := skip(parser, lexer.TokenKind[lexer.BRACE_R]); err != nil {
 			return nil, err
 		} else if skp {
 			break
 		}
-		field, fieldName, err := parseObjectField(parser, isConst, fieldNames)
+		field, err := parseObjectField(parser, isConst)
 		if err != nil {
 			return nil, err
 		}
-		fieldNames[fieldName] = true
 		fields = append(fields, field)
 	}
 	return ast.NewObjectValue(&ast.ObjectValue{
@@ -729,30 +727,25 @@ func parseObject(parser *Parser, isConst bool) (*ast.ObjectValue, error) {
 /**
  * ObjectField[Const] : Name : Value[?Const]
  */
-func parseObjectField(parser *Parser, isConst bool, fieldNames map[string]bool) (*ast.ObjectField, string, error) {
+func parseObjectField(parser *Parser, isConst bool) (*ast.ObjectField, error) {
 	start := parser.Token.Start
 	name, err := parseName(parser)
 	if err != nil {
-		return nil, "", err
-	}
-	fieldName := name.Value
-	if _, ok := fieldNames[fieldName]; ok {
-		descp := fmt.Sprintf("Duplicate input object field %v.", fieldName)
-		return nil, "", gqlerrors.NewSyntaxError(parser.Source, start, descp)
+		return nil, err
 	}
 	_, err = expect(parser, lexer.TokenKind[lexer.COLON])
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	value, err := parseValueLiteral(parser, isConst)
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 	return ast.NewObjectField(&ast.ObjectField{
 		Name:  name,
 		Value: value,
 		Loc:   loc(parser, start),
-	}), fieldName, nil
+	}), nil
 }
 
 /* Implements the parsing rules in the Directives section. */

language/parser/parser_test.go

@@ -161,15 +161,6 @@ func TestParsesConstantDefaultValues(t *testing.T) {
 	testErrorMessage(t, test)
 }
 
-func TestDuplicatedKeysInInputObject(t *testing.T) {
-	test := errorMessageTest{
-		`{ field(arg: { a: 1, a: 2 }) }'`,
-		`Syntax Error GraphQL (1:22) Duplicate input object field a.`,
-		false,
-	}
-	testErrorMessage(t, test)
-}
-
 func TestDoesNotAcceptFragmentsNameOn(t *testing.T) {
 	test := errorMessageTest{
 		`fragment on on on { on }`,

rules.go

@@ -34,6 +34,7 @@ var SpecifiedRules = []ValidationRuleFn{
 	ScalarLeafsRule,
 	UniqueArgumentNamesRule,
 	UniqueFragmentNamesRule,
+	UniqueInputFieldNamesRule,
 	UniqueOperationNamesRule,
 	VariablesAreInputTypesRule,
 	VariablesInAllowedPositionRule,
@@ -1660,6 +1661,59 @@ func UniqueFragmentNamesRule(context *ValidationContext) *ValidationRuleInstance
 	}
 }
 
+/**
+ * UniqueInputFieldNamesRule
+ *
+ * A GraphQL input object value is only valid if all supplied fields are
+ * uniquely named.
+ */
+func UniqueInputFieldNamesRule(context *ValidationContext) *ValidationRuleInstance {
+	knownNameStack := []map[string]*ast.Name{}
+	knownNames := map[string]*ast.Name{}
+
+	visitorOpts := &visitor.VisitorOptions{
+		KindFuncMap: map[string]visitor.NamedVisitFuncs{
+			kinds.ObjectValue: visitor.NamedVisitFuncs{
+				Enter: func(p visitor.VisitFuncParams) (string, interface{}) {
+					knownNameStack = append(knownNameStack, knownNames)
+					knownNames = map[string]*ast.Name{}
+					return visitor.ActionNoChange, nil
+				},
+				Leave: func(p visitor.VisitFuncParams) (string, interface{}) {
+					// pop
+					knownNames, knownNameStack = knownNameStack[len(knownNameStack)-1], knownNameStack[:len(knownNameStack)-1]
+					return visitor.ActionNoChange, nil
+				},
+			},
+			kinds.ObjectField: visitor.NamedVisitFuncs{
+				Kind: func(p visitor.VisitFuncParams) (string, interface{}) {
+					var action = visitor.ActionNoChange
+					var result interface{}
+					if node, ok := p.Node.(*ast.ObjectField); ok {
+						fieldName := ""
+						if node.Name != nil {
+							fieldName = node.Name.Value
+						}
+						if knownNameAST, ok := knownNames[fieldName]; ok {
+							return newValidationRuleError(
+								fmt.Sprintf(`There can be only one input field named "%v".`, fieldName),
+								[]ast.Node{knownNameAST, node.Name},
+							)
+						} else {
+							knownNames[fieldName] = node.Name
+						}
+
+					}
+					return action, result
+				},
+			},
+		},
+	}
+	return &ValidationRuleInstance{
+		VisitorOpts: visitorOpts,
+	}
+}
+
 /**
  * UniqueOperationNamesRule
  * Unique operation names

rules_unique_input_field_names_test.go

@@ -0,0 +1,65 @@
+package graphql_test
+
+import (
+	"testing"
+
+	"github.com/graphql-go/graphql"
+	"github.com/graphql-go/graphql/gqlerrors"
+	"github.com/graphql-go/graphql/testutil"
+)
+
+func TestValidate_UniqueInputFieldNames_InputObjectWithFields(t *testing.T) {
+	testutil.ExpectPassesRule(t, graphql.UniqueInputFieldNamesRule, `
+      {
+        field(arg: { f: true })
+      }
+    `)
+}
+func TestValidate_UniqueInputFieldNames_SameInputObjectWithinTwoArgs(t *testing.T) {
+	testutil.ExpectPassesRule(t, graphql.UniqueInputFieldNamesRule, `
+      {
+        field(arg1: { f: true }, arg2: { f: true })
+      }
+    `)
+}
+func TestValidate_UniqueInputFieldNames_MultipleInputObjectFields(t *testing.T) {
+	testutil.ExpectPassesRule(t, graphql.UniqueInputFieldNamesRule, `
+      {
+        field(arg: { f1: "value", f2: "value", f3: "value" })
+      }
+    `)
+}
+func TestValidate_UniqueInputFieldNames_AllowsForNestedInputObjectsWithSimilarFields(t *testing.T) {
+	testutil.ExpectPassesRule(t, graphql.UniqueInputFieldNamesRule, `
+      {
+        field(arg: {
+          deep: {
+            deep: {
+              id: 1
+            }
+            id: 1
+          }
+          id: 1
+        })
+      }
+    `)
+}
+func TestValidate_UniqueInputFieldNames_DuplicateInputObjectFields(t *testing.T) {
+	testutil.ExpectFailsRule(t, graphql.UniqueInputFieldNamesRule, `
+      {
+        field(arg: { f1: "value", f1: "value" })
+      }
+    `, []gqlerrors.FormattedError{
+		testutil.RuleError(`There can be only one input field named "f1".`, 3, 22, 3, 35),
+	})
+}
+func TestValidate_UniqueInputFieldNames_ManyDuplicateInputObjectFields(t *testing.T) {
+	testutil.ExpectFailsRule(t, graphql.UniqueInputFieldNamesRule, `
+      {
+        field(arg: { f1: "value", f1: "value", f1: "value" })
+      }
+    `, []gqlerrors.FormattedError{
+		testutil.RuleError(`There can be only one input field named "f1".`, 3, 22, 3, 35),
+		testutil.RuleError(`There can be only one input field named "f1".`, 3, 22, 3, 48),
+	})
+}