Retool validator utils realType and unaliasType

Previous commits focused on checking for pointers and aliases more appropriately.

What emerged was two functions `realType` which gave us the concrete
native type (non-pointer, non-alias) an `unaliasType` which stripped
aliasing but only until it hit pointerness.  They were sort of
inconsistently applied and in a few places I found the composition of
them to be weird.

I want to reframe those as:

1) `func nativeType(t) t` which removes all aliasing but retains pointerness.  So for example:
```
type T1 string
type T2 *T1
type T3 *T2

type Foo struct {
    Name *T3
}
```

Calling `nativeType()` on the `Name` member would yield `***string`.
This is what we want in most places.  We need to retain pointerness
because (e.g.) we DO NOT support pointer to slice, and we want that to
fail, but we DO support pointer to string and we want that to succeed,
but `optional` needs to handle pointers and non-pointers differently.

We filter totally disallowed things early on, but if a pointer to slice
should sneak in somehow, this will refuse to generate bad code (fail in
an obvious way).

2) `func nonPointer(t) t` which removes all pointerness (if present),
but does not look past aliases. We needed to compose these both and call
`nonPointer(nativeType(t))` in a few places. E.g. when applying
`+k8s:format`, we need to know that the field is a string OR pointer to
string.

These two are more composable, I think.  If you agree I can finish that
off this weekend and ping the PR.  If not, would like to hear your
thoughts.  Mostly the goal is put as many defensive checks in place as
possible.  New validators will copy these ones and I want it to be as
clear as possible.

Author: thockin

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

