[Validation] Report errors rather than return them

syrusakbary · syrusakbary · commit 891d4a66c328 · 2016-04-12T23:05:32.000-07:00
Related GraphQL-js commit graphql/graphql-js@5e545cc
diff --git a/graphql/core/validation/__init__.py b/graphql/core/validation/__init__.py
@@ -19,4 +19,4 @@ def visit_using_rules(schema, type_info, ast, rules):
     errors = []
     rules = [rule(context) for rule in rules]
     visit(ast, ValidationVisitor(rules, context, type_info, errors))
-    return errors
+    return context.get_errors()
diff --git a/graphql/core/validation/context.py b/graphql/core/validation/context.py
@@ -31,18 +31,25 @@ def leave(self, node, key, parent, path, ancestors):
 
 
 class ValidationContext(object):
-    __slots__ = '_schema', '_ast', '_type_info', '_fragments', '_fragment_spreads', '_recursively_referenced_fragments', '_variable_usages', '_recursive_variable_usages'
+    __slots__ = '_schema', '_ast', '_type_info', '_errors', '_fragments', '_fragment_spreads', '_recursively_referenced_fragments', '_variable_usages', '_recursive_variable_usages'
 
     def __init__(self, schema, ast, type_info):
         self._schema = schema
         self._ast = ast
         self._type_info = type_info
+        self._errors = []
         self._fragments = None
         self._fragment_spreads = {}
         self._recursively_referenced_fragments = {}
         self._variable_usages = {}
         self._recursive_variable_usages = {}
 
+    def report_error(self, error):
+        self._errors.append(error)
+
+    def get_errors(self):
+        return self._errors
+
     def get_schema(self):
         return self._schema
 
diff --git a/graphql/core/validation/rules/arguments_of_correct_type.py b/graphql/core/validation/rules/arguments_of_correct_type.py
@@ -10,11 +10,11 @@ def enter_Argument(self, node, key, parent, path, ancestors):
         if arg_def:
             errors = is_valid_literal_value(arg_def.type, node.value)
             if errors:
-                return GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.bad_value_message(node.name.value, arg_def.type,
                                            print_ast(node.value), errors),
                     [node.value]
-                )
+                ))
         return False
 
     @staticmethod
diff --git a/graphql/core/validation/rules/default_values_of_correct_type.py b/graphql/core/validation/rules/default_values_of_correct_type.py
@@ -12,18 +12,18 @@ def enter_VariableDefinition(self, node, key, parent, path, ancestors):
         type = self.context.get_input_type()
 
         if isinstance(type, GraphQLNonNull) and default_value:
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.default_for_non_null_arg_message(name, type, type.of_type),
                 [default_value]
-            )
+            ))
 
         if type and default_value:
             errors = is_valid_literal_value(type, default_value)
             if errors:
-                return GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.bad_value_for_default_arg_message(name, type, print_ast(default_value), errors),
                     [default_value]
-                )
+                ))
         return False
 
     def enter_SelectionSet(self, node, key, parent, path, ancestors):
diff --git a/graphql/core/validation/rules/fields_on_correct_type.py b/graphql/core/validation/rules/fields_on_correct_type.py
@@ -10,10 +10,10 @@ def enter_Field(self, node, key, parent, path, ancestors):
 
         field_def = self.context.get_field_def()
         if not field_def:
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.undefined_field_message(node.name.value, type.name),
                 [node]
-            )
+            ))
 
     @staticmethod
     def undefined_field_message(field_name, type):
diff --git a/graphql/core/validation/rules/fragments_on_composite_types.py b/graphql/core/validation/rules/fragments_on_composite_types.py
@@ -9,19 +9,19 @@ def enter_InlineFragment(self, node, key, parent, path, ancestors):
         type = self.context.get_type()
 
         if node.type_condition and type and not is_composite_type(type):
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.inline_fragment_on_non_composite_error_message(print_ast(node.type_condition)),
                 [node.type_condition]
-            )
+            ))
 
     def enter_FragmentDefinition(self, node, key, parent, path, ancestors):
         type = self.context.get_type()
 
         if type and not is_composite_type(type):
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.fragment_on_non_composite_error_message(node.name.value, print_ast(node.type_condition)),
                 [node.type_condition]
-            )
+            ))
 
     @staticmethod
     def inline_fragment_on_non_composite_error_message(type):
