Skip to content

Commit 97b9a8f

Browse files
JoviDeCroockJoviDeCroock
committed
Implement validation
1 parent aba2bbb commit 97b9a8f

17 files changed

+832
-104
lines changed

‎src/validation/ValidationContext.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type {
99
FragmentSpreadNode,
1010
OperationDefinitionNode,
1111
SelectionSetNode,
12+
VariableDefinitionNode,
1213
VariableNode,
1314
} from '../language/ast';
1415
import { Kind } from '../language/kinds';
@@ -26,13 +27,15 @@ import type {
2627
import type { GraphQLDirective } from '../type/directives';
2728
import type { GraphQLSchema } from '../type/schema';
2829

29-
import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo';
30+
import type { TypeInfo } from '../utilities/TypeInfo';
31+
import { visitWithTypeInfo } from '../utilities/TypeInfo';
3032

3133
type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode;
3234
interface VariableUsage {
3335
readonly node: VariableNode;
3436
readonly type: Maybe<GraphQLInputType>;
3537
readonly defaultValue: Maybe<unknown>;
38+
readonly fragmentVarDef: Maybe<VariableDefinitionNode>;
3639
}
3740

3841
/**
@@ -199,16 +202,25 @@ export class ValidationContext extends ASTValidationContext {
199202
let usages = this._variableUsages.get(node);
200203
if (!usages) {
201204
const newUsages: Array<VariableUsage> = [];
202-
const typeInfo = new TypeInfo(this._schema);
205+
const typeInfo = this._typeInfo;
206+
const fragmentVariableDefinitions =
207+
node.kind === Kind.FRAGMENT_DEFINITION
208+
? node.variableDefinitions
209+
: undefined;
210+
203211
visit(
204212
node,
205213
visitWithTypeInfo(typeInfo, {
206214
VariableDefinition: () => false,
207215
Variable(variable) {
216+
const fragmentVarDef = fragmentVariableDefinitions?.find(
217+
(varDef) => varDef.variable.name.value === variable.name.value,
218+
);
208219
newUsages.push({
209220
node: variable,
210221
type: typeInfo.getInputType(),
211222
defaultValue: typeInfo.getDefaultValue(),
223+
fragmentVarDef,
212224
});
213225
},
214226
}),
@@ -233,6 +245,20 @@ export class ValidationContext extends ASTValidationContext {
233245
return usages;
234246
}
235247

248+
getOperationVariableUsages(
249+
operation: OperationDefinitionNode,
250+
): ReadonlyArray<VariableUsage> {
251+
let usages = this._recursiveVariableUsages.get(operation);
252+
if (!usages) {
253+
usages = this.getVariableUsages(operation);
254+
for (const frag of this.getRecursivelyReferencedFragments(operation)) {
255+
usages = usages.concat(this.getVariableUsages(frag));
256+
}
257+
this._recursiveVariableUsages.set(operation, usages);
258+
}
259+
return usages.filter(({ fragmentVarDef }) => !fragmentVarDef);
260+
}
261+
236262
getType(): Maybe<GraphQLOutputType> {
237263
return this._typeInfo.getType();
238264
}

‎src/validation/__tests__/NoUndefinedVariablesRule-test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,4 +404,67 @@ describe('Validate: No undefined variables', () => {
404404
},
405405
]);
406406
});
407+
408+
it('fragment defined arguments are not undefined variables', () => {
409+
expectValid(`
410+
query Foo {
411+
...FragA
412+
}
413+
fragment FragA($a: String) on Type {
414+
field1(a: $a)
415+
}
416+
`);
417+
});
418+
419+
it('defined variables used as fragment arguments are not undefined variables', () => {
420+
expectValid(`
421+
query Foo($b: String) {
422+
...FragA(a: $b)
423+
}
424+
fragment FragA($a: String) on Type {
425+
field1
426+
}
427+
`);
428+
});
429+
430+
it('variables used as fragment arguments may be undefined variables', () => {
431+
expectErrors(`
432+
query Foo {
433+
...FragA(a: $a)
434+
}
435+
fragment FragA($a: String) on Type {
436+
field1
437+
}
438+
`).toDeepEqual([
439+
{
440+
message: 'Variable "$a" is not defined by operation "Foo".',
441+
locations: [
442+
{ line: 3, column: 21 },
443+
{ line: 2, column: 7 },
444+
],
445+
},
446+
]);
447+
});
448+
449+
it('variables shadowed by parent fragment arguments are still undefined variables', () => {
450+
expectErrors(`
451+
query Foo {
452+
...FragA
453+
}
454+
fragment FragA($a: String) on Type {
455+
...FragB
456+
}
457+
fragment FragB on Type {
458+
field1(a: $a)
459+
}
460+
`).toDeepEqual([
461+
{
462+
message: 'Variable "$a" is not defined by operation "Foo".',
463+
locations: [
464+
{ line: 9, column: 19 },
465+
{ line: 2, column: 7 },
466+
],
467+
},
468+
]);
469+
});
407470
});

‎src/validation/__tests__/NoUnusedFragmentVariablesRule-test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, it } from 'mocha';
2+
3+
import { NoUnusedFragmentVariablesRule } from '../rules/NoUnusedFragmentVariablesRule';
4+
5+
import { expectValidationErrors } from './harness';
6+
7+
function expectErrors(queryStr: string) {
8+
return expectValidationErrors(NoUnusedFragmentVariablesRule, queryStr);
9+
}
10+
11+
function expectValid(queryStr: string) {
12+
expectErrors(queryStr).toDeepEqual([]);
13+
}
14+
15+
describe('Validate: No unused variables', () => {
16+
it('fragment defined arguments are not unused variables', () => {
17+
expectValid(`
18+
query Foo {
19+
...FragA
20+
}
21+
fragment FragA($a: String) on Type {
22+
field1(a: $a)
23+
}
24+
`);
25+
});
26+
27+
it('defined variables used as fragment arguments are not unused variables', () => {
28+
expectErrors(`
29+
query Foo($b: String) {
30+
...FragA(a: $b)
31+
}
32+
fragment FragA($a: String) on Type {
33+
field1
34+
}
35+
`);
36+
});
37+
});

‎src/validation/__tests__/NoUnusedVariablesRule-test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,26 @@ describe('Validate: No unused variables', () => {
230230
},
231231
]);
232232
});
233+
234+
it('fragment defined arguments are not unused variables', () => {
235+
expectValid(`
236+
query Foo {
237+
...FragA
238+
}
239+
fragment FragA($a: String) on Type {
240+
field1(a: $a)
241+
}
242+
`);
243+
});
244+
245+
it('defined variables used as fragment arguments are not unused variables', () => {
246+
expectValid(`
247+
query Foo($b: String) {
248+
...FragA(a: $b)
249+
}
250+
fragment FragA($a: String) on Type {
251+
field1
252+
}
253+
`);
254+
});
233255
});

‎src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,4 +1138,136 @@ describe('Validate: Overlapping fields can be merged', () => {
11381138
},
11391139
]);
11401140
});
1141+
1142+
describe('fragment arguments must produce fields that can be merged', () => {
1143+
it('allows conflicting spreads at different depths', () => {
1144+
expectValid(`
1145+
query ValidDifferingFragmentArgs($command1: DogCommand, $command2: DogCommand) {
1146+
dog {
1147+
...DoesKnowCommand(command: $command1)
1148+
mother {
1149+
...DoesKnowCommand(command: $command2)
1150+
}
1151+
}
1152+
}
1153+
fragment DoesKnowCommand($command: DogCommand) on Dog {
1154+
doesKnowCommand(dogCommand: $command)
1155+
}
1156+
`);
1157+
});
1158+
1159+
it('encounters conflict in fragments', () => {
1160+
expectErrors(`
1161+
{
1162+
...WithArgs(x: 3)
1163+
...WithArgs(x: 4)
1164+
}
1165+
fragment WithArgs($x: Int) on Type {
1166+
a(x: $x)
1167+
}
1168+
`).toDeepEqual([
1169+
{
1170+
message:
1171+
'Spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments.',
1172+
locations: [
1173+
{ line: 3, column: 11 },
1174+
{ line: 4, column: 11 },
1175+
],
1176+
},
1177+
]);
1178+
});
1179+
1180+
it('encounters conflict in fragment - field no args', () => {
1181+
expectErrors(`
1182+
query ($y: Int = 1) {
1183+
a(x: $y)
1184+
...WithArgs
1185+
}
1186+
fragment WithArgs on Type {
1187+
a(x: $y)
1188+
}
1189+
`).toDeepEqual([]);
1190+
});
1191+
1192+
it('encounters conflict in fragment/field', () => {
1193+
expectErrors(`
1194+
query ($y: Int = 1) {
1195+
a(x: $y)
1196+
...WithArgs(x: $y)
1197+
}
1198+
fragment WithArgs($x: Int) on Type {
1199+
a(x: $x)
1200+
}
1201+
`).toDeepEqual([]);
1202+
});
1203+
1204+
// This is currently not validated, should we?
1205+
it('encounters nested field conflict in fragments that could otherwise merge', () => {
1206+
expectErrors(`
1207+
query ValidDifferingFragmentArgs($command1: DogCommand, $command2: DogCommand) {
1208+
dog {
1209+
...DoesKnowCommandNested(command: $command1)
1210+
mother {
1211+
...DoesKnowCommandNested(command: $command2)
1212+
}
1213+
}
1214+
}
1215+
fragment DoesKnowCommandNested($command: DogCommand) on Dog {
1216+
doesKnowCommand(dogCommand: $command)
1217+
mother {
1218+
doesKnowCommand(dogCommand: $command)
1219+
}
1220+
}
1221+
`).toDeepEqual([
1222+
{
1223+
message:
1224+
'Fields "mother" conflict because subfields "doesKnowCommand" conflict because they have differing arguments. Use different aliases on the fields to fetch both if this was intentional.',
1225+
locations: [
1226+
{ line: 5, column: 13 },
1227+
{ line: 13, column: 13 },
1228+
{ line: 12, column: 11 },
1229+
{ line: 11, column: 11 },
1230+
],
1231+
},
1232+
]);
1233+
});
1234+
1235+
it('encounters nested conflict in fragments', () => {
1236+
expectErrors(`
1237+
{
1238+
connection {
1239+
edges {
1240+
...WithArgs(x: 3)
1241+
}
1242+
}
1243+
...Connection
1244+
}
1245+
fragment Connection on Type {
1246+
connection {
1247+
edges {
1248+
...WithArgs(x: 4)
1249+
}
1250+
}
1251+
}
1252+
fragment WithArgs($x: Int) on Type {
1253+
a(x: $x)
1254+
}
1255+
`).toDeepEqual([
1256+
{
1257+
message:
1258+
'Spreads "WithArgs" conflict because WithArgs(x: 3) and WithArgs(x: 4) have different fragment arguments.',
1259+
locations: [
1260+
{
1261+
column: 15,
1262+
line: 5,
1263+
},
1264+
{
1265+
column: 15,
1266+
line: 13,
1267+
},
1268+
],
1269+
},
1270+
]);
1271+
});
1272+
});
11411273
});

0 commit comments

Comments
 (0)