Add type information to tag Docs() and use it to typecheck tags

This commit is contained in:
Joe Betz
2025-06-03 11:17:48 -04:00
parent 31aa3c2029
commit 28e99ef2ee
7 changed files with 300 additions and 28 deletions

View File

@@ -116,6 +116,8 @@ func (lttv listTypeTagValidator) Docs() TagDoc {
Description: "<type>",
Docs: "map | atomic",
}},
PayloadsType: codetags.ValueTypeString,
PayloadsRequired: true,
}
return doc
}
@@ -174,6 +176,8 @@ func (lmktv listMapKeyTagValidator) Docs() TagDoc {
Description: "<field-json-name>",
Docs: "The name of the field.",
}},
PayloadsType: codetags.ValueTypeString,
PayloadsRequired: true,
}
return doc
}
@@ -318,6 +322,8 @@ func (evtv eachValTagValidator) Docs() TagDoc {
Description: "<validation-tag>",
Docs: "The tag to evaluate for each value.",
}},
PayloadsType: codetags.ValueTypeTag,
PayloadsRequired: true,
}
return doc
}
@@ -355,9 +361,6 @@ func (ektv eachKeyTagValidator) GetValidations(context Context, tag codetags.Tag
Parent: t,
Path: context.Path.Child("(keys)"),
}
if tag.ValueTag == nil {
return Validations{}, fmt.Errorf("missing validation tag")
}
if validations, err := ektv.validator.ExtractValidations(elemContext, *tag.ValueTag); err != nil {
return Validations{}, err
} else {
@@ -394,6 +397,8 @@ func (ektv eachKeyTagValidator) Docs() TagDoc {
Description: "<validation-tag>",
Docs: "The tag to evaluate for each value.",
}},
PayloadsType: codetags.ValueTypeTag,
PayloadsRequired: true,
}
return doc
}

View File

@@ -79,5 +79,7 @@ func (mtv minimumTagValidator) Docs() TagDoc {
Description: "<integer>",
Docs: "This field must be greater than or equal to x.",
}},
PayloadsType: codetags.ValueTypeInt,
PayloadsRequired: true,
}
}

View File

