Skip to content

Commit 5e545cc

Browse files
leebyronleebyron
committed
[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.
1 parent ef7c755 commit 5e545cc

26 files changed

+156
-173
lines changed

‎src/validation/__tests__/FieldsOnCorrectType.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,20 @@ describe('Validate: Fields on correct type', () => {
7575
`);
7676
});
7777

78+
it('reports errors when type is known again', () => {
79+
expectFailsRule(FieldsOnCorrectType, `
80+
fragment typeKnownAgain on Pet {
81+
unknown_pet_field {
82+
... on Cat {
83+
unknown_cat_field
84+
}
85+
}
86+
}`,
87+
[ undefinedField('unknown_pet_field', 'Pet', 3, 9),
88+
undefinedField('unknown_cat_field', 'Cat', 5, 13) ]
89+
);
90+
});
91+
7892
it('Field not defined on fragment', () => {
7993
expectFailsRule(FieldsOnCorrectType, `
8094
fragment fieldNotDefined on Dog {
@@ -84,7 +98,7 @@ describe('Validate: Fields on correct type', () => {
8498
);
8599
});
86100

87-
it('Field not defined deeply, only reports first', () => {
101+
it('Ignores deeply unknown field', () => {
88102
expectFailsRule(FieldsOnCorrectType, `
89103
fragment deepFieldNotDefined on Dog {
90104
unknown_field {

‎src/validation/__tests__/validation.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ describe('Validate: Supports full validation', () => {
6565
);
6666

6767
expect(errors).to.deep.equal([
68-
{ message: 'Cannot query field "catOrDog" on "QueryRoot".' }
68+
{ message: 'Cannot query field "catOrDog" on "QueryRoot".' },
69+
{ message: 'Cannot query field "furColor" on "Cat".' },
70+
{ message: 'Cannot query field "isHousetrained" on "Dog".' }
6971
]);
7072
});
7173

‎src/validation/rules/ArgumentsOfCorrectType.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ export function ArgumentsOfCorrectType(context: ValidationContext): any {
2929
Argument(argAST) {
3030
var argDef = context.getArgument();
3131
if (argDef && !isValidLiteralValue(argDef.type, argAST.value)) {
32-
return new GraphQLError(
32+
context.reportError(new GraphQLError(
3333
badValueMessage(
3434
argAST.name.value,
3535
argDef.type,
3636
print(argAST.value)
3737
),
3838
[ argAST.value ]
39-
);
39+
));
4040
}
4141
return false;
4242
}

‎src/validation/rules/DefaultValuesOfCorrectType.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,16 @@ export function DefaultValuesOfCorrectType(context: ValidationContext): any {
4646
var defaultValue = varDefAST.defaultValue;
4747
var type = context.getInputType();
4848
if (type instanceof GraphQLNonNull && defaultValue) {
49-
return new GraphQLError(
49+
context.reportError(new GraphQLError(
5050
defaultForNonNullArgMessage(name, type, type.ofType),
5151
[ defaultValue ]
52-
);
52+
));
5353
}
5454
if (type && defaultValue && !isValidLiteralValue(type, defaultValue)) {
55-
return new GraphQLError(
55+
context.reportError(new GraphQLError(
5656
badValueForDefaultArgMessage(name, type, print(defaultValue)),
5757
[ defaultValue ]
58-
);
58+
));
5959
}
6060
return false;
6161
},

‎src/validation/rules/FieldsOnCorrectType.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ export function FieldsOnCorrectType(context: ValidationContext): any {
3030
if (type) {
3131
var fieldDef = context.getFieldDef();
3232
if (!fieldDef) {
33-
return new GraphQLError(
33+
context.reportError(new GraphQLError(
3434
undefinedFieldMessage(node.name.value, type.name),
3535
[ node ]
36-
);
36+
));
3737
}
3838
}
3939
}

‎src/validation/rules/FragmentsOnCompositeTypes.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,22 @@ export function FragmentsOnCompositeTypes(context: ValidationContext): any {
4040
InlineFragment(node) {
4141
var type = context.getType();
4242
if (node.typeCondition && type && !isCompositeType(type)) {
43-
return new GraphQLError(
43+
context.reportError(new GraphQLError(
4444
inlineFragmentOnNonCompositeErrorMessage(print(node.typeCondition)),
4545
[ node.typeCondition ]
46-
);
46+
));
4747
}
4848
},
4949
FragmentDefinition(node) {
5050
var type = context.getType();
5151
if (type && !isCompositeType(type)) {
52-
return new GraphQLError(
52+
context.reportError(new GraphQLError(
5353
fragmentOnNonCompositeErrorMessage(
5454
node.name.value,
5555
print(node.typeCondition)
5656
),
5757
[ node.typeCondition ]
58-
);
58+
));
5959
}
6060
}
6161
};

‎src/validation/rules/KnownArgumentNames.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,14 @@ export function KnownArgumentNames(context: ValidationContext): any {
5454
if (!fieldArgDef) {
5555
var parentType = context.getParentType();
5656
invariant(parentType);
57-
return new GraphQLError(
57+
context.reportError(new GraphQLError(
5858
unknownArgMessage(
5959
node.name.value,
6060
fieldDef.name,
6161
parentType.name
6262
),
6363
[ node ]
64-
);
64+
));
6565
}
6666
}
6767
} else if (argumentOf.kind === DIRECTIVE) {
@@ -72,10 +72,10 @@ export function KnownArgumentNames(context: ValidationContext): any {
7272
arg => arg.name === node.name.value
7373
);
7474
if (!directiveArgDef) {
75-
return new GraphQLError(
75+
context.reportError(new GraphQLError(
7676
unknownDirectiveArgMessage(node.name.value, directive.name),
7777
[ node ]
78-
);
78+
));
7979
}
8080
}
8181
}

‎src/validation/rules/KnownDirectives.js

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -45,33 +45,40 @@ export function KnownDirectives(context: ValidationContext): any {
4545
def => def.name === node.name.value
4646
);
4747
if (!directiveDef) {
48-
return new GraphQLError(
48+
context.reportError(new GraphQLError(
4949
unknownDirectiveMessage(node.name.value),
5050
[ node ]
51-
);
51+
));
52+
return;
5253
}
53-
var appliedTo = ancestors[ancestors.length - 1];
54-
if (appliedTo.kind === OPERATION_DEFINITION &&
55-
!directiveDef.onOperation) {
56-
return new GraphQLError(
57-
misplacedDirectiveMessage(node.name.value, 'operation'),
58-
[ node ]
59-
);
60-
}
61-
if (appliedTo.kind === FIELD && !directiveDef.onField) {
62-
return new GraphQLError(
63-
misplacedDirectiveMessage(node.name.value, 'field'),
64-
[ node ]
65-
);
66-
}
67-
if ((appliedTo.kind === FRAGMENT_SPREAD ||
68-
appliedTo.kind === INLINE_FRAGMENT ||
69-
appliedTo.kind === FRAGMENT_DEFINITION) &&
70-
!directiveDef.onFragment) {
71-
return new GraphQLError(
72-
misplacedDirectiveMessage(node.name.value, 'fragment'),
73-
[ node ]
74-
);
54+
const appliedTo = ancestors[ancestors.length - 1];
55+
switch (appliedTo.kind) {
56+
case OPERATION_DEFINITION:
57+
if (!directiveDef.onOperation) {
58+
context.reportError(new GraphQLError(
59+
misplacedDirectiveMessage(node.name.value, 'operation'),
60+
[ node ]
61+
));
62+
}
63+
break;
64+
case FIELD:
65+
if (!directiveDef.onField) {
66+
context.reportError(new GraphQLError(
67+
misplacedDirectiveMessage(node.name.value, 'field'),
68+
[ node ]
69+
));
70+
}
71+
break;
72+
case FRAGMENT_SPREAD:
73+
case INLINE_FRAGMENT:
74+
case FRAGMENT_DEFINITION:
75+
if (!directiveDef.onFragment) {
76+
context.reportError(new GraphQLError(
77+
misplacedDirectiveMessage(node.name.value, 'fragment'),
78+
[ node ]
79+
));
80+
}
81+
break;
7582
}
7683
}
7784
};

‎src/validation/rules/KnownFragmentNames.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ export function KnownFragmentNames(context: ValidationContext): any {
2828
var fragmentName = node.name.value;
2929
var fragment = context.getFragment(fragmentName);
3030
if (!fragment) {
31-
return new GraphQLError(
31+
context.reportError(new GraphQLError(
3232
unknownFragmentMessage(fragmentName),
3333
[ node.name ]
34-
);
34+
));
3535
}
3636
}
3737
};

‎src/validation/rules/KnownTypeNames.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export function KnownTypeNames(context: ValidationContext): any {
2828
var typeName = node.name.value;
2929
var type = context.getSchema().getType(typeName);
3030
if (!type) {
31-
return new GraphQLError(unknownTypeMessage(typeName), [ node ]);
31+
context.reportError(
32+
new GraphQLError(unknownTypeMessage(typeName), [ node ])
33+
);
3234
}
3335
}
3436
};

‎src/validation/rules/LoneAnonymousOperation.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* of patent rights can be found in the PATENTS file in the same directory.
99
*/
1010

11+
import type { ValidationContext } from '../index';
1112
import { GraphQLError } from '../../error';
1213
import { OPERATION_DEFINITION } from '../../language/kinds';
1314

@@ -22,7 +23,7 @@ export function anonOperationNotAloneMessage(): string {
2223
* A GraphQL document is only valid if when it contains an anonymous operation
2324
* (the query short-hand) that it contains only that one operation definition.
2425
*/
25-
export function LoneAnonymousOperation(): any {
26+
export function LoneAnonymousOperation(context: ValidationContext): any {
2627
var operationCount = 0;
2728
return {
2829
Document(node) {
@@ -32,7 +33,9 @@ export function LoneAnonymousOperation(): any {
3233
},
3334
OperationDefinition(node) {
3435
if (!node.name && operationCount > 1) {
35-
return new GraphQLError(anonOperationNotAloneMessage(), [ node ]);
36+
context.reportError(
37+
new GraphQLError(anonOperationNotAloneMessage(), [ node ])
38+
);
3639
}
3740
}
3841
};

‎src/validation/rules/NoFragmentCycles.js

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ export function cycleErrorMessage(
2222
}
2323

2424
export function NoFragmentCycles(context: ValidationContext): any {
25-
var errors = [];
26-
2725
// Tracks already visited fragments to maintain O(N) and to ensure that cycles
2826
// are not redundantly reported.
2927
var visitedFrags = Object.create(null);
@@ -35,13 +33,6 @@ export function NoFragmentCycles(context: ValidationContext): any {
3533
var spreadPathIndexByName = Object.create(null);
3634

3735
return {
38-
Document: {
39-
leave() {
40-
if (errors.length) {
41-
return errors;
42-
}
43-
}
44-
},
4536
OperationDefinition: () => false,
4637
FragmentDefinition(node) {
4738
if (!visitedFrags[node.name.value]) {
@@ -81,7 +72,7 @@ export function NoFragmentCycles(context: ValidationContext): any {
8172
spreadPath.pop();
8273
} else {
8374
const cyclePath = spreadPath.slice(cycleIndex);
84-
errors.push(new GraphQLError(
75+
context.reportError(new GraphQLError(
8576
cycleErrorMessage(
8677
spreadName,
8778
cyclePath.map(s => s.name.value)

‎src/validation/rules/NoUndefinedVariables.js

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,26 +34,19 @@ export function NoUndefinedVariables(context: ValidationContext): any {
3434
},
3535
leave(operation) {
3636
const usages = context.getRecursiveVariableUsages(operation);
37-
const errors = [];
3837

3938
usages.forEach(({ node }) => {
4039
var varName = node.name.value;
4140
if (variableNameDefined[varName] !== true) {
42-
errors.push(
43-
new GraphQLError(
44-
undefinedVarMessage(
45-
varName,
46-
operation.name && operation.name.value
47-
),
48-
[ node, operation ]
49-
)
50-
);
41+
context.reportError(new GraphQLError(
42+
undefinedVarMessage(
43+
varName,
44+
operation.name && operation.name.value
45+
),
46+
[ node, operation ]
47+
));
5148
}
5249
});
53-
54-
if (errors.length > 0) {
55-
return errors;
56-
}
5750
}
5851
},
5952
VariableDefinition(varDefAST) {

‎src/validation/rules/NoUnusedFragments.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@ export function NoUnusedFragments(context: ValidationContext): any {
4444
);
4545
});
4646

47-
var errors = fragmentDefs
48-
.filter(def => fragmentNameUsed[def.name.value] !== true)
49-
.map(def => new GraphQLError(
50-
unusedFragMessage(def.name.value),
51-
[ def ]
52-
));
53-
if (errors.length > 0) {
54-
return errors;
55-
}
47+
fragmentDefs.forEach(fragmentDef => {
48+
const fragName = fragmentDef.name.value;
49+
if (fragmentNameUsed[fragName] !== true) {
50+
context.reportError(new GraphQLError(
51+
unusedFragMessage(fragName),
52+
[ fragmentDef ]
53+
));
54+
}
55+
});
5656
}
5757
}
5858
};

‎src/validation/rules/NoUnusedVariables.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,22 @@ export function NoUnusedVariables(context: ValidationContext): any {
3131
variableDefs = [];
3232
},
3333
leave(operation) {
34-
3534
const variableNameUsed = Object.create(null);
3635
const usages = context.getRecursiveVariableUsages(operation);
3736

3837
usages.forEach(({ node }) => {
3938
variableNameUsed[node.name.value] = true;
4039
});
4140

42-
var errors = variableDefs
43-
.filter(def => variableNameUsed[def.variable.name.value] !== true)
44-
.map(def => new GraphQLError(
45-
unusedVariableMessage(def.variable.name.value),
46-
[ def ]
47-
));
48-
if (errors.length > 0) {
49-
return errors;
50-
}
41+
variableDefs.forEach(variableDef => {
42+
const variableName = variableDef.variable.name.value;
43+
if (variableNameUsed[variableName] !== true) {
44+
context.reportError(new GraphQLError(
45+
unusedVariableMessage(variableName),
46+
[ variableDef ]
47+
));
48+
}
49+
});
5150
}
5251
},
5352
VariableDefinition(def) {

0 commit comments

Comments
 (0)