diff --git a/graphql/core/validation/rules/known_argument_names.py b/graphql/core/validation/rules/known_argument_names.py
@@ -17,10 +17,10 @@ def enter_Argument(self, node, key, parent, path, ancestors):
             if not field_arg_def:
                 parent_type = self.context.get_parent_type()
                 assert parent_type
-                return GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.unknown_arg_message(node.name.value, field_def.name, parent_type.name),
                     [node]
-                )
+                ))
 
         elif isinstance(argument_of, ast.Directive):
             directive = self.context.get_directive()
@@ -30,10 +30,10 @@ def enter_Argument(self, node, key, parent, path, ancestors):
             directive_arg_def = next((arg for arg in directive.args if arg.name == node.name.value), None)
 
             if not directive_arg_def:
-                return GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.unknown_directive_arg_message(node.name.value, directive.name),
                     [node]
-                )
+                ))
 
     @staticmethod
     def unknown_arg_message(arg_name, field_name, type):
diff --git a/graphql/core/validation/rules/known_directives.py b/graphql/core/validation/rules/known_directives.py
@@ -11,31 +11,31 @@ def enter_Directive(self, node, key, parent, path, ancestors):
         ), None)
 
         if not directive_def:
-            return GraphQLError(
+            return self.context.report_error(GraphQLError(
                 self.unknown_directive_message(node.name.value),
                 [node]
-            )
+            ))
 
         applied_to = ancestors[-1]
 
         if isinstance(applied_to, ast.OperationDefinition) and not directive_def.on_operation:
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.misplaced_directive_message(node.name.value, 'operation'),
                 [node]
-            )
+            ))
 
-        if isinstance(applied_to, ast.Field) and not directive_def.on_field:
-            return GraphQLError(
+        elif isinstance(applied_to, ast.Field) and not directive_def.on_field:
+            self.context.report_error(GraphQLError(
                 self.misplaced_directive_message(node.name.value, 'field'),
                 [node]
-            )
+            ))
 
-        if (isinstance(applied_to, (ast.FragmentSpread, ast.InlineFragment, ast.FragmentDefinition)) and
+        elif (isinstance(applied_to, (ast.FragmentSpread, ast.InlineFragment, ast.FragmentDefinition)) and
                 not directive_def.on_fragment):
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.misplaced_directive_message(node.name.value, 'fragment'),
                 [node]
-            )
+            ))
 
     @staticmethod
     def unknown_directive_message(directive_name):
diff --git a/graphql/core/validation/rules/known_fragment_names.py b/graphql/core/validation/rules/known_fragment_names.py
@@ -8,10 +8,10 @@ def enter_FragmentSpread(self, node, key, parent, path, ancestors):
         fragment = self.context.get_fragment(fragment_name)
 
         if not fragment:
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.unknown_fragment_message(fragment_name),
                 [node.name]
-            )
+            ))
 
     @staticmethod
     def unknown_fragment_message(fragment_name):
diff --git a/graphql/core/validation/rules/known_type_names.py b/graphql/core/validation/rules/known_type_names.py
@@ -8,7 +8,7 @@ def enter_NamedType(self, node, *args):
         type = self.context.get_schema().get_type(type_name)
 
         if not type:
-            return GraphQLError(self.unknown_type_message(type_name), [node])
+            self.context.report_error(GraphQLError(self.unknown_type_message(type_name), [node]))
 
     @staticmethod
     def unknown_type_message(type):
diff --git a/graphql/core/validation/rules/lone_anonymous_operation.py b/graphql/core/validation/rules/lone_anonymous_operation.py
@@ -16,7 +16,7 @@ def enter_Document(self, node, key, parent, path, ancestors):
 
     def enter_OperationDefinition(self, node, key, parent, path, ancestors):
         if not node.name and self.operation_count > 1:
-            return GraphQLError(self.anonymous_operation_not_alone_message(), [node])
+            self.context.report_error(GraphQLError(self.anonymous_operation_not_alone_message(), [node]))
 
     @staticmethod
     def anonymous_operation_not_alone_message():
diff --git a/graphql/core/validation/rules/no_fragment_cycles.py b/graphql/core/validation/rules/no_fragment_cycles.py
@@ -14,10 +14,6 @@ def __init__(self, context):
         self.spread_path = []
         self.spread_path_index_by_name = {}
 
