From 5e545cce0104708a4ac6e994dd5f837d1d30a09b Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Mon, 16 Nov 2015 19:32:13 -0800 Subject: [PATCH] [Validation] Report errors rather than return them This replaces the mechanism of returning errors or lists of errors from a validator step to instead report those errors via calling a function on the context. This simplifies the implementation of the visitor mechanism, but also opens the doors for future rules being specified as warnings instead of errors. --- .../__tests__/FieldsOnCorrectType.js | 16 +++++- src/validation/__tests__/validation.js | 4 +- .../rules/ArgumentsOfCorrectType.js | 4 +- .../rules/DefaultValuesOfCorrectType.js | 8 +-- src/validation/rules/FieldsOnCorrectType.js | 4 +- .../rules/FragmentsOnCompositeTypes.js | 8 +-- src/validation/rules/KnownArgumentNames.js | 8 +-- src/validation/rules/KnownDirectives.js | 55 +++++++++++-------- src/validation/rules/KnownFragmentNames.js | 4 +- src/validation/rules/KnownTypeNames.js | 4 +- .../rules/LoneAnonymousOperation.js | 7 ++- src/validation/rules/NoFragmentCycles.js | 11 +--- src/validation/rules/NoUndefinedVariables.js | 21 +++---- src/validation/rules/NoUnusedFragments.js | 18 +++--- src/validation/rules/NoUnusedVariables.js | 19 +++---- .../rules/OverlappingFieldsCanBeMerged.js | 16 +++--- .../rules/PossibleFragmentSpreads.js | 8 +-- .../rules/ProvidedNonNullArguments.js | 14 +---- src/validation/rules/ScalarLeafs.js | 8 +-- src/validation/rules/UniqueArgumentNames.js | 10 ++-- src/validation/rules/UniqueFragmentNames.js | 10 ++-- src/validation/rules/UniqueInputFieldNames.js | 10 ++-- src/validation/rules/UniqueOperationNames.js | 10 ++-- .../rules/VariablesAreInputTypes.js | 4 +- .../rules/VariablesInAllowedPosition.js | 7 +-- src/validation/validate.js | 41 ++++---------- 26 files changed, 156 insertions(+), 173 deletions(-) diff --git a/src/validation/__tests__/FieldsOnCorrectType.js b/src/validation/__tests__/FieldsOnCorrectType.js index e2ac55e459..4d594982a6 100644 --- a/src/validation/__tests__/FieldsOnCorrectType.js +++ b/src/validation/__tests__/FieldsOnCorrectType.js @@ -75,6 +75,20 @@ describe('Validate: Fields on correct type', () => { `); }); + it('reports errors when type is known again', () => { + expectFailsRule(FieldsOnCorrectType, ` + fragment typeKnownAgain on Pet { + unknown_pet_field { + ... on Cat { + unknown_cat_field + } + } + }`, + [ undefinedField('unknown_pet_field', 'Pet', 3, 9), + undefinedField('unknown_cat_field', 'Cat', 5, 13) ] + ); + }); + it('Field not defined on fragment', () => { expectFailsRule(FieldsOnCorrectType, ` fragment fieldNotDefined on Dog { @@ -84,7 +98,7 @@ describe('Validate: Fields on correct type', () => { ); }); - it('Field not defined deeply, only reports first', () => { + it('Ignores deeply unknown field', () => { expectFailsRule(FieldsOnCorrectType, ` fragment deepFieldNotDefined on Dog { unknown_field { diff --git a/src/validation/__tests__/validation.js b/src/validation/__tests__/validation.js index ded2a01fe6..21495f0540 100644 --- a/src/validation/__tests__/validation.js +++ b/src/validation/__tests__/validation.js @@ -65,7 +65,9 @@ describe('Validate: Supports full validation', () => { ); expect(errors).to.deep.equal([ - { message: 'Cannot query field "catOrDog" on "QueryRoot".' } + { message: 'Cannot query field "catOrDog" on "QueryRoot".' }, + { message: 'Cannot query field "furColor" on "Cat".' }, + { message: 'Cannot query field "isHousetrained" on "Dog".' } ]); }); diff --git a/src/validation/rules/ArgumentsOfCorrectType.js b/src/validation/rules/ArgumentsOfCorrectType.js index 8fbee82205..963dfbb8c7 100644 --- a/src/validation/rules/ArgumentsOfCorrectType.js +++ b/src/validation/rules/ArgumentsOfCorrectType.js @@ -29,14 +29,14 @@ export function ArgumentsOfCorrectType(context: ValidationContext): any { Argument(argAST) { var argDef = context.getArgument(); if (argDef && !isValidLiteralValue(argDef.type, argAST.value)) { - return new GraphQLError( + context.reportError(new GraphQLError( badValueMessage( argAST.name.value, argDef.type, print(argAST.value) ), [ argAST.value ] - ); + )); } return false; } diff --git a/src/validation/rules/DefaultValuesOfCorrectType.js b/src/validation/rules/DefaultValuesOfCorrectType.js index 47738bf46a..070b4a289c 100644 --- a/src/validation/rules/DefaultValuesOfCorrectType.js +++ b/src/validation/rules/DefaultValuesOfCorrectType.js @@ -46,16 +46,16 @@ export function DefaultValuesOfCorrectType(context: ValidationContext): any { var defaultValue = varDefAST.defaultValue; var type = context.getInputType(); if (type instanceof GraphQLNonNull && defaultValue) { - return new GraphQLError( + context.reportError(new GraphQLError( defaultForNonNullArgMessage(name, type, type.ofType), [ defaultValue ] - ); + )); } if (type && defaultValue && !isValidLiteralValue(type, defaultValue)) { - return new GraphQLError( + context.reportError(new GraphQLError( badValueForDefaultArgMessage(name, type, print(defaultValue)), [ defaultValue ] - ); + )); } return false; }, diff --git a/src/validation/rules/FieldsOnCorrectType.js b/src/validation/rules/FieldsOnCorrectType.js index fb0b63505e..8c01486090 100644 --- a/src/validation/rules/FieldsOnCorrectType.js +++ b/src/validation/rules/FieldsOnCorrectType.js @@ -30,10 +30,10 @@ export function FieldsOnCorrectType(context: ValidationContext): any { if (type) { var fieldDef = context.getFieldDef(); if (!fieldDef) { - return new GraphQLError( + context.reportError(new GraphQLError( undefinedFieldMessage(node.name.value, type.name), [ node ] - ); + )); } } } diff --git a/src/validation/rules/FragmentsOnCompositeTypes.js b/src/validation/rules/FragmentsOnCompositeTypes.js index 5c9f56bcce..2636af6c45 100644 --- a/src/validation/rules/FragmentsOnCompositeTypes.js +++ b/src/validation/rules/FragmentsOnCompositeTypes.js @@ -40,22 +40,22 @@ export function FragmentsOnCompositeTypes(context: ValidationContext): any { InlineFragment(node) { var type = context.getType(); if (node.typeCondition && type && !isCompositeType(type)) { - return new GraphQLError( + context.reportError(new GraphQLError( inlineFragmentOnNonCompositeErrorMessage(print(node.typeCondition)), [ node.typeCondition ] - ); + )); } }, FragmentDefinition(node) { var type = context.getType(); if (type && !isCompositeType(type)) { - return new GraphQLError( + context.reportError(new GraphQLError( fragmentOnNonCompositeErrorMessage( node.name.value, print(node.typeCondition) ), [ node.typeCondition ] - ); + )); } } }; diff --git a/src/validation/rules/KnownArgumentNames.js b/src/validation/rules/KnownArgumentNames.js index 25a816d72f..00eb439b95 100644 --- a/src/validation/rules/KnownArgumentNames.js +++ b/src/validation/rules/KnownArgumentNames.js @@ -54,14 +54,14 @@ export function KnownArgumentNames(context: ValidationContext): any { if (!fieldArgDef) { var parentType = context.getParentType(); invariant(parentType); - return new GraphQLError( + context.reportError(new GraphQLError( unknownArgMessage( node.name.value, fieldDef.name, parentType.name ), [ node ] - ); + )); } } } else if (argumentOf.kind === DIRECTIVE) { @@ -72,10 +72,10 @@ export function KnownArgumentNames(context: ValidationContext): any { arg => arg.name === node.name.value ); if (!directiveArgDef) { - return new GraphQLError( + context.reportError(new GraphQLError( unknownDirectiveArgMessage(node.name.value, directive.name), [ node ] - ); + )); } } } diff --git a/src/validation/rules/KnownDirectives.js b/src/validation/rules/KnownDirectives.js index 8d4cd5649b..a2efd12559 100644 --- a/src/validation/rules/KnownDirectives.js +++ b/src/validation/rules/KnownDirectives.js @@ -45,33 +45,40 @@ export function KnownDirectives(context: ValidationContext): any { def => def.name === node.name.value ); if (!directiveDef) { - return new GraphQLError( + context.reportError(new GraphQLError( unknownDirectiveMessage(node.name.value), [ node ] - ); + )); + return; } - var appliedTo = ancestors[ancestors.length - 1]; - if (appliedTo.kind === OPERATION_DEFINITION && - !directiveDef.onOperation) { - return new GraphQLError( - misplacedDirectiveMessage(node.name.value, 'operation'), - [ node ] - ); - } - if (appliedTo.kind === FIELD && !directiveDef.onField) { - return new GraphQLError( - misplacedDirectiveMessage(node.name.value, 'field'), - [ node ] - ); - } - if ((appliedTo.kind === FRAGMENT_SPREAD || - appliedTo.kind === INLINE_FRAGMENT || - appliedTo.kind === FRAGMENT_DEFINITION) && - !directiveDef.onFragment) { - return new GraphQLError( - misplacedDirectiveMessage(node.name.value, 'fragment'), - [ node ] - ); + const appliedTo = ancestors[ancestors.length - 1]; + switch (appliedTo.kind) { + case OPERATION_DEFINITION: + if (!directiveDef.onOperation) { + context.reportError(new GraphQLError( + misplacedDirectiveMessage(node.name.value, 'operation'), + [ node ] + )); + } + break; + case FIELD: + if (!directiveDef.onField) { + context.reportError(new GraphQLError( + misplacedDirectiveMessage(node.name.value, 'field'), + [ node ] + )); + } + break; + case FRAGMENT_SPREAD: + case INLINE_FRAGMENT: + case FRAGMENT_DEFINITION: + if (!directiveDef.onFragment) { + context.reportError(new GraphQLError( + misplacedDirectiveMessage(node.name.value, 'fragment'), + [ node ] + )); + } + break; } } }; diff --git a/src/validation/rules/KnownFragmentNames.js b/src/validation/rules/KnownFragmentNames.js index 38c022ce66..ebcadc60a9 100644 --- a/src/validation/rules/KnownFragmentNames.js +++ b/src/validation/rules/KnownFragmentNames.js @@ -28,10 +28,10 @@ export function KnownFragmentNames(context: ValidationContext): any { var fragmentName = node.name.value; var fragment = context.getFragment(fragmentName); if (!fragment) { - return new GraphQLError( + context.reportError(new GraphQLError( unknownFragmentMessage(fragmentName), [ node.name ] - ); + )); } } }; diff --git a/src/validation/rules/KnownTypeNames.js b/src/validation/rules/KnownTypeNames.js index 99c6025a5c..1a34e56e09 100644 --- a/src/validation/rules/KnownTypeNames.js +++ b/src/validation/rules/KnownTypeNames.js @@ -28,7 +28,9 @@ export function KnownTypeNames(context: ValidationContext): any { var typeName = node.name.value; var type = context.getSchema().getType(typeName); if (!type) { - return new GraphQLError(unknownTypeMessage(typeName), [ node ]); + context.reportError( + new GraphQLError(unknownTypeMessage(typeName), [ node ]) + ); } } }; diff --git a/src/validation/rules/LoneAnonymousOperation.js b/src/validation/rules/LoneAnonymousOperation.js index 2dc406b88c..ee5d905765 100644 --- a/src/validation/rules/LoneAnonymousOperation.js +++ b/src/validation/rules/LoneAnonymousOperation.js @@ -8,6 +8,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; import { OPERATION_DEFINITION } from '../../language/kinds'; @@ -22,7 +23,7 @@ export function anonOperationNotAloneMessage(): string { * A GraphQL document is only valid if when it contains an anonymous operation * (the query short-hand) that it contains only that one operation definition. */ -export function LoneAnonymousOperation(): any { +export function LoneAnonymousOperation(context: ValidationContext): any { var operationCount = 0; return { Document(node) { @@ -32,7 +33,9 @@ export function LoneAnonymousOperation(): any { }, OperationDefinition(node) { if (!node.name && operationCount > 1) { - return new GraphQLError(anonOperationNotAloneMessage(), [ node ]); + context.reportError( + new GraphQLError(anonOperationNotAloneMessage(), [ node ]) + ); } } }; diff --git a/src/validation/rules/NoFragmentCycles.js b/src/validation/rules/NoFragmentCycles.js index 1b8baf976f..dd9461e772 100644 --- a/src/validation/rules/NoFragmentCycles.js +++ b/src/validation/rules/NoFragmentCycles.js @@ -22,8 +22,6 @@ export function cycleErrorMessage( } export function NoFragmentCycles(context: ValidationContext): any { - var errors = []; - // Tracks already visited fragments to maintain O(N) and to ensure that cycles // are not redundantly reported. var visitedFrags = Object.create(null); @@ -35,13 +33,6 @@ export function NoFragmentCycles(context: ValidationContext): any { var spreadPathIndexByName = Object.create(null); return { - Document: { - leave() { - if (errors.length) { - return errors; - } - } - }, OperationDefinition: () => false, FragmentDefinition(node) { if (!visitedFrags[node.name.value]) { @@ -81,7 +72,7 @@ export function NoFragmentCycles(context: ValidationContext): any { spreadPath.pop(); } else { const cyclePath = spreadPath.slice(cycleIndex); - errors.push(new GraphQLError( + context.reportError(new GraphQLError( cycleErrorMessage( spreadName, cyclePath.map(s => s.name.value) diff --git a/src/validation/rules/NoUndefinedVariables.js b/src/validation/rules/NoUndefinedVariables.js index 73c2ccb721..6cd205927f 100644 --- a/src/validation/rules/NoUndefinedVariables.js +++ b/src/validation/rules/NoUndefinedVariables.js @@ -34,26 +34,19 @@ export function NoUndefinedVariables(context: ValidationContext): any { }, leave(operation) { const usages = context.getRecursiveVariableUsages(operation); - const errors = []; usages.forEach(({ node }) => { var varName = node.name.value; if (variableNameDefined[varName] !== true) { - errors.push( - new GraphQLError( - undefinedVarMessage( - varName, - operation.name && operation.name.value - ), - [ node, operation ] - ) - ); + context.reportError(new GraphQLError( + undefinedVarMessage( + varName, + operation.name && operation.name.value + ), + [ node, operation ] + )); } }); - - if (errors.length > 0) { - return errors; - } } }, VariableDefinition(varDefAST) { diff --git a/src/validation/rules/NoUnusedFragments.js b/src/validation/rules/NoUnusedFragments.js index 75ac2184b4..a85b5ae36b 100644 --- a/src/validation/rules/NoUnusedFragments.js +++ b/src/validation/rules/NoUnusedFragments.js @@ -44,15 +44,15 @@ export function NoUnusedFragments(context: ValidationContext): any { ); }); - var errors = fragmentDefs - .filter(def => fragmentNameUsed[def.name.value] !== true) - .map(def => new GraphQLError( - unusedFragMessage(def.name.value), - [ def ] - )); - if (errors.length > 0) { - return errors; - } + fragmentDefs.forEach(fragmentDef => { + const fragName = fragmentDef.name.value; + if (fragmentNameUsed[fragName] !== true) { + context.reportError(new GraphQLError( + unusedFragMessage(fragName), + [ fragmentDef ] + )); + } + }); } } }; diff --git a/src/validation/rules/NoUnusedVariables.js b/src/validation/rules/NoUnusedVariables.js index 4169ea0aeb..eaba06f026 100644 --- a/src/validation/rules/NoUnusedVariables.js +++ b/src/validation/rules/NoUnusedVariables.js @@ -31,7 +31,6 @@ export function NoUnusedVariables(context: ValidationContext): any { variableDefs = []; }, leave(operation) { - const variableNameUsed = Object.create(null); const usages = context.getRecursiveVariableUsages(operation); @@ -39,15 +38,15 @@ export function NoUnusedVariables(context: ValidationContext): any { variableNameUsed[node.name.value] = true; }); - var errors = variableDefs - .filter(def => variableNameUsed[def.variable.name.value] !== true) - .map(def => new GraphQLError( - unusedVariableMessage(def.variable.name.value), - [ def ] - )); - if (errors.length > 0) { - return errors; - } + variableDefs.forEach(variableDef => { + const variableName = variableDef.variable.name.value; + if (variableNameUsed[variableName] !== true) { + context.reportError(new GraphQLError( + unusedVariableMessage(variableName), + [ variableDef ] + )); + } + }); } }, VariableDefinition(def) { diff --git a/src/validation/rules/OverlappingFieldsCanBeMerged.js b/src/validation/rules/OverlappingFieldsCanBeMerged.js index 0225aedd68..8dd18ad2e2 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMerged.js +++ b/src/validation/rules/OverlappingFieldsCanBeMerged.js @@ -180,15 +180,13 @@ export function OverlappingFieldsCanBeMerged(context: ValidationContext): any { selectionSet ); var conflicts = findConflicts(fieldMap); - if (conflicts.length) { - return conflicts.map( - ([ [ responseName, reason ], fields1, fields2 ]) => - new GraphQLError( - fieldsConflictMessage(responseName, reason), - fields1.concat(fields2) - ) - ); - } + conflicts.forEach( + ([ [ responseName, reason ], fields1, fields2 ]) => + context.reportError(new GraphQLError( + fieldsConflictMessage(responseName, reason), + fields1.concat(fields2) + )) + ); } } }; diff --git a/src/validation/rules/PossibleFragmentSpreads.js b/src/validation/rules/PossibleFragmentSpreads.js index 1c10e99032..320a21c4ed 100644 --- a/src/validation/rules/PossibleFragmentSpreads.js +++ b/src/validation/rules/PossibleFragmentSpreads.js @@ -49,10 +49,10 @@ export function PossibleFragmentSpreads(context: ValidationContext): any { var fragType = context.getType(); var parentType = context.getParentType(); if (fragType && parentType && !doTypesOverlap(fragType, parentType)) { - return new GraphQLError( + context.reportError(new GraphQLError( typeIncompatibleAnonSpreadMessage(parentType, fragType), [ node ] - ); + )); } }, FragmentSpread(node) { @@ -60,10 +60,10 @@ export function PossibleFragmentSpreads(context: ValidationContext): any { var fragType = getFragmentType(context, fragName); var parentType = context.getParentType(); if (fragType && parentType && !doTypesOverlap(fragType, parentType)) { - return new GraphQLError( + context.reportError(new GraphQLError( typeIncompatibleSpreadMessage(fragName, parentType, fragType), [ node ] - ); + )); } } }; diff --git a/src/validation/rules/ProvidedNonNullArguments.js b/src/validation/rules/ProvidedNonNullArguments.js index c7e118a8d9..f85bc02324 100644 --- a/src/validation/rules/ProvidedNonNullArguments.js +++ b/src/validation/rules/ProvidedNonNullArguments.js @@ -47,14 +47,13 @@ export function ProvidedNonNullArguments(context: ValidationContext): any { if (!fieldDef) { return false; } - var errors = []; var argASTs = fieldAST.arguments || []; var argASTMap = keyMap(argASTs, arg => arg.name.value); fieldDef.args.forEach(argDef => { var argAST = argASTMap[argDef.name]; if (!argAST && argDef.type instanceof GraphQLNonNull) { - errors.push(new GraphQLError( + context.reportError(new GraphQLError( missingFieldArgMessage( fieldAST.name.value, argDef.name, @@ -64,10 +63,6 @@ export function ProvidedNonNullArguments(context: ValidationContext): any { )); } }); - - if (errors.length > 0) { - return errors; - } } }, @@ -78,14 +73,13 @@ export function ProvidedNonNullArguments(context: ValidationContext): any { if (!directiveDef) { return false; } - var errors = []; var argASTs = directiveAST.arguments || []; var argASTMap = keyMap(argASTs, arg => arg.name.value); directiveDef.args.forEach(argDef => { var argAST = argASTMap[argDef.name]; if (!argAST && argDef.type instanceof GraphQLNonNull) { - errors.push(new GraphQLError( + context.reportError(new GraphQLError( missingDirectiveArgMessage( directiveAST.name.value, argDef.name, @@ -95,10 +89,6 @@ export function ProvidedNonNullArguments(context: ValidationContext): any { )); } }); - - if (errors.length > 0) { - return errors; - } } } }; diff --git a/src/validation/rules/ScalarLeafs.js b/src/validation/rules/ScalarLeafs.js index 33d54c13ce..a11e82339a 100644 --- a/src/validation/rules/ScalarLeafs.js +++ b/src/validation/rules/ScalarLeafs.js @@ -35,16 +35,16 @@ export function ScalarLeafs(context: ValidationContext): any { if (type) { if (isLeafType(type)) { if (node.selectionSet) { - return new GraphQLError( + context.reportError(new GraphQLError( noSubselectionAllowedMessage(node.name.value, type), [ node.selectionSet ] - ); + )); } } else if (!node.selectionSet) { - return new GraphQLError( + context.reportError(new GraphQLError( requiredSubselectionMessage(node.name.value, type), [ node ] - ); + )); } } } diff --git a/src/validation/rules/UniqueArgumentNames.js b/src/validation/rules/UniqueArgumentNames.js index fd448d7a09..510a474c05 100644 --- a/src/validation/rules/UniqueArgumentNames.js +++ b/src/validation/rules/UniqueArgumentNames.js @@ -8,6 +8,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; @@ -21,7 +22,7 @@ export function duplicateArgMessage(argName: any): string { * A GraphQL field or directive is only valid if all supplied arguments are * uniquely named. */ -export function UniqueArgumentNames(): any { +export function UniqueArgumentNames(context: ValidationContext): any { var knownArgNames = Object.create(null); return { Field() { @@ -33,12 +34,13 @@ export function UniqueArgumentNames(): any { Argument(node) { var argName = node.name.value; if (knownArgNames[argName]) { - return new GraphQLError( + context.reportError(new GraphQLError( duplicateArgMessage(argName), [ knownArgNames[argName], node.name ] - ); + )); + } else { + knownArgNames[argName] = node.name; } - knownArgNames[argName] = node.name; return false; } }; diff --git a/src/validation/rules/UniqueFragmentNames.js b/src/validation/rules/UniqueFragmentNames.js index 467a57dbcb..78ceebc601 100644 --- a/src/validation/rules/UniqueFragmentNames.js +++ b/src/validation/rules/UniqueFragmentNames.js @@ -8,6 +8,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; @@ -20,19 +21,20 @@ export function duplicateFragmentNameMessage(fragName: any): string { * * A GraphQL document is only valid if all defined fragments have unique names. */ -export function UniqueFragmentNames(): any { +export function UniqueFragmentNames(context: ValidationContext): any { var knownFragmentNames = Object.create(null); return { OperationDefinition: () => false, FragmentDefinition(node) { var fragmentName = node.name.value; if (knownFragmentNames[fragmentName]) { - return new GraphQLError( + context.reportError(new GraphQLError( duplicateFragmentNameMessage(fragmentName), [ knownFragmentNames[fragmentName], node.name ] - ); + )); + } else { + knownFragmentNames[fragmentName] = node.name; } - knownFragmentNames[fragmentName] = node.name; return false; } }; diff --git a/src/validation/rules/UniqueInputFieldNames.js b/src/validation/rules/UniqueInputFieldNames.js index a43fa801ea..c3b684b1bc 100644 --- a/src/validation/rules/UniqueInputFieldNames.js +++ b/src/validation/rules/UniqueInputFieldNames.js @@ -8,6 +8,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; @@ -21,7 +22,7 @@ export function duplicateInputFieldMessage(fieldName: any): string { * A GraphQL input object value is only valid if all supplied fields are * uniquely named. */ -export function UniqueInputFieldNames(): any { +export function UniqueInputFieldNames(context: ValidationContext): any { let knownNameStack = []; let knownNames = Object.create(null); @@ -38,12 +39,13 @@ export function UniqueInputFieldNames(): any { ObjectField(node) { var fieldName = node.name.value; if (knownNames[fieldName]) { - return new GraphQLError( + context.reportError(new GraphQLError( duplicateInputFieldMessage(fieldName), [ knownNames[fieldName], node.name ] - ); + )); + } else { + knownNames[fieldName] = node.name; } - knownNames[fieldName] = node.name; return false; } }; diff --git a/src/validation/rules/UniqueOperationNames.js b/src/validation/rules/UniqueOperationNames.js index d1f8b62aa0..763bb6cca4 100644 --- a/src/validation/rules/UniqueOperationNames.js +++ b/src/validation/rules/UniqueOperationNames.js @@ -8,6 +8,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +import type { ValidationContext } from '../index'; import { GraphQLError } from '../../error'; @@ -20,19 +21,20 @@ export function duplicateOperationNameMessage(operationName: any): string { * * A GraphQL document is only valid if all defined operations have unique names. */ -export function UniqueOperationNames(): any { +export function UniqueOperationNames(context: ValidationContext): any { var knownOperationNames = Object.create(null); return { OperationDefinition(node) { var operationName = node.name; if (operationName) { if (knownOperationNames[operationName.value]) { - return new GraphQLError( + context.reportError(new GraphQLError( duplicateOperationNameMessage(operationName.value), [ knownOperationNames[operationName.value], operationName ] - ); + )); + } else { + knownOperationNames[operationName.value] = operationName; } - knownOperationNames[operationName.value] = operationName; } return false; }, diff --git a/src/validation/rules/VariablesAreInputTypes.js b/src/validation/rules/VariablesAreInputTypes.js index 35a08ad989..ad17e930f7 100644 --- a/src/validation/rules/VariablesAreInputTypes.js +++ b/src/validation/rules/VariablesAreInputTypes.js @@ -37,10 +37,10 @@ export function VariablesAreInputTypes(context: ValidationContext): any { // If the variable type is not an input type, return an error. if (type && !isInputType(type)) { var variableName = node.variable.name.value; - return new GraphQLError( + context.reportError(new GraphQLError( nonInputTypeOnVarMessage(variableName, print(node.type)), [ node.type ] - ); + )); } } }; diff --git a/src/validation/rules/VariablesInAllowedPosition.js b/src/validation/rules/VariablesInAllowedPosition.js index 315a8c47b7..367e7a4bf5 100644 --- a/src/validation/rules/VariablesInAllowedPosition.js +++ b/src/validation/rules/VariablesInAllowedPosition.js @@ -36,7 +36,6 @@ export function VariablesInAllowedPosition(context: ValidationContext): any { }, leave(operation) { const usages = context.getRecursiveVariableUsages(operation); - const errors = []; usages.forEach(({ node, type }) => { const varName = node.name.value; @@ -45,16 +44,12 @@ export function VariablesInAllowedPosition(context: ValidationContext): any { varDef && typeFromAST(context.getSchema(), varDef.type); if (varType && type && !varTypeAllowedForType(effectiveType(varType, varDef), type)) { - errors.push(new GraphQLError( + context.reportError(new GraphQLError( badVarPosMessage(varName, varType, type), [ node ] )); } }); - - if (errors.length > 0) { - return errors; - } } }, VariableDefinition(varDefAST) { diff --git a/src/validation/validate.js b/src/validation/validate.js index 132cda1dfd..f7a8c92a86 100644 --- a/src/validation/validate.js +++ b/src/validation/validate.js @@ -79,7 +79,6 @@ export function visitUsingRules( rules: Array ): Array { var context = new ValidationContext(schema, documentAST, typeInfo); - var errors = []; function visitInstance(ast, instance) { visit(ast, { @@ -104,13 +103,6 @@ export function visitUsingRules( var enter = getVisitFn(instance, false, node.kind); result = enter ? enter.apply(instance, arguments) : undefined; - // If the visitor returned an error, log it and do not visit any - // deeper nodes. - if (result && isError(result)) { - append(errors, result); - result = false; - } - // If any validation instances provide the flag `visitSpreadFragments` // and this node is a fragment spread, visit the fragment definition // from this point. @@ -137,13 +129,6 @@ export function visitUsingRules( var leave = getVisitFn(instance, true, node.kind); var result = leave ? leave.apply(instance, arguments) : undefined; - // If the visitor returned an error, log it and do not visit any - // deeper nodes. - if (result && isError(result)) { - append(errors, result); - result = false; - } - // Update typeInfo. typeInfo.leave(node); @@ -158,21 +143,7 @@ export function visitUsingRules( visitInstance(documentAST, instance); }); - return errors; -} - -function isError(value) { - return Array.isArray(value) ? - value.every(item => item instanceof GraphQLError) : - value instanceof GraphQLError; -} - -function append(arr, items) { - if (Array.isArray(items)) { - arr.push.apply(arr, items); - } else { - arr.push(items); - } + return context.getErrors(); } type HasSelectionSet = OperationDefinition | FragmentDefinition; @@ -187,6 +158,7 @@ export class ValidationContext { _schema: GraphQLSchema; _ast: Document; _typeInfo: TypeInfo; + _errors: Array; _fragments: {[name: string]: FragmentDefinition}; _fragmentSpreads: Map>; _recursivelyReferencedFragments: @@ -198,12 +170,21 @@ export class ValidationContext { this._schema = schema; this._ast = ast; this._typeInfo = typeInfo; + this._errors = []; this._fragmentSpreads = new Map(); this._recursivelyReferencedFragments = new Map(); this._variableUsages = new Map(); this._recursiveVariableUsages = new Map(); } + reportError(error: GraphQLError): void { + this._errors.push(error); + } + + getErrors(): Array { + return this._errors; + } + getSchema(): GraphQLSchema { return this._schema; }