Skip to content

Commit 4631744

Browse files
IvanGoncharovIvanGoncharov
authored
Validate directive arguments inside SDL (#1463)
1 parent be42622 commit 4631744

6 files changed

+321
-63
lines changed

‎src/validation/__tests__/KnownArgumentNames-test.js

+97-1
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,24 @@
66
*/
77

88
import { describe, it } from 'mocha';
9-
import { expectPassesRule, expectFailsRule } from './harness';
9+
import { buildSchema } from '../../utilities';
10+
import {
11+
expectPassesRule,
12+
expectFailsRule,
13+
expectSDLErrorsFromRule,
14+
} from './harness';
1015
import {
1116
KnownArgumentNames,
17+
KnownArgumentNamesOnDirectives,
1218
unknownArgMessage,
1319
unknownDirectiveArgMessage,
1420
} from '../rules/KnownArgumentNames';
1521

22+
const expectSDLErrors = expectSDLErrorsFromRule.bind(
23+
undefined,
24+
KnownArgumentNamesOnDirectives,
25+
);
26+
1627
function unknownArg(argName, fieldName, typeName, suggestedArgs, line, column) {
1728
return {
1829
message: unknownArgMessage(argName, fieldName, typeName, suggestedArgs),
@@ -215,4 +226,89 @@ describe('Validate: Known argument names', () => {
215226
],
216227
);
217228
});
229+
230+
describe('within SDL', () => {
231+
it('known arg on directive defined inside SDL', () => {
232+
expectSDLErrors(`
233+
type Query {
234+
foo: String @test(arg: "")
235+
}
236+
237+
directive @test(arg: String) on FIELD_DEFINITION
238+
`).to.deep.equal([]);
239+
});
240+
241+
it('unknown arg on directive defined inside SDL', () => {
242+
expectSDLErrors(`
243+
type Query {
244+
foo: String @test(unknown: "")
245+
}
246+
247+
directive @test(arg: String) on FIELD_DEFINITION
248+
`).to.deep.equal([unknownDirectiveArg('unknown', 'test', [], 3, 29)]);
249+
});
250+
251+
it('misspelled arg name is reported on directive defined inside SDL', () => {
252+
expectSDLErrors(`
253+
type Query {
254+
foo: String @test(agr: "")
255+
}
256+
257+
directive @test(arg: String) on FIELD_DEFINITION
258+
`).to.deep.equal([unknownDirectiveArg('agr', 'test', ['arg'], 3, 29)]);
259+
});
260+
261+
it('unknown arg on standard directive', () => {
262+
expectSDLErrors(`
263+
type Query {
264+
foo: String @deprecated(unknown: "")
265+
}
266+
`).to.deep.equal([
267+
unknownDirectiveArg('unknown', 'deprecated', [], 3, 35),
268+
]);
269+
});
270+
271+
it('unknown arg on overrided standard directive', () => {
272+
expectSDLErrors(`
273+
type Query {
274+
foo: String @deprecated(reason: "")
275+
}
276+
directive @deprecated(arg: String) on FIELD
277+
`).to.deep.equal([
278+
unknownDirectiveArg('reason', 'deprecated', [], 3, 35),
279+
]);
280+
});
281+
282+
it('unknown arg on directive defined in schema extension', () => {
283+
const schema = buildSchema(`
284+
type Query {
285+
foo: String
286+
}
287+
`);
288+
expectSDLErrors(
289+
`
290+
directive @test(arg: String) on OBJECT
291+
292+
extend type Query @test(unknown: "")
293+
`,
294+
schema,
295+
).to.deep.equal([unknownDirectiveArg('unknown', 'test', [], 4, 36)]);
296+
});
297+
298+
it('unknown arg on directive used in schema extension', () => {
299+
const schema = buildSchema(`
300+
directive @test(arg: String) on OBJECT
301+
302+
type Query {
303+
foo: String
304+
}
305+
`);
306+
expectSDLErrors(
307+
`
308+
extend type Query @test(unknown: "")
309+
`,
310+
schema,
311+
).to.deep.equal([unknownDirectiveArg('unknown', 'test', [], 2, 35)]);
312+
});
313+
});
218314
});

