Skip to content

Commit 0a848cc

Browse files
authored
Ensure we validate for using nullable variables in oneOf input fields (#4363)
In the spec we explicitly disallow `nullable` variables to be passed to the fields of a oneOf input object. This is present in v17 but seems missing from v16.
1 parent 6b253e7 commit 0a848cc

File tree

3 files changed

+52
-2
lines changed

3 files changed

+52
-2
lines changed

src/validation/ValidationContext.ts

+2
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ interface VariableUsage {
3333
readonly node: VariableNode;
3434
readonly type: Maybe<GraphQLInputType>;
3535
readonly defaultValue: Maybe<unknown>;
36+
readonly parentType: Maybe<GraphQLInputType>;
3637
}
3738

3839
/**
@@ -209,6 +210,7 @@ export class ValidationContext extends ASTValidationContext {
209210
node: variable,
210211
type: typeInfo.getInputType(),
211212
defaultValue: typeInfo.getDefaultValue(),
213+
parentType: typeInfo.getParentInputType(),
212214
});
213215
},
214216
}),

src/validation/__tests__/VariablesInAllowedPositionRule-test.ts

+31
Original file line numberDiff line numberDiff line change
@@ -358,3 +358,34 @@ describe('Validate: Variables are in allowed positions', () => {
358358
});
359359
});
360360
});
361+
362+
describe('Validates OneOf Input Objects', () => {
363+
it('Allows exactly one non-nullable variable', () => {
364+
expectValid(`
365+
query ($string: String!) {
366+
complicatedArgs {
367+
oneOfArgField(oneOfArg: { stringField: $string })
368+
}
369+
}
370+
`);
371+
});
372+
373+
it('Forbids one nullable variable', () => {
374+
expectErrors(`
375+
query ($string: String) {
376+
complicatedArgs {
377+
oneOfArgField(oneOfArg: { stringField: $string })
378+
}
379+
}
380+
`).toDeepEqual([
381+
{
382+
message:
383+
'Variable "$string" is of type "String" but must be non-nullable to be used for OneOf Input Object "OneOfInput".',
384+
locations: [
385+
{ line: 2, column: 14 },
386+
{ line: 4, column: 50 },
387+
],
388+
},
389+
]);
390+
});
391+
});

src/validation/rules/VariablesInAllowedPositionRule.ts

+19-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import { Kind } from '../../language/kinds';
88
import type { ASTVisitor } from '../../language/visitor';
99

1010
import type { GraphQLType } from '../../type/definition';
11-
import { isNonNullType } from '../../type/definition';
11+
import {
12+
isInputObjectType,
13+
isNonNullType,
14+
isNullableType,
15+
} from '../../type/definition';
1216
import type { GraphQLSchema } from '../../type/schema';
1317

1418
import { isTypeSubTypeOf } from '../../utilities/typeComparators';
@@ -36,7 +40,7 @@ export function VariablesInAllowedPositionRule(
3640
leave(operation) {
3741
const usages = context.getRecursiveVariableUsages(operation);
3842

39-
for (const { node, type, defaultValue } of usages) {
43+
for (const { node, type, defaultValue, parentType } of usages) {
4044
const varName = node.name.value;
4145
const varDef = varDefMap[varName];
4246
if (varDef && type) {
@@ -66,6 +70,19 @@ export function VariablesInAllowedPositionRule(
6670
),
6771
);
6872
}
73+
74+
if (
75+
isInputObjectType(parentType) &&
76+
parentType.isOneOf &&
77+
isNullableType(varType)
78+
) {
79+
context.reportError(
80+
new GraphQLError(
81+
`Variable "$${varName}" is of type "${varType}" but must be non-nullable to be used for OneOf Input Object "${parentType}".`,
82+
{ nodes: [varDef, node] },
83+
),
84+
);
85+
}
6986
}
7087
}
7188
},

0 commit comments

Comments
 (0)