Add automatic fixes to nonpointerstructs

Author: JoelSpeed

docs/linters.md

@@ -529,6 +529,21 @@ If there are no required fields, the struct is implicitly optional and must be m
 To have an optional struct field that includes required fields, the struct must be a pointer.
 To have a required struct field that includes no required fields, the struct must be a pointer.
 
+### Configuration
+
+```yaml
+lintersConfig:
+  nonpointerstructs:
+    preferredRequiredMarker: required | kubebuilder:validation:Required | k8s:required # The preferred required marker to use for required fields when providing fixes. Defaults to `required`.
+    preferredOptionalMarker: optional | kubebuilder:validation:Optional | k8s:optional # The preferred optional marker to use for optional fields when providing fixes. Defaults to `optional`.
+```
+
+### Fixes
+
+The `nonpointerstructs` linter can automatically fix non-pointer struct fields that are not marked as required or optional.
+
+It will suggest to mark the field as required or optional, depending on the fields within the non-pointer struct.
+
 ## NoNullable
 
 The `nonullable` linter ensures that types and fields do not have the `nullable` marker.

pkg/analysis/nonpointerstructs/analyzer.go

@@ -30,29 +30,45 @@ import (
 
 const name = "nonpointerstructs"
 
-func newAnalyzer() *analysis.Analyzer {
+func newAnalyzer(cfg *Config) *analysis.Analyzer {
+	if cfg == nil {
+		cfg = &Config{}
+	}
+
+	defaultConfig(cfg)
+
+	a := &analyzer{
+		preferredRequiredMarker: cfg.PreferredRequiredMarker,
+		preferredOptionalMarker: cfg.PreferredOptionalMarker,
+	}
+
 	return &analysis.Analyzer{
 		Name:     name,
 		Doc:      "Checks that non-pointer structs that contain required fields are marked as required. Non-pointer structs that contain no required fields are marked as optional.",
-		Run:      run,
+		Run:      a.run,
 		Requires: []*analysis.Analyzer{inspector.Analyzer},
 	}
 }
 
-func run(pass *analysis.Pass) (any, error) {
+type analyzer struct {
+	preferredRequiredMarker string
+	preferredOptionalMarker string
+}
+
+func (a *analyzer) run(pass *analysis.Pass) (any, error) {
 	inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
 	if !ok {
 		return nil, kalerrors.ErrCouldNotGetInspector
 	}
 
 	inspect.InspectFields(func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, qualifiedFieldName string) {
-		checkField(pass, field, markersAccess, jsonTagInfo, qualifiedFieldName)
+		a.checkField(pass, field, markersAccess, jsonTagInfo, qualifiedFieldName)
 	})
 
 	return nil, nil //nolint:nilnil
 }
 
-func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, jsonTagInfo extractjsontags.FieldTagInfo, qualifiedFieldName string) {
+func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, jsonTagInfo extractjsontags.FieldTagInfo, qualifiedFieldName string) {
 	if field.Type == nil {
 		return
 	}
@@ -74,9 +90,9 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp
 	case hasRequiredField && isRequired, !hasRequiredField && isOptional:
 		// This is the desired case.
 	case hasRequiredField:
-		pass.Reportf(field.Pos(), "field %s is a non-pointer struct with required fields. It must be marked as required.", qualifiedFieldName)
+		a.handleShouldBeRequired(pass, field, markersAccess, qualifiedFieldName)
 	case !hasRequiredField:
-		pass.Reportf(field.Pos(), "field %s is a non-pointer struct with no required fields. It must be marked as optional.", qualifiedFieldName)
+		a.handleShouldBeOptional(pass, field, markersAccess, qualifiedFieldName)
 	}
 }
 
@@ -113,3 +129,80 @@ func hasRequiredField(structType *ast.StructType, markersAccess markershelper.Ma
 
 	return false
 }
