diff --git a/src/language/__tests__/schema-kitchen-sink.graphql b/src/language/__tests__/schema-kitchen-sink.graphql index 8b94ab2b9e..2895984e19 100644 --- a/src/language/__tests__/schema-kitchen-sink.graphql +++ b/src/language/__tests__/schema-kitchen-sink.graphql @@ -142,6 +142,10 @@ directive @include2(if: Boolean!) on | FRAGMENT_SPREAD | INLINE_FRAGMENT +directive @myRepeatableDir(name: String!) repeatable on + | OBJECT + | INTERFACE + extend schema @onSchema extend schema @onSchema { diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index 058b7b0ba4..65abe8bc1c 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -814,6 +814,108 @@ input Hello { ); }); + it('Directive definition', () => { + const body = 'directive @foo on OBJECT | INTERFACE'; + const doc = parse(body); + + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'DirectiveDefinition', + description: undefined, + name: { + kind: 'Name', + value: 'foo', + loc: { + start: 11, + end: 14, + }, + }, + arguments: [], + repeatable: false, + locations: [ + { + kind: 'Name', + value: 'OBJECT', + loc: { + start: 18, + end: 24, + }, + }, + { + kind: 'Name', + value: 'INTERFACE', + loc: { + start: 27, + end: 36, + }, + }, + ], + loc: { + start: 0, + end: 36, + }, + }, + ], + loc: { + start: 0, + end: 36, + }, + }); + }); + + it('Repeatable directive definition', () => { + const body = 'directive @foo repeatable on OBJECT | INTERFACE'; + const doc = parse(body); + + expect(toJSONDeep(doc)).to.deep.equal({ + kind: 'Document', + definitions: [ + { + kind: 'DirectiveDefinition', + description: undefined, + name: { + kind: 'Name', + value: 'foo', + loc: { + start: 11, + end: 14, + }, + }, + arguments: [], + repeatable: true, + locations: [ + { + kind: 'Name', + value: 'OBJECT', + loc: { + start: 29, + end: 35, + }, + }, + { + kind: 'Name', + value: 'INTERFACE', + loc: { + start: 38, + end: 47, + }, + }, + ], + loc: { + start: 0, + end: 47, + }, + }, + ], + loc: { + start: 0, + end: 47, + }, + }); + }); + it('Directive with incorrect locations', () => { expectSyntaxError( ` diff --git a/src/language/__tests__/schema-printer-test.js b/src/language/__tests__/schema-printer-test.js index 85dd5b0fb7..1c7b02b775 100644 --- a/src/language/__tests__/schema-printer-test.js +++ b/src/language/__tests__/schema-printer-test.js @@ -179,6 +179,8 @@ describe('Printer: SDL document', () => { directive @include2(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + directive @myRepeatableDir(name: String!) repeatable on OBJECT | INTERFACE + extend schema @onSchema extend schema @onSchema { diff --git a/src/language/ast.js b/src/language/ast.js index 22169c327d..8dee33c8a3 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -508,6 +508,7 @@ export type DirectiveDefinitionNode = { +description?: StringValueNode, +name: NameNode, +arguments?: $ReadOnlyArray, + +repeatable: boolean, +locations: $ReadOnlyArray, }; diff --git a/src/language/parser.js b/src/language/parser.js index 3802448e0a..c37043be57 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -1351,7 +1351,7 @@ function parseInputObjectTypeExtension( /** * DirectiveDefinition : - * - Description? directive @ Name ArgumentsDefinition? on DirectiveLocations + * - Description? directive @ Name ArgumentsDefinition? `repeatable`? on DirectiveLocations */ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { const start = lexer.token; @@ -1360,6 +1360,7 @@ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { expect(lexer, TokenKind.AT); const name = parseName(lexer); const args = parseArgumentDefs(lexer); + const repeatable = expectOptionalKeyword(lexer, 'repeatable'); expectKeyword(lexer, 'on'); const locations = parseDirectiveLocations(lexer); return { @@ -1367,6 +1368,7 @@ function parseDirectiveDefinition(lexer: Lexer<*>): DirectiveDefinitionNode { description, name, arguments: args, + repeatable, locations, loc: loc(lexer, start), }; @@ -1512,6 +1514,21 @@ function expectKeyword(lexer: Lexer<*>, value: string): void { } } +/** + * If the next token is a keyword with the given value, return true after advancing + * the lexer. Otherwise, do not change the parser state and return false. + */ +function expectOptionalKeyword(lexer: Lexer<*>, value: string): boolean { + const token = lexer.token; + const match = token.kind === TokenKind.NAME && token.value === value; + + if (match) { + lexer.advance(); + } + + return match; +} + /** * Helper function for creating an error when an unexpected lexed token * is encountered. diff --git a/src/language/printer.js b/src/language/printer.js index 6ade5f6235..2f6390ffeb 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -181,12 +181,13 @@ const printDocASTReducer = { ), DirectiveDefinition: addDescription( - ({ name, arguments: args, locations }) => + ({ name, arguments: args, locations, repeatable }) => 'directive @' + name + (hasMultilineItems(args) ? wrap('(\n', indent(join(args, '\n')), '\n)') : wrap('(', join(args, ', '), ')')) + + (repeatable ? ' repeatable' : '') + ' on ' + join(locations, ' | '), ), diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index 532a19db3e..2dc82784f4 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -700,6 +700,21 @@ describe('Introspection', () => { isDeprecated: false, deprecationReason: null, }, + { + args: [], + deprecationReason: null, + isDeprecated: false, + name: 'isRepeatable', + type: { + kind: 'NON_NULL', + name: null, + ofType: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + }, + }, ], inputFields: null, interfaces: [], @@ -872,6 +887,81 @@ describe('Introspection', () => { }); }); + it('includes repeatable flag on directives', () => { + const EmptySchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'QueryRoot', + fields: { + onlyField: { type: GraphQLString }, + }, + }), + }); + const query = getIntrospectionQuery({ + descriptions: false, + directiveRepeatableFlag: true, + }); + const result = graphqlSync(EmptySchema, query); + + expect((result.data: any).__schema.directives).to.deep.equal([ + { + name: 'include', + locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], + args: [ + { + name: 'if', + type: { + kind: 'NON_NULL', + name: null, + ofType: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + }, + defaultValue: null, + }, + ], + isRepeatable: false, + }, + { + name: 'skip', + locations: ['FIELD', 'FRAGMENT_SPREAD', 'INLINE_FRAGMENT'], + args: [ + { + name: 'if', + type: { + kind: 'NON_NULL', + name: null, + ofType: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + }, + defaultValue: null, + }, + ], + isRepeatable: false, + }, + { + name: 'deprecated', + locations: ['FIELD_DEFINITION', 'ENUM_VALUE'], + args: [ + { + name: 'reason', + type: { + kind: 'SCALAR', + name: 'String', + ofType: null, + }, + defaultValue: '"No longer supported"', + }, + ], + isRepeatable: false, + }, + ]); + }); + it('introspects on input object', () => { const TestInputObject = new GraphQLInputObjectType({ name: 'TestInputObject', diff --git a/src/type/directives.js b/src/type/directives.js index e8a7b74fdb..f24603fda3 100644 --- a/src/type/directives.js +++ b/src/type/directives.js @@ -44,12 +44,15 @@ export class GraphQLDirective { locations: Array; args: Array; astNode: ?DirectiveDefinitionNode; + repeatable: boolean; constructor(config: GraphQLDirectiveConfig): void { this.name = config.name; this.description = config.description; this.locations = config.locations; this.astNode = config.astNode; + this.repeatable = config.repeatable == null ? false : config.repeatable; + invariant(config.name, 'Directive must be named.'); invariant( Array.isArray(config.locations), @@ -92,6 +95,7 @@ export type GraphQLDirectiveConfig = {| locations: Array, args?: ?GraphQLFieldConfigArgumentMap, astNode?: ?DirectiveDefinitionNode, + repeatable?: ?boolean, |}; /** diff --git a/src/type/introspection.js b/src/type/introspection.js index 63630d95c9..6c60295f14 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -98,6 +98,12 @@ export const __Directive = new GraphQLObjectType({ type: GraphQLNonNull(GraphQLList(GraphQLNonNull(__InputValue))), resolve: directive => directive.args || [], }, + isRepeatable: { + type: GraphQLNonNull(GraphQLBoolean), + description: + 'Permits using the directive multiple times at the same location.', + resolve: directive => directive.repeatable, + }, }), }); diff --git a/src/utilities/__tests__/buildASTSchema-test.js b/src/utilities/__tests__/buildASTSchema-test.js index 9305cd31a9..7b5b43809f 100644 --- a/src/utilities/__tests__/buildASTSchema-test.js +++ b/src/utilities/__tests__/buildASTSchema-test.js @@ -91,6 +91,22 @@ describe('Schema Builder', () => { expect(output).to.equal(body); }); + it('With repeatable directives', () => { + const body = dedent` + directive @foo(arg: Int) repeatable on FIELD + + type Query { + str: String + } + `; + + const output = cycleOutput(body); + expect(output).to.equal(body); + + const schema = buildASTSchema(parse(body)); + expect(schema.getDirective('foo').repeatable).to.equal(true); + }); + it('Supports descriptions', () => { const body = dedent` """This is a directive""" diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index 5bc7075c17..5429d5f8d7 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -37,9 +37,19 @@ import { // query against the client-side schema, it should get a result identical to // what was returned by the server. function testSchema(serverSchema) { - const initialIntrospection = introspectionFromSchema(serverSchema); + const introspectionOptions = { + descriptions: true, + directiveRepeatableFlag: true, + }; + const initialIntrospection = introspectionFromSchema( + serverSchema, + introspectionOptions, + ); const clientSchema = buildClientSchema(initialIntrospection); - const secondIntrospection = introspectionFromSchema(clientSchema); + const secondIntrospection = introspectionFromSchema( + clientSchema, + introspectionOptions, + ); expect(secondIntrospection).to.deep.equal(initialIntrospection); } @@ -583,6 +593,12 @@ describe('Type System: build schema from introspection', () => { description: 'This is a custom directive', locations: ['FIELD'], }), + new GraphQLDirective({ + name: 'customRepeatableDirective', + description: 'This is a custom repeatable directive', + repeatable: true, + locations: ['FIELD'], + }), ], }); diff --git a/src/utilities/__tests__/extendSchema-test.js b/src/utilities/__tests__/extendSchema-test.js index e9bfa43fe7..eb13748b7a 100644 --- a/src/utilities/__tests__/extendSchema-test.js +++ b/src/utilities/__tests__/extendSchema-test.js @@ -115,6 +115,15 @@ const FooDirective = new GraphQLDirective({ ], }); +const RepeatableDirective = new GraphQLDirective({ + name: 'repeatableDirective', + args: { + input: { type: SomeInputType }, + }, + repeatable: true, + locations: [DirectiveLocation.OBJECT, DirectiveLocation.INTERFACE], +}); + const testSchema = new GraphQLSchema({ query: new GraphQLObjectType({ name: 'Query', @@ -134,7 +143,7 @@ const testSchema = new GraphQLSchema({ }), }), types: [FooType, BarType], - directives: specifiedDirectives.concat([FooDirective]), + directives: specifiedDirectives.concat([FooDirective, RepeatableDirective]), }); function extendTestSchema(sdl, options) { diff --git a/src/utilities/__tests__/findBreakingChanges-test.js b/src/utilities/__tests__/findBreakingChanges-test.js index 32d969a193..066a3f4344 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.js +++ b/src/utilities/__tests__/findBreakingChanges-test.js @@ -35,6 +35,7 @@ import { findAddedNonNullDirectiveArgs, findRemovedLocationsForDirective, findRemovedDirectiveLocations, + findRemovedDirectiveRepeatable, } from '../findBreakingChanges'; import { @@ -1007,6 +1008,23 @@ describe('findBreakingChanges', () => { }, ]); }); + + it('should detect removal of repeatable flag', () => { + const oldSchema = buildSchema(` + directive @foo repeatable on OBJECT + `); + + const newSchema = buildSchema(` + directive @foo on OBJECT + `); + + expect(findRemovedDirectiveRepeatable(oldSchema, newSchema)).to.eql([ + { + type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, + description: 'Repeatable flag was removed from foo', + }, + ]); + }); }); describe('findDangerousChanges', () => { diff --git a/src/utilities/__tests__/schemaPrinter-test.js b/src/utilities/__tests__/schemaPrinter-test.js index e4a1a6c1ad..419f990972 100644 --- a/src/utilities/__tests__/schemaPrinter-test.js +++ b/src/utilities/__tests__/schemaPrinter-test.js @@ -650,6 +650,9 @@ describe('Type System Printer', () => { description: String locations: [__DirectiveLocation!]! args: [__InputValue!]! + + """Permits using the directive multiple times at the same location.""" + isRepeatable: Boolean! } """ @@ -883,6 +886,9 @@ describe('Type System Printer', () => { description: String locations: [__DirectiveLocation!]! args: [__InputValue!]! + + # Permits using the directive multiple times at the same location. + isRepeatable: Boolean! } # A Directive can be adjacent to many parts of the GraphQL language, a diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index 2319e740b3..3354198db6 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -282,6 +282,7 @@ export class ASTDefinitionBuilder { args: directiveNode.arguments && this._makeInputValues(directiveNode.arguments), + repeatable: directiveNode.repeatable, astNode: directiveNode, }); } diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index 531036dca3..791c26518d 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -355,6 +355,10 @@ export function buildClientSchema( description: directiveIntrospection.description, locations: directiveIntrospection.locations.slice(), args: buildInputValueDefMap(directiveIntrospection.args), + repeatable: + directiveIntrospection.isRepeatable === undefined + ? false + : directiveIntrospection.isRepeatable, }); } diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js index 62b184a5be..21ce29973c 100644 --- a/src/utilities/extendSchema.js +++ b/src/utilities/extendSchema.js @@ -320,6 +320,7 @@ export function extendSchema( locations: directive.locations, args: extendArgs(directive.args), astNode: directive.astNode, + repeatable: directive.repeatable, }); } diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index 9f7962138f..6a77981a4b 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -50,6 +50,7 @@ export const BreakingChangeType = { DIRECTIVE_REMOVED: 'DIRECTIVE_REMOVED', DIRECTIVE_ARG_REMOVED: 'DIRECTIVE_ARG_REMOVED', DIRECTIVE_LOCATION_REMOVED: 'DIRECTIVE_LOCATION_REMOVED', + DIRECTIVE_REPEATABLE_REMOVED: 'DIRECTIVE_REPEATABLE_REMOVED', REQUIRED_DIRECTIVE_ARG_ADDED: 'REQUIRED_DIRECTIVE_ARG_ADDED', }; @@ -94,6 +95,7 @@ export function findBreakingChanges( ...findRemovedDirectiveArgs(oldSchema, newSchema), ...findAddedNonNullDirectiveArgs(oldSchema, newSchema), ...findRemovedDirectiveLocations(oldSchema, newSchema), + ...findRemovedDirectiveRepeatable(oldSchema, newSchema), ]; } @@ -840,6 +842,31 @@ export function findRemovedDirectiveLocations( return removedLocations; } +export function findRemovedDirectiveRepeatable( + oldSchema: GraphQLSchema, + newSchema: GraphQLSchema, +): Array { + const removedRepeatable = []; + const oldSchemaDirectiveMap = getDirectiveMapForSchema(oldSchema); + + for (const newDirective of newSchema.getDirectives()) { + const oldDirective = oldSchemaDirectiveMap[newDirective.name]; + + if (!oldDirective) { + continue; + } + + if (oldDirective.repeatable && !newDirective.repeatable) { + removedRepeatable.push({ + type: BreakingChangeType.DIRECTIVE_REPEATABLE_REMOVED, + description: `Repeatable flag was removed from ${newDirective.name}`, + }); + } + } + + return removedRepeatable; +} + function getDirectiveMapForSchema( schema: GraphQLSchema, ): ObjMap { diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index b6ebe58485..f3ed594d1a 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -13,10 +13,15 @@ export type IntrospectionOptions = {| // Whether to include descriptions in the introspection result. // Default: true descriptions: boolean, + // Whether to include `isRepeatable` flag on directives. + // Default: false + directiveRepeatableFlag?: ?boolean, |}; export function getIntrospectionQuery(options?: IntrospectionOptions): string { const descriptions = !(options && options.descriptions === false); + const directiveRepeatableFlag = + options && options.directiveRepeatableFlag === true; return ` query IntrospectionQuery { __schema { @@ -33,6 +38,7 @@ export function getIntrospectionQuery(options?: IntrospectionOptions): string { args { ...InputValue } + ${directiveRepeatableFlag ? 'isRepeatable' : ''} } } } @@ -273,4 +279,5 @@ export type IntrospectionDirective = {| +description?: ?string, +locations: $ReadOnlyArray, +args: $ReadOnlyArray, + +isRepeatable?: boolean, |}; diff --git a/src/utilities/schemaPrinter.js b/src/utilities/schemaPrinter.js index aa9b01c096..f6572a7283 100644 --- a/src/utilities/schemaPrinter.js +++ b/src/utilities/schemaPrinter.js @@ -309,6 +309,7 @@ function printDirective(directive, options) { 'directive @' + directive.name + printArgs(options, directive.args) + + (directive.repeatable ? ' repeatable' : '') + ' on ' + directive.locations.join(' | ') ); diff --git a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js index d0c07813fe..d1b8cd00cc 100644 --- a/src/validation/__tests__/UniqueDirectivesPerLocation-test.js +++ b/src/validation/__tests__/UniqueDirectivesPerLocation-test.js @@ -47,8 +47,8 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directiveA { - field @directiveB + fragment Test on Type @genericDirectiveA { + field @genericDirectiveB } `, ); @@ -58,8 +58,8 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directiveA @directiveB { - field @directiveA @directiveB + fragment Test on Type @genericDirectiveA @genericDirectiveB { + field @genericDirectiveA @genericDirectiveB } `, ); @@ -69,8 +69,8 @@ describe('Validate: Directives Are Unique Per Location', () => { expectPassesRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directiveA { - field @directiveA + fragment Test on Type @genericDirectiveA { + field @genericDirectiveA } `, ); @@ -81,8 +81,49 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directive - field @directive + field @genericDirectiveA + field @genericDirectiveA + } + `, + ); + }); + + it('repeatable directives in same location', () => { + expectPassesRule( + UniqueDirectivesPerLocation, + ` + type Test @repeatableDirective(id: 1) @repeatableDirective(id: 2) { + field: String! + } + `, + ); + }); + + it('repeatable directives in similar locations', () => { + expectPassesRule( + UniqueDirectivesPerLocation, + ` + type Test @repeatableDirective(id: 1) { + field: String! + } + + extend type Test @repeatableDirective(id: 2) { + anotherField: String! + } + `, + ); + }); + + it('unknown directives must be ignored', () => { + expectPassesRule( + UniqueDirectivesPerLocation, + ` + type Test @unknownDirective @unknownDirective { + field: String! + } + + extend type Test @unknownDirective { + anotherField: String! } `, ); @@ -93,10 +134,10 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directive @directive + field @genericDirectiveA @genericDirectiveA } `, - [duplicateDirective('directive', 3, 15, 3, 26)], + [duplicateDirective('genericDirectiveA', 3, 15, 3, 34)], ); }); @@ -105,12 +146,12 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directive @directive @directive + field @genericDirectiveA @genericDirectiveA @genericDirectiveA } `, [ - duplicateDirective('directive', 3, 15, 3, 26), - duplicateDirective('directive', 3, 15, 3, 37), + duplicateDirective('genericDirectiveA', 3, 15, 3, 34), + duplicateDirective('genericDirectiveA', 3, 15, 3, 53), ], ); }); @@ -120,12 +161,12 @@ describe('Validate: Directives Are Unique Per Location', () => { UniqueDirectivesPerLocation, ` fragment Test on Type { - field @directiveA @directiveB @directiveA @directiveB + field @genericDirectiveA @genericDirectiveB @genericDirectiveA @genericDirectiveB } `, [ - duplicateDirective('directiveA', 3, 15, 3, 39), - duplicateDirective('directiveB', 3, 27, 3, 51), + duplicateDirective('genericDirectiveA', 3, 15, 3, 53), + duplicateDirective('genericDirectiveB', 3, 34, 3, 72), ], ); }); @@ -134,19 +175,27 @@ describe('Validate: Directives Are Unique Per Location', () => { expectFailsRule( UniqueDirectivesPerLocation, ` - fragment Test on Type @directive @directive { - field @directive @directive + fragment Test on Type @genericDirectiveA @genericDirectiveA { + field @genericDirectiveA @genericDirectiveA } `, [ - duplicateDirective('directive', 2, 29, 2, 40), - duplicateDirective('directive', 3, 15, 3, 26), + duplicateDirective('genericDirectiveA', 2, 29, 2, 48), + duplicateDirective('genericDirectiveA', 3, 15, 3, 34), ], ); }); it('duplicate directives on SDL definitions', () => { expectSDLErrors(` + directive @directive on + | SCHEMA + | SCALAR + | OBJECT + | INTERFACE + | UNION + | INPUT_OBJECT + schema @directive @directive { query: Dummy } extend schema @directive @directive @@ -165,18 +214,18 @@ describe('Validate: Directives Are Unique Per Location', () => { input TestInput @directive @directive extend input TestInput @directive @directive `).to.deep.equal([ - duplicateDirective('directive', 2, 14, 2, 25), - duplicateDirective('directive', 3, 21, 3, 32), - duplicateDirective('directive', 5, 25, 5, 36), - duplicateDirective('directive', 6, 32, 6, 43), - duplicateDirective('directive', 8, 23, 8, 34), - duplicateDirective('directive', 9, 30, 9, 41), - duplicateDirective('directive', 11, 31, 11, 42), - duplicateDirective('directive', 12, 38, 12, 49), - duplicateDirective('directive', 14, 23, 14, 34), - duplicateDirective('directive', 15, 30, 15, 41), - duplicateDirective('directive', 17, 23, 17, 34), - duplicateDirective('directive', 18, 30, 18, 41), + duplicateDirective('directive', 10, 14, 10, 25), + duplicateDirective('directive', 11, 21, 11, 32), + duplicateDirective('directive', 13, 25, 13, 36), + duplicateDirective('directive', 14, 32, 14, 43), + duplicateDirective('directive', 16, 23, 16, 34), + duplicateDirective('directive', 17, 30, 17, 41), + duplicateDirective('directive', 19, 31, 19, 42), + duplicateDirective('directive', 20, 38, 20, 49), + duplicateDirective('directive', 22, 23, 22, 34), + duplicateDirective('directive', 23, 30, 23, 41), + duplicateDirective('directive', 25, 23, 25, 34), + duplicateDirective('directive', 26, 30, 26, 41), ]); }); }); diff --git a/src/validation/__tests__/harness.js b/src/validation/__tests__/harness.js index cec09455cd..2b4c4a6f14 100644 --- a/src/validation/__tests__/harness.js +++ b/src/validation/__tests__/harness.js @@ -380,6 +380,25 @@ export const testSchema = new GraphQLSchema({ name: 'onVariableDefinition', locations: ['VARIABLE_DEFINITION'], }), + new GraphQLDirective({ + name: 'genericDirectiveA', + locations: ['FIELD', 'FRAGMENT_DEFINITION'], + }), + new GraphQLDirective({ + name: 'genericDirectiveB', + locations: ['FIELD', 'FRAGMENT_DEFINITION'], + }), + new GraphQLDirective({ + name: 'repeatableDirective', + args: { + id: { + type: GraphQLNonNull(GraphQLInt), + description: 'Some generic ID.', + }, + }, + repeatable: true, + locations: ['OBJECT'], + }), ], }); diff --git a/src/validation/rules/UniqueDirectivesPerLocation.js b/src/validation/rules/UniqueDirectivesPerLocation.js index a866d81e74..d3c85cb6e5 100644 --- a/src/validation/rules/UniqueDirectivesPerLocation.js +++ b/src/validation/rules/UniqueDirectivesPerLocation.js @@ -7,10 +7,15 @@ * @flow strict */ -import type { ASTValidationContext } from '../ValidationContext'; +import type { + SDLValidationContext, + ValidationContext, +} from '../ValidationContext'; import { GraphQLError } from '../../error/GraphQLError'; import type { DirectiveNode } from '../../language/ast'; import type { ASTVisitor } from '../../language/visitor'; +import { Kind } from '../../language/kinds'; +import { specifiedDirectives } from '../../type/directives'; export function duplicateDirectiveMessage(directiveName: string): string { return ( @@ -22,12 +27,29 @@ export function duplicateDirectiveMessage(directiveName: string): string { /** * Unique directive names per location * - * A GraphQL document is only valid if all directives at a given location + * A GraphQL document is only valid if directives, that are not marked as `isRepeatable`, at a given location * are uniquely named. */ export function UniqueDirectivesPerLocation( - context: ASTValidationContext, + context: ValidationContext | SDLValidationContext, ): ASTVisitor { + const uniqueDirectiveMap = Object.create(null); + + const schema = context.getSchema(); + const definedDirectives = schema + ? schema.getDirectives() + : specifiedDirectives; + for (const directive of definedDirectives) { + uniqueDirectiveMap[directive.name] = !directive.repeatable; + } + + const astDefinitions = context.getDocument().definitions; + for (const def of astDefinitions) { + if (def.kind === Kind.DIRECTIVE_DEFINITION) { + uniqueDirectiveMap[def.name.value] = !def.repeatable; + } + } + return { // Many different AST nodes may contain directives. Rather than listing // them all, just listen for entering any node, and check to see if it @@ -40,15 +62,18 @@ export function UniqueDirectivesPerLocation( const knownDirectives = Object.create(null); for (const directive of directives) { const directiveName = directive.name.value; - if (knownDirectives[directiveName]) { - context.reportError( - new GraphQLError(duplicateDirectiveMessage(directiveName), [ - knownDirectives[directiveName], - directive, - ]), - ); - } else { - knownDirectives[directiveName] = directive; + + if (uniqueDirectiveMap[directiveName]) { + if (knownDirectives[directiveName]) { + context.reportError( + new GraphQLError(duplicateDirectiveMessage(directiveName), [ + knownDirectives[directiveName], + directive, + ]), + ); + } else { + knownDirectives[directiveName] = directive; + } } } }