Skip to content

Commit 32107eb

Browse files
Fragment arguments validation (#5)
* Fragment args validation * add experimental substitution * validation suggestions (#12) * validation suggestions * remove OperationSignature from ValidationContext only used in one rule, does not need to be on context remove dependency on getVariableSignature move FRAGMENT_ARGUMENT below ARGUMENT fragment arguments do not have location defaults only variable defaults => so getDefaultValue(), which returns location defaults for use with the allowedVariableUsage helper, should never be called by getVariableUsages with respect to fragment arguments fragment arguments therefore need not add anything to the default value stack reduce diff from main these diffs crept in before Kind.FRAGMENT_ARGUMENT was separated out --------- Co-authored-by: Yaacov Rydzinski <[email protected]>
1 parent 2fb5f1d commit 32107eb

21 files changed

+1051
-170
lines changed

src/utilities/TypeInfo.ts

Lines changed: 83 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ import type { Maybe } from '../jsutils/Maybe.js';
22

33
import type {
44
ASTNode,
5+
DocumentNode,
56
FieldNode,
67
FragmentDefinitionNode,
7-
FragmentSpreadNode,
8+
VariableDefinitionNode,
89
} from '../language/ast.js';
910
import { isNode } from '../language/ast.js';
1011
import { Kind } from '../language/kinds.js';
@@ -36,7 +37,11 @@ import type { GraphQLDirective } from '../type/directives.js';
3637
import type { GraphQLSchema } from '../type/schema.js';
3738

3839
import { typeFromAST } from './typeFromAST.js';
39-
import { valueFromAST } from './valueFromAST.js';
40+
41+
export interface FragmentSignature {
42+
readonly definition: FragmentDefinitionNode;
43+
readonly variableDefinitions: Map<string, VariableDefinitionNode>;
44+
}
4045

4146
/**
4247
* TypeInfo is a utility class which, given a GraphQL schema, can keep track
@@ -53,8 +58,12 @@ export class TypeInfo {
5358
private _directive: Maybe<GraphQLDirective>;
5459
private _argument: Maybe<GraphQLArgument>;
5560
private _enumValue: Maybe<GraphQLEnumValue>;
56-
private _fragmentSpread: Maybe<FragmentSpreadNode>;
57-
private _fragmentDefinitions: Map<string, FragmentDefinitionNode>;
61+
private _fragmentSignaturesByName: (
62+
fragmentName: string,
63+
) => Maybe<FragmentSignature>;
64+
65+
private _fragmentSignature: Maybe<FragmentSignature>;
66+
private _fragmentArgument: Maybe<VariableDefinitionNode>;
5867
private _getFieldDef: GetFieldDefFn;
5968

6069
constructor(
@@ -66,7 +75,10 @@ export class TypeInfo {
6675
initialType?: Maybe<GraphQLType>,
6776

6877
/** @deprecated will be removed in 17.0.0 */
69-
getFieldDefFn?: GetFieldDefFn,
78+
getFieldDefFn?: Maybe<GetFieldDefFn>,
79+
fragmentSignatures?: Maybe<
80+
(fragmentName: string) => Maybe<FragmentSignature>
81+
>,
7082
) {
7183
this._schema = schema;
7284
this._typeStack = [];
@@ -77,8 +89,9 @@ export class TypeInfo {
7789
this._directive = null;
7890
this._argument = null;
7991
this._enumValue = null;
80-
this._fragmentSpread = null;
81-
this._fragmentDefinitions = new Map();
92+
this._fragmentSignaturesByName = fragmentSignatures ?? (() => null);
93+
this._fragmentSignature = null;
94+
this._fragmentArgument = null;
8295
this._getFieldDef = getFieldDefFn ?? getFieldDef;
8396
if (initialType) {
8497
if (isInputType(initialType)) {
@@ -129,6 +142,20 @@ export class TypeInfo {
129142
return this._argument;
130143
}
131144

145+
getFragmentSignature(): Maybe<FragmentSignature> {
146+
return this._fragmentSignature;
147+
}
148+
149+
getFragmentSignatureByName(): (
150+
fragmentName: string,
151+
) => Maybe<FragmentSignature> {
152+
return this._fragmentSignaturesByName;
153+
}
154+
155+
getFragmentArgument(): Maybe<VariableDefinitionNode> {
156+
return this._fragmentArgument;
157+
}
158+
132159
getEnumValue(): Maybe<GraphQLEnumValue> {
133160
return this._enumValue;
134161
}
@@ -141,14 +168,9 @@ export class TypeInfo {
141168
// which occurs before guarantees of schema and document validity.
142169
switch (node.kind) {
143170
case Kind.DOCUMENT: {
144-
// A document's fragment definitions are type signatures
145-
// referenced via fragment spreads. Ensure we can use definitions
146-
// before visiting their call sites.
147-
for (const astNode of node.definitions) {
148-
if (astNode.kind === Kind.FRAGMENT_DEFINITION) {
149-
this._fragmentDefinitions.set(astNode.name.value, astNode);
150-
}
151-
}
171+
const fragmentSignatures = getFragmentSignatures(node);
172+
this._fragmentSignaturesByName = (fragmentName: string) =>
173+
fragmentSignatures.get(fragmentName);
152174
break;
153175
}
154176
case Kind.SELECTION_SET: {
@@ -181,7 +203,9 @@ export class TypeInfo {
181203
break;
182204
}
183205
case Kind.FRAGMENT_SPREAD: {
184-
this._fragmentSpread = node;
206+
this._fragmentSignature = this.getFragmentSignatureByName()(
207+
node.name.value,
208+
);
185209
break;
186210
}
187211
case Kind.INLINE_FRAGMENT:
@@ -200,69 +224,33 @@ export class TypeInfo {
200224
);
201225
break;
202226
}
203-
case Kind.FRAGMENT_ARGUMENT: {
227+
case Kind.ARGUMENT: {
204228
let argDef;
205229
let argType: unknown;
206-
const fragmentSpread = this._fragmentSpread;
207-
208-
const fragmentDef = this._fragmentDefinitions.get(
209-
fragmentSpread!.name.value,
210-
);
211-
const fragVarDef = fragmentDef?.variableDefinitions?.find(
212-
(varDef) => varDef.variable.name.value === node.name.value,
213-
);
214-
if (fragVarDef) {
215-
const fragVarType = typeFromAST(schema, fragVarDef.type);
216-
if (isInputType(fragVarType)) {
217-
const fragVarDefault = fragVarDef.defaultValue
218-
? valueFromAST(fragVarDef.defaultValue, fragVarType)
219-
: undefined;
220-
221-
// Minor hack: transform the FragmentArgDef
222-
// into a schema Argument definition, to
223-
// enable visiting identically to field/directive args
224-
const schemaArgDef: GraphQLArgument = {
225-
name: fragVarDef.variable.name.value,
226-
type: fragVarType,
227-
defaultValue: fragVarDefault,
228-
description: undefined,
229-
deprecationReason: undefined,
230-
extensions: {},
231-
astNode: {
232-
...fragVarDef,
233-
kind: Kind.INPUT_VALUE_DEFINITION,
234-
name: fragVarDef.variable.name,
235-
},
236-
};
237-
argDef = schemaArgDef;
230+
const fieldOrDirective = this.getDirective() ?? this.getFieldDef();
231+
if (fieldOrDirective) {
232+
argDef = fieldOrDirective.args.find(
233+
(arg) => arg.name === node.name.value,
234+
);
235+
if (argDef) {
236+
argType = argDef.type;
238237
}
239238
}
240-
241-
if (argDef) {
242-
argType = argDef.type;
243-
}
244-
245239
this._argument = argDef;
246240
this._defaultValueStack.push(argDef ? argDef.defaultValue : undefined);
247241
this._inputTypeStack.push(isInputType(argType) ? argType : undefined);
248242
break;
249243
}
250-
case Kind.ARGUMENT: {
251-
let argDef;
244+
case Kind.FRAGMENT_ARGUMENT: {
245+
const fragmentSignature = this.getFragmentSignature();
246+
const argDef = fragmentSignature?.variableDefinitions.get(
247+
node.name.value,
248+
);
249+
this._fragmentArgument = argDef;
252250
let argType: unknown;
253-
const directive = this.getDirective();
254-
const fieldDef = this.getFieldDef();
255-
if (directive) {
256-
argDef = directive.args.find((arg) => arg.name === node.name.value);
257-
} else if (fieldDef) {
258-
argDef = fieldDef.args.find((arg) => arg.name === node.name.value);
259-
}
260251
if (argDef) {
261-
argType = argDef.type;
252+
argType = typeFromAST(this._schema, argDef.type);
262253
}
263-
264-
this._argument = argDef;
265-
this._defaultValueStack.push(argDef ? argDef.defaultValue : undefined);
266254
this._inputTypeStack.push(isInputType(argType) ? argType : undefined);
267255
break;
268256
}
@@ -311,7 +299,8 @@ export class TypeInfo {
311299
leave(node: ASTNode) {
312300
switch (node.kind) {
313301
case Kind.DOCUMENT:
314-
this._fragmentDefinitions = new Map();
302+
this._fragmentSignaturesByName = /* c8 ignore start */ () =>
303+
null /* c8 ignore end */;
315304
break;
316305
case Kind.SELECTION_SET:
317306
this._parentTypeStack.pop();
@@ -324,7 +313,7 @@ export class TypeInfo {
324313
this._directive = null;
325314
break;
326315
case Kind.FRAGMENT_SPREAD:
327-
this._fragmentSpread = null;
316+
this._fragmentSignature = null;
328317
break;
329318
case Kind.OPERATION_DEFINITION:
330319
case Kind.INLINE_FRAGMENT:
@@ -334,12 +323,17 @@ export class TypeInfo {
334323
case Kind.VARIABLE_DEFINITION:
335324
this._inputTypeStack.pop();
336325
break;
337-
case Kind.FRAGMENT_ARGUMENT:
338326
case Kind.ARGUMENT:
339327
this._argument = null;
340328
this._defaultValueStack.pop();
341329
this._inputTypeStack.pop();
342330
break;
331+
case Kind.FRAGMENT_ARGUMENT: {
332+
this._fragmentArgument = null;
333+
this._defaultValueStack.pop();
334+
this._inputTypeStack.pop();
335+
break;
336+
}
343337
case Kind.LIST:
344338
case Kind.OBJECT_FIELD:
345339
this._defaultValueStack.pop();
@@ -368,6 +362,25 @@ function getFieldDef(
368362
return schema.getField(parentType, fieldNode.name.value);
369363
}
370364

365+
function getFragmentSignatures(
366+
document: DocumentNode,
367+
): Map<string, FragmentSignature> {
368+
const fragmentSignatures = new Map<string, FragmentSignature>();
369+
for (const definition of document.definitions) {
370+
if (definition.kind === Kind.FRAGMENT_DEFINITION) {
371+
const variableDefinitions = new Map<string, VariableDefinitionNode>();
372+
if (definition.variableDefinitions) {
373+
for (const varDef of definition.variableDefinitions) {
374+
variableDefinitions.set(varDef.variable.name.value, varDef);
375+
}
376+
}
377+
const signature = { definition, variableDefinitions };
378+
fragmentSignatures.set(definition.name.value, signature);
379+
}
380+
}
381+
return fragmentSignatures;
382+
}
383+
371384
/**
372385
* Creates a new visitor instance which maintains a provided TypeInfo instance
373386
* along with visiting visitor.

src/utilities/__tests__/TypeInfo-test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ describe('TypeInfo', () => {
6868
expect(typeInfo.getFieldDef()).to.equal(undefined);
6969
expect(typeInfo.getDefaultValue()).to.equal(undefined);
7070
expect(typeInfo.getDirective()).to.equal(null);
71+
expect(typeInfo.getFragmentSignature()).to.equal(null);
72+
expect(typeInfo.getFragmentSignatureByName()('')).to.equal(null);
73+
expect(typeInfo.getFragmentArgument()).to.equal(null);
7174
expect(typeInfo.getArgument()).to.equal(null);
7275
expect(typeInfo.getEnumValue()).to.equal(null);
7376
});

src/validation/ValidationContext.ts

Lines changed: 37 additions & 6 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.js';
1415
import { Kind } from '../language/kinds.js';
@@ -26,13 +27,15 @@ import type {
2627
import type { GraphQLDirective } from '../type/directives.js';
2728
import type { GraphQLSchema } from '../type/schema.js';
2829

30+
import type { FragmentSignature } from '../utilities/TypeInfo.js';
2931
import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js';
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 fragmentVariableDefinition: Maybe<VariableDefinitionNode>;
3639
}
3740

3841
/**
@@ -199,17 +202,41 @@ 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 = new TypeInfo(
206+
this._schema,
207+
undefined,
208+
undefined,
209+
this._typeInfo.getFragmentSignatureByName(),
210+
);
211+
const fragmentDefinition =
212+
node.kind === Kind.FRAGMENT_DEFINITION ? node : undefined;
203213
visit(
204214
node,
205215
visitWithTypeInfo(typeInfo, {
206216
VariableDefinition: () => false,
207217
Variable(variable) {
208-
newUsages.push({
209-
node: variable,
210-
type: typeInfo.getInputType(),
211-
defaultValue: typeInfo.getDefaultValue(),
212-
});
218+
let fragmentVariableDefinition;
219+
if (fragmentDefinition) {
220+
const fragmentSignature = typeInfo.getFragmentSignatureByName()(
221+
fragmentDefinition.name.value,
222+
);
223+
224+
fragmentVariableDefinition =
225+
fragmentSignature?.variableDefinitions.get(variable.name.value);
226+
newUsages.push({
227+
node: variable,
228+
type: typeInfo.getInputType(),
229+
defaultValue: undefined, // fragment variables have a variable default but no location default, which is what this default value represents
230+
fragmentVariableDefinition,
231+
});
232+
} else {
233+
newUsages.push({
234+
node: variable,
235+
type: typeInfo.getInputType(),
236+
defaultValue: typeInfo.getDefaultValue(),
237+
fragmentVariableDefinition: undefined,
238+
});
239+
}
213240
},
214241
}),
215242
);
@@ -261,6 +288,10 @@ export class ValidationContext extends ASTValidationContext {
261288
return this._typeInfo.getArgument();
262289
}
263290

291+
getFragmentSignature(): Maybe<FragmentSignature> {
292+
return this._typeInfo.getFragmentSignature();
293+
}
294+
264295
getEnumValue(): Maybe<GraphQLEnumValue> {
265296
return this._typeInfo.getEnumValue();
266297
}

0 commit comments

Comments
 (0)