+
+func defaultConfig(cfg *Config) {
+	if cfg.PreferredRequiredMarker == "" {
+		cfg.PreferredRequiredMarker = markers.RequiredMarker
+	}
+
+	if cfg.PreferredOptionalMarker == "" {
+		cfg.PreferredOptionalMarker = markers.OptionalMarker
+	}
+}
+
+func (a *analyzer) handleShouldBeRequired(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, qualifiedFieldName string) {
+	fieldMarkers := markersAccess.FieldMarkers(field)
+
+	textEdits := []analysis.TextEdit{}
+
+	for _, marker := range []string{markers.OptionalMarker, markers.KubebuilderOptionalMarker, markers.K8sOptionalMarker} {
+		for _, m := range fieldMarkers.Get(marker) {
+			textEdits = append(textEdits, analysis.TextEdit{
+				Pos:     m.Pos,
+				End:     m.End + 1, // Add 1 to include the newline character
+				NewText: nil,
+			})
+		}
+	}
+
+	textEdits = append(textEdits, analysis.TextEdit{
+		Pos:     field.Pos(),
+		End:     field.Pos(),
+		NewText: fmt.Appendf(nil, "// +%s\n", a.preferredRequiredMarker),
+	})
+
+	pass.Report(analysis.Diagnostic{
+		Pos:     field.Pos(),
+		End:     field.Pos(),
+		Message: fmt.Sprintf("field %s is a non-pointer struct with required fields. It must be marked as required.", qualifiedFieldName),
+		SuggestedFixes: []analysis.SuggestedFix{
+			{
+				Message:   "should mark the field as required",
+				TextEdits: textEdits,
+			},
+		},
+	})
+}
+
+func (a *analyzer) handleShouldBeOptional(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, qualifiedFieldName string) {
+	fieldMarkers := markersAccess.FieldMarkers(field)
+
+	textEdits := []analysis.TextEdit{}
+
+	for _, marker := range []string{markers.RequiredMarker, markers.KubebuilderRequiredMarker, markers.K8sRequiredMarker} {
+		for _, m := range fieldMarkers.Get(marker) {
+			textEdits = append(textEdits, analysis.TextEdit{
+				Pos:     m.Pos,
+				End:     m.End + 1, // Add 1 to include the newline character
+				NewText: nil,
+			})
+		}
+	}
+
+	textEdits = append(textEdits, analysis.TextEdit{
+		Pos:     field.Pos(),
+		End:     field.Pos(),
+		NewText: fmt.Appendf(nil, "// +%s\n", a.preferredOptionalMarker),
+	})
+
+	pass.Report(analysis.Diagnostic{
+		Pos:     field.Pos(),
+		Message: fmt.Sprintf("field %s is a non-pointer struct with no required fields. It must be marked as optional.", qualifiedFieldName),
+		SuggestedFixes: []analysis.SuggestedFix{
+			{
+				Message:   "should mark the field as optional",
+				TextEdits: textEdits,
+			},
+		},
+	})
+}

pkg/analysis/nonpointerstructs/analyzer_test.go

@@ -25,10 +25,10 @@ import (
 func Test(t *testing.T) {
 	testdata := analysistest.TestData()
 
-	analyzer, err := nonpointerstructs.Initializer().Init(nil)
+	analyzer, err := nonpointerstructs.Initializer().Init(&nonpointerstructs.Config{})
 	if err != nil {
 		t.Fatalf("initializing nonpointerstructs linter: %v", err)
 	}
 
-	analysistest.Run(t, testdata, analyzer, "a")
+	analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "a")
 }

pkg/analysis/nonpointerstructs/config.go

