[Validation] Memoize collecting variable usage.

Related GraphQL-js commit: graphql/graphql-js@2afbff7

Author: syrusakbary

graphql/core/validation/context.py

@@ -1,8 +1,37 @@
-from ..language.ast import FragmentDefinition, FragmentSpread
+from ..language.ast import FragmentDefinition, FragmentSpread, VariableDefinition, Variable, OperationDefinition
+from ..utils.type_info import TypeInfo
+from ..language.visitor import Visitor, visit
+
+
+class VariableUsage(object):
+    __slots__ = 'node', 'type'
+
+    def __init__(self, node, type):
+        self.node = node
+        self.type = type
+
+
+class UsageVisitor(Visitor):
+    __slots__ = 'context', 'usages', 'type_info'
+
+    def __init__(self, usages, type_info):
+        self.usages = usages
+        self.type_info = type_info
+
+    def enter(self, node, key, parent, path, ancestors):
+        self.type_info.enter(node)
+        if isinstance(node, VariableDefinition):
+            return False
+        elif isinstance(node, Variable):
+            usage = VariableUsage(node, type=self.type_info.get_input_type())
+            self.usages.append(usage)
+
+    def leave(self, node, key, parent, path, ancestors):
+        self.type_info.leave(node)
 
 
 class ValidationContext(object):
-    __slots__ = '_schema', '_ast', '_type_info', '_fragments', '_fragment_spreads', '_recursively_referenced_fragments'
+    __slots__ = '_schema', '_ast', '_type_info', '_fragments', '_fragment_spreads', '_recursively_referenced_fragments', '_variable_usages', '_recursive_variable_usages'
 
     def __init__(self, schema, ast, type_info):
         self._schema = schema
@@ -11,11 +40,36 @@ def __init__(self, schema, ast, type_info):
         self._fragments = None
         self._fragment_spreads = {}
         self._recursively_referenced_fragments = {}
+        self._variable_usages = {}
+        self._recursive_variable_usages = {}
 
     def get_schema(self):
         return self._schema
 
+    def get_variable_usages(self, node):
+        usages = self._variable_usages.get(node)
+        if usages is None:
+            usages = []
+            sub_visitor = UsageVisitor(usages, self._type_info)
+            visit(node, sub_visitor)
+            self._variable_usages[node] = usages
+
+        return usages
+
+    def get_recursive_variable_usages(self, operation):
+        assert isinstance(operation, OperationDefinition)
+        usages = self._recursive_variable_usages.get(operation)
+        if usages is None:
+            usages = self.get_variable_usages(operation)
+            fragments = self.get_recursively_referenced_fragments(operation)
+            for fragment in fragments:
+                usages.extend(self.get_variable_usages(fragment))
+            self._recursive_variable_usages[operation] = usages
+
+        return usages
+
     def get_recursively_referenced_fragments(self, operation):
+        assert isinstance(operation, OperationDefinition)
         fragments = self._recursively_referenced_fragments.get(operation)
         if not fragments:
             fragments = []

graphql/core/validation/rules/no_undefined_variables.py

@@ -4,51 +4,38 @@
 
 
 class NoUndefinedVariables(ValidationRule):
-    __slots__ = 'visited_fragment_names', 'defined_variable_names', 'operation',
-    visit_spread_fragments = True
+    __slots__ = 'defined_variable_names',
 
     def __init__(self, context):
-        self.visited_fragment_names = set()
         self.defined_variable_names = set()
-        self.operation = None
-
         super(NoUndefinedVariables, self).__init__(context)
 
     @staticmethod
-    def undefined_var_message(var_name):
+    def undefined_var_message(var_name, op_name=None):
+        if op_name:
+            return 'Variable "${}" is not defined by operation "{}".'.format(
+                var_name, op_name
+            )
         return 'Variable "${}" is not defined.'.format(var_name)
 
-    @staticmethod
-    def undefined_var_by_op_message(var_name, op_name):
-        return 'Variable "${}" is not defined by operation "{}".'.format(
-            var_name, op_name
-        )
-
-    def enter_OperationDefinition(self, node, key, parent, path, ancestors):
-        self.operation = node
-        self.visited_fragment_names = set()
+    def enter_OperationDefinition(self, operation, key, parent, path, ancestors):
         self.defined_variable_names = set()
 
-    def enter_VariableDefinition(self, node, key, parent, path, ancestors):
-        self.defined_variable_names.add(node.variable.name.value)
+    def leave_OperationDefinition(self, operation, key, parent, path, ancestors):
+        usages = self.context.get_recursive_variable_usages(operation)
+        errors = []
 
-    def enter_Variable(self, variable, key, parent, path, ancestors):
-        var_name = variable.name.value
-        if var_name not in self.defined_variable_names:
-            within_fragment = any(isinstance(node, ast.FragmentDefinition) for node in ancestors)
-            if within_fragment and self.operation and self.operation.name:
-                return GraphQLError(
-                    self.undefined_var_by_op_message(var_name, self.operation.name.value),
-                    [variable, self.operation]
-                )
-
-            return GraphQLError(
-                self.undefined_var_message(var_name),
-                [variable]
-            )
+        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.undefined_var_message(var_name, operation.name and operation.name.value),
+                    [node, operation]
+                ))
 
