fix: fix validation of variables used in nested fields of type list o…

…f an input object (#1099)

fix validation if lists with the variable value nested in the input
object; merge valid arguments rule with the values validation rule

closes #1096

v2/pkg/astprinter/astprinter_test.go

@@ -974,7 +974,7 @@ type __Field {
     deprecationReason: String
 }
 
-"""ValidArguments provided to FieldSelections or Directives and the input fields of an
+"""Arguments provided to FieldSelections or Directives and the input fields of an
 InputObject are represented as Input Values which describe their type and
 optionally a default value.
 """

v2/pkg/astvalidation/operation_rule_valid_arguments.go

@@ -2,70 +2,19 @@ package astvalidation
 
 import (
 	"bytes"
-	"fmt"
 
 	"github.com/wundergraph/graphql-go-tools/v2/pkg/ast"
-	"github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor"
 	"github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport"
 )
 
-// ValidArguments validates if arguments are valid: values and variables has compatible types
-// deep variables comparison is handled by Values
-func ValidArguments() Rule {
-	return func(walker *astvisitor.Walker) {
-		visitor := validArgumentsVisitor{
-			Walker: walker,
-		}
-		walker.RegisterEnterDocumentVisitor(&visitor)
-		walker.RegisterEnterArgumentVisitor(&visitor)
-	}
-}
-
-type validArgumentsVisitor struct {
-	*astvisitor.Walker
-	operation, definition *ast.Document
-}
-
-func (v *validArgumentsVisitor) EnterDocument(operation, definition *ast.Document) {
-	v.operation = operation
-	v.definition = definition
-}
-
-func (v *validArgumentsVisitor) EnterArgument(ref int) {
-	definitionRef, exists := v.ArgumentInputValueDefinition(ref)
-
-	if !exists {
-		return
-	}
-
-	value := v.operation.ArgumentValue(ref)
-	v.validateIfValueSatisfiesInputFieldDefinition(value, definitionRef)
-}
-
-func (v *validArgumentsVisitor) validateIfValueSatisfiesInputFieldDefinition(value ast.Value, inputValueDefinitionRef int) {
+func (v *valuesVisitor) validateIfValueSatisfiesInputFieldDefinition(value ast.Value, inputValueDefinitionRef int) {
 	var (
 		satisfied             bool
 		operationTypeRef      int
 		variableDefinitionRef int
 	)
 
-	switch value.Kind {
-	case ast.ValueKindVariable:
-		satisfied, operationTypeRef, variableDefinitionRef = v.variableValueSatisfiesInputValueDefinition(value.Ref, inputValueDefinitionRef)
-	case ast.ValueKindEnum,
-		ast.ValueKindNull,
-		ast.ValueKindBoolean,
-		ast.ValueKindInteger,
-		ast.ValueKindString,
-		ast.ValueKindFloat,
-		ast.ValueKindObject,
-		ast.ValueKindList:
-		// this types of values are covered by Values() / valuesVisitor
-		return
-	default:
-		v.StopWithInternalErr(fmt.Errorf("validateIfValueSatisfiesInputFieldDefinition: not implemented for value.Kind: %s", value.Kind))
-		return
-	}
+	satisfied, operationTypeRef, variableDefinitionRef = v.variableValueSatisfiesInputValueDefinition(value.Ref, inputValueDefinitionRef)
 
 	if satisfied {
 		return
@@ -95,7 +44,7 @@ func (v *validArgumentsVisitor) validateIfValueSatisfiesInputFieldDefinition(val
 	v.StopWithExternalErr(operationreport.ErrVariableTypeDoesntSatisfyInputValueDefinition(printedValue, actualTypeName, expectedTypeName, value.Position, v.operation.VariableDefinitions[variableDefinitionRef].VariableValue.Position))
 }
 
-func (v *validArgumentsVisitor) variableValueSatisfiesInputValueDefinition(variableValue, inputValueDefinition int) (satisfies bool, operationTypeRef int, variableDefRef int) {
+func (v *valuesVisitor) variableValueSatisfiesInputValueDefinition(variableValue, inputValueDefinition int) (satisfies bool, operationTypeRef int, variableDefRef int) {
 	variableDefinitionRef, exists := v.variableDefinition(variableValue)
 	if !exists {
 		return false, ast.InvalidRef, variableDefinitionRef
@@ -110,7 +59,7 @@ func (v *validArgumentsVisitor) variableValueSatisfiesInputValueDefinition(varia
 	return v.operationTypeSatisfiesDefinitionType(operationTypeRef, definitionTypeRef, hasDefaultValue), operationTypeRef, variableDefinitionRef
 }
 
-func (v *validArgumentsVisitor) variableDefinition(variableValueRef int) (ref int, exists bool) {
+func (v *valuesVisitor) variableDefinition(variableValueRef int) (ref int, exists bool) {
 	variableName := v.operation.VariableValueNameBytes(variableValueRef)
 
 	if v.Ancestors[0].Kind == ast.NodeKindOperationDefinition {
@@ -127,11 +76,11 @@ func (v *validArgumentsVisitor) variableDefinition(variableValueRef int) (ref in
 	return ast.InvalidRef, false
 }
 
-func (v *validArgumentsVisitor) validDefaultValue(value ast.DefaultValue) bool {
+func (v *valuesVisitor) validDefaultValue(value ast.DefaultValue) bool {
 	return value.IsDefined && value.Value.Kind != ast.ValueKindNull
 }
 
-func (v *validArgumentsVisitor) operationTypeSatisfiesDefinitionType(operationTypeRef int, definitionTypeRef int, hasDefaultValue bool) bool {
+func (v *valuesVisitor) operationTypeSatisfiesDefinitionType(operationTypeRef int, definitionTypeRef int, hasDefaultValue bool) bool {
 	opKind := v.operation.Types[operationTypeRef].TypeKind
 	defKind := v.definition.Types[definitionTypeRef].TypeKind
 

v2/pkg/astvalidation/operation_rule_values.go

@@ -45,27 +45,24 @@ func (v *valuesVisitor) EnterVariableDefinition(ref int) {
 
 func (v *valuesVisitor) EnterArgument(ref int) {
 
-	definition, exists := v.ArgumentInputValueDefinition(ref)
+	definitionRef, exists := v.ArgumentInputValueDefinition(ref)
 	if !exists {
 		return
 	}
 
 	value := v.operation.ArgumentValue(ref)
 	if value.Kind == ast.ValueKindVariable {
-		variableName := v.operation.VariableValueNameBytes(value.Ref)
-		variableDefinition, exists := v.operation.VariableDefinitionByNameAndOperation(v.Ancestors[0].Ref, variableName)
+		_, exists := v.variableDefinition(value.Ref)
 		if !exists {
-			operationName := v.operation.OperationDefinitionNameBytes(v.Ancestors[0].Ref)
-			v.StopWithExternalErr(operationreport.ErrVariableNotDefinedOnOperation(variableName, operationName))
+			v.StopWithExternalErr(operationreport.ErrVariableNotDefinedOnOperation(v.operation.VariableValueNameBytes(value.Ref), []byte("")))
 			return
 		}
-		if !v.operation.VariableDefinitions[variableDefinition].DefaultValue.IsDefined {
-			return // variable has no default value, deep type check not required
-		}
-		value = v.operation.VariableDefinitions[variableDefinition].DefaultValue.Value
+
+		v.validateIfValueSatisfiesInputFieldDefinition(value, definitionRef)
+		return
 	}
 
-	v.valueSatisfiesInputValueDefinitionType(value, v.definition.InputValueDefinitions[definition].Type)
+	v.valueSatisfiesInputValueDefinitionType(value, v.definition.InputValueDefinitions[definitionRef].Type)
 }
 
 func (v *valuesVisitor) valueSatisfiesOperationType(value ast.Value, operationTypeRef int) bool {
@@ -160,35 +157,41 @@ func (v *valuesVisitor) valueSatisfiesInputValueDefinitionType(value ast.Value,
 	case ast.TypeKindNamed:
 		return v.valuesSatisfiesNamedType(value, definitionTypeRef)
 	case ast.TypeKindList:
-		return v.valueSatisfiesListType(value, definitionTypeRef, v.definition.Types[definitionTypeRef].OfType)
+		return v.valueSatisfiesListType(value, definitionTypeRef)
 	default:
 		v.handleTypeError(value, definitionTypeRef)
 		return false
 	}
 }
 
+func (v *valuesVisitor) variableValueSatisfiesDefinitionType(value ast.Value, definitionTypeRef int) bool {
+	variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(value.Ref)
+	if !ok {
+		v.handleTypeError(value, definitionTypeRef)
+		return false
+	}
+
+	hasDefaultValue := v.operation.VariableDefinitionHasDefaultValue(variableDefinitionRef)
+
+	if !v.operationTypeSatisfiesDefinitionType(variableTypeRef, definitionTypeRef, hasDefaultValue) {
+		v.handleVariableHasIncompatibleTypeError(value, definitionTypeRef)
+		return false
+	}
+
+	if hasDefaultValue {
+		return v.valueSatisfiesInputValueDefinitionType(v.operation.VariableDefinitions[variableDefinitionRef].DefaultValue.Value, definitionTypeRef)
+	}
+
+	return true
+}
+
 func (v *valuesVisitor) valuesSatisfiesNonNullType(value ast.Value, definitionTypeRef int) bool {
 	switch value.Kind {
 	case ast.ValueKindNull:
 		v.handleUnexpectedNullError(value, definitionTypeRef)
 		return false
 	case ast.ValueKindVariable:
-		variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(value.Ref)
-		if !ok {
-			v.handleTypeError(value, definitionTypeRef)
-			return false
-		}
-
-		if v.operation.VariableDefinitionHasDefaultValue(variableDefinitionRef) {
-			return v.valueSatisfiesInputValueDefinitionType(v.operation.VariableDefinitions[variableDefinitionRef].DefaultValue.Value, definitionTypeRef)
-		}
-
-		importedDefinitionType := v.importer.ImportType(definitionTypeRef, v.definition, v.operation)
-		if !v.operation.TypesAreEqualDeep(importedDefinitionType, variableTypeRef) {
-			v.handleVariableHasIncompatibleTypeError(value, definitionTypeRef)
-			return false
-		}
-		return true
+		return v.variableValueSatisfiesDefinitionType(value, definitionTypeRef)
 	}
 	return v.valueSatisfiesInputValueDefinitionType(value, v.definition.Types[definitionTypeRef].OfType)
 }
@@ -209,27 +212,11 @@ func (v *valuesVisitor) valuesSatisfiesNamedType(value ast.Value, definitionType
 	return v.valueSatisfiesTypeDefinitionNode(value, definitionTypeRef, node)
 }
 
-func (v *valuesVisitor) valueSatisfiesListType(value ast.Value, definitionTypeRef int, listItemType int) bool {
+func (v *valuesVisitor) valueSatisfiesListType(value ast.Value, definitionTypeRef int) bool {
+	listItemType := v.definition.Types[definitionTypeRef].OfType
 
 	if value.Kind == ast.ValueKindVariable {
-		variableDefinitionRef, actualType, _, ok := v.operationVariableType(value.Ref)
-		if !ok {
-			v.handleTypeError(value, definitionTypeRef)
-			return false
-		}
-
-		if v.operation.VariableDefinitionHasDefaultValue(variableDefinitionRef) {
-			return v.valueSatisfiesInputValueDefinitionType(v.operation.VariableDefinitions[variableDefinitionRef].DefaultValue.Value, definitionTypeRef)
-		}
-
-		expectedType := v.importer.ImportType(listItemType, v.definition, v.operation)
-		actualType = v.operation.ResolveUnderlyingType(actualType)
-
-		if !v.operation.TypesAreEqualDeep(expectedType, actualType) {
-			v.handleVariableHasIncompatibleTypeError(value, definitionTypeRef)
-			return false
-		}
-		return true
+		return v.variableValueSatisfiesDefinitionType(value, definitionTypeRef)
 	}
 
 	if value.Kind == ast.ValueKindNull {
@@ -259,7 +246,6 @@ func (v *valuesVisitor) valueSatisfiesListType(value ast.Value, definitionTypeRe
 
 	return valid
 }
-
 func (v *valuesVisitor) valueSatisfiesTypeDefinitionNode(value ast.Value, definitionTypeRef int, node ast.Node) bool {
 	switch node.Kind {
 	case ast.NodeKindEnumTypeDefinition:
@@ -587,11 +573,16 @@ func (v *valuesVisitor) handleVariableHasIncompatibleTypeError(value ast.Value,
 		return
 	}
 
-	variableDefinitionRef, _, actualTypeName, ok := v.operationVariableType(value.Ref)
+	variableDefinitionRef, variableTypeRef, _, ok := v.operationVariableType(value.Ref)
 	if !ok {
 		return
 	}
 
+	actualTypeName, err := v.operation.PrintTypeBytes(variableTypeRef, nil)
+	if v.HandleInternalErr(err) {
+		return
+	}
+
 	v.Report.AddExternalError(operationreport.ErrVariableTypeDoesntSatisfyInputValueDefinition(
 		printedValue,
 		actualTypeName,

v2/pkg/astvalidation/operation_validation.go

@@ -39,7 +39,6 @@ func DefaultOperationValidator(options ...Option) *OperationValidator {
 	validator.RegisterRule(FieldSelections(opts))
 	validator.RegisterRule(FieldSelectionMerging())
 	validator.RegisterRule(KnownArguments())
-	validator.RegisterRule(ValidArguments())
 	validator.RegisterRule(Values())
 	validator.RegisterRule(ArgumentUniqueness())
 	validator.RegisterRule(RequiredArguments())