@@ -0,0 +1,29 @@
+/*
+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 nonpointerstructs
+
+// Config is the configuration for the nonpointerstructs linter.
+type Config struct {
+	// preferredRequiredMarker is the preferred marker to use for required fields when providing fixes.
+	// If this field is not set, the default value is "required".
+	// Valid values are "required" and "kubebuilder:validation:Required" and "k8s:required".
+	PreferredRequiredMarker string `json:"preferredRequiredMarker"`
+
+	// preferredOptionalMarker is the preferred marker to use for optional fields when providing fixes.
+	// If this field is not set, the default value is "optional".
+	// Valid values are "optional" and "kubebuilder:validation:Optional" and "k8s:optional".
+	PreferredOptionalMarker string `json:"preferredOptionalMarker"`
+}

pkg/analysis/nonpointerstructs/initializer.go

@@ -16,8 +16,13 @@ limitations under the License.
 package nonpointerstructs
 
 import (
+	"fmt"
+
+	"golang.org/x/tools/go/analysis"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/initializer"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/registry"
+	"sigs.k8s.io/kube-api-linter/pkg/markers"
 )
 
 func init() {
@@ -27,9 +32,36 @@ func init() {
 // Initializer returns the AnalyzerInitializer for this
 // Analyzer so that it can be added to the registry.
 func Initializer() initializer.AnalyzerInitializer {
-	return initializer.NewInitializer(
+	return initializer.NewConfigurableInitializer(
 		name,
-		newAnalyzer(),
+		initAnalyzer,
 		true,
+		validateConfig,
 	)
 }
+
+func initAnalyzer(cfg *Config) (*analysis.Analyzer, error) {
+	return newAnalyzer(cfg), nil
+}
+
+func validateConfig(cfg *Config, fldPath *field.Path) field.ErrorList {
+	if cfg == nil {
+		return field.ErrorList{}
+	}
+
+	fieldErrors := field.ErrorList{}
+
+	switch cfg.PreferredRequiredMarker {
+	case "", markers.RequiredMarker, markers.KubebuilderRequiredMarker, markers.K8sRequiredMarker:
+	default:
+		fieldErrors = append(fieldErrors, field.Invalid(fldPath.Child("preferredRequiredMarker"), cfg.PreferredRequiredMarker, fmt.Sprintf("invalid value, must be one of %q, %q, %q or omitted", markers.RequiredMarker, markers.KubebuilderRequiredMarker, markers.K8sRequiredMarker)))
+	}
+
+	switch cfg.PreferredOptionalMarker {
+	case "", markers.OptionalMarker, markers.KubebuilderOptionalMarker, markers.K8sOptionalMarker:
+	default:
+		fieldErrors = append(fieldErrors, field.Invalid(fldPath.Child("preferredOptionalMarker"), cfg.PreferredOptionalMarker, fmt.Sprintf("invalid value, must be one of %q, %q, %q or omitted", markers.OptionalMarker, markers.KubebuilderOptionalMarker, markers.K8sOptionalMarker)))
+	}
+
+	return fieldErrors
+}

pkg/analysis/nonpointerstructs/testdata/src/a/a.go.golden

@@ -0,0 +1,75 @@
+package a
+
+type A struct {
+	// +required
+	WithRequired WithRequiredField `json:"withRequired"` // want "field A.WithRequired is a non-pointer struct with required fields. It must be marked as required."
+	// +optional
+	WithOptional WithOptionalField `json:"withOptional"` // want "field A.WithOptional is a non-pointer struct with no required fields. It must be marked as optional."
+	// +required
+	WithRequiredAndOptional WithRequiredAndOptionalField `json:"withRequiredAndOptional"` // want "field A.WithRequiredAndOptional is a non-pointer struct with required fields. It must be marked as required."
+	// +required
+	WithOptionalAndMinProperties WithOptionalFieldsAndMinProperties `json:"withOptionalAndMinProperties"` // want "field A.WithOptionalAndMinProperties is a non-pointer struct with required fields. It must be marked as required."
+
+	// No diagnostic with correct markers
+
+	// +required
+	WithRequiredFieldCorrect WithRequiredField `json:"withRequiredFieldCorrect"`
+
+	// +optional
+	WithOptionalFieldCorrect WithOptionalField `json:"withOptionalFieldCorrect"`
+
+	// +kubebuilder:validation:Required
+	WithRequiredAndOptionalFieldCorrect WithRequiredAndOptionalField `json:"withRequiredAndOptionalFieldCorrect"`
+
+	// +k8s:required
+	WithOptionalFieldsAndMinPropertiesCorrect WithOptionalFieldsAndMinProperties `json:"withOptionalFieldsAndMinPropertiesCorrect"`
+
+	// Diagnostic with incorrect markers
+
+	// +required
+	WithRequiredFieldIncorrect WithRequiredField `json:"withRequiredFieldIncorrect"` // want "field A.WithRequiredFieldIncorrect is a non-pointer struct with required fields. It must be marked as required."
+
+	// +optional
+	WithOptionalFieldIncorrect WithOptionalField `json:"withOptionalFieldIncorrect"` // want "field A.WithOptionalFieldIncorrect is a non-pointer struct with no required fields. It must be marked as optional."
+
+	// +required
+	WithRequiredAndOptionalFieldIncorrect WithRequiredAndOptionalField `json:"withRequiredAndOptionalFieldIncorrect"` // want "field A.WithRequiredAndOptionalFieldIncorrect is a non-pointer struct with required fields. It must be marked as required."
+
+	// +required
+	WithOptionalFieldsAndMinPropertiesIncorrect WithOptionalFieldsAndMinProperties `json:"withOptionalFieldsAndMinPropertiesIncorrect"` // want "field A.WithOptionalFieldsAndMinPropertiesIncorrect is a non-pointer struct with required fields. It must be marked as required."
+
+	// Pointers are ignored in this linter.
+	WithRequiredFieldAndPointer                  *WithRequiredField                  `json:"withRequiredFieldAndPointer"`
+	WithOptionalFieldAndPointer                  *WithOptionalField                  `json:"withOptionalFieldAndPointer"`
+	WithRequiredAndOptionalFieldAndPointer       *WithRequiredAndOptionalField       `json:"withRequiredAndOptionalFieldAndPointer"`
+	WithOptionalFieldsAndMinPropertiesAndPointer *WithOptionalFieldsAndMinProperties `json:"withOptionalFieldsAndMinPropertiesAndPointer"`
+
+	// Inline structs are ignored in this linter.
+	WithRequiredField                  `json:",inline"`
+	WithOptionalField                  `json:",inline"`
+	WithRequiredAndOptionalField       `json:",inline"`
+	WithOptionalFieldsAndMinProperties `json:",inline"`
+}
+
+type WithRequiredField struct {
+	// +required
+	RequiredField string `json:"requiredField"`
+}
+
+type WithOptionalField struct {
+	// +optional
+	OptionalField string `json:"optionalField"`
+}
+
+type WithRequiredAndOptionalField struct {
+	// +k8s:required
+	RequiredField string `json:"requiredField"`
+	// +k8s:optional
+	OptionalField string `json:"optionalField"`
+}
+
+// +kubebuilder:validation:MinProperties=1
+type WithOptionalFieldsAndMinProperties struct {
+	// +k8s:optional
+	OptionalField string `json:"optionalField"`
+}