Merge pull request #39 from jhgg/master

Clean ups

Author: dittos

graphql/core/validation/rules.py

@@ -18,75 +18,79 @@ def __init__(self, context):
 
     def enter_OperationDefinition(self, node, *args):
         operation_name = node.name
-        if operation_name:
-            if operation_name.value in self.known_operation_names:
-                return GraphQLError(
-                    self.message(operation_name.value),
-                    [self.known_operation_names[operation_name.value], operation_name]
-                )
-            self.known_operation_names[operation_name.value] = operation_name
+        if not operation_name:
+            return
+
+        if operation_name.value in self.known_operation_names:
+            return GraphQLError(
+                self.duplicate_operation_name_message(operation_name.value),
+                [self.known_operation_names[operation_name.value], operation_name]
+            )
+
+        self.known_operation_names[operation_name.value] = operation_name
 
     @staticmethod
-    def message(operation_name):
+    def duplicate_operation_name_message(operation_name):
         return 'There can only be one operation named "{}".'.format(operation_name)
 
 
 class LoneAnonymousOperation(ValidationRule):
+    operation_count = 0
+
     def __init__(self, context):
         super(LoneAnonymousOperation, self).__init__(context)
-        self._op_count = 0
 
     def enter_Document(self, node, *args):
-        n = 0
-        for definition in node.definitions:
-            if isinstance(definition, ast.OperationDefinition):
-                n += 1
-        self._op_count = n
+        self.operation_count = \
+            sum(1 for definition in node.definitions if isinstance(definition, ast.OperationDefinition))
 
     def enter_OperationDefinition(self, node, *args):
-        if not node.name and self._op_count > 1:
-            return GraphQLError(self.message(), [node])
+        if not node.name and self.operation_count > 1:
+            return GraphQLError(self.anonymous_operation_not_alone_message(), [node])
 
     @staticmethod
-    def message():
+    def anonymous_operation_not_alone_message():
         return 'This anonymous operation must be the only defined operation.'
 
 
 class KnownTypeNames(ValidationRule):
     def enter_NamedType(self, node, *args):
         type_name = node.name.value
         type = self.context.get_schema().get_type(type_name)
+
         if not type:
-            return GraphQLError(self.message(type_name), [node])
+            return GraphQLError(self.unknown_type_message(type_name), [node])
 
     @staticmethod
-    def message(type):
+    def unknown_type_message(type):
         return 'Unknown type "{}".'.format(type)
 
 
 class FragmentsOnCompositeTypes(ValidationRule):
     def enter_InlineFragment(self, node, *args):
         type = self.context.get_type()
+
         if type and not is_composite_type(type):
             return GraphQLError(
-                self.inline_message(print_ast(node.type_condition)),
+                self.inline_fragment_on_non_composite_error_message(print_ast(node.type_condition)),
                 [node.type_condition]
             )
 
     def enter_FragmentDefinition(self, node, *args):
         type = self.context.get_type()
+
         if type and not is_composite_type(type):
             return GraphQLError(
-                self.message(node.name.value, print_ast(node.type_condition)),
+                self.fragment_on_non_composite_error_message(node.name.value, print_ast(node.type_condition)),
                 [node.type_condition]
             )
 
     @staticmethod
-    def inline_message(type):
+    def inline_fragment_on_non_composite_error_message(type):
         return 'Fragment cannot condition on non composite type "{}".'.format(type)
 
     @staticmethod
-    def message(frag_name, type):
+    def fragment_on_non_composite_error_message(frag_name, type):
         return 'Fragment "{}" cannot condition on non composite type "{}".'.format(frag_name, type)
 
 
@@ -95,55 +99,60 @@ def enter_VariableDefinition(self, node, *args):
         type = type_from_ast(self.context.get_schema(), node.type)
 
         if type and not is_input_type(type):
-            variable_name = node.variable.name.value
             return GraphQLError(
-                self.message(variable_name, print_ast(node.type)),
+                self.non_input_type_on_variable_message(node.variable.name.value, print_ast(node.type)),
                 [node.type]
             )
 
     @staticmethod
