diff --git a/src/jsutils/groupBy.ts b/src/jsutils/groupBy.ts new file mode 100644 index 0000000000..f3b0c076d1 --- /dev/null +++ b/src/jsutils/groupBy.ts @@ -0,0 +1,19 @@ +/** + * Groups array items into a Map, given a function to produce grouping key. + */ +export function groupBy( + list: ReadonlyArray, + keyFn: (item: T) => K, +): Map> { + const result = new Map>(); + 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; +} diff --git a/src/validation/__tests__/UniqueArgumentNamesRule-test.ts b/src/validation/__tests__/UniqueArgumentNamesRule-test.ts index a18c56bd8f..e8114948b5 100644 --- a/src/validation/__tests__/UniqueArgumentNamesRule-test.ts +++ b/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 }, ], }, diff --git a/src/validation/__tests__/UniqueVariableNamesRule-test.ts b/src/validation/__tests__/UniqueVariableNamesRule-test.ts index 4b950fb4cf..d4d39bcc55 100644 --- a/src/validation/__tests__/UniqueVariableNamesRule-test.ts +++ b/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 }, ], }, diff --git a/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts b/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts index db40ac17e3..4db179696b 100644 --- a/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts +++ b/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( - list: ReadonlyArray, - keyFn: (item: T) => K, -): Map> { - const result = new Map>(); - 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; -} diff --git a/src/validation/rules/UniqueArgumentNamesRule.ts b/src/validation/rules/UniqueArgumentNamesRule.ts index 54b090c940..c045c316c7 100644 --- a/src/validation/rules/UniqueArgumentNamesRule.ts +++ b/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; + }) { + // 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; - }, - }; + } + } } diff --git a/src/validation/rules/UniqueVariableNamesRule.ts b/src/validation/rules/UniqueVariableNamesRule.ts index 6035cdfa3e..dd77d49cef 100644 --- a/src/validation/rules/UniqueVariableNamesRule.ts +++ b/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), + ), + ); + } } }, };