From 4125a304dae68502008ab8accbbb9342d591d6f3 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sat, 8 Feb 2020 21:31:03 +0800 Subject: [PATCH] UniqueDirectivesPerLocation: check for directive uniqueness in extensions Fixes #2440 #2442 --- .../UniqueDirectivesPerLocationRule-test.js | 134 ++++++++++++++---- .../rules/UniqueDirectivesPerLocationRule.js | 56 +++++--- 2 files changed, 144 insertions(+), 46 deletions(-) diff --git a/src/validation/__tests__/UniqueDirectivesPerLocationRule-test.js b/src/validation/__tests__/UniqueDirectivesPerLocationRule-test.js index 9edf45d9845..2def655e469 100644 --- a/src/validation/__tests__/UniqueDirectivesPerLocationRule-test.js +++ b/src/validation/__tests__/UniqueDirectivesPerLocationRule-test.js @@ -201,22 +201,12 @@ describe('Validate: Directives Are Unique Per Location', () => { SCHEMA | SCALAR | OBJECT | INTERFACE | UNION | INPUT_OBJECT schema @nonRepeatable @nonRepeatable { query: Dummy } - extend schema @nonRepeatable @nonRepeatable scalar TestScalar @nonRepeatable @nonRepeatable - extend scalar TestScalar @nonRepeatable @nonRepeatable - type TestObject @nonRepeatable @nonRepeatable - extend type TestObject @nonRepeatable @nonRepeatable - interface TestInterface @nonRepeatable @nonRepeatable - extend interface TestInterface @nonRepeatable @nonRepeatable - union TestUnion @nonRepeatable @nonRepeatable - extend union TestUnion @nonRepeatable @nonRepeatable - input TestInput @nonRepeatable @nonRepeatable - extend input TestInput @nonRepeatable @nonRepeatable `).to.deep.equal([ { message: @@ -230,24 +220,32 @@ describe('Validate: Directives Are Unique Per Location', () => { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 6, column: 21 }, - { line: 6, column: 36 }, + { line: 7, column: 25 }, + { line: 7, column: 40 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 8, column: 25 }, - { line: 8, column: 40 }, + { line: 8, column: 23 }, + { line: 8, column: 38 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 9, column: 32 }, - { line: 9, column: 47 }, + { line: 9, column: 31 }, + { line: 9, column: 46 }, + ], + }, + { + message: + 'The directive "@nonRepeatable" can only be used once at this location.', + locations: [ + { line: 10, column: 23 }, + { line: 10, column: 38 }, ], }, { @@ -258,60 +256,136 @@ describe('Validate: Directives Are Unique Per Location', () => { { line: 11, column: 38 }, ], }, + ]); + }); + + it('duplicate directives on SDL extensions', () => { + expectSDLErrors(` + directive @nonRepeatable on + SCHEMA | SCALAR | OBJECT | INTERFACE | UNION | INPUT_OBJECT + + extend schema @nonRepeatable @nonRepeatable + + extend scalar TestScalar @nonRepeatable @nonRepeatable + extend type TestObject @nonRepeatable @nonRepeatable + extend interface TestInterface @nonRepeatable @nonRepeatable + extend union TestUnion @nonRepeatable @nonRepeatable + extend input TestInput @nonRepeatable @nonRepeatable + `).to.deep.equal([ { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 12, column: 30 }, - { line: 12, column: 45 }, + { line: 5, column: 21 }, + { line: 5, column: 36 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 14, column: 31 }, - { line: 14, column: 46 }, + { line: 7, column: 32 }, + { line: 7, column: 47 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 15, column: 38 }, - { line: 15, column: 53 }, + { line: 8, column: 30 }, + { line: 8, column: 45 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 17, column: 23 }, - { line: 17, column: 38 }, + { line: 9, column: 38 }, + { line: 9, column: 53 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 18, column: 30 }, - { line: 18, column: 45 }, + { line: 10, column: 30 }, + { line: 10, column: 45 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 20, column: 23 }, - { line: 20, column: 38 }, + { line: 11, column: 30 }, + { line: 11, column: 45 }, + ], + }, + ]); + }); + + it('duplicate directives beetween SDL definitions and extensions', () => { + expectSDLErrors(` + directive @nonRepeatable on SCHEMA + + schema @nonRepeatable { query: Dummy } + extend schema @nonRepeatable + `).to.deep.equal([ + { + message: + 'The directive "@nonRepeatable" can only be used once at this location.', + locations: [ + { line: 4, column: 14 }, + { line: 5, column: 21 }, + ], + }, + ]); + + expectSDLErrors(` + directive @nonRepeatable on SCALAR + + scalar TestScalar @nonRepeatable + extend scalar TestScalar @nonRepeatable + scalar TestScalar @nonRepeatable + `).to.deep.equal([ + { + message: + 'The directive "@nonRepeatable" can only be used once at this location.', + locations: [ + { line: 4, column: 25 }, + { line: 5, column: 32 }, + ], + }, + { + message: + 'The directive "@nonRepeatable" can only be used once at this location.', + locations: [ + { line: 4, column: 25 }, + { line: 6, column: 25 }, + ], + }, + ]); + + expectSDLErrors(` + directive @nonRepeatable on OBJECT + + extend type TestObject @nonRepeatable + type TestObject @nonRepeatable + extend type TestObject @nonRepeatable + `).to.deep.equal([ + { + message: + 'The directive "@nonRepeatable" can only be used once at this location.', + locations: [ + { line: 4, column: 30 }, + { line: 5, column: 23 }, ], }, { message: 'The directive "@nonRepeatable" can only be used once at this location.', locations: [ - { line: 21, column: 30 }, - { line: 21, column: 45 }, + { line: 4, column: 30 }, + { line: 6, column: 30 }, ], }, ]); diff --git a/src/validation/rules/UniqueDirectivesPerLocationRule.js b/src/validation/rules/UniqueDirectivesPerLocationRule.js index 142866dbe46..4302f727638 100644 --- a/src/validation/rules/UniqueDirectivesPerLocationRule.js +++ b/src/validation/rules/UniqueDirectivesPerLocationRule.js @@ -4,6 +4,10 @@ import { GraphQLError } from '../../error/GraphQLError'; import { Kind } from '../../language/kinds'; import { type ASTVisitor } from '../../language/visitor'; +import { + isTypeDefinitionNode, + isTypeExtensionNode, +} from '../../language/predicates'; import { specifiedDirectives } from '../../type/directives'; @@ -38,27 +42,47 @@ export function UniqueDirectivesPerLocationRule( } } + const schemaDirectives = Object.create(null); + const typeDirectivesMap = Object.create(null); + 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 // defines any directives. - enter(node) { - if (node.directives != null) { - const knownDirectives = Object.create(null); - for (const directive of node.directives) { - const directiveName = directive.name.value; + enter(node, _key, _parent, _path, ancestors) { + if (node.directives == null) { + return; + } + + let seenDirectives; + if ( + node.kind == Kind.SCHEMA_DEFINITION || + node.kind === Kind.SCHEMA_EXTENSION + ) { + seenDirectives = schemaDirectives; + } else if (isTypeDefinitionNode(node) || isTypeExtensionNode(node)) { + const typeName = node.name.value; + seenDirectives = typeDirectivesMap[typeName]; + if (seenDirectives === undefined) { + typeDirectivesMap[typeName] = seenDirectives = Object.create(null); + } + } else { + seenDirectives = Object.create(null); + } + + for (const directive of node.directives) { + const directiveName = directive.name.value; - if (uniqueDirectiveMap[directiveName]) { - if (knownDirectives[directiveName]) { - context.reportError( - new GraphQLError( - `The directive "@${directiveName}" can only be used once at this location.`, - [knownDirectives[directiveName], directive], - ), - ); - } else { - knownDirectives[directiveName] = directive; - } + if (uniqueDirectiveMap[directiveName]) { + if (seenDirectives[directiveName]) { + context.reportError( + new GraphQLError( + `The directive "@${directiveName}" can only be used once at this location.`, + [seenDirectives[directiveName], directive], + ), + ); + } else { + seenDirectives[directiveName] = directive; } } }