-    def message(variable_name, type_name):
+    def non_input_type_on_variable_message(variable_name, type_name):
         return 'Variable "${}" cannot be non-input type "{}".'.format(variable_name, type_name)
 
 
 class ScalarLeafs(ValidationRule):
     def enter_Field(self, node, *args):
         type = self.context.get_type()
-        if type:
-            if is_leaf_type(type):
-                if node.selection_set:
-                    return GraphQLError(
-                        self.not_allowed_message(node.name.value, type),
-                        [node.selection_set]
-                    )
-            elif not node.selection_set:
+
+        if not type:
+            return
+
+        if is_leaf_type(type):
+            if node.selection_set:
                 return GraphQLError(
-                    self.required_message(node.name.value, type),
-                    [node]
+                    self.no_subselection_allowed_message(node.name.value, type),
+                    [node.selection_set]
                 )
 
+        elif not node.selection_set:
+            return GraphQLError(
+                self.required_subselection_message(node.name.value, type),
+                [node]
+            )
+
     @staticmethod
-    def not_allowed_message(field, type):
+    def no_subselection_allowed_message(field, type):
         return 'Field "{}" of type "{}" must not have a sub selection.'.format(field, type)
 
     @staticmethod
-    def required_message(field, type):
+    def required_subselection_message(field, type):
         return 'Field "{}" of type "{}" must have a sub selection.'.format(field, type)
 
 
 class FieldsOnCorrectType(ValidationRule):
     def enter_Field(self, node, *args):
         type = self.context.get_parent_type()
-        if type:
-            field_def = self.context.get_field_def()
-            if not field_def:
-                return GraphQLError(
-                    self.message(node.name.value, type.name),
-                    [node]
-                )
+        if not type:
+            return
+
+        field_def = self.context.get_field_def()
+        if not field_def:
+            return GraphQLError(
+                self.undefined_field_message(node.name.value, type.name),
+                [node]
+            )
 
     @staticmethod
-    def message(field_name, type):
+    def undefined_field_message(field_name, type):
         return 'Cannot query field "{}" on "{}".'.format(field_name, type)
 
 
@@ -159,17 +168,19 @@ def enter_FragmentDefinition(self, node, *args):
                 self.duplicate_fragment_name_message(fragment_name),
                 [self.known_fragment_names[fragment_name], node.name]
             )
+
         self.known_fragment_names[fragment_name] = node.name
 
     @staticmethod
     def duplicate_fragment_name_message(field):
-        return 'There can only be one fragment named {}'.format(field)
+        return 'There can only be one fragment named "{}".'.format(field)
 
 
 class KnownFragmentNames(ValidationRule):
     def enter_FragmentSpread(self, node, *args):
         fragment_name = node.name.value
         fragment = self.context.get_fragment(fragment_name)
