fix: follow the specification for absent, null, and default input values

executor.go

@@ -481,10 +481,7 @@ func collectFields(p collectFieldsParams) (fields map[string][]*ast.Field) {
 // Determines if a field should be included based on the @include and @skip
 // directives, where @skip has higher precedence than @include.
 func shouldIncludeNode(eCtx *executionContext, directives []*ast.Directive) bool {
-	var (
-		skipAST, includeAST *ast.Directive
-		argValues           map[string]interface{}
-	)
+	var skipAST, includeAST *ast.Directive
 	for _, directive := range directives {
 		if directive == nil || directive.Name == nil {
 			continue
@@ -498,13 +495,24 @@ func shouldIncludeNode(eCtx *executionContext, directives []*ast.Directive) bool
 	}
 	// precedence: skipAST > includeAST
 	if skipAST != nil {
-		argValues = getArgumentValues(SkipDirective.Args, skipAST.Arguments, eCtx.VariableValues)
+		argValues, err := getArgumentValues(SkipDirective.Args, skipAST.Arguments, eCtx.VariableValues, eCtx.Schema.nonSpecArgumentHandling)
+		if err != nil {
+			// @skip declares "if: Boolean!" with no default, so a null can only get
+			// here if both variable coercion and validation let it through. Record
+			// the error rather than dropping it, and leave the node out.
+			eCtx.Errors = append(eCtx.Errors, gqlerrors.FormatErrorsFromError(err)...)
+			return false // excluded selectionSet's fields
+		}
 		if skipIf, ok := argValues["if"].(bool); ok && skipIf {
 			return false // excluded selectionSet's fields
 		}
 	}
 	if includeAST != nil {
-		argValues = getArgumentValues(IncludeDirective.Args, includeAST.Arguments, eCtx.VariableValues)
+		argValues, err := getArgumentValues(IncludeDirective.Args, includeAST.Arguments, eCtx.VariableValues, eCtx.Schema.nonSpecArgumentHandling)
+		if err != nil {
+			eCtx.Errors = append(eCtx.Errors, gqlerrors.FormatErrorsFromError(err)...)
+			return false // excluded selectionSet's fields
+		}
 		if includeIf, ok := argValues["if"].(bool); ok && !includeIf {
 			return false // excluded selectionSet's fields
 		}
@@ -624,7 +632,12 @@ func resolveField(eCtx *executionContext, parentType *Object, source interface{}
 	// Build a map of arguments from the field.arguments AST, using the
 	// variables scope to fulfill any variable references.
 	// TODO: find a way to memoize, in case this field is within a List type.
-	args := getArgumentValues(fieldDef.Args, fieldAST.Arguments, eCtx.VariableValues)
+	args, argErr := getArgumentValues(fieldDef.Args, fieldAST.Arguments, eCtx.VariableValues, eCtx.Schema.nonSpecArgumentHandling)
+	if argErr != nil {
+		// Same idiom as a failing resolver below: the deferred recover turns this
+		// into a field error via handleFieldError.
+		panic(argErr)
+	}
 
 	info := ResolveInfo{
 		FieldName:      fieldName,

rules.go

@@ -68,13 +68,15 @@ func reportError(context *ValidationContext, message string, nodes []ast.Node) (
 // A GraphQL document is only valid if all field argument literal values are
 // of the type expected by their position.
 func ArgumentsOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance {
+	nonSpec := context.Schema().nonSpecArgumentHandling
+
 	visitorOpts := &visitor.VisitorOptions{
 		KindFuncMap: map[string]visitor.NamedVisitFuncs{
 			kinds.Argument: {
 				Kind: func(p visitor.VisitFuncParams) (string, interface{}) {
 					if argAST, ok := p.Node.(*ast.Argument); ok {
 						if argDef := context.Argument(); argDef != nil {
-							if isValid, messages := isValidLiteralValue(argDef.Type, argAST.Value); !isValid {
+							if isValid, messages := isValidLiteralValue(argDef.Type, argAST.Value, nonSpec); !isValid {
 								var messagesStr, argNameValue string
 								if argAST.Name != nil {
 									argNameValue = argAST.Name.Value
@@ -108,6 +110,8 @@ func ArgumentsOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInsta
 // A GraphQL document is only valid if all variable default values are of the
 // type expected by their definition.
 func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance {
+	nonSpec := context.Schema().nonSpecArgumentHandling
+
 	visitorOpts := &visitor.VisitorOptions{
 		KindFuncMap: map[string]visitor.NamedVisitFuncs{
 			kinds.VariableDefinition: {
@@ -123,16 +127,21 @@ func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleI
 						}
 						ttype := context.InputType()
 
-						// when input variable value must be nonNull, and set default value is unnecessary
-						if ttype, ok := ttype.(*NonNull); ok && defaultValue != nil {
-							reportError(
-								context,
-								fmt.Sprintf(`Variable "$%v" of type "%v" is required and will not use the default value. Perhaps you meant to use type "%v".`,
-									name, ttype, ttype.OfType),
-								[]ast.Node{defaultValue},
-							)
+						// Spec §6.1.2 applies a variable's default before checking the
+						// non-null requirement, so a non-null variable may declare one and
+						// nothing in the Validation section forbids it. Only a schema that
+						// opted into NonSpecArgumentHandling keeps rejecting it.
+						if nonSpec {
+							if ttype, ok := ttype.(*NonNull); ok && defaultValue != nil {
+								reportError(
+									context,
+									fmt.Sprintf(`Variable "$%v" of type "%v" is required and will not use the default value. Perhaps you meant to use type "%v".`,
+										name, ttype, ttype.OfType),
+									[]ast.Node{defaultValue},
+								)
+							}
 						}
-						if isValid, messages := isValidLiteralValue(ttype, defaultValue); !isValid && defaultValue != nil {
+						if isValid, messages := isValidLiteralValue(ttype, defaultValue, nonSpec); !isValid && defaultValue != nil {
 							if len(messages) > 0 {
 								messagesStr = "\n" + strings.Join(messages, "\n")
 							}
@@ -1247,6 +1256,12 @@ func PossibleFragmentSpreadsRule(context *ValidationContext) *ValidationRuleInst
 // have been provided.
 func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleInstance {
 
+	// Spec §5.4.2.1: an argument is required only when its type is non-null and
+	// it declares no default value. A schema that opted into
+	// NonSpecArgumentHandling keeps the older, stricter reading, under which
+	// every non-null argument is required whether or not it has a default.
+	nonSpec := context.Schema().nonSpecArgumentHandling
+
 	visitorOpts := &visitor.VisitorOptions{
 		KindFuncMap: map[string]visitor.NamedVisitFuncs{
 			kinds.Field: {
@@ -1271,7 +1286,7 @@ func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleIns
 						for _, argDef := range fieldDef.Args {
 							argAST, _ := argASTMap[argDef.Name()]
 							if argAST == nil {
-								if argDefType, ok := argDef.Type.(*NonNull); ok {
+								if argDefType, ok := argDef.Type.(*NonNull); ok && (nonSpec || argDef.DefaultValue == nil) {
 									fieldName := ""
 									if fieldAST.Name != nil {
 										fieldName = fieldAST.Name.Value
@@ -1312,7 +1327,7 @@ func ProvidedNonNullArgumentsRule(context *ValidationContext) *ValidationRuleIns
 						for _, argDef := range directiveDef.Args {
 							argAST, _ := argASTMap[argDef.Name()]
 							if argAST == nil {
-								if argDefType, ok := argDef.Type.(*NonNull); ok {
+								if argDefType, ok := argDef.Type.(*NonNull); ok && (nonSpec || argDef.DefaultValue == nil) {
 									directiveName := ""
 									if directiveAST.Name != nil {
 										directiveName = directiveAST.Name.Value
@@ -1656,8 +1671,31 @@ func effectiveType(varType Type, varDef *ast.VariableDefinition) Type {
 	return NewNonNull(varType)
 }
 
+// allowedVariableUsage implements spec §5.8.5 IsVariableUsageAllowed. A nullable
+// variable is allowed at a non-null location when either the variable or the
+// location declares a default value: whichever one exists covers the case where
+// the caller supplies nothing.
+func allowedVariableUsage(schema *Schema, varType Type, varDefaultValue ast.Value,
+	locationType Input, locationDefaultValue interface{}) bool {
+	if locType, ok := locationType.(*NonNull); ok {
+		if _, isNonNull := varType.(*NonNull); !isNonNull {
+			// The parser does not accept the null literal, so a default value that
+			// exists is necessarily not null. Revisit if that ever changes.
+			hasNonNullVariableDefaultValue := varDefaultValue != nil
+			hasLocationDefaultValue := locationDefaultValue != nil
+			if !hasNonNullVariableDefaultValue && !hasLocationDefaultValue {
+				return false
+			}
+			ofType, _ := locType.OfType.(Input)
+			return isTypeSubTypeOf(schema, varType, ofType)
+		}
+	}
+	return isTypeSubTypeOf(schema, varType, locationType)
+}
+
 // VariablesInAllowedPositionRule Variables passed to field arguments conform to type
 func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleInstance {
+	nonSpec := context.Schema().nonSpecArgumentHandling
 
 	varDefMap := map[string]*ast.VariableDefinition{}
 
@@ -1683,7 +1721,15 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI
 								if err != nil {
 									varType = nil
 								}
-								if varType != nil && !isTypeSubTypeOf(context.Schema(), effectiveType(varType, varDef), usage.Type) {
+								allowed := true
+								if varType != nil {
+									if nonSpec {
+										allowed = isTypeSubTypeOf(context.Schema(), effectiveType(varType, varDef), usage.Type)
+									} else {
+										allowed = allowedVariableUsage(context.Schema(), varType, varDef.DefaultValue, usage.Type, usage.LocationDefaultValue)
+									}
+								}
+								if varType != nil && !allowed {
 									reportError(
 										context,
 										fmt.Sprintf(`Variable "$%v" of type "%v" used in position `+
@@ -1724,7 +1770,7 @@ func VariablesInAllowedPositionRule(context *ValidationContext) *ValidationRuleI
 //
 // Note that this only validates literal values, variables are assumed to
 // provide values of the correct type.
-func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) {
+func isValidLiteralValue(ttype Input, valueAST ast.Value, nonSpec bool) (bool, []string) {
 	if _, ok := ttype.(*NonNull); !ok {
 		if valueAST == nil {
 			return true, nil
@@ -1749,21 +1795,21 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) {
 			return false, []string{"Expected non-null value, found null."}
 		}
 		ofType, _ := ttype.OfType.(Input)
-		return isValidLiteralValue(ofType, valueAST)
+		return isValidLiteralValue(ofType, valueAST, nonSpec)
 	case *List:
 		// Lists accept a non-list value as a list of one.
 		itemType, _ := ttype.OfType.(Input)
 		if valueAST, ok := valueAST.(*ast.ListValue); ok {
 			messagesReduce := []string{}
 			for _, value := range valueAST.Values {
-				_, messages := isValidLiteralValue(itemType, value)
+				_, messages := isValidLiteralValue(itemType, value, nonSpec)
 				for idx, message := range messages {
 					messagesReduce = append(messagesReduce, fmt.Sprintf(`In element #%v: %v`, idx+1, message))
 				}
 			}
 			return (len(messagesReduce) == 0), messagesReduce
 		}
-		return isValidLiteralValue(itemType, valueAST)
+		return isValidLiteralValue(itemType, valueAST, nonSpec)
 	case *InputObject:
 		// Input objects check each defined field and look for undefined fields.
 		valueAST, ok := valueAST.(*ast.ObjectValue)
@@ -1786,10 +1832,17 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) {
 		// Ensure every defined field is valid.
 		for fieldName, field := range fields {
 			var fieldASTValue ast.Value
-			if fieldAST := fieldASTMap[fieldName]; fieldAST != nil {
+			fieldAST, ok := fieldASTMap[fieldName]
+			if ok && fieldAST != nil {
 				fieldASTValue = fieldAST.Value
 			}
-			if isValid, messages := isValidLiteralValue(field.Type, fieldASTValue); !isValid {
+			// Spec §3.10 Input Coercion and §5.6.4 Input Object Required Fields: a
+			// field that declares a default value is optional even when its type is
+			// non-null. A field written in the literal is still validated.
+			if !nonSpec && !ok && field.DefaultValue != nil {
+				continue
+			}
+			if isValid, messages := isValidLiteralValue(field.Type, fieldASTValue, nonSpec); !isValid {
 				for _, message := range messages {
 					messagesReduce = append(messagesReduce, fmt.Sprintf("In field \"%v\": %v", fieldName, message))
 				}

rules_arguments_of_correct_type_test.go

@@ -799,3 +799,64 @@ func TestValidate_ArgValuesOfCorrectType_DirectiveArguments_WithDirectivesWithIn
 			),
 		})
 }
+
+// One field whose input object declares a non-null field with a default value,
+// under whichever mode the caller asks for.
+func nonNullInputFieldWithDefaultSchema(t *testing.T, nonSpec bool) graphql.Schema {
+	t.Helper()
+	input := graphql.NewInputObject(graphql.InputObjectConfig{
+		Name: "FieldDefaultInput",
+		Fields: graphql.InputObjectConfigFieldMap{
+			"a": &graphql.InputObjectFieldConfig{
+				Type:         graphql.NewNonNull(graphql.String),
+				DefaultValue: "FIELDDEF",
+			},
+		},
+	})
+	schema, err := graphql.NewSchema(graphql.SchemaConfig{
+		Query: graphql.NewObject(graphql.ObjectConfig{
+			Name: "Query",
+			Fields: graphql.Fields{
+				"f": &graphql.Field{
+					Type: graphql.String,
+					Args: graphql.FieldConfigArgument{
+						"input": &graphql.ArgumentConfig{Type: input},
+					},
+				},
+			},
+		}),
+		NonSpecArgumentHandling: nonSpec,
+	})
+	if err != nil {
+		t.Fatalf("Unexpected error, got: %v", err)
+	}
+	return schema
+}
+
+// Spec §5.6.4: an input field is required only when its type is non-null AND it
+// declares no default value.
+func TestValidate_ArgumentsOfCorrectType_NonNullInputFieldWithDefaultMayBeOmitted(t *testing.T) {
+	schema := nonNullInputFieldWithDefaultSchema(t, false)
+	testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.ArgumentsOfCorrectTypeRule, `
+        {
+          f(input: {})
+        }
+    `)
+}
+
+// NonSpecArgumentHandling keeps the older reading, under which every non-null
+// input field is required whether or not it declares a default.
+func TestValidate_ArgumentsOfCorrectType_NonSpecKeepsNonNullInputFieldRequired(t *testing.T) {
+	schema := nonNullInputFieldWithDefaultSchema(t, true)
+	testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.ArgumentsOfCorrectTypeRule, `
+        {
+          f(input: {})
+        }
+    `, []gqlerrors.FormattedError{
+		testutil.RuleError(
+			`Argument "input" has invalid value {}.`+
+				"\nIn field \"a\": Expected \"String!\", found null.",
+			3, 20,
+		),
+	})
+}

rules_default_values_of_correct_type_test.go

@@ -33,24 +33,18 @@ func TestValidate_VariableDefaultValuesOfCorrectType_VariablesWithValidDefaultVa
       }
     `)
 }
-func TestValidate_VariableDefaultValuesOfCorrectType_NoRequiredVariablesWithDefaultValues(t *testing.T) {
-	testutil.ExpectFailsRule(t, graphql.DefaultValuesOfCorrectTypeRule, `
-      query UnreachableDefaultValues($a: Int! = 3, $b: String! = "default") {
+
+// Spec §6.1.2 applies a variable's default before checking the non-null
+// requirement, so a non-null variable may declare one and the default is
+// reached. This test asserted the opposite before the coercion fix; the
+// rejection now lives behind NonSpecArgumentHandling and is covered by
+// TestValidate_VariableDefaultValuesOfCorrectType_NonSpecRejectsNonNullVariableDefault.
+func TestValidate_VariableDefaultValuesOfCorrectType_NonNullVariablesWithDefaultValues(t *testing.T) {
+	testutil.ExpectPassesRule(t, graphql.DefaultValuesOfCorrectTypeRule, `
+      query NonNullDefaultValues($a: Int! = 3, $b: String! = "default") {
         dog { name }
       }
-    `,
-		[]gqlerrors.FormattedError{
-			testutil.RuleError(
-				`Variable "$a" of type "Int!" is required and will not `+
-					`use the default value. Perhaps you meant to use type "Int".`,
-				2, 49,
-			),
-			testutil.RuleError(
-				`Variable "$b" of type "String!" is required and will not `+
-					`use the default value. Perhaps you meant to use type "String".`,
-				2, 66,
-			),
-		})
+    `)
 }
 func TestValidate_VariableDefaultValuesOfCorrectType_VariablesWithInvalidDefaultValues(t *testing.T) {
 	testutil.ExpectFailsRule(t, graphql.DefaultValuesOfCorrectTypeRule, `
@@ -105,3 +99,51 @@ func TestValidate_VariableDefaultValuesOfCorrectType_ListVariablesWithInvalidIte
 func TestValidate_VariableDefaultValuesOfCorrectType_InvalidNonNull(t *testing.T) {
 	testutil.ExpectPassesRule(t, graphql.DefaultValuesOfCorrectTypeRule, `query($g:e!){a}`)
 }
+
+// One nullable argument, so a non-null variable can be used at it, under
+// whichever mode the caller asks for.
+func nonNullVariableDefaultSchema(t *testing.T, nonSpec bool) graphql.Schema {
+	t.Helper()
+	schema, err := graphql.NewSchema(graphql.SchemaConfig{
+		Query: graphql.NewObject(graphql.ObjectConfig{
+			Name: "Query",
+			Fields: graphql.Fields{
+				"f": &graphql.Field{
+					Type: graphql.String,
+					Args: graphql.FieldConfigArgument{
+						"a": &graphql.ArgumentConfig{Type: graphql.String},
+					},
+				},
+			},
+		}),
+		NonSpecArgumentHandling: nonSpec,
+	})
+	if err != nil {
+		t.Fatalf("Unexpected error, got: %v", err)
+	}
+	return schema
+}
+
+// Spec §6.1.2 applies a variable's default before checking the non-null
+// requirement, and nothing in the Validation section forbids a non-null variable
+// from declaring one.
+func TestValidate_VariableDefaultValuesOfCorrectType_NonNullVariableMayHaveDefault(t *testing.T) {
+	schema := nonNullVariableDefaultSchema(t, false)
+	testutil.ExpectPassesRuleWithSchema(t, &schema, graphql.DefaultValuesOfCorrectTypeRule, `
+      query Probe($a: String! = "VARDEF") {
+        f(a: $a)
+      }
+    `)
+}
+
+// NonSpecArgumentHandling keeps rejecting it.
+func TestValidate_VariableDefaultValuesOfCorrectType_NonSpecRejectsNonNullVariableDefault(t *testing.T) {
+	schema := nonNullVariableDefaultSchema(t, true)
+	testutil.ExpectFailsRuleWithSchema(t, &schema, graphql.DefaultValuesOfCorrectTypeRule, `
+      query Probe($a: String! = "VARDEF") {
+        f(a: $a)
+      }
+    `, []gqlerrors.FormattedError{
+		testutil.RuleError(`Variable "$a" of type "String!" is required and will not use the default value. Perhaps you meant to use type "String".`, 2, 33),
+	})
+}