‎src/validation/__tests__/ProvidedRequiredArguments-test.js

+87-1
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,24 @@
66
*/
77

88
import { describe, it } from 'mocha';
9-
import { expectPassesRule, expectFailsRule } from './harness';
9+
import { buildSchema } from '../../utilities';
10+
import {
11+
expectPassesRule,
12+
expectFailsRule,
13+
expectSDLErrorsFromRule,
14+
} from './harness';
1015
import {
1116
ProvidedRequiredArguments,
17+
ProvidedRequiredArgumentsOnDirectives,
1218
missingFieldArgMessage,
1319
missingDirectiveArgMessage,
1420
} from '../rules/ProvidedRequiredArguments';
1521

22+
const expectSDLErrors = expectSDLErrorsFromRule.bind(
23+
undefined,
24+
ProvidedRequiredArgumentsOnDirectives,
25+
);
26+
1627
function missingFieldArg(fieldName, argName, typeName, line, column) {
1728
return {
1829
message: missingFieldArgMessage(fieldName, argName, typeName),
@@ -278,4 +289,79 @@ describe('Validate: Provided required arguments', () => {
278289
);
279290
});
280291
});
292+
293+
describe('within SDL', () => {
294+
it('Missing optional args on directive defined inside SDL', () => {
295+
expectSDLErrors(`
296+
type Query {
297+
foo: String @test
298+
}
299+
300+
directive @test(arg1: String, arg2: String! = "") on FIELD_DEFINITION
301+
`).to.deep.equal([]);
302+
});
303+
304+
it('Missing arg on directive defined inside SDL', () => {
305+
expectSDLErrors(`
306+
type Query {
307+
foo: String @test
308+
}
309+
310+
directive @test(arg: String!) on FIELD_DEFINITION
311+
`).to.deep.equal([missingDirectiveArg('test', 'arg', 'String!', 3, 23)]);
312+
});
313+
314+
it('Missing arg on standard directive', () => {
315+
expectSDLErrors(`
316+
type Query {
317+
foo: String @include
318+
}
319+
`).to.deep.equal([
320+
missingDirectiveArg('include', 'if', 'Boolean!', 3, 23),
321+
]);
322+
});
323+
324+
it('Missing arg on overrided standard directive', () => {
325+
expectSDLErrors(`
326+
type Query {
327+
foo: String @deprecated
328+
}
329+
directive @deprecated(reason: String!) on FIELD
330+
`).to.deep.equal([
331+
missingDirectiveArg('deprecated', 'reason', 'String!', 3, 23),
332+
]);
333+
});
334+
335+
it('Missing arg on directive defined in schema extension', () => {
336+
const schema = buildSchema(`
337+
type Query {
338+
foo: String
339+
}
340+
`);
341+
expectSDLErrors(
342+
`
343+
directive @test(arg: String!) on OBJECT
344+
345+
extend type Query @test
346+
`,
347+
schema,
348+
).to.deep.equal([missingDirectiveArg('test', 'arg', 'String!', 4, 30)]);
349+
});
350+
351+
it('Missing arg on directive used in schema extension', () => {
352+
const schema = buildSchema(`
353+
directive @test(arg: String!) on OBJECT
354+
355+
type Query {
356+
foo: String
357+
}
358+
`);
359+
expectSDLErrors(
360+
`
361+
extend type Query @test
362+
`,
363+
schema,
364+
).to.deep.equal([missingDirectiveArg('test', 'arg', 'String!', 2, 29)]);
365+
});
366+
});
281367
});

‎src/validation/rules/KnownArgumentNames.js

+66-35
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@
77
* @flow strict
88
*/
99

10-
import type { ValidationContext } from '../ValidationContext';
10+
import type {
11+
ValidationContext,
12+
SDLValidationContext,
13+
} from '../ValidationContext';
1114
import { GraphQLError } from '../../error/GraphQLError';
1215
import type { ASTVisitor } from '../../language/visitor';
1316
import suggestionList from '../../jsutils/suggestionList';
1417
import quotedOrList from '../../jsutils/quotedOrList';
1518
import { Kind } from '../../language/kinds';
19+
import { specifiedDirectives } from '../../type/directives';
1620

