From 1a5126be1c9d8a27afe47649cbda610d7b63a8d1 Mon Sep 17 00:00:00 2001 From: mjmahone Date: Thu, 29 Dec 2022 18:23:39 -0500 Subject: [PATCH] Comment in VariablesAreInputTypesRule --- src/validation/ValidationContext.ts | 13 +++++++- .../VariablesInAllowedPositionRule-test.ts | 31 ++++++++++++++++--- .../rules/NoUndefinedVariablesRule.ts | 5 ++- .../rules/VariablesAreInputTypesRule.ts | 2 ++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/validation/ValidationContext.ts b/src/validation/ValidationContext.ts index 2467e2141fc..2c261abded9 100644 --- a/src/validation/ValidationContext.ts +++ b/src/validation/ValidationContext.ts @@ -35,6 +35,7 @@ interface VariableUsage { readonly node: VariableNode; readonly type: Maybe; readonly defaultValue: Maybe; + readonly argumentDefinition: Maybe; } interface DefinedVariable { readonly variableDefinition: VariableDefinitionNode; @@ -151,7 +152,9 @@ export class ASTValidationContext { const variableDefinedOperations: ObjMap> = {}; visit(this._ast, { OperationDefinition(operation) { - for (const varDef of operation.variableDefinitions ?? []) { + /* c8 ignore next */ + const variableDefinitions = operation.variableDefinitions ?? []; + for (const varDef of variableDefinitions) { if (!variableDefinedOperations[varDef.variable.name.value]) { variableDefinedOperations[varDef.variable.name.value] = []; } @@ -238,10 +241,18 @@ export class ValidationContext extends ASTValidationContext { visitWithTypeInfo(typeInfo, { VariableDefinition: () => false, Variable(variable) { + const argumentDefinition = + node.kind === Kind.FRAGMENT_DEFINITION + ? node.argumentDefinitions?.find( + (argDef) => + argDef.variable.name.value === variable.name.value, + ) + : undefined; newUsages.push({ node: variable, type: typeInfo.getInputType(), defaultValue: typeInfo.getDefaultValue(), + argumentDefinition, }); }, }), diff --git a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts index e2368787374..140ff2cfe88 100644 --- a/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts +++ b/src/validation/__tests__/VariablesInAllowedPositionRule-test.ts @@ -356,6 +356,13 @@ describe('Validate: Variables are in allowed positions', () => { dog @include(if: $boolVar) }`); }); + + it('undefined in directive with default value with option', () => { + expectValid(` + { + dog @include(if: $x) + }`); + }); }); describe('Fragment arguments are validated', () => { @@ -373,12 +380,26 @@ describe('Validate: Variables are in allowed positions', () => { `); }); + it('Boolean => Boolean with default value', () => { + expectValid(` + query Query($booleanArg: Boolean) + { + complicatedArgs { + ...A(b: $booleanArg) + } + } + fragment A($b: Boolean = true) on ComplicatedArgs { + booleanArgField(booleanArg: $b) + } + `); + }); + it('Boolean => Boolean!', () => { expectErrors(` - query Query($fb: Boolean) + query Query($ab: Boolean) { complicatedArgs { - ...A(b: $fb) + ...A(b: $ab) } } fragment A($b: Boolean!) on ComplicatedArgs { @@ -387,10 +408,10 @@ describe('Validate: Variables are in allowed positions', () => { `).toDeepEqual([ { message: - 'Variable "$booleanArg" of type "Boolean" used in position expecting type "Boolean!".', + 'Variable "$ab" of type "Boolean" used in position expecting type "Boolean!".', locations: [ { line: 2, column: 21 }, - { line: 4, column: 47 }, + { line: 5, column: 21 }, ], }, ]); @@ -412,7 +433,7 @@ describe('Validate: Variables are in allowed positions', () => { 'Variable "$intVar" of type "Int" used in position expecting type "Int!".', locations: [ { line: 2, column: 21 }, - { line: 4, column: 47 }, + { line: 4, column: 21 }, ], }, ]); diff --git a/src/validation/rules/NoUndefinedVariablesRule.ts b/src/validation/rules/NoUndefinedVariablesRule.ts index d1672ecd0bd..a90dde2128c 100644 --- a/src/validation/rules/NoUndefinedVariablesRule.ts +++ b/src/validation/rules/NoUndefinedVariablesRule.ts @@ -22,7 +22,10 @@ export function NoUndefinedVariablesRule( ); const usages = context.getRecursiveVariableUsages(operation); - for (const { node } of usages) { + for (const { node, argumentDefinition } of usages) { + if (argumentDefinition) { + continue; + } const varName = node.name.value; if (!variableNameDefined.has(varName)) { context.reportError( diff --git a/src/validation/rules/VariablesAreInputTypesRule.ts b/src/validation/rules/VariablesAreInputTypesRule.ts index 019f3e762ca..9a3a649ad83 100644 --- a/src/validation/rules/VariablesAreInputTypesRule.ts +++ b/src/validation/rules/VariablesAreInputTypesRule.ts @@ -16,6 +16,8 @@ import type { ValidationContext } from '../ValidationContext.js'; * A GraphQL operation is only valid if all the variables it defines are of * input types (scalar, enum, or input object). * + * Similarly, all fragment defined arguments must be of input types. + * * See https://spec.graphql.org/draft/#sec-Variables-Are-Input-Types */ export function VariablesAreInputTypesRule(