-    def leave_Document(self, node, key, parent, path, ancestors):
-        if self.errors:
-            return self.errors
-
     def enter_OperationDefinition(self, node, key, parent, path, ancestors):
         return False
 
@@ -49,7 +45,7 @@ def detect_cycle_recursive(self, fragment):
                 self.spread_path.pop()
             else:
                 cycle_path = self.spread_path[cycle_index:]
-                self.errors.append(GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.cycle_error_message(
                         spread_name,
                         [s.name.value for s in cycle_path]
diff --git a/graphql/core/validation/rules/no_undefined_variables.py b/graphql/core/validation/rules/no_undefined_variables.py
@@ -23,19 +23,15 @@ def enter_OperationDefinition(self, operation, key, parent, path, ancestors):
 
     def leave_OperationDefinition(self, operation, key, parent, path, ancestors):
         usages = self.context.get_recursive_variable_usages(operation)
-        errors = []
 
         for variable_usage in usages:
             node = variable_usage.node
             var_name = node.name.value
             if var_name not in self.defined_variable_names:
-                errors.append(GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.undefined_var_message(var_name, operation.name and operation.name.value),
                     [node, operation]
                 ))
 
-        if errors:
-            return errors
-
     def enter_VariableDefinition(self, node, key, parent, path, ancestors):
         self.defined_variable_names.add(node.variable.name.value)
diff --git a/graphql/core/validation/rules/no_unused_fragments.py b/graphql/core/validation/rules/no_unused_fragments.py
@@ -26,17 +26,12 @@ def leave_Document(self, node, key, parent, path, ancestors):
             for fragment in fragments:
                 fragment_names_used.add(fragment.name.value)
 
-        errors = [
-            GraphQLError(
-                self.unused_fragment_message(fragment_definition.name.value),
-                [fragment_definition]
-            )
-            for fragment_definition in self.fragment_definitions
-            if fragment_definition.name.value not in fragment_names_used
-            ]
-
-        if errors:
-            return errors
+        for fragment_definition in self.fragment_definitions:
+            if fragment_definition.name.value not in fragment_names_used:
+                self.context.report_error(GraphQLError(
+                    self.unused_fragment_message(fragment_definition.name.value),
+                    [fragment_definition]
+                ))
 
     @staticmethod
     def unused_fragment_message(fragment_name):
diff --git a/graphql/core/validation/rules/no_unused_variables.py b/graphql/core/validation/rules/no_unused_variables.py
@@ -19,17 +19,12 @@ def leave_OperationDefinition(self, operation, key, parent, path, ancestors):
         for variable_usage in usages:
             variable_name_used.add(variable_usage.node.name.value)
 
-        errors = [
-            GraphQLError(
-                self.unused_variable_message(variable_definition.variable.name.value),
-                [variable_definition]
-            )
-            for variable_definition in self.variable_definitions
-            if variable_definition.variable.name.value not in variable_name_used
-            ]
-
-        if errors:
-            return errors
+        for variable_definition in self.variable_definitions:
+            if variable_definition.variable.name.value not in variable_name_used:
+                self.context.report_error(GraphQLError(
+                    self.unused_variable_message(variable_definition.variable.name.value),
+                    [variable_definition]
+                ))
 
     def enter_VariableDefinition(self, node, key, parent, path, ancestors):
         self.variable_definitions.append(node)
diff --git a/graphql/core/validation/rules/overlapping_fields_can_be_merged.py b/graphql/core/validation/rules/overlapping_fields_can_be_merged.py
@@ -123,10 +123,8 @@ def leave_SelectionSet(self, node, key, parent, path, ancestors):
 
         conflicts = self.find_conflicts(field_map)
         if conflicts:
-            return [
-                GraphQLError(self.fields_conflict_message(reason_name, reason), list(fields1)+list(fields2)) for
-                (reason_name, reason), fields1, fields2 in conflicts
-                ]
+            for (reason_name, reason), fields1, fields2 in conflicts:
+                self.context.report_error(GraphQLError(self.fields_conflict_message(reason_name, reason), list(fields1)+list(fields2)))
 
     @staticmethod
     def same_type(type1, type2):
diff --git a/graphql/core/validation/rules/possible_fragment_spreads.py b/graphql/core/validation/rules/possible_fragment_spreads.py
@@ -13,20 +13,20 @@ def enter_InlineFragment(self, node, key, parent, path, ancestors):
         frag_type = self.context.get_type()
         parent_type = self.context.get_parent_type()
         if frag_type and parent_type and not self.do_types_overlap(frag_type, parent_type):
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.type_incompatible_anon_spread_message(parent_type, frag_type),
                 [node]
-            )
+            ))
 
     def enter_FragmentSpread(self, node, key, parent, path, ancestors):
         frag_name = node.name.value
         frag_type = self.get_fragment_type(self.context, frag_name)
         parent_type = self.context.get_parent_type()
         if frag_type and parent_type and not self.do_types_overlap(frag_type, parent_type):
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.type_incompatible_spread_message(frag_name, parent_type, frag_type),
                 [node]
