Merge pull request #130827 from thockin/kk_refactor_FunctionGen

validation-gen: Simplify FunctionGen and VariableGen

staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go

@@ -1100,7 +1100,7 @@ func emitCallsToValidators(c *generator.Context, validations []validators.Functi
 		later := make([]validators.FunctionGen, 0, len(in))
 
 		for _, fg := range in {
-			isShortCircuit := (fg.Flags().IsSet(validators.ShortCircuit))
+			isShortCircuit := (fg.Flags.IsSet(validators.ShortCircuit))
 
 			if isShortCircuit {
 				sooner = append(sooner, fg)
@@ -1116,19 +1116,17 @@ func emitCallsToValidators(c *generator.Context, validations []validators.Functi
 	validations = sort(validations)
 
 	for _, v := range validations {
-		isShortCircuit := v.Flags().IsSet(validators.ShortCircuit)
+		isShortCircuit := v.Flags.IsSet(validators.ShortCircuit)
-		isNonError := v.Flags().IsSet(validators.NonError)
+		isNonError := v.Flags.IsSet(validators.NonError)
 
-		fn, extraArgs := v.SignatureAndArgs()
 		targs := generator.Args{
-			"funcName": c.Universe.Type(fn),
+			"funcName": c.Universe.Type(v.Function),
 			"field":    mkSymbolArgs(c, fieldPkgSymbols),
 		}
 
 		emitCall := func() {
 			sw.Do("$.funcName|raw$", targs)
-			typeArgs := v.TypeArgs()
+			if typeArgs := v.TypeArgs; len(typeArgs) > 0 {
-			if len(typeArgs) > 0 {
 				sw.Do("[", nil)
 				for i, typeArg := range typeArgs {
 					sw.Do("$.|raw$", c.Universe.Type(typeArg))
@@ -1139,7 +1137,7 @@ func emitCallsToValidators(c *generator.Context, validations []validators.Functi
 				sw.Do("]", nil)
 			}
 			sw.Do("(ctx, op, fldPath, obj, oldObj", targs)
-			for _, arg := range extraArgs {
+			for _, arg := range v.Args {
 				sw.Do(", ", nil)
 				toGolangSourceDataLiteral(sw, c, arg)
 			}
@@ -1147,21 +1145,21 @@ func emitCallsToValidators(c *generator.Context, validations []validators.Functi
 		}
 
 		// If validation is conditional, wrap the validation function with a conditions check.
-		if !v.Conditions().Empty() {
+		if !v.Conditions.Empty() {
 			emitBaseFunction := emitCall
 			emitCall = func() {
 				sw.Do("func() $.field.ErrorList|raw$ {\n", targs)
 				sw.Do("  if ", nil)
 				firstCondition := true
-				if len(v.Conditions().OptionEnabled) > 0 {
+				if len(v.Conditions.OptionEnabled) > 0 {
-					sw.Do("op.Options.Has($.$)", strconv.Quote(v.Conditions().OptionEnabled))
+					sw.Do("op.Options.Has($.$)", strconv.Quote(v.Conditions.OptionEnabled))
 					firstCondition = false
 				}
-				if len(v.Conditions().OptionDisabled) > 0 {
+				if len(v.Conditions.OptionDisabled) > 0 {
 					if !firstCondition {
 						sw.Do(" && ", nil)
 					}
-					sw.Do("!op.Options.Has($.$)", strconv.Quote(v.Conditions().OptionDisabled))
+					sw.Do("!op.Options.Has($.$)", strconv.Quote(v.Conditions.OptionDisabled))
 				}
 				sw.Do(" {\n", nil)
 				sw.Do("    return ", nil)
@@ -1174,7 +1172,7 @@ func emitCallsToValidators(c *generator.Context, validations []validators.Functi
 			}
 		}
 
-		for _, comment := range v.Comments() {
+		for _, comment := range v.Comments {
 			sw.Do("// $.$\n", comment)
 		}
 		if isShortCircuit {
@@ -1214,21 +1212,19 @@ func (g *genValidations) emitValidationVariables(c *generator.Context, t *types.
 
 	variables := tn.typeValidations.Variables
 	slices.SortFunc(variables, func(a, b validators.VariableGen) int {
-		return cmp.Compare(a.Var().Name, b.Var().Name)
+		return cmp.Compare(a.Variable.Name, b.Variable.Name)
 	})
 	for _, variable := range variables {
-		fn := variable.Init()
+		fn := variable.InitFunc
-		supportInitFn, supportInitArgs := fn.SignatureAndArgs()
 		targs := generator.Args{
-			"varName": c.Universe.Type(types.Name(variable.Var())),
+			"varName": c.Universe.Type(types.Name(variable.Variable)),
-			"initFn":  c.Universe.Type(supportInitFn),
+			"initFn":  c.Universe.Type(fn.Function),
 		}
-		for _, comment := range fn.Comments() {
+		for _, comment := range fn.Comments {
 			sw.Do("// $.$\n", comment)
 		}
 		sw.Do("var $.varName|private$ = $.initFn|raw$", targs)
-		typeArgs := variable.Init().TypeArgs()
+		if typeArgs := fn.TypeArgs; len(typeArgs) > 0 {
-		if len(typeArgs) > 0 {
 			sw.Do("[", nil)
 			for i, typeArg := range typeArgs {
 				sw.Do("$.|raw$", c.Universe.Type(typeArg))
@@ -1239,11 +1235,11 @@ func (g *genValidations) emitValidationVariables(c *generator.Context, t *types.
 			sw.Do("]", nil)
 		}
 		sw.Do("(", targs)
-		for i, arg := range supportInitArgs {
+		for i, arg := range fn.Args {
-			toGolangSourceDataLiteral(sw, c, arg)
+			if i != 0 {
-			if i < len(supportInitArgs)-1 {
 				sw.Do(", ", nil)
 			}
+			toGolangSourceDataLiteral(sw, c, arg)
 		}
 		sw.Do(")\n", nil)
 
@@ -1277,14 +1273,13 @@ func toGolangSourceDataLiteral(sw *generator.SnippetWriter, c *generator.Context
 	case *validators.PrivateVar:
 		sw.Do("$.|private$", c.Universe.Type(types.Name(*v)))
 	case validators.WrapperFunction:
-		fn, extraArgs := v.Function.SignatureAndArgs()
+		if extraArgs := v.Function.Args; len(extraArgs) == 0 {
-		if len(extraArgs) == 0 {
 			// If the function to be wrapped has no additional arguments, we can
 			// just use it directly.
 			targs := generator.Args{
-				"funcName": c.Universe.Type(fn),
+				"funcName": c.Universe.Type(v.Function.Function),
 			}
-			for _, comment := range v.Function.Comments() {
+			for _, comment := range v.Function.Comments {
 				sw.Do("// $.$\n", comment)
 			}
 			sw.Do("$.funcName|raw$", targs)
@@ -1292,7 +1287,7 @@ func toGolangSourceDataLiteral(sw *generator.SnippetWriter, c *generator.Context
 			// If the function to be wrapped has additional arguments, we need
 			// a "standard signature" validation function to wrap it.
 			targs := generator.Args{
-				"funcName":   c.Universe.Type(fn),
+				"funcName":   c.Universe.Type(v.Function.Function),
 				"field":      mkSymbolArgs(c, fieldPkgSymbols),
 				"operation":  mkSymbolArgs(c, operationPkgSymbols),
 				"context":    mkSymbolArgs(c, contextPkgSymbols),
@@ -1305,7 +1300,7 @@ func toGolangSourceDataLiteral(sw *generator.SnippetWriter, c *generator.Context
 
 			emitCall := func() {
 				sw.Do("return $.funcName|raw$", targs)
-				typeArgs := v.Function.TypeArgs()
+				typeArgs := v.Function.TypeArgs
 				if len(typeArgs) > 0 {
 					sw.Do("[", nil)
 					for i, typeArg := range typeArgs {

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/each.go

@@ -286,7 +286,7 @@ func (evtv eachValTagValidator) getListValidations(fldPath *field.Path, t *types
 			cmpFn.Body = buf.String()
 			cmpArg = cmpFn
 		}
-		f := Function(eachValTagName, vfn.Flags(), validateEachSliceVal, cmpArg, WrapperFunction{vfn, t.Elem})
+		f := Function(eachValTagName, vfn.Flags, validateEachSliceVal, cmpArg, WrapperFunction{vfn, t.Elem})
 		result.Functions = append(result.Functions, f)
 	}
 
@@ -298,7 +298,7 @@ func (evtv eachValTagValidator) getMapValidations(t *types.Type, validations Val
 	result.OpaqueValType = validations.OpaqueType
 
 	for _, vfn := range validations.Functions {
-		f := Function(eachValTagName, vfn.Flags(), validateEachMapVal, WrapperFunction{vfn, t.Elem})
+		f := Function(eachValTagName, vfn.Flags, validateEachMapVal, WrapperFunction{vfn, t.Elem})
 		result.Functions = append(result.Functions, f)
 	}
 
@@ -369,7 +369,7 @@ func (ektv eachKeyTagValidator) getValidations(t *types.Type, validations Valida
 	result := Validations{}
 	result.OpaqueKeyType = validations.OpaqueType
 	for _, vfn := range validations.Functions {
-		f := Function(eachKeyTagName, vfn.Flags(), validateEachMapKey, WrapperFunction{vfn, t.Key})
+		f := Function(eachKeyTagName, vfn.Flags, validateEachMapKey, WrapperFunction{vfn, t.Key})
 		result.Functions = append(result.Functions, f)
 	}
 	return result, nil

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/registry.go

@@ -165,7 +165,7 @@ func (reg *registry) sortTagsIntoPhases(tags map[string][]gengo.Tag) [][]string
 	// Tag extraction will retain the relative order between 111 and 222, but
 	// 333 is extracted as tag "k8s:ifOptionEnabled".  Those are all in a map,
 	// which we iterate (in a random order).  When it reaches the emit stage,
-	// the "ifOptionEnabled" part is gone, and we will have 3 functionGen
+	// the "ifOptionEnabled" part is gone, and we will have 3 FunctionGen
 	// objects, all with tag "k8s:validateFalse", in a non-deterministic order
 	// because of the map iteration.  If we sort them at that point, we won't
 	// have enough information to do something smart, unless we look at the

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go

@@ -156,7 +156,7 @@ func (rtv requirednessTagValidator) doOptional(context Context) (Validations, er
 			return Validations{}, err
 		}
 		for i, fn := range validations.Functions {
-			validations.Functions[i] = WithComment(fn, "optional fields with default values are effectively required")
+			validations.Functions[i] = fn.WithComment("optional fields with default values are effectively required")
 		}
 		return validations, nil
 	}

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/subfield.go

@@ -100,7 +100,7 @@ func (stv subfieldTagValidator) GetValidations(context Context, args []string, p
 				Results:    []ParamResult{{"", nilableFieldType}},
 			}
 			getFn.Body = fmt.Sprintf("return %so.%s", fieldExprPrefix, submemb.Name)
-			f := Function(subfieldTagName, vfn.Flags(), validateSubfield, subname, getFn, WrapperFunction{vfn, submemb.Type})
+			f := Function(subfieldTagName, vfn.Flags, validateSubfield, subname, getFn, WrapperFunction{vfn, submemb.Type})
 			result.Functions = append(result.Functions, f)
 			result.Variables = append(result.Variables, validations.Variables...)
 		}

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/testing.go

@@ -73,7 +73,7 @@ func (frtv fixedResultTagValidator) GetValidations(context Context, _ []string,
 	if err != nil {
 		return result, fmt.Errorf("can't decode tag payload: %w", err)
 	}
-	result.AddFunction(GenericFunction(frtv.TagName(), tag.flags, fixedResultValidator, tag.typeArgs, frtv.result, tag.msg))
+	result.AddFunction(Function(frtv.TagName(), tag.flags, fixedResultValidator, frtv.result, tag.msg).WithTypeArgs(tag.typeArgs...))
 
 	return result, nil
 }

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go

@@ -207,13 +207,47 @@ type TagPayloadSchema struct {
 	Default string
 }
 
-// Validations defines the function calls and variables to generate to perform validation.
+// Validations defines the function calls and variables to generate to perform
+// validation.
 type Validations struct {
-	Functions     []FunctionGen
+	// Functions holds the function calls that should be generated to perform
-	Variables     []VariableGen
+	// validation.  These functions may not be called in order - they may be
-	Comments      []string
+	// sorted based on their flags and other criteria.
-	OpaqueType    bool
+	//
+	// Each function's signature must be of the form:
+	//   func(
+	//        // standard arguments
+	//        ctx context.Context
+	//        op operation.Operation,
+	//        fldPath field.Path,
+	//        value, oldValue <ValueType>, // always nilable
+	//        // additional arguments (optional)
+	//        Args[0] <Args[0]Type>,
+	//        Args[1] <Args[1]Type>,
+	//        ...
+	//        Args[N] <Args[N]Type>)
+	//
+	// The standard arguments are not included in the FunctionGen.Args list.
+	Functions []FunctionGen
+
+	// Variables holds any variables which must be generated to perform
+	// validation.  Variables are not permitted in every context.
+	Variables []VariableGen
+
+	// Comments holds comments to emit (without the leanding "//").
+	Comments []string
+
+	// OpaqueType indicates that the type being validated is opaque, and that
+	// any validations defined on it should not be emitted.
+	OpaqueType bool
+
+	// OpaqueKeyType indicates that the key type of a map being validated is
+	// opaque, and that any validations defined on it should not be emitted.
 	OpaqueKeyType bool
+
+	// OpaqueValType indicates that the key type of a map or slice being
+	// validated is opaque, and that any validations defined on it should not
+	// be emitted.
 	OpaqueValType bool
 }
 
@@ -225,12 +259,12 @@ func (v *Validations) Len() int {
 	return len(v.Functions) + len(v.Variables) + len(v.Comments)
 }
 
-func (v *Validations) AddFunction(f FunctionGen) {
+func (v *Validations) AddFunction(fn FunctionGen) {
-	v.Functions = append(v.Functions, f)
+	v.Functions = append(v.Functions, fn)
 }
 
-func (v *Validations) AddVariable(variable VariableGen) {
+func (v *Validations) AddVariable(vr VariableGen) {
-	v.Variables = append(v.Variables, variable)
+	v.Variables = append(v.Variables, vr)
 }
 
 func (v *Validations) AddComment(comment string) {
@@ -269,46 +303,6 @@ const (
 	NonError
 )
 
-// FunctionGen provides validation-gen with the information needed to generate a
-// validation function invocation.
-type FunctionGen interface {
-	// TagName returns the tag which triggers this validator.
-	TagName() string
-
-	// SignatureAndArgs returns the function name and all extraArg value literals that are passed when the function
-	// invocation is generated.
-	//
-	// The function signature must be of the form:
-	//   func(op operation.Operation,
-	//        fldPath field.Path,
-	//        value, oldValue <ValueType>,     // always nilable
-	//        extraArgs[0] <extraArgs[0]Type>, // optional
-	//        ...,
-	//        extraArgs[N] <extraArgs[N]Type>)
-	//
-	// extraArgs may contain:
-	// - data literals comprised of maps, slices, strings, ints, floats and bools
-	// - references, represented by types.Type (to reference any type in the universe), and types.Member (to reference members of the current value)
-	//
-	// If validation function to be called does not have a signature of this form, please introduce
-	// a function that does and use that function to call the validation function.
-	SignatureAndArgs() (function types.Name, extraArgs []any)
-
-	// TypeArgs assigns types to the type parameters of the function, for invocation.
-	TypeArgs() []types.Name
-
-	// Flags returns the options for this validator function.
-	Flags() FunctionFlags
-
-	// Conditions returns the conditions that must true for a resource to be
-	// validated by this function.
-	Conditions() Conditions
-
-	// Comments returns optional comments that should be added to the generated
-	// code (without the leading "//").
-	Comments() []string
-}
-
 // Conditions defines what conditions must be true for a resource to be validated.
 // If any of the conditions are not true, the resource is not validated.
 type Conditions struct {
@@ -331,115 +325,85 @@ type Identifier types.Name
 // PrivateVars are generated using the PrivateNamer strategy.
 type PrivateVar types.Name
 
-// VariableGen provides validation-gen with the information needed to generate variable.
-// Variables typically support generated functions by providing static information such
-// as the list of supported symbols for an enum.
-type VariableGen interface {
-	// TagName returns the tag which triggers this validator.
-	TagName() string
-
-	// Var returns the variable identifier.
-	Var() PrivateVar
-
-	// Init generates the function call that the variable is assigned to.
-	Init() FunctionGen
-}
-
 // Function creates a FunctionGen for a given function name and extraArgs.
 func Function(tagName string, flags FunctionFlags, function types.Name, extraArgs ...any) FunctionGen {
-	return GenericFunction(tagName, flags, function, nil, extraArgs...)
+	return FunctionGen{
-}
+		TagName:  tagName,
-
+		Flags:    flags,
-func GenericFunction(tagName string, flags FunctionFlags, function types.Name, typeArgs []types.Name, extraArgs ...any) FunctionGen {
+		Function: function,
-	// Callers of Signature don't care if the args are all of a known type, it just
+		Args:     extraArgs,
-	// makes it easier to declare validators.
-	var anyArgs []any
-	if len(extraArgs) > 0 {
-		anyArgs = make([]any, len(extraArgs))
-		copy(anyArgs, extraArgs)
-	}
-	return &functionGen{tagName: tagName, flags: flags, function: function, extraArgs: anyArgs, typeArgs: typeArgs}
-}
-
-// WithCondition adds a condition to a FunctionGen.
-func WithCondition(fn FunctionGen, conditions Conditions) FunctionGen {
-	name, args := fn.SignatureAndArgs()
-	return &functionGen{
-		tagName:    fn.TagName(),
-		flags:      fn.Flags(),
-		function:   name,
-		extraArgs:  args,
-		typeArgs:   fn.TypeArgs(),
-		comments:   fn.Comments(),
-		conditions: conditions,
 	}
 }
 
-// WithComment adds a comment to a FunctionGen.
+// FunctionGen describes a function call that should be generated.
-func WithComment(fn FunctionGen, comment string) FunctionGen {
+type FunctionGen struct {
-	name, args := fn.SignatureAndArgs()
+	// TagName is the tag which triggered this function.
-	return &functionGen{
+	TagName string
-		tagName:    fn.TagName(),
+
-		flags:      fn.Flags(),
+	// Flags holds the options for this validator function.
-		function:   name,
+	Flags FunctionFlags
-		extraArgs:  args,
+
-		typeArgs:   fn.TypeArgs(),
+	// Function is the name of the function to call.
-		comments:   append(fn.Comments(), comment),
+	Function types.Name
-		conditions: fn.Conditions(),
+
-	}
+	// Args holds arguments to pass to the function, and may conatin:
+	// - data literals comprised of maps, slices, strings, ints, floats, and bools
+	// - types.Type (to reference any type in the universe)
+	// - types.Member (to reference members of the current value)
+	// - types.Identifier (to reference any identifier in the universe)
+	// - validators.WrapperFunction (to call another validation function)
+	// - validators.Literal (to pass a literal value)
+	// - validators.FunctionLiteral (to pass a function literal)
+	// - validators.PrivateVar (to reference a variable)
+	//
+	// See toGolangSourceDataLiteral for details.
+	Args []any
+
+	// TypeArgs assigns types to the type parameters of the function, for
+	// generic function calls which require explicit type arguments.
+	TypeArgs []types.Name
+
+	// Conditions holds any conditions that must true for a field to be
+	// validated by this function.
+	Conditions Conditions
+
+	// Comments holds optional comments that should be added to the generated
+	// code (without the leading "//").
+	Comments []string
 }
 
-type functionGen struct {
+// WithTypeArgs returns a derived FunctionGen with type arguments.
-	tagName    string
+func (fg FunctionGen) WithTypeArgs(typeArgs ...types.Name) FunctionGen {
-	function   types.Name
+	fg.TypeArgs = typeArgs
-	extraArgs  []any
+	return fg
-	typeArgs   []types.Name
-	flags      FunctionFlags
-	conditions Conditions
-	comments   []string
 }
 
-func (v *functionGen) TagName() string {
+// WithConditions returns a derived FunctionGen with conditions.
-	return v.tagName
+func (fg FunctionGen) WithConditions(conditions Conditions) FunctionGen {
+	fg.Conditions = conditions
+	return fg
 }
 
-func (v *functionGen) SignatureAndArgs() (function types.Name, args []any) {
+// WithComment returns a new FunctionGen with a comment.
-	return v.function, v.extraArgs
+func (fg FunctionGen) WithComment(comment string) FunctionGen {
+	fg.Comments = append(fg.Comments, comment)
+	return fg
 }
 
-func (v *functionGen) TypeArgs() []types.Name { return v.typeArgs }
-
-func (v *functionGen) Flags() FunctionFlags {
-	return v.flags
-}
-
-func (v *functionGen) Conditions() Conditions { return v.conditions }
-
-func (v *functionGen) Comments() []string { return v.comments }
-
 // Variable creates a VariableGen for a given function name and extraArgs.
-func Variable(variable PrivateVar, init FunctionGen) VariableGen {
+func Variable(variable PrivateVar, initFunc FunctionGen) VariableGen {
-	return &variableGen{
+	return VariableGen{
-		variable: variable,
+		Variable: variable,
-		init:     init,
+		InitFunc: initFunc,
 	}
 }
 
-type variableGen struct {
+type VariableGen struct {
-	variable PrivateVar
+	// Variable holds the variable identifier.
-	init     FunctionGen
+	Variable PrivateVar
-}
 
-func (v variableGen) TagName() string {
+	// InitFunc describes the function call that the variable is assigned to.
-	return v.init.TagName()
+	InitFunc FunctionGen
-}
-
-func (v variableGen) Var() PrivateVar {
-	return v.variable
-}
-
-func (v variableGen) Init() FunctionGen {
-	return v.init
 }
 
 // WrapperFunction describes a function literal which has the fingerprint of a