Invert VariablesInAllowedPositionRule to visit every variable, rather than every operation

Author: mjmahone

src/validation/ValidationContext.ts

@@ -4,11 +4,13 @@ import type { ObjMap } from '../jsutils/ObjMap.js';
 import type { GraphQLError } from '../error/GraphQLError.js';
 
 import type {
+  ConstValueNode,
   DocumentNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
   OperationDefinitionNode,
   SelectionSetNode,
+  VariableDefinitionNode,
   VariableNode,
 } from '../language/ast.js';
 import { Kind } from '../language/kinds.js';
@@ -34,6 +36,10 @@ interface VariableUsage {
   readonly type: Maybe<GraphQLInputType>;
   readonly defaultValue: Maybe<unknown>;
 }
+interface DefinedVariable {
+  readonly variableDefinition: VariableDefinitionNode;
+  readonly operation: OperationDefinitionNode;
+}
 
 /**
  * An instance of this class is passed as the "this" context to all validators,
@@ -50,11 +56,16 @@ export class ASTValidationContext {
     Array<FragmentDefinitionNode>
   >;
 
+  private _variableDefinedOperations:
+    | ObjMap<ReadonlyArray<DefinedVariable>>
+    | undefined;
+
   constructor(ast: DocumentNode, onError: (error: GraphQLError) => void) {
     this._ast = ast;
     this._fragments = undefined;
     this._fragmentSpreads = new Map();
     this._recursivelyReferencedFragments = new Map();
+    this._variableDefinedOperations = undefined;
     this._onError = onError;
   }
 
@@ -134,6 +145,28 @@ export class ASTValidationContext {
     }
     return fragments;
   }
+
+  getVariableDefinitions(node: VariableNode): ReadonlyArray<DefinedVariable> {
+    if (!this._variableDefinedOperations) {
+      const variableDefinedOperations: ObjMap<Array<DefinedVariable>> = {};
+      visit(this._ast, {
+        OperationDefinition(operation) {
+          for (const varDef of operation.variableDefinitions ?? []) {
+            if (!variableDefinedOperations[varDef.variable.name.value]) {
+              variableDefinedOperations[varDef.variable.name.value] = [];
+            }
+            variableDefinedOperations[varDef.variable.name.value].push({
+              variableDefinition: varDef,
+              operation,
+            });
+          }
+        },
+      });
+
+      this._variableDefinedOperations = variableDefinedOperations;
+    }
+    return this._variableDefinedOperations[node.name.value] ?? [];
+  }
 }
 
 export type ASTValidationRule = (context: ASTValidationContext) => ASTVisitor;
@@ -245,6 +278,10 @@ export class ValidationContext extends ASTValidationContext {
     return this._typeInfo.getInputType();
   }
 
+  getDefaultValue(): Maybe<ConstValueNode> {
+    return this._typeInfo.getDefaultValue() as Maybe<ConstValueNode>;
+  }
+
   getParentInputType(): Maybe<GraphQLInputType> {
     return this._typeInfo.getParentInputType();
   }

src/validation/rules/VariablesInAllowedPositionRule.ts

@@ -26,52 +26,36 @@ import type { ValidationContext } from '../ValidationContext.js';
 export function VariablesInAllowedPositionRule(
   context: ValidationContext,
 ): ASTVisitor {
-  let varDefMap = Object.create(null);
-
   return {
-    OperationDefinition: {
-      enter() {
-        varDefMap = Object.create(null);
-      },
-      leave(operation) {
-        const usages = context.getRecursiveVariableUsages(operation);
-
-        for (const { node, type, defaultValue } of usages) {
-          const varName = node.name.value;
-          const varDef = varDefMap[varName];
-          if (varDef && type) {
-            // A var type is allowed if it is the same or more strict (e.g. is
-            // a subtype of) than the expected type. It can be more strict if
-            // the variable type is non-null when the expected type is nullable.
-            // If both are list types, the variable item type can be more strict
-            // than the expected item type (contravariant).
-            const schema = context.getSchema();
-            const varType = typeFromAST(schema, varDef.type);
-            if (
-              varType &&
-              !allowedVariableUsage(
-                schema,
-                varType,
-                varDef.defaultValue,
-                type,
-                defaultValue,
-              )
-            ) {
-              const varTypeStr = inspect(varType);
-              const typeStr = inspect(type);
-              context.reportError(
-                new GraphQLError(
-                  `Variable "$${varName}" of type "${varTypeStr}" used in position expecting type "${typeStr}".`,
-                  { nodes: [varDef, node] },
-                ),
-              );
-            }
-          }
+    Variable(node) {
+      const varDefs = context.getVariableDefinitions(node);
+      const type = context.getInputType();
+      const defaultValue = context.getDefaultValue();
+      const varName = node.name.value;
+      for (const { variableDefinition } of varDefs) {
+        const schema = context.getSchema();
+        const varType = typeFromAST(schema, variableDefinition.type);
+        if (
+          type &&
+          varType &&
+          !allowedVariableUsage(
+            schema,
+            varType,
+            variableDefinition.defaultValue,
+            type,
+            defaultValue,
+          )
+        ) {
+          const varTypeStr = inspect(varType);
+          const typeStr = inspect(type);
+          context.reportError(
+            new GraphQLError(
+              `Variable "$${varName}" of type "${varTypeStr}" used in position expecting type "${typeStr}".`,
+              { nodes: [variableDefinition, node] },
+            ),
+          );
         }
-      },
-    },
-    VariableDefinition(node) {
-      varDefMap[node.variable.name.value] = node;
+      }
     },
   };
 }