Merge pull request #60 from jhgg/unique-input-field-names

Implement [RFC] Move input field uniqueness validator

Author: dittos

graphql/core/language/parser.py

@@ -379,25 +379,16 @@ def parse_array(parser, is_const):
 def parse_object(parser, is_const):
     start = parser.token.start
     expect(parser, TokenKind.BRACE_L)
-    field_names = set()
     fields = []
     while not skip(parser, TokenKind.BRACE_R):
-        fields.append(parse_object_field(parser, is_const, field_names))
+        fields.append(parse_object_field(parser, is_const))
     return ast.ObjectValue(fields=fields, loc=loc(parser, start))
 
 
-def parse_object_field(parser, is_const, field_names):
+def parse_object_field(parser, is_const):
     start = parser.token.start
-    name = parse_name(parser)
-    if name.value in field_names:
-        raise LanguageError(
-            parser.source,
-            start,
-            "Duplicate input object field {}.".format(name.value)
-        )
-    field_names.add(name.value)
     return ast.ObjectField(
-        name=name,
+        name=parse_name(parser),
         value=(
             expect(parser, TokenKind.COLON),
             parse_value(parser, is_const))[1],

graphql/core/validation/__init__.py

@@ -28,6 +28,7 @@
     Rules.DefaultValuesOfCorrectType,
     Rules.VariablesInAllowedPosition,
     Rules.OverlappingFieldsCanBeMerged,
+    Rules.UniqueInputFieldNames
 ]
 
 

graphql/core/validation/rules.py

@@ -246,7 +246,7 @@ def reduce_spread_fragments(spreads):
             )
             for fragment_definition in self.fragment_definitions
             if fragment_definition.name.value not in fragment_names_used
-            ]
+        ]
 
         if errors:
             return errors
@@ -315,7 +315,7 @@ def __init__(self, context):
             node.name.value: self.gather_spreads(node)
             for node in context.get_ast().definitions
             if isinstance(node, ast.FragmentDefinition)
-            }
+        }
         self.known_to_lead_to_cycle = set()
 
     def enter_FragmentDefinition(self, node, *args):
@@ -450,7 +450,7 @@ def leave_OperationDefinition(self, *args):
             )
             for variable_definition in self.variable_definitions
             if variable_definition.variable.name.value not in self.variable_name_used
-            ]
+        ]
 
         if errors:
             return errors
@@ -851,7 +851,7 @@ def leave_SelectionSet(self, node, *args):
             return [
                 GraphQLError(self.fields_conflict_message(reason_name, reason), list(fields)) for
                 (reason_name, reason), fields in conflicts
-                ]
+            ]
 
     @staticmethod
     def same_type(type1, type2):
@@ -967,3 +967,26 @@ def reason_message(cls, reason):
                                 for reason_name, sub_reason in reason)
 
         return reason
+
+
+class UniqueInputFieldNames(ValidationRule):
+    def __init__(self, context):
+        super(UniqueInputFieldNames, self).__init__(context)
+        self.known_names = {}
+
+    def enter_ObjectValue(self, *args):
+        self.known_names = {}
+
+    def enter_ObjectField(self, node, *args):
+        field_name = node.name.value
+        if field_name in self.known_names:
+            return GraphQLError(
+                self.duplicate_input_field_message(field_name),
+                [self.known_names[field_name], node.name]
+            )
+
+        self.known_names[field_name] = node.name
+
+    @staticmethod
+    def duplicate_input_field_message(field_name):
+        return 'There can only be one input field named "{}".'.format(field_name)

tests/core_language/test_parser.py

@@ -42,12 +42,6 @@ def test_parses_constant_default_values():
     assert 'Syntax Error GraphQL (1:37) Unexpected $' in str(excinfo.value)
 
 
-def test_duplicate_keys_in_input_object_is_syntax_error():
-    with raises(LanguageError) as excinfo:
-        parse('{ field(arg: { a: 1, a: 2 }) }')
-    assert 'Syntax Error GraphQL (1:22) Duplicate input object field a.' in str(excinfo.value)
-
-
 def test_parses_kitchen_sink():
     parse(KITCHEN_SINK)
 

tests/core_validation/test_unique_input_field_names.py

@@ -0,0 +1,55 @@
+from graphql.core.language.location import SourceLocation as L
+from graphql.core.validation.rules import UniqueInputFieldNames
+from utils import expect_passes_rule, expect_fails_rule
+
+
+def duplicate_field(name, l1, l2):
+    return {
+        'message': UniqueInputFieldNames.duplicate_input_field_message(name),
+        'locations': [l1, l2]
+    }
+
+
+def test_input_object_with_fields():
+    expect_passes_rule(UniqueInputFieldNames, '''
+    {
+        field(arg: { f: true })
+    }
+    ''')
+
+
+def test_same_input_object_within_two_args():
+    expect_passes_rule(UniqueInputFieldNames, '''
+    {
+        field(arg1: { f: true }, arg2: { f: true })
+    }
+    ''')
+
+
+def test_multiple_input_object_fields():
+    expect_passes_rule(UniqueInputFieldNames, '''
+    {
+        field(arg: { f1: "value", f2: "value", f3: "value" })
+    }
+    ''')
+
+
+def test_duplicate_input_object_fields():
+    expect_fails_rule(UniqueInputFieldNames, '''
+    {
+        field(arg: { f1: "value", f1: "value" })
+    }
+    ''', [
+        duplicate_field("f1", L(3, 22), L(3, 35))
+    ])
+
+
+def test_many_duplicate_input_object_fields():
+    expect_fails_rule(UniqueInputFieldNames, '''
+    {
+        field(arg: { f1: "value", f1: "value", f1: "value" })
+    }
+    ''', [
+        duplicate_field('f1', L(3, 22), L(3, 35)),
+        duplicate_field('f1', L(3, 22), L(3, 48))
+    ])