-            )
+            ))
 
     @staticmethod
     def get_fragment_type(context, name):
diff --git a/graphql/core/validation/rules/provided_non_null_arguments.py b/graphql/core/validation/rules/provided_non_null_arguments.py
@@ -9,41 +9,33 @@ def leave_Field(self, node, key, parent, path, ancestors):
         if not field_def:
             return False
 
-        errors = []
         arg_asts = node.arguments or []
         arg_ast_map = {arg.name.value: arg for arg in arg_asts}
 
         for arg_def in field_def.args:
             arg_ast = arg_ast_map.get(arg_def.name, None)
             if not arg_ast and isinstance(arg_def.type, GraphQLNonNull):
-                errors.append(GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.missing_field_arg_message(node.name.value, arg_def.name, arg_def.type),
                     [node]
                 ))
 
-        if errors:
-            return errors
-
     def leave_Directive(self, node, key, parent, path, ancestors):
         directive_def = self.context.get_directive()
         if not directive_def:
             return False
 
-        errors = []
         arg_asts = node.arguments or []
         arg_ast_map = {arg.name.value: arg for arg in arg_asts}
 
         for arg_def in directive_def.args:
             arg_ast = arg_ast_map.get(arg_def.name, None)
             if not arg_ast and isinstance(arg_def.type, GraphQLNonNull):
-                errors.append(GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.missing_directive_arg_message(node.name.value, arg_def.name, arg_def.type),
                     [node]
                 ))
 
-        if errors:
-            return errors
-
     @staticmethod
     def missing_field_arg_message(name, arg_name, type):
         return 'Field "{}" argument "{}" of type "{}" is required but not provided.'.format(name, arg_name, type)
diff --git a/graphql/core/validation/rules/scalar_leafs.py b/graphql/core/validation/rules/scalar_leafs.py
@@ -12,16 +12,16 @@ def enter_Field(self, node, key, parent, path, ancestors):
 
         if is_leaf_type(type):
             if node.selection_set:
-                return GraphQLError(
+                self.context.report_error(GraphQLError(
                     self.no_subselection_allowed_message(node.name.value, type),
                     [node.selection_set]
-                )
+                ))
 
         elif not node.selection_set:
-            return GraphQLError(
+            self.context.report_error(GraphQLError(
                 self.required_subselection_message(node.name.value, type),
                 [node]
-            )
+            ))
 
     @staticmethod
     def no_subselection_allowed_message(field, type):
diff --git a/graphql/core/validation/rules/unique_argument_names.py b/graphql/core/validation/rules/unique_argument_names.py
diff --git a/graphql/core/validation/rules/unique_fragment_names.py b/graphql/core/validation/rules/unique_fragment_names.py
diff --git a/graphql/core/validation/rules/unique_input_field_names.py b/graphql/core/validation/rules/unique_input_field_names.py
diff --git a/graphql/core/validation/rules/unique_operation_names.py b/graphql/core/validation/rules/unique_operation_names.py
diff --git a/graphql/core/validation/rules/variables_are_input_types.py b/graphql/core/validation/rules/variables_are_input_types.py
diff --git a/graphql/core/validation/rules/variables_in_allowed_position.py b/graphql/core/validation/rules/variables_in_allowed_position.py
diff --git a/tests/core_validation/test_fields_on_correct_type.py b/tests/core_validation/test_fields_on_correct_type.py
diff --git a/tests/core_validation/test_validation.py b/tests/core_validation/test_validation.py