@@ -40,37 +40,64 @@ func getMemberByJSON(t *types.Type, jsonName string) *types.Member {
 
 // isNilableType returns true if the argument type can be compared to nil.
 func isNilableType(t *types.Type) bool {
-	switch unaliasType(t).Kind {
+	for t.Kind == types.Alias {
+		t = t.Underlying
+	}
+	switch t.Kind {
 	case types.Pointer, types.Map, types.Slice, types.Interface: // Note: Arrays are not nilable
 		return true
 	}
 	return false
 }
 
-// unaliasType returns the underlying type of the specified type if it is an
-// alias, otherwise returns the type itself.
-func unaliasType(t *types.Type) *types.Type {
-	for t.Kind == types.Alias {
-		t = t.Underlying
-	}
-	return t
-}
-
-// realType returns the underlying type of a type, unwrapping aliases and
-// dropping pointerness.
-func realType(t *types.Type) *types.Type {
+// nativeType returns the Go native type of the argument type, with any
+// intermediate typedefs removed. Go itself already flattens typedefs, but this
+// handles it in the unlikely event that we ever fix that.
+//
+// Examples:
+// * Trivial:
+//   - given `int`, returns `int`
+//   - given `*int`, returns `*int`
+//   - given `[]int`, returns `[]int`
+//
+// * Typedefs
+//   - given `type X int; X`, returns `int`
+//   - given `type X int; []X`, returns `[]X`
+//
+// * Typedefs and pointers:
+//   - given `type X int; *X`, returns `*int`
+//   - given `type X *int; *X`, returns `**int`
+//   - given `type X []int; X`, returns `[]int`
+//   - given `type X []int; *X`, returns `*[]int`
+func nativeType(t *types.Type) *types.Type {
+	ptrs := 0
 	for {
 		if t.Kind == types.Alias {
 			t = t.Underlying
 		} else if t.Kind == types.Pointer {
+			ptrs++
 			t = t.Elem
 		} else {
 			break
 		}
 	}
+	for range ptrs {
+		t = types.PointerTo(t)
+	}
+	return t
+}
+
+// nonPointer returns the value-type of a possibly pointer type. If type is not
+// a pointer, it returns the input type.
+func nonPointer(t *types.Type) *types.Type {
+	for t.Kind == types.Pointer {
+		t = t.Elem
+	}
 	return t
 }
 
+// rootTypeString returns a string representation of the relationship between
+// src and dst types, for use in error messages.
 func rootTypeString(src, dst *types.Type) string {
 	if src == dst {
 		return src.String()

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

@@ -76,9 +76,8 @@ func (listTypeTagValidator) ValidScopes() sets.Set[Scope] {
 }
 
 func (lttv listTypeTagValidator) GetValidations(context Context, _ []string, payload string) (Validations, error) {
-	// We don't support pointers to lists, but other validators use realType()
-	// for this sort of check, so let's be consistent.
-	t := realType(context.Type)
+	// NOTE: pointers to lists are not supported, so we should never see a pointer here.
+	t := nativeType(context.Type)
 	if t.Kind != types.Slice && t.Kind != types.Array {
 		return Validations{}, fmt.Errorf("can only be used on list types")
 	}
@@ -87,8 +86,8 @@ func (lttv listTypeTagValidator) GetValidations(context Context, _ []string, pay
 	case "atomic", "set":
 		// Allowed but no special handling.
 	case "map":
-		// NOTE: maps of pointers are not supported, and that is enforced way before this point.
-		if realType(t.Elem).Kind != types.Struct {
+		// NOTE: maps of pointers are not supported, so we should never see a pointer here.
+		if nativeType(t.Elem).Kind != types.Struct {
 			return Validations{}, fmt.Errorf("only lists of structs can be list-maps")
 		}
 
@@ -135,23 +134,21 @@ func (listMapKeyTagValidator) ValidScopes() sets.Set[Scope] {
 }
 
 func (lmktv listMapKeyTagValidator) GetValidations(context Context, _ []string, payload string) (Validations, error) {
-	// We don't support pointers to lists, but other validators use realType()
-	// for this sort of check, so let's be consistent.
-	t := realType(context.Type)
+	// NOTE: pointers to lists are not supported, so we should never see a pointer here.
+	t := nativeType(context.Type)
 	if t.Kind != types.Slice && t.Kind != types.Array {
 		return Validations{}, fmt.Errorf("can only be used on list types")
 	}
-	if realType(t.Elem).Kind != types.Struct {
+	// NOTE: lists of pointers are not supported, so we should never see a pointer here.
+	if nativeType(t.Elem).Kind != types.Struct {
 		return Validations{}, fmt.Errorf("only lists of structs can be list-maps")
 	}
 
 	var fieldName string
-	if memb := getMemberByJSON(realType(t.Elem), payload); memb == nil {
+	if memb := getMemberByJSON(nativeType(t.Elem), payload); memb == nil {
 		return Validations{}, fmt.Errorf("no field for JSON name %q", payload)
-	} else if k := realType(memb.Type).Kind; k != types.Builtin {
-		return Validations{}, fmt.Errorf("only primitive types can be list-map keys, not %s", k)
-	} else if isPointer(memb.Type) {
-		return Validations{}, fmt.Errorf("pointer types cannot be list-map keys")
+	} else if k := nativeType(memb.Type).Kind; k != types.Builtin {
+		return Validations{}, fmt.Errorf("only primitive types can be list-map keys (%s)", k)
 	} else {
 		fieldName = memb.Name
 	}
@@ -167,16 +164,6 @@ func (lmktv listMapKeyTagValidator) GetValidations(context Context, _ []string,
 	return Validations{}, nil
 }
 
-func isPointer(t *types.Type) bool {
-	if t.Kind == types.Pointer {
-		return true
-	}
-	if t.Kind == types.Alias {
-		return isPointer(t.Underlying)
-	}
-	return false
-}
-
 func (lmktv listMapKeyTagValidator) Docs() TagDoc {
 	doc := TagDoc{
 		Tag:         lmktv.TagName(),
@@ -217,9 +204,8 @@ var (
 )
 
 func (evtv eachValTagValidator) GetValidations(context Context, _ []string, payload string) (Validations, error) {
-	// We don't support pointers to lists, but other validators use realType()
-	// for this sort of check, so let's be consistent.
-	t := realType(context.Type)
+	// NOTE: pointers to lists and maps are not supported, so we should never see a pointer here.
+	t := nativeType(context.Type)
 	switch t.Kind {
 	case types.Slice, types.Array, types.Map:
 	default:
@@ -351,9 +337,8 @@ var (
 )
 
 func (ektv eachKeyTagValidator) GetValidations(context Context, _ []string, payload string) (Validations, error) {
-	// We don't support pointers to lists, but other validators use realType()
-	// for this sort of check, so let's be consistent.
-	t := realType(context.Type)
+	// NOTE: pointers to lists are not supported, so we should never see a pointer here.
+	t := nativeType(context.Type)
 	if t.Kind != types.Map {
 		return Validations{}, fmt.Errorf("can only be used on map types")
 	}

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

@@ -51,7 +51,7 @@ var (
 func (immutableTagValidator) GetValidations(context Context, _ []string, payload string) (Validations, error) {
 	var result Validations
 
-	if realType(context.Type).Kind == types.Builtin {
+	if nonPointer(nativeType(context.Type)).Kind == types.Builtin {
 		// This is a minor optimization to just compare primitive values when
 		// possible. Slices and maps are not comparable, and structs might hold
 		// pointer fields, which are directly comparable but not what we need.

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

@@ -57,7 +57,7 @@ func (minimumTagValidator) GetValidations(context Context, _ []string, payload s
 
 	// This tag can apply to value and pointer fields, as well as typedefs
 	// (which should never be pointers). We need to check the concrete type.
-	if t := realType(context.Type); !types.IsInteger(t) {
+	if t := nonPointer(nativeType(context.Type)); !types.IsInteger(t) {
 		return result, fmt.Errorf("can only be used on integer types (%s)", rootTypeString(context.Type, t))
 	}
 

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

@@ -90,7 +90,7 @@ func (rtv requirednessTagValidator) doRequired(context Context) (Validations, er
 	// originally defined as a value-type or a pointer-type in the API.  This
 	// one does.  Since Go doesn't do partial specialization of templates, we
 	// do manual dispatch here.
-	switch unaliasType(context.Type).Kind {
+	switch nativeType(context.Type).Kind {
 	case types.Slice:
 		return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredSliceValidator)}}, nil
 	case types.Map:
@@ -161,7 +161,7 @@ func (rtv requirednessTagValidator) doOptional(context Context) (Validations, er
 	// originally defined as a value-type or a pointer-type in the API.  This
 	// one does.  Since Go doesn't do partial specialization of templates, we
 	// do manual dispatch here.
-	switch unaliasType(context.Type).Kind {
+	switch nativeType(context.Type).Kind {
 	case types.Slice:
 		return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalSliceValidator)}}, nil
 	case types.Map:
@@ -182,7 +182,7 @@ func (rtv requirednessTagValidator) doOptional(context Context) (Validations, er
 // hasZeroDefault returns whether the field has a default value and whether
 // that default value is the zero value for the field's type.
 func (rtv requirednessTagValidator) hasZeroDefault(context Context) (bool, bool, error) {
-	t := realType(context.Type)
+	t := nonPointer(nativeType(context.Type))
 	// This validator only applies to fields, so Member must be valid.
 	tagsByName, err := gengo.ExtractFunctionStyleCommentTags("+", []string{defaultTagName}, context.Member.CommentLines)
 	if err != nil {
@@ -261,7 +261,7 @@ func (requirednessTagValidator) doForbidden(context Context) (Validations, error
 	// optional check and short-circuit (but without error).  Why?  For
 	// example, this prevents any further validation from trying to run on a
 	// nil pointer.
-	switch unaliasType(context.Type).Kind {
+	switch nativeType(context.Type).Kind {
 	case types.Slice:
 		return Validations{
 			Functions: []FunctionGen{

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

@@ -56,7 +56,7 @@ var (
 func (stv subfieldTagValidator) GetValidations(context Context, args []string, payload string) (Validations, error) {
 	// This tag can apply to value and pointer fields, as well as typedefs
 	// (which should never be pointers). We need to check the concrete type.
-	t := realType(context.Type)
+	t := nonPointer(nativeType(context.Type))
 	if t.Kind != types.Struct {
 		return Validations{}, fmt.Errorf("can only be used on struct types")
 	}