+
         if not fragment:
             return GraphQLError(
                 self.unknown_fragment_message(fragment_name),
@@ -310,9 +321,9 @@ def collect_fragment_spread_nodes(self):
 
 class NoUndefinedVariables(ValidationRule):
     visit_spread_fragments = True
+    operation = None
 
     def __init__(self, context):
-        self.operation = None
         self.visited_fragment_names = set()
         self.defined_variable_names = set()
         super(NoUndefinedVariables, self).__init__(context)
@@ -409,27 +420,31 @@ def unused_variable_message(variable_name):
 
 class KnownDirectives(ValidationRule):
     def enter_Directive(self, node, key, parent, path, ancestors):
-        directive_def = None
-        for definition in self.context.get_schema().get_directives():
-            if definition.name == node.name.value:
-                directive_def = definition
-                break
+        directive_def = next((
+            definition for definition in self.context.get_schema().get_directives()
+            if definition.name == node.name.value
+        ), None)
+
         if not directive_def:
             return GraphQLError(
-                self.message(node.name.value),
+                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.misplaced_directive_message(node.name.value, 'operation'),
                 [node]
             )
+
         if isinstance(applied_to, ast.Field) and not directive_def.on_field:
             return GraphQLError(
                 self.misplaced_directive_message(node.name.value, 'field'),
                 [node]
             )
+
         if (isinstance(applied_to, (ast.FragmentSpread, ast.InlineFragment, ast.FragmentDefinition)) and
                 not directive_def.on_fragment):
             return GraphQLError(
@@ -438,7 +453,7 @@ def enter_Directive(self, node, key, parent, path, ancestors):
             )
 
     @staticmethod
-    def message(directive_name):
+    def unknown_directive_message(directive_name):
         return 'Unknown directive "{}".'.format(directive_name)
 
     @staticmethod
@@ -449,41 +464,41 @@ def misplaced_directive_message(directive_name, placement):
 class KnownArgumentNames(ValidationRule):
     def enter_Argument(self, node, key, parent, path, ancestors):
         argument_of = ancestors[-1]
+
         if isinstance(argument_of, ast.Field):
             field_def = self.context.get_field_def()
-            if field_def:
-                field_arg_def = None
-                for arg in field_def.args:
-                    if arg.name == node.name.value:
-                        field_arg_def = arg
-                        break
-                if not field_arg_def:
-                    parent_type = self.context.get_parent_type()
-                    assert parent_type
-                    return GraphQLError(
-                        self.message(node.name.value, field_def.name, parent_type.name),
-                        [node]
-                    )
+            if not field_def:
+                return
+
+            field_arg_def = next((arg for arg in field_def.args if arg.name == node.name.value), None)
+
+            if not field_arg_def:
+                parent_type = self.context.get_parent_type()
+                assert parent_type
+                return 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()
-            if directive:
-                directive_arg_def = None
-                for arg in directive.args:
-                    if arg.name == node.name.value:
-                        directive_arg_def = arg
-                        break
-                if not directive_arg_def:
-                    return GraphQLError(
-                        self.directive_message(node.name.value, directive.name),
-                        [node]
-                    )
+            if not directive:
+                return
+
+            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.unknown_directive_arg_message(node.name.value, directive.name),
+                    [node]
+                )
 
     @staticmethod
-    def message(arg_name, field_name, type):
+    def unknown_arg_message(arg_name, field_name, type):
         return 'Unknown argument "{}" on field "{}" of type "{}".'.format(arg_name, field_name, type)
 
     @staticmethod
-    def directive_message(arg_name, directive_name):
+    def unknown_directive_arg_message(arg_name, directive_name):
         return 'Unknown argument "{}" on directive "@{}".'.format(arg_name, directive_name)
 
 
@@ -492,24 +507,26 @@ def __init__(self, context):
         super(UniqueArgumentNames, self).__init__(context)
         self.known_arg_names = {}
 
-    def enter_Field(self, node, *args):
+    def enter_Field(self, *args):
         self.known_arg_names = {}
 
-    def enter_Directive(self, node, key, parent, path, ancestors):
+    def enter_Directive(self, *args):
         self.known_arg_names = {}
 
     def enter_Argument(self, node, *args):
         arg_name = node.name.value
+
         if arg_name in self.known_arg_names:
             return GraphQLError(
                 self.duplicate_arg_message(arg_name),
                 [self.known_arg_names[arg_name], node.name]
             )
+
         self.known_arg_names[arg_name] = node.name
 
     @staticmethod
     def duplicate_arg_message(field):
-        return 'There can only be one argument named {}'.format(field)
+        return 'There can only be one argument named "{}".'.format(field)
 
 
 class ArgumentsOfCorrectType(ValidationRule):
@@ -528,7 +545,7 @@ def bad_value_message(arg_name, type, value):
 
 
 class ProvidedNonNullArguments(ValidationRule):
-    def leave_Field(self, node, key, parent, path, ancestors):
+    def leave_Field(self, node, *args):
         field_def = self.context.get_field_def()
         if not field_def:
             return False
@@ -544,10 +561,11 @@ def leave_Field(self, node, key, parent, path, ancestors):
                     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):
+    def leave_Directive(self, node, *args):
         directive_def = self.context.get_directive()
         if not directive_def:
             return False
@@ -581,6 +599,7 @@ def enter_VariableDefinition(self, node, *args):
         name = node.variable.name.value
         default_value = node.default_value
         type = self.context.get_input_type()
+
         if isinstance(type, GraphQLNonNull) and default_value:
             return GraphQLError(
                 self.default_for_non_null_arg_message(name, type, type.of_type),