@@ -134,6 +134,9 @@ func (reg *registry) ExtractValidations(context Context, tags ...codetags.Tag) (
if scopes := tv.ValidScopes(); !scopes.Has(context.Scope) && !scopes.Has(ScopeAny) {
return Validations{}, fmt.Errorf("tag %q cannot be specified on %s", tv.TagName(), context.Scope)
}
if err := typeCheck(tag, tv.Docs()); err != nil {
return Validations{}, fmt.Errorf("tag %q: %w", tv.TagName(), err)
}
if theseValidations, err := tv.GetValidations(context, tag); err != nil {
return Validations{}, fmt.Errorf("tag %q: %w", tv.TagName(), err)
} else {

View File

@@ -62,26 +62,18 @@ func (stv subfieldTagValidator) GetValidations(context Context, tag codetags.Tag
if t.Kind != types.Struct {
return Validations{}, fmt.Errorf("can only be used on struct types")
}
if len(args) != 1 || len(args[0].Name) != 0 || args[0].Type != codetags.ArgTypeString {
return Validations{}, fmt.Errorf("requires exactly 1 positional string argument")
}
subname := args[0].Value
submemb := getMemberByJSON(t, subname)
if submemb == nil {
return Validations{}, fmt.Errorf("no field for json name %q", subname)
}
result := Validations{}
subContext := Context{
Scope: ScopeField,
Type: submemb.Type,
Parent: t,
Path: context.Path.Child(subname),
}
if tag.ValueTag == nil {
return Validations{}, fmt.Errorf("missing validation tag")
}
if validations, err := stv.validator.ExtractValidations(subContext, *tag.ValueTag); err != nil {
return Validations{}, err
} else {
@@ -121,12 +113,16 @@ func (stv subfieldTagValidator) Docs() TagDoc {
Description: "Declares a validation for a subfield of a struct.",
Args: []TagArgDoc{{
Description: "<field-json-name>",
Type: codetags.ArgTypeString,
Required: true,
}},
Docs: "The named subfield must be a direct field of the struct, or of an embedded struct.",
Payloads: []TagPayloadDoc{{
Description: "<validation-tag>",
Docs: "The tag to evaluate for the subfield.",
}},
PayloadsType: codetags.ValueTypeTag,
PayloadsRequired: true,
}
return doc
}

View File

@@ -124,6 +124,7 @@ func (frtv fixedResultTagValidator) Docs() TagDoc {
Tag: frtv.TagName(),
Scopes: frtv.ValidScopes().UnsortedList(),
}
doc.PayloadsType = codetags.ValueTypeString
if frtv.error {
doc.Description = "Always fails code generation (useful for testing)."
doc.Payloads = []TagPayloadDoc{{
@@ -135,21 +136,19 @@ func (frtv fixedResultTagValidator) Docs() TagDoc {
doc.Payloads = []TagPayloadDoc{{
Description: "<none>",
}, {
Description: "<quoted-string>",
Description: "<string>",
Docs: "The generated code will include this string.",
}}
doc.Args = []TagArgDoc{{
Name: "flags",
Description: "<list-of-flag-string>",
Description: "<comma-separated-list-of-flag-string>",
Docs: `values: ShortCircuit, NonError`,
}, {
Name: "msg",
Description: "<string>",
Docs: "The generated code will include this string.",
Type: codetags.ArgTypeString,
}, {
Name: "typeArg",
Description: "<string>",
Docs: "The type arg in generated code (must be the value-type, not pointer).",
Type: codetags.ArgTypeString,
}}
if frtv.result {
doc.Description = "Always passes validation (useful for testing)."

View File

@@ -17,6 +17,8 @@ limitations under the License.
package validators
import (
"fmt"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/gengo/v2/codetags"
@@ -176,6 +178,19 @@ type TagDoc struct {
// never has a payload, this list should be empty, but if the payload is
// optional, this list should include an entry for "<none>".
Payloads []TagPayloadDoc
// PayloadsType is the type of the payloads.
PayloadsType codetags.ValueType
// PayloadsRequired is true if a payload is required.
PayloadsRequired bool
}
func (td TagDoc) Arg(name string) (TagArgDoc, bool) {
for _, arg := range td.Args {
if arg.Name == name {
return arg, true
}
}
return TagArgDoc{}, false
}
// TagArgDoc describes an argument for a tag.
@@ -189,9 +204,13 @@ type TagArgDoc struct {
Name string
// Description is a short description of this arg (e.g. `<name>`).
Description string
// Type is the type of the arg.
Type codetags.ArgType
// Required is true if the argument is required.
Required bool
// Default is the effective value if no value is provided.
Default string
// Docs is a human-orientd string explaining this payload.
// Docs is a human-oriented string explaining this arg.
Docs string
}
@@ -202,16 +221,6 @@ type TagPayloadDoc struct {
Description string
// Docs is a human-oriented string explaining this payload.
Docs string
// Schema details a JSON payload's contents.
Schema []TagPayloadSchema
}
// TagPayloadSchema describes a JSON tag payload.
type TagPayloadSchema struct {
Key string
Value string
Docs string
Default string
}
// Validations define the function calls and variables to generate to perform
@@ -440,3 +449,42 @@ type ParamResult struct {
Name string
Type *types.Type
}
// typeCheck checks that the argument and value types of the tag match the types
// declared in the doc.
func typeCheck(tag codetags.Tag, doc TagDoc) error {
for _, docArg := range doc.Args {
hasArg := false
for _, tagArg := range tag.Args {
if tagArg.Name == docArg.Name {
hasArg = true
if docArg.Type != tagArg.Type {
return fmt.Errorf("argument %q has wrong type: got %s, want %s",
tagArg, tagArg.Type, docArg.Type)
}
break
}
}
if !hasArg && docArg.Required {
if docArg.Name == "" {
return fmt.Errorf("missing required positional argument of type %s", docArg.Type)
} else {
return fmt.Errorf("missing named argument %q of type %s", docArg.Name, docArg.Type)
}
}
}
for _, tagArg := range tag.Args {
if _, ok := doc.Arg(tagArg.Name); !ok {
return fmt.Errorf("unrecognized named argument %q", tagArg)
}
}
if tag.ValueType == codetags.ValueTypeNone {
if doc.PayloadsRequired {
return fmt.Errorf("missing required tag value of type %s", doc.PayloadsType)
}
} else if tag.ValueType != doc.PayloadsType {
return fmt.Errorf("tag value has wrong type: got %s, want %s", tag.ValueType, doc.PayloadsType)
}
return nil
}

View File

@@ -0,0 +1,219 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package validators
import (
"strings"
"testing"
"k8s.io/gengo/v2/codetags"
)
func TestTypeCheck(t *testing.T) {
mkt := func(args []codetags.Arg, valueType codetags.ValueType) codetags.Tag {
return codetags.Tag{
Args: args,
ValueType: valueType,
}
}
mkta := func(name string, value string, argType codetags.ArgType) codetags.Arg {
return codetags.Arg{
Name: name,
Value: value,
Type: argType,
}
}
mkd := func(args []TagArgDoc, payloadsRequired bool, payloadsType codetags.ValueType) TagDoc {
return TagDoc{
Args: args,
PayloadsRequired: payloadsRequired,
PayloadsType: payloadsType,
}
}
mkda := func(name string, argType codetags.ArgType, required bool) TagArgDoc {
return TagArgDoc{
Name: name,
Type: argType,
Required: required,
}
}
tests := []struct {
name string
tag codetags.Tag
doc TagDoc
wantErr string
}{
{
name: "valid tag with no args",
tag: mkt(nil, codetags.ValueTypeNone),
doc: mkd(nil, false, codetags.ValueTypeNone),
},
// named args
{
name: "valid tag with required named args",
tag: mkt([]codetags.Arg{
mkta("arg1", "value1", codetags.ArgTypeString),
mkta("arg2", "value2", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("arg1", codetags.ArgTypeString, true),
mkda("arg2", codetags.ArgTypeString, true),
}, false, codetags.ValueTypeNone),
},
{
name: "valid tag with optional named args",
tag: mkt([]codetags.Arg{
mkta("arg1", "value1", codetags.ArgTypeString),
mkta("arg2", "value2", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("arg1", codetags.ArgTypeString, false),
mkda("arg2", codetags.ArgTypeString, false),
}, false, codetags.ValueTypeNone),
},
{
name: "valid tag without optional named args",
tag: mkt([]codetags.Arg{}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("arg1", codetags.ArgTypeString, false),
mkda("arg2", codetags.ArgTypeString, false),
}, false, codetags.ValueTypeNone),
},
{
name: "missing required named argument",
tag: mkt(nil, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("arg1", codetags.ArgTypeString, true),
}, false, codetags.ValueTypeNone),
wantErr: "missing named argument",
},
{
name: "named argument with wrong type",
tag: mkt([]codetags.Arg{
mkta("arg1", "value1", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("arg1", codetags.ArgTypeInt, true),
}, false, codetags.ValueTypeNone),
wantErr: "has wrong type: got string, want int",
},
{
name: "unrecognized named argument",
tag: mkt([]codetags.Arg{
mkta("arg1", "value1", codetags.ArgTypeString),
mkta("arg2", "value2", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("arg1", codetags.ArgTypeString, true),
}, false, codetags.ValueTypeNone),
wantErr: "unrecognized named argument",
},
// positional arg
{
name: "valid tag with required positional arg",
tag: mkt([]codetags.Arg{
mkta("", "value1", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("", codetags.ArgTypeString, true),
}, false, codetags.ValueTypeNone),
},
{
name: "valid tag with optional positional arg",
tag: mkt([]codetags.Arg{
mkta("", "value1", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("", codetags.ArgTypeString, false),
}, false, codetags.ValueTypeNone),
},
{
name: "valid tag without optional positional arg",
tag: mkt([]codetags.Arg{}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("", codetags.ArgTypeString, false),
}, false, codetags.ValueTypeNone),
},
{
name: "missing required positional argument",
tag: mkt(nil, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("", codetags.ArgTypeString, true),
}, false, codetags.ValueTypeNone),
wantErr: "missing required positional argument",
},
{
name: "positional argument with wrong type",
tag: mkt([]codetags.Arg{
mkta("", "value1", codetags.ArgTypeString),
}, codetags.ValueTypeNone),
doc: mkd([]TagArgDoc{
mkda("", codetags.ArgTypeInt, true),
}, false, codetags.ValueTypeNone),
wantErr: "has wrong type: got string, want int",
},
// values
{
name: "valid required tag value",
tag: mkt(nil, codetags.ValueTypeString),
doc: mkd(nil, true, codetags.ValueTypeString),
},
{
name: "valid with optional tag value",
tag: mkt(nil, codetags.ValueTypeString),
doc: mkd(nil, false, codetags.ValueTypeString),
},
{
name: "valid without optional tag value",
tag: mkt(nil, codetags.ValueTypeNone),
doc: mkd(nil, false, codetags.ValueTypeString),
},
{
name: "missing required tag value",
tag: mkt(nil, codetags.ValueTypeNone),
doc: mkd(nil, true, codetags.ValueTypeString),
wantErr: "missing required tag value",
},
{
name: "tag value with wrong type",
tag: mkt(nil, codetags.ValueTypeString),
doc: mkd(nil, false, codetags.ValueTypeInt),
wantErr: "tag value has wrong type",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := typeCheck(tt.tag, tt.doc)
if (len(tt.wantErr) == 0) != (err == nil) {
t.Errorf("typeCheck() error = %v, wantErr = %v", err, tt.wantErr)
}
if err != nil && tt.wantErr != "" {
if !strings.Contains(err.Error(), tt.wantErr) {
t.Errorf("typeCheck() error = %v, wantErr = %v", err, tt.wantErr)
}
}
})
}
}