From 742bfa405e61983512e283ac0bdd9dcae930583a Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 6 Aug 2015 17:09:26 -0700 Subject: [PATCH] Validate directive arguments Fix #110 Argument validation was assuming field arguments which led to spurious error. --- .../__tests__/KnownArgumentNames.js | 27 ++++++++++- src/validation/errors.js | 4 ++ src/validation/rules/KnownArgumentNames.js | 46 ++++++++++++++----- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/src/validation/__tests__/KnownArgumentNames.js b/src/validation/__tests__/KnownArgumentNames.js index 7d41f9b9df..1541475131 100644 --- a/src/validation/__tests__/KnownArgumentNames.js +++ b/src/validation/__tests__/KnownArgumentNames.js @@ -10,7 +10,7 @@ import { describe, it } from 'mocha'; import { expectPassesRule, expectFailsRule } from './harness'; import KnownArgumentNames from '../rules/KnownArgumentNames'; -import { unknownArgMessage } from '../errors'; +import { unknownArgMessage, unknownDirectiveArgMessage } from '../errors'; function unknownArg(argName, fieldName, typeName, line, column) { return { @@ -19,6 +19,13 @@ function unknownArg(argName, fieldName, typeName, line, column) { }; } +function unknownDirectiveArg(argName, directiveName, line, column) { + return { + message: unknownDirectiveArgMessage(argName, directiveName), + locations: [ { line, column } ], + }; +} + describe('Validate: Known argument names', () => { it('single arg is known', () => { @@ -78,6 +85,24 @@ describe('Validate: Known argument names', () => { `); }); + it('directive args are known', () => { + expectPassesRule(KnownArgumentNames, ` + { + dog @skip(if: true) + } + `); + }); + + it('undirective args are invalid', () => { + expectFailsRule(KnownArgumentNames, ` + { + dog @skip(unless: true) + } + `, [ + unknownDirectiveArg('unless', 'skip', 3, 19), + ]); + }); + it('invalid arg name', () => { expectFailsRule(KnownArgumentNames, ` fragment invalidArgName on Dog { diff --git a/src/validation/errors.js b/src/validation/errors.js index b6f5480b9f..b38c1d96e4 100644 --- a/src/validation/errors.js +++ b/src/validation/errors.js @@ -50,6 +50,10 @@ export function unknownArgMessage(argName, fieldName, typeName) { `of type ${typeName}.`; } +export function unknownDirectiveArgMessage(argName, fieldName) { + return `Unknown argument ${argName} on directive ${fieldName}.`; +} + export function unknownTypeMessage(typeName) { return `Unknown type ${typeName}.`; } diff --git a/src/validation/rules/KnownArgumentNames.js b/src/validation/rules/KnownArgumentNames.js index 34b7c970f8..109cf231ad 100644 --- a/src/validation/rules/KnownArgumentNames.js +++ b/src/validation/rules/KnownArgumentNames.js @@ -10,7 +10,7 @@ import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; -import { unknownArgMessage } from '../errors'; +import { unknownArgMessage, unknownDirectiveArgMessage } from '../errors'; import invariant from '../../jsutils/invariant'; import find from '../../jsutils/find'; @@ -23,17 +23,41 @@ import find from '../../jsutils/find'; */ export default function KnownArgumentNames(context: ValidationContext): any { return { - Argument(node) { - var fieldDef = context.getFieldDef(); - if (fieldDef) { - var argDef = find(fieldDef.args, arg => arg.name === node.name.value); - if (!argDef) { - var parentType = context.getParentType(); - invariant(parentType); - return new GraphQLError( - unknownArgMessage(node.name.value, fieldDef.name, parentType.name), - [node] + Argument(node, key, parent, path, ancestors) { + var argumentOf = ancestors[ancestors.length - 1]; + if (argumentOf.kind === 'Field') { + var fieldDef = context.getFieldDef(); + if (fieldDef) { + var fieldArgDef = find( + fieldDef.args, + arg => arg.name === node.name.value ); + if (!fieldArgDef) { + var parentType = context.getParentType(); + invariant(parentType); + return new GraphQLError( + unknownArgMessage( + node.name.value, + fieldDef.name, + parentType.name + ), + [node] + ); + } + } + } else if (argumentOf.kind === 'Directive') { + var directive = context.getDirective(); + if (directive) { + var directiveArgDef = find( + directive.args, + arg => arg.name === node.name.value + ); + if (!directiveArgDef) { + return new GraphQLError( + unknownDirectiveArgMessage(node.name.value, directive.name), + [node] + ); + } } } }