From 2643c704b749d192c8055cee9b9dac885e97a05f Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Thu, 1 Jul 2021 19:57:55 +0300 Subject: [PATCH] Add 'UniqueArgumentDefinitionNamesRule' Background https://github.com/graphql/graphql-wg/issues/505 --- src/index.ts | 1 + .../UniqueArgumentDefinitionNamesRule-test.ts | 120 ++++++++++++++++++ src/validation/index.ts | 1 + .../UniqueArgumentDefinitionNamesRule.ts | 91 +++++++++++++ src/validation/specifiedRules.ts | 2 + 5 files changed, 215 insertions(+) create mode 100644 src/validation/__tests__/UniqueArgumentDefinitionNamesRule-test.ts create mode 100644 src/validation/rules/UniqueArgumentDefinitionNamesRule.ts diff --git a/src/index.ts b/src/index.ts index be0eda9b0f..0f5238bf50 100644 --- a/src/index.ts +++ b/src/index.ts @@ -357,6 +357,7 @@ export { UniqueTypeNamesRule, UniqueEnumValueNamesRule, UniqueFieldDefinitionNamesRule, + UniqueArgumentDefinitionNamesRule, UniqueDirectiveNamesRule, PossibleTypeExtensionsRule, /** Custom validation rules */ diff --git a/src/validation/__tests__/UniqueArgumentDefinitionNamesRule-test.ts b/src/validation/__tests__/UniqueArgumentDefinitionNamesRule-test.ts new file mode 100644 index 0000000000..cbcd2bd82c --- /dev/null +++ b/src/validation/__tests__/UniqueArgumentDefinitionNamesRule-test.ts @@ -0,0 +1,120 @@ +import { describe, it } from 'mocha'; + +import { UniqueArgumentDefinitionNamesRule } from '../rules/UniqueArgumentDefinitionNamesRule'; + +import { expectSDLValidationErrors } from './harness'; + +function expectSDLErrors(sdlStr: string) { + return expectSDLValidationErrors( + undefined, + UniqueArgumentDefinitionNamesRule, + sdlStr, + ); +} + +function expectValidSDL(sdlStr: string) { + expectSDLErrors(sdlStr).to.deep.equal([]); +} + +describe('Validate: Unique argument definition names', () => { + it('no args', () => { + expectValidSDL(` + type SomeObject { + someField: String + } + + interface SomeInterface { + someField: String + } + + directive @someDirective on QUERY + `); + }); + + it('one argument', () => { + expectValidSDL(` + type SomeObject { + someField(foo: String): String + } + + interface SomeInterface { + someField(foo: String): String + } + + directive @someDirective(foo: String) on QUERY + `); + }); + + it('multiple arguments', () => { + expectValidSDL(` + type SomeObject { + someField( + foo: String + bar: String + ): String + } + + interface SomeInterface { + someField( + foo: String + bar: String + ): String + } + + directive @someDirective( + foo: String + bar: String + ) on QUERY + `); + }); + + it('duplicating arguments', () => { + expectSDLErrors(` + type SomeObject { + someField( + foo: String + bar: String + foo: String + ): String + } + + interface SomeInterface { + someField( + foo: String + bar: String + foo: String + ): String + } + + directive @someDirective( + foo: String + bar: String + foo: String + ) on QUERY + `).to.deep.equal([ + { + message: + 'Argument "SomeObject.someField(foo:)" can only be defined once.', + locations: [ + { line: 4, column: 11 }, + { line: 6, column: 11 }, + ], + }, + { + message: + 'Argument "SomeInterface.someField(foo:)" can only be defined once.', + locations: [ + { line: 12, column: 11 }, + { line: 14, column: 11 }, + ], + }, + { + message: 'Argument "@someDirective(foo:)" can only be defined once.', + locations: [ + { line: 19, column: 9 }, + { line: 21, column: 9 }, + ], + }, + ]); + }); +}); diff --git a/src/validation/index.ts b/src/validation/index.ts index 81a10eea00..584faf3440 100644 --- a/src/validation/index.ts +++ b/src/validation/index.ts @@ -90,6 +90,7 @@ export { UniqueOperationTypesRule } from './rules/UniqueOperationTypesRule'; export { UniqueTypeNamesRule } from './rules/UniqueTypeNamesRule'; export { UniqueEnumValueNamesRule } from './rules/UniqueEnumValueNamesRule'; export { UniqueFieldDefinitionNamesRule } from './rules/UniqueFieldDefinitionNamesRule'; +export { UniqueArgumentDefinitionNamesRule } from './rules/UniqueArgumentDefinitionNamesRule'; export { UniqueDirectiveNamesRule } from './rules/UniqueDirectiveNamesRule'; export { PossibleTypeExtensionsRule } from './rules/PossibleTypeExtensionsRule'; diff --git a/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts b/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts new file mode 100644 index 0000000000..db40ac17e3 --- /dev/null +++ b/src/validation/rules/UniqueArgumentDefinitionNamesRule.ts @@ -0,0 +1,91 @@ +import { GraphQLError } from '../../error/GraphQLError'; + +import type { ASTVisitor } from '../../language/visitor'; +import type { + NameNode, + FieldDefinitionNode, + InputValueDefinitionNode, +} from '../../language/ast'; + +import type { SDLValidationContext } from '../ValidationContext'; + +/** + * Unique argument definition names + * + * A GraphQL Object or Interface type is only valid if all its fields have uniquely named arguments. + * A GraphQL Directive is only valid if all its arguments are uniquely named. + */ +export function UniqueArgumentDefinitionNamesRule( + context: SDLValidationContext, +): ASTVisitor { + return { + DirectiveDefinition(directiveNode) { + // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203') + const argumentNodes = directiveNode.arguments ?? []; + + return checkArgUniqueness(`@${directiveNode.name.value}`, argumentNodes); + }, + InterfaceTypeDefinition: checkArgUniquenessPerField, + InterfaceTypeExtension: checkArgUniquenessPerField, + ObjectTypeDefinition: checkArgUniquenessPerField, + ObjectTypeExtension: checkArgUniquenessPerField, + }; + + function checkArgUniquenessPerField(typeNode: { + readonly name: NameNode; + readonly fields?: ReadonlyArray; + }) { + const typeName = typeNode.name.value; + + // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203') + const fieldNodes = typeNode.fields ?? []; + + for (const fieldDef of fieldNodes) { + const fieldName = fieldDef.name.value; + + // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203') + const argumentNodes = fieldDef.arguments ?? []; + + checkArgUniqueness(`${typeName}.${fieldName}`, argumentNodes); + } + + return false; + } + + function checkArgUniqueness( + parentName: string, + argumentNodes: ReadonlyArray, + ) { + const seenArgs = groupBy(argumentNodes, (arg) => arg.name.value); + + for (const [argName, argNodes] of seenArgs) { + if (argNodes.length > 1) { + context.reportError( + new GraphQLError( + `Argument "${parentName}(${argName}:)" can only be defined once.`, + argNodes.map((node) => node.name), + ), + ); + } + } + + 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/specifiedRules.ts b/src/validation/specifiedRules.ts index 083c9f63f5..930e9f0f58 100644 --- a/src/validation/specifiedRules.ts +++ b/src/validation/specifiedRules.ts @@ -90,6 +90,7 @@ import { UniqueOperationTypesRule } from './rules/UniqueOperationTypesRule'; import { UniqueTypeNamesRule } from './rules/UniqueTypeNamesRule'; import { UniqueEnumValueNamesRule } from './rules/UniqueEnumValueNamesRule'; import { UniqueFieldDefinitionNamesRule } from './rules/UniqueFieldDefinitionNamesRule'; +import { UniqueArgumentDefinitionNamesRule } from './rules/UniqueArgumentDefinitionNamesRule'; import { UniqueDirectiveNamesRule } from './rules/UniqueDirectiveNamesRule'; import { PossibleTypeExtensionsRule } from './rules/PossibleTypeExtensionsRule'; @@ -138,6 +139,7 @@ export const specifiedSDLRules: ReadonlyArray = UniqueTypeNamesRule, UniqueEnumValueNamesRule, UniqueFieldDefinitionNamesRule, + UniqueArgumentDefinitionNamesRule, UniqueDirectiveNamesRule, KnownTypeNamesRule, KnownDirectivesRule,