Reuse groupBy in validation rules (#3307)

Author: IvanGoncharov

src/jsutils/groupBy.ts

@@ -0,0 +1,19 @@
+/**
+ * Groups array items into a Map, given a function to produce grouping key.
+ */
+export function groupBy<K, T>(
+  list: ReadonlyArray<T>,
+  keyFn: (item: T) => K,
+): Map<K, ReadonlyArray<T>> {
+  const result = new Map<K, Array<T>>();
+  for (const item of list) {
+    const key = keyFn(item);
+    const group = result.get(key);
+    if (group === undefined) {
+      result.set(key, [item]);
+    } else {
+      group.push(item);
+    }
+  }
+  return result;
+}

src/validation/__tests__/UniqueArgumentNamesRule-test.ts

@@ -113,12 +113,6 @@ describe('Validate: Unique argument names', () => {
         locations: [
           { line: 3, column: 15 },
           { line: 3, column: 30 },
-        ],
-      },
-      {
-        message: 'There can be only one argument named "arg1".',
-        locations: [
-          { line: 3, column: 15 },
           { line: 3, column: 45 },
         ],
       },
@@ -152,12 +146,6 @@ describe('Validate: Unique argument names', () => {
         locations: [
           { line: 3, column: 26 },
           { line: 3, column: 41 },
-        ],
-      },
-      {
-        message: 'There can be only one argument named "arg1".',
-        locations: [
-          { line: 3, column: 26 },
           { line: 3, column: 56 },
         ],
       },

src/validation/__tests__/UniqueVariableNamesRule-test.ts

@@ -31,12 +31,6 @@ describe('Validate: Unique variable names', () => {
         locations: [
           { line: 2, column: 16 },
           { line: 2, column: 25 },
-        ],
-      },
-      {
-        message: 'There can be only one variable named "$x".',
-        locations: [
-          { line: 2, column: 16 },
           { line: 2, column: 34 },
         ],
       },

src/validation/rules/UniqueArgumentDefinitionNamesRule.ts

@@ -1,3 +1,5 @@
+import { groupBy } from '../../jsutils/groupBy';
+
 import { GraphQLError } from '../../error/GraphQLError';
 
 import type { ASTVisitor } from '../../language/visitor';
@@ -72,20 +74,3 @@ export function UniqueArgumentDefinitionNamesRule(
     return false;
   }
 }
-
-function groupBy<K, T>(
-  list: ReadonlyArray<T>,
-  keyFn: (item: T) => K,
-): Map<K, Array<T>> {
-  const result = new Map<K, Array<T>>();
-  for (const item of list) {
-    const key = keyFn(item);
-    const group = result.get(key);
-    if (group === undefined) {
-      result.set(key, [item]);
-    } else {
-      group.push(item);
-    }
-  }
-  return result;
-}

src/validation/rules/UniqueArgumentNamesRule.ts

@@ -1,4 +1,8 @@
+import { groupBy } from '../../jsutils/groupBy';
+
 import { GraphQLError } from '../../error/GraphQLError';
+
+import type { ArgumentNode } from '../../language/ast';
 import type { ASTVisitor } from '../../language/visitor';
 
 import type { ASTValidationContext } from '../ValidationContext';
@@ -14,27 +18,28 @@ import type { ASTValidationContext } from '../ValidationContext';
 export function UniqueArgumentNamesRule(
   context: ASTValidationContext,
 ): ASTVisitor {
-  let knownArgNames = Object.create(null);
   return {
-    Field() {
-      knownArgNames = Object.create(null);
-    },
-    Directive() {
-      knownArgNames = Object.create(null);
-    },
-    Argument(node) {
-      const argName = node.name.value;
-      if (knownArgNames[argName]) {
+    Field: checkArgUniqueness,
+    Directive: checkArgUniqueness,
+  };
+
+  function checkArgUniqueness(parentNode: {
+    arguments?: ReadonlyArray<ArgumentNode>;
+  }) {
+    // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
+    const argumentNodes = parentNode.arguments ?? [];
+
+    const seenArgs = groupBy(argumentNodes, (arg) => arg.name.value);
+
+    for (const [argName, argNodes] of seenArgs) {
+      if (argNodes.length > 1) {
         context.reportError(
           new GraphQLError(
             `There can be only one argument named "${argName}".`,
-            [knownArgNames[argName], node.name],
+            argNodes.map((node) => node.name),
           ),
         );
-      } else {
-        knownArgNames[argName] = node.name;
       }
-      return false;
-    },
-  };
+    }
+  }
 }

src/validation/rules/UniqueVariableNamesRule.ts

@@ -1,7 +1,8 @@
+import { groupBy } from '../../jsutils/groupBy';
+
 import { GraphQLError } from '../../error/GraphQLError';
 
 import type { ASTVisitor } from '../../language/visitor';
-import type { VariableDefinitionNode } from '../../language/ast';
 
 import type { ASTValidationContext } from '../ValidationContext';
 
@@ -13,22 +14,25 @@ import type { ASTValidationContext } from '../ValidationContext';
 export function UniqueVariableNamesRule(
   context: ASTValidationContext,
 ): ASTVisitor {
-  let knownVariableNames = Object.create(null);
   return {
-    OperationDefinition() {
-      knownVariableNames = Object.create(null);
-    },
-    VariableDefinition(node: VariableDefinitionNode) {
-      const variableName = node.variable.name.value;
-      if (knownVariableNames[variableName]) {
-        context.reportError(
-          new GraphQLError(
-            `There can be only one variable named "$${variableName}".`,
-            [knownVariableNames[variableName], node.variable.name],
-          ),
-        );
-      } else {
-        knownVariableNames[variableName] = node.variable.name;
+    OperationDefinition(operationNode) {
+      // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
+      const variableDefinitions = operationNode.variableDefinitions ?? [];
+
+      const seenVariableDefinitions = groupBy(
+        variableDefinitions,
+        (node) => node.variable.name.value,
+      );
+
+      for (const [variableName, variableNodes] of seenVariableDefinitions) {
+        if (variableNodes.length > 1) {
+          context.reportError(
+            new GraphQLError(
+              `There can be only one variable named "$${variableName}".`,
+              variableNodes.map((node) => node.variable.name),
+            ),
+          );
+        }
       }
     },
   };