Make validation-gen lint errors easier to read

This started with fixing the "+default" tag, but I realized the errors
could be more readable.

* Move lint rules to lint_rules.go
* Make lintErrors a map of type -> []error
* Keep 1 linter for all pkgs, rather than create/destroy
* Make conflictingTagsRule() return a lintRule, making the default rules
  simpler and fail at startup if too few tags
* Added a human-friendly "message" arg to conflictingTagsRule() to be
  printed when the rule fails

Now we get:

```
go run k8s.io/code-generator/cmd/validation-gen --output-file zz_generated.validations.go --go-header-file=../../../examples/hack/boilerplate.go.txt ./...
F1228 15:38:48.953915   82030 targets.go:309] lint failed:
  type k8s.io/code-generator/cmd/validation-gen/output_tests/optional.T1:
    field S: conflicting tags: {+k8s:optional, +k8s:required}: fields cannot be both optional and required
    field S: conflicting tags: {+k8s:required, +default}: fields with default values are always optional
```

Author: thockin

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

@@ -18,6 +18,7 @@ package main
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"k8s.io/gengo/v2/types"
@@ -30,30 +31,34 @@ import (
 type linter struct {
 	linted map[*types.Type]bool
 	rules  []lintRule
-	// lintErrors is a list of errors that occurred during the linting process.
-	// lintErrors would be in the format:
-	// field <field_name>: <lint broken message>
-	// type <type_name>: <lint broken message>
-	lintErrors []error
+	// lintErrors is all the errors, grouped by type, that occurred during the
+	// linting process.
+	lintErrors map[*types.Type][]error
 }
 
-var defaultRules = []lintRule{
-	ruleOptionalAndRequired,
-	ruleRequiredAndDefault,
-}
+// lintRule is a function that validates a slice of comments.
+// It returns a string as an error message if the comments are invalid,
+// and an error there is an error happened during the linting process.
+type lintRule func(comments []string) (string, error)
 
-func (l *linter) AddError(field, msg string) {
-	l.lintErrors = append(l.lintErrors, fmt.Errorf("%s: %s", field, msg))
+func (l *linter) AddError(t *types.Type, field, msg string) {
+	var err error
+	if field == "" {
+		err = fmt.Errorf("%s", msg)
+	} else {
+		err = fmt.Errorf("field %s: %s", field, msg)
+	}
+	l.lintErrors[t] = append(l.lintErrors[t], err)
 }
 
 func newLinter(rules ...lintRule) *linter {
 	if len(rules) == 0 {
-		rules = defaultRules
+		rules = defaultLintRules
 	}
 	return &linter{
 		linted:     make(map[*types.Type]bool),
 		rules:      rules,
-		lintErrors: []error{},
+		lintErrors: map[*types.Type][]error{},
 	}
 }
 
@@ -70,7 +75,7 @@ func (l *linter) lintType(t *types.Type) error {
 			return err
 		}
 		for _, lintErr := range lintErrs {
-			l.AddError("type "+t.Name.String(), lintErr)
+			l.AddError(t, "", lintErr)
 		}
 	}
 	switch t.Kind {
@@ -88,7 +93,7 @@ func (l *linter) lintType(t *types.Type) error {
 				return err
 			}
 			for _, lintErr := range lintErrs {
-				l.AddError("type "+t.Name.String(), lintErr)
+				l.AddError(t, member.Name, lintErr)
 			}
 			if err := l.lintType(member.Type); err != nil {
 				return err
@@ -111,11 +116,6 @@ func (l *linter) lintType(t *types.Type) error {
 	return nil
 }
 
-// lintRule is a function that validates a slice of comments.
-// It returns a string as an error message if the comments are invalid,
-// and an error there is an error happened during the linting process.
-type lintRule func(comments []string) (string, error)
-
 // lintComments runs all registered rules on a slice of comments.
 func (l *linter) lintComments(comments []string) ([]string, error) {
 	var lintErrs []string
@@ -130,31 +130,29 @@ func (l *linter) lintComments(comments []string) ([]string, error) {
 	return lintErrs, nil
 }
 
-// conflictingTagsRule checks for conflicting tags in a slice of comments.
-func conflictingTagsRule(comments []string, tags ...string) (string, error) {
+// conflictingTagsRule creates a lintRule which checks for conflicting tags.
+func conflictingTagsRule(msg string, tags ...string) lintRule {
 	if len(tags) < 2 {
-		return "", fmt.Errorf("at least two tags must be provided")
+		panic("conflictingTagsRule: at least 2 tags must be specified")
 	}
-	tagCount := make(map[string]bool)
-	for _, comment := range comments {
-		for _, tag := range tags {
-			if strings.HasPrefix(comment, tag) {
-				tagCount[tag] = true
+
+	return func(comments []string) (string, error) {
+		found := make(map[string]bool)
+		for _, comment := range comments {
+			for _, tag := range tags {
+				if strings.HasPrefix(comment, tag) {
+					found[tag] = true
+				}
 			}
 		}
+		if len(found) > 1 {
+			keys := make([]string, 0, len(found))
+			for k := range found {
+				keys = append(keys, k)
+			}
+			sort.Strings(keys)
+			return fmt.Sprintf("conflicting tags: {%s}: %s", strings.Join(keys, ", "), msg), nil
+		}
+		return "", nil
 	}
-	if len(tagCount) > 1 {
-		return fmt.Sprintf("conflicting tags: {%s}", strings.Join(tags, ", ")), nil
-	}
-	return "", nil
-}
-
-// ruleOptionalAndRequired checks for conflicting tags +k8s:optional and +k8s:required in a slice of comments.
-func ruleOptionalAndRequired(comments []string) (string, error) {
-	return conflictingTagsRule(comments, "+k8s:optional", "+k8s:required")
-}
-
-// ruleRequiredAndDefault checks for conflicting tags +k8s:required and +default in a slice of comments.
-func ruleRequiredAndDefault(comments []string) (string, error) {
-	return conflictingTagsRule(comments, "+k8s:required", "+default")
 }

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

@@ -0,0 +1,30 @@
+/*
+Copyright 2025 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 main
+
+var ruleOptionalAndRequired = conflictingTagsRule(
+	"fields cannot be both optional and required",
+	"+k8s:optional", "+k8s:required")
+
+var ruleRequiredAndDefault = conflictingTagsRule(
+	"fields with default values are always optional",
+	"+k8s:required", "+default")
+
+var defaultLintRules = []lintRule{
+	ruleOptionalAndRequired,
+	ruleRequiredAndDefault,
+}

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

@@ -18,6 +18,7 @@ package main
 
 import (
 	"errors"
+	"regexp"
 	"testing"
 
 	"k8s.io/gengo/v2/types"
@@ -121,7 +122,6 @@ func TestRuleOptionalAndRequired(t *testing.T) {
 		name     string
 		comments []string
 		wantMsg  string
-		wantErr  bool
 	}{
 		{
 			name:     "no comments",
@@ -139,22 +139,37 @@ func TestRuleOptionalAndRequired(t *testing.T) {
 			wantMsg:  "",
 		},
 		{
-			name:     "optional and required",
+			name:     "optional required",
 			comments: []string{"+k8s:optional", "+k8s:required"},
-			wantMsg:  "conflicting tags: {+k8s:optional, +k8s:required}",
+			wantMsg:  `conflicting tags: {\+k8s:optional, \+k8s:required}`,
 		},
 		{
-			name:     "optional, empty, required",
+			name:     "required optional",
+			comments: []string{"+k8s:optional", "+k8s:required"},
+			wantMsg:  `conflicting tags: {\+k8s:optional, \+k8s:required}`,
+		},
+		{
+			name:     "optional empty required",
 			comments: []string{"+k8s:optional", "", "+k8s:required"},
-			wantMsg:  "conflicting tags: {+k8s:optional, +k8s:required}",
+			wantMsg:  `conflicting tags: {\+k8s:optional, \+k8s:required}`,
+		},
+		{
+			name:     "empty required empty empty optional empty",
+			comments: []string{"", "+k8s:optional", "", "", "+k8s:required", ""},
+			wantMsg:  `conflicting tags: {\+k8s:optional, \+k8s:required}`,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			msg, _ := ruleOptionalAndRequired(tt.comments)
-			if msg != tt.wantMsg {
-				t.Errorf("ruleOptionalAndRequired() msg = %v, wantMsg %v", msg, tt.wantMsg)
+			msg, err := ruleOptionalAndRequired(tt.comments)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			} else if tt.wantMsg != "" {
+				re := regexp.MustCompile(tt.wantMsg)
+				if !re.MatchString(msg) {
+					t.Errorf("message:\n\t%s\ndoes not match:\n\t%s", msg, re.String())
+				}
 			}
 		})
 	}
@@ -182,22 +197,37 @@ func TestRuleRequiredAndDefault(t *testing.T) {
 			wantMsg:  "",
 		},
 		{
-			name:     "required and default",
+			name:     "required default",
 			comments: []string{"+k8s:required", "+default=somevalue"},
-			wantMsg:  "conflicting tags: {+k8s:required, +default}",
+			wantMsg:  `conflicting tags: {\+default, \+k8s:required}`,
+		},
+		{
+			name:     "default required",
+			comments: []string{"+default=somevalue", "+k8s:required"},
+			wantMsg:  `conflicting tags: {\+default, \+k8s:required}`,
 		},
 		{
-			name:     "required, empty, default",
+			name:     "required empty default",
 			comments: []string{"+k8s:required", "", "+default=somevalue"},
-			wantMsg:  "conflicting tags: {+k8s:required, +default}",
+			wantMsg:  `conflicting tags: {\+default, \+k8s:required}`,
+		},
+		{
+			name:     "empty default empty empty required empty",
+			comments: []string{"", "+default=somevalue", "", "", "+k8s:required", ""},
+			wantMsg:  `conflicting tags: {\+default, \+k8s:required}`,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			msg, _ := ruleRequiredAndDefault(tt.comments)
-			if msg != tt.wantMsg {
-				t.Errorf("ruleRequiredAndDefault() msg = %v, wantMsg %v", msg, tt.wantMsg)
+			msg, err := ruleRequiredAndDefault(tt.comments)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			} else if tt.wantMsg != "" {
+				re := regexp.MustCompile(tt.wantMsg)
+				if !re.MatchString(msg) {
+					t.Errorf("message:\n\t%s\ndoes not match:\n\t%s", msg, re.String())
+				}
 			}
 		})
 	}
@@ -209,7 +239,6 @@ func TestConflictingTagsRule(t *testing.T) {
 		comments []string
 		tags     []string
 		wantMsg  string
-		wantErr  bool
 	}{
 		{
 			name:     "no comments",
@@ -227,32 +256,32 @@ func TestConflictingTagsRule(t *testing.T) {
 			name:     "tag1, empty, tag2",
 			comments: []string{"+tag1", "", "+tag2"},
 			tags:     []string{"+tag1", "+tag2"},
-			wantMsg:  "conflicting tags: {+tag1, +tag2}",
+			wantMsg:  `conflicting tags: {\+tag1, \+tag2}`,
 		},
 		{
-			name:     "3 tags",
-			comments: []string{"tag1", "+tag2", "+tag3=value"},
+			name:     "3 lines 2 tags match",
+			comments: []string{"tag3", "+tag1", "+tag2=value"},
 			tags:     []string{"+tag1", "+tag2", "+tag3"},
-			wantMsg:  "conflicting tags: {+tag1, +tag2, +tag3}",
+			wantMsg:  `conflicting tags: {\+tag1, \+tag2}`,
 		},
 		{
-			name:     "less than 2 tags",
-			comments: []string{"+tag1"},
-			tags:     []string{"+tag1"},
-			wantMsg:  "",
-			wantErr:  true,
+			name:     "3 tags all match",
+			comments: []string{"+tag3", "+tag1", "+tag2=value"},
+			tags:     []string{"+tag1", "+tag2", "+tag3"},
+			wantMsg:  `conflicting tags: {\+tag1, \+tag2, \+tag3}`,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			msg, err := conflictingTagsRule(tt.comments, tt.tags...)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("conflictingTagsRule() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if msg != tt.wantMsg {
-				t.Errorf("conflictingTagsRule() msg = %v, wantMsg %v", msg, tt.wantMsg)
+			msg, err := conflictingTagsRule("test", tt.tags...)(tt.comments)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			} else if tt.wantMsg != "" {
+				re := regexp.MustCompile(tt.wantMsg)
+				if !re.MatchString(msg) {
+					t.Errorf("message:\n\t%s\ndoes not match:\n\t%s", msg, re.String())
+				}
 			}
 		})
 	}

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

@@ -78,7 +78,7 @@ type Args struct {
 	ReadOnlyPkgs []string // Always consider these as last-ditch possibilities for validations.
 	GoHeaderFile string
 	PrintDocs    bool
-	Lint         bool
+	LintOnly     bool
 }
 
 // AddFlags add the generator flags to the flag set.
@@ -91,7 +91,7 @@ func (args *Args) AddFlags(fs *pflag.FlagSet) {
 		"the path to a file containing boilerplate header text; the string \"YEAR\" will be replaced with the current 4-digit year")
 	fs.BoolVar(&args.PrintDocs, "docs", false,
 		"print documentation for supported declarative validations, and then exit")
-	fs.BoolVar(&args.Lint, "lint", false,
+	fs.BoolVar(&args.LintOnly, "lint", false,
 		"only run linting checks, do not generate code")
 }