-    def enter_FragmentSpread(self, spread_ast, *args):
-        if spread_ast.name.value in self.visited_fragment_names:
-            return False
+        if errors:
+            return errors
 
-        self.visited_fragment_names.add(spread_ast.name.value)
+    def enter_VariableDefinition(self, node, key, parent, path, ancestors):
+        self.defined_variable_names.add(node.variable.name.value)

graphql/core/validation/rules/no_unused_variables.py

@@ -3,50 +3,36 @@
 
 
 class NoUnusedVariables(ValidationRule):
-    __slots__ = 'visited_fragment_names', 'variable_definitions', 'variable_name_used', 'visit_spread_fragments'
+    __slots__ = 'variable_definitions'
 
     def __init__(self, context):
-        self.visited_fragment_names = None
-        self.variable_definitions = None
-        self.variable_name_used = None
-        self.visit_spread_fragments = True
+        self.variable_definitions = []
         super(NoUnusedVariables, self).__init__(context)
 
     def enter_OperationDefinition(self, node, key, parent, path, ancestors):
-        self.visited_fragment_names = set()
         self.variable_definitions = []
-        self.variable_name_used = set()
 
-    def leave_OperationDefinition(self, node, key, parent, path, ancestors):
+    def leave_OperationDefinition(self, operation, key, parent, path, ancestors):
+        variable_name_used = set()
+        usages = self.context.get_recursive_variable_usages(operation)
+
+        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 self.variable_name_used
+            if variable_definition.variable.name.value not in variable_name_used
             ]
 
         if errors:
             return errors
 
     def enter_VariableDefinition(self, node, key, parent, path, ancestors):
-        if self.variable_definitions is not None:
-            self.variable_definitions.append(node)
-
-        return False
-
-    def enter_Variable(self, node, key, parent, path, ancestors):
-        if self.variable_name_used is not None:
-            self.variable_name_used.add(node.name.value)
-
-    def enter_FragmentSpread(self, node, key, parent, path, ancestors):
-        if self.visited_fragment_names is not None:
-            spread_name = node.name.value
-            if spread_name in self.visited_fragment_names:
-                return False
-
-            self.visited_fragment_names.add(spread_name)
+        self.variable_definitions.append(node)
 
     @staticmethod
     def unused_variable_message(variable_name):

graphql/core/validation/rules/variables_in_allowed_position.py

@@ -8,36 +8,37 @@
 
 
 class VariablesInAllowedPosition(ValidationRule):
-    visit_spread_fragments = True
-    __slots__ = 'var_def_map', 'visited_fragment_names'
+    __slots__ = 'var_def_map'
 
     def __init__(self, context):
         super(VariablesInAllowedPosition, self).__init__(context)
         self.var_def_map = {}
-        self.visited_fragment_names = set()
 
     def enter_OperationDefinition(self, node, key, parent, path, ancestors):
         self.var_def_map = {}
-        self.visited_fragment_names = set()
+
+    def leave_OperationDefinition(self, operation, key, parent, path, ancestors):
+        usages = self.context.get_recursive_variable_usages(operation)
+        errors = []
+
+        for usage in usages:
+            node = usage.node
+            type = usage.type
+            var_name = node.name.value
+            var_def = self.var_def_map.get(var_name)
+            var_type = var_def and type_from_ast(self.context.get_schema(), var_def.type)
+            if var_type and type and not self.var_type_allowed_for_type(self.effective_type(var_type, var_def), type):
+                errors.append(GraphQLError(
+                    self.bad_var_pos_message(var_name, var_type, type),
+                    [node]
+                ))
+
+        if errors:
+            return errors
 
     def enter_VariableDefinition(self, node, key, parent, path, ancestors):
         self.var_def_map[node.variable.name.value] = node
 
-    def enter_Variable(self, node, key, parent, path, ancestors):
-        var_name = node.name.value
-        var_def = self.var_def_map.get(var_name)
-        var_type = var_def and type_from_ast(self.context.get_schema(), var_def.type)
-        input_type = self.context.get_input_type()
-        if var_type and input_type and not self.var_type_allowed_for_type(self.effective_type(var_type, var_def),
-                                                                          input_type):
-            return GraphQLError(self.bad_var_pos_message(var_name, var_type, input_type),
-                                [node])
-
-    def enter_FragmentSpread(self, node, key, parent, path, ancestors):
-        if node.name.value in self.visited_fragment_names:
-            return False
-        self.visited_fragment_names.add(node.name.value)
-
     @staticmethod
     def effective_type(var_type, var_def):
         if not var_def.default_value or isinstance(var_type, GraphQLNonNull):

tests/core_validation/test_no_undefined_variables.py

@@ -3,17 +3,9 @@
 from utils import expect_passes_rule, expect_fails_rule
 
 
-def undefined_var(var_name, line, column):
+def undefined_var(var_name, l1, c1, op_name, l2, c2):
     return {
-        'message': NoUndefinedVariables.undefined_var_message(var_name),
-        'locations': [SourceLocation(line, column)]
-    }
-
-
-def undefined_var_by_op(var_name, l1, c1, op_name, l2, c2):
-    return {
-        'message': NoUndefinedVariables.undefined_var_by_op_message(
-            var_name, op_name),
+        'message': NoUndefinedVariables.undefined_var_message(var_name, op_name),
         'locations': [
             SourceLocation(l1, c1),
             SourceLocation(l2, c2),
@@ -128,7 +120,7 @@ def test_variable_not_defined():
         field(a: $a, b: $b, c: $c, d: $d)
       }
     ''', [
-        undefined_var('d', 3, 39)
+        undefined_var('d', 3, 39, 'Foo', 2, 7)
     ])
 
 
@@ -138,7 +130,7 @@ def variable_not_defined_by_unnamed_query():
         field(a: $a)
       }
     ''', [
-        undefined_var('a', 3, 18)
+        undefined_var('a', 3, 18, '', 2, 7)
     ])
 
 
@@ -148,8 +140,8 @@ def test_multiple_variables_not_defined():
         field(a: $a, b: $b, c: $c)
       }
     ''', [
-        undefined_var('a', 3, 18),
-        undefined_var('c', 3, 32)
+        undefined_var('a', 3, 18, 'Foo', 2, 7),
+        undefined_var('c', 3, 32, 'Foo', 2, 7)
     ])
 
 
@@ -162,7 +154,7 @@ def test_variable_in_fragment_not_defined_by_unnamed_query():
         field(a: $a)
       }
     ''', [
-        undefined_var('a', 6, 18)
+        undefined_var('a', 6, 18, '', 2, 7)
     ])
 
 
@@ -185,7 +177,7 @@ def test_variable_in_fragment_not_defined_by_operation():
         field(c: $c)
       }
     ''', [
-        undefined_var_by_op('c', 16, 18, 'Foo', 2, 7)
+        undefined_var('c', 16, 18, 'Foo', 2, 7)
     ])
 
 
@@ -208,8 +200,8 @@ def test_multiple_variables_in_fragments_not_defined():
         field(c: $c)
       }
     ''', [
-        undefined_var_by_op('a', 6, 18, 'Foo', 2, 7),
-        undefined_var_by_op('c', 16, 18, 'Foo', 2, 7)
+        undefined_var('a', 6, 18, 'Foo', 2, 7),
+        undefined_var('c', 16, 18, 'Foo', 2, 7)
     ])
 
 
@@ -225,8 +217,8 @@ def test_single_variable_in_fragment_not_defined_by_multiple_operations():
         field(a: $a, b: $b)
       }
     ''', [
-        undefined_var_by_op('b', 9, 25, 'Foo', 2, 7),
-        undefined_var_by_op('b', 9, 25, 'Bar', 5, 7)
+        undefined_var('b', 9, 25, 'Foo', 2, 7),
+        undefined_var('b', 9, 25, 'Bar', 5, 7)
     ])
 
 
@@ -242,8 +234,8 @@ def test_variables_in_fragment_not_defined_by_multiple_operations():
         field(a: $a, b: $b)
       }
     ''', [
-        undefined_var_by_op('a', 9, 18, 'Foo', 2, 7),
-        undefined_var_by_op('b', 9, 25, 'Bar', 5, 7)
+        undefined_var('a', 9, 18, 'Foo', 2, 7),
+        undefined_var('b', 9, 25, 'Bar', 5, 7)
     ])
 
 
@@ -262,8 +254,8 @@ def test_variable_in_fragment_used_by_other_operation():
         field(b: $b)
       }
     ''', [
-        undefined_var_by_op('a', 9, 18, 'Foo', 2, 7),
-        undefined_var_by_op('b', 12, 18, 'Bar', 5, 7)
+        undefined_var('a', 9, 18, 'Foo', 2, 7),
+        undefined_var('b', 12, 18, 'Bar', 5, 7)
     ])
 
 
@@ -284,10 +276,10 @@ def test_multiple_undefined_variables_produce_multiple_errors():
         field2(c: $c)
       }
     ''', [
-        undefined_var_by_op('a', 9, 19, 'Foo', 2, 7),
-        undefined_var_by_op('c', 14, 19, 'Foo', 2, 7),
-        undefined_var_by_op('a', 11, 19, 'Foo', 2, 7),
-        undefined_var_by_op('b', 9, 26, 'Bar', 5, 7),
-        undefined_var_by_op('c', 14, 19, 'Bar', 5, 7),
-        undefined_var_by_op('b', 11, 26, 'Bar', 5, 7),
+        undefined_var('a', 9, 19, 'Foo', 2, 7),
+        undefined_var('a', 11, 19, 'Foo', 2, 7),
+        undefined_var('c', 14, 19, 'Foo', 2, 7),
+        undefined_var('b', 9, 26, 'Bar', 5, 7),
+        undefined_var('b', 11, 26, 'Bar', 5, 7),
+        undefined_var('c', 14, 19, 'Bar', 5, 7),
     ])