1721
export function unknownArgMessage(
1822
argName: string,
@@ -49,48 +53,75 @@ export function unknownDirectiveArgMessage(
4953
*/
5054
export function KnownArgumentNames(context: ValidationContext): ASTVisitor {
5155
return {
52-
Argument(node, key, parent, path, ancestors) {
56+
...KnownArgumentNamesOnDirectives(context),
57+
Argument(argNode) {
5358
const argDef = context.getArgument();
54-
if (!argDef) {
55-
const argumentOf = ancestors[ancestors.length - 1];
56-
if (argumentOf.kind === Kind.FIELD) {
57-
const fieldDef = context.getFieldDef();
58-
const parentType = context.getParentType();
59-
if (fieldDef && parentType) {
60-
context.reportError(
61-
new GraphQLError(
62-
unknownArgMessage(
63-
node.name.value,
64-
fieldDef.name,
65-
parentType.name,
66-
suggestionList(
67-
node.name.value,
68-
fieldDef.args.map(arg => arg.name),
69-
),
70-
),
71-
[node],
72-
),
73-
);
74-
}
75-
} else if (argumentOf.kind === Kind.DIRECTIVE) {
76-
const directive = context.getDirective();
77-
if (directive) {
59+
const fieldDef = context.getFieldDef();
60+
const parentType = context.getParentType();
61+
62+
if (!argDef && fieldDef && parentType) {
63+
const argName = argNode.name.value;
64+
const knownArgsNames = fieldDef.args.map(arg => arg.name);
65+
context.reportError(
66+
new GraphQLError(
67+
unknownArgMessage(
68+
argName,
69+
fieldDef.name,
70+
parentType.name,
71+
suggestionList(argName, knownArgsNames),
72+
),
73+
argNode,
74+
),
75+
);
76+
}
77+
},
78+
};
79+
}
80+
81+
// @internal
82+
export function KnownArgumentNamesOnDirectives(
83+
context: ValidationContext | SDLValidationContext,
84+
): ASTVisitor {
85+
const directiveArgs = Object.create(null);
86+
87+
const schema = context.getSchema();
88+
const definedDirectives = schema
89+
? schema.getDirectives()
90+
: specifiedDirectives;
91+
for (const directive of definedDirectives) {
92+
directiveArgs[directive.name] = directive.args.map(arg => arg.name);
93+
}
94+
95+
const astDefinitions = context.getDocument().definitions;
96+
for (const def of astDefinitions) {
97+
if (def.kind === Kind.DIRECTIVE_DEFINITION) {
98+
directiveArgs[def.name.value] = def.arguments
99+
? def.arguments.map(arg => arg.name.value)
100+
: [];
101+
}
102+
}
103+
104+
return {
105+
Directive(directiveNode) {
106+
const directiveName = directiveNode.name.value;
107+
const knownArgs = directiveArgs[directiveName];
108+
109+
if (directiveNode.arguments && knownArgs) {
110+
for (const argNode of directiveNode.arguments) {
111+
const argName = argNode.name.value;
112+
if (knownArgs.indexOf(argName) === -1) {
113+
const suggestions = suggestionList(argName, knownArgs);
78114
context.reportError(
79115
new GraphQLError(
80-
unknownDirectiveArgMessage(
81-
node.name.value,
82-
directive.name,
83-
suggestionList(
84-
node.name.value,
85-
directive.args.map(arg => arg.name),
86-
),
87-
),
88-
[node],
116+
unknownDirectiveArgMessage(argName, directiveName, suggestions),
117+
argNode,
89118
),
90119
);
91120
}
92121
}
93122
}
123+
124+
return false;
94125
},
95126
};
96127
}

‎src/validation/rules/KnownDirectives.js

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export function KnownDirectives(
3838
context: ValidationContext | SDLValidationContext,
3939
): ASTVisitor {
4040
const locationsMap = Object.create(null);
41+
4142
const schema = context.getSchema();
4243
const definedDirectives = schema
4344
? schema.getDirectives()

0 commit comments

Comments
 (0)