Handle min properties for structs

Author: JoelSpeed

pkg/analysis/optionalfields/analyzer.go

@@ -247,26 +247,60 @@ func (a *analyzer) checkFieldPointersPreferenceWhenRequiredIdentObj(pass *analys
 
 func (a *analyzer) checkFieldPointersPreferenceWhenRequiredStructType(pass *analysis.Pass, field *ast.Field, fieldName string, isStarExpr bool, typeExpr *ast.StructType, markersAccess markers.Markers, jsonTags extractjsontags.FieldTagInfo) {
 	if a.omitEmptyPolicy == config.OptionalFieldsOmitEmptyPolicyIgnore && !jsonTags.OmitEmpty {
-		if isStarExpr {
-			reportShouldRemovePointer(pass, field, a.pointerPolicy, fieldName, "field %s is an optional struct without omitempty. It should not be a pointer")
-		}
+		a.checkFieldPointersPreferenceWhenRequiredStructTypeWithoutOmitEmpty(pass, field, fieldName, isStarExpr, typeExpr, markersAccess)
+		return
+	}
+
+	hasRequiredFields := structContainsRequiredFields(typeExpr, markersAccess)
 
+	hasMinimumProperties, err := structHasGreaterThanZeroMinProperties(typeExpr, markersAccess.StructMarkers(typeExpr))
+	if err != nil {
+		pass.Reportf(field.Pos(), "error checking struct for min properties: %v", err)
 		return
 	}
 
-	mustBePointer := structContainsRequiredFields(typeExpr, markersAccess)
-	if mustBePointer == isStarExpr {
-		// The field is already a pointer and should be a pointer, or it should not be a pointer and isn't a pointer.
+	fieldHasMinimumProperties, err := structHasGreaterThanZeroMinProperties(typeExpr, markersAccess.FieldMarkers(field))
+	if err != nil {
+		pass.Reportf(field.Pos(), "error checking field for min properties: %v", err)
 		return
 	}
 
-	if mustBePointer {
+	switch {
+	case hasRequiredFields && !isStarExpr:
 		reportShouldAddPointer(pass, field, a.pointerPolicy, fieldName, "field %s is optional, but contains required field(s) and should be a pointer")
-	} else {
+	case hasMinimumProperties && !isStarExpr, fieldHasMinimumProperties && !isStarExpr:
+		reportShouldAddPointer(pass, field, a.pointerPolicy, fieldName, "field %s has a greater than zero minimum number of properties and should be a pointer")
+	case isStarExpr && !hasRequiredFields && !hasMinimumProperties && !fieldHasMinimumProperties:
 		reportShouldRemovePointer(pass, field, a.pointerPolicy, fieldName, "field %s is optional, and contains no required field(s) and does not need to be a pointer")
 	}
 }
 
+func (a *analyzer) checkFieldPointersPreferenceWhenRequiredStructTypeWithoutOmitEmpty(pass *analysis.Pass, field *ast.Field, fieldName string, isStarExpr bool, typeExpr *ast.StructType, markersAccess markers.Markers) {
+	hasMinimumProperties, err := structHasGreaterThanZeroMinProperties(typeExpr, markersAccess.StructMarkers(typeExpr))
+	if err != nil {
+		pass.Reportf(field.Pos(), "error checking struct for min properties: %v", err)
+		return
+	}
+
+	fieldHasMinimumProperties, err := structHasGreaterThanZeroMinProperties(typeExpr, markersAccess.FieldMarkers(field))
+	if err != nil {
+		pass.Reportf(field.Pos(), "error checking field for min properties: %v", err)
+		return
+	}
+
+	switch {
+	case hasMinimumProperties && isStarExpr, fieldHasMinimumProperties && isStarExpr:
+		// The field is already a pointer and should be a pointer, so we don't need to do anything.
+	case hasMinimumProperties && !isStarExpr:
+		reportShouldAddPointer(pass, field, a.pointerPolicy, fieldName, "field %s has a greater than zero minimum number of properties and should be a pointer")
+	case fieldHasMinimumProperties && !isStarExpr:
+		reportShouldRemoveMarker(pass, field, markersAccess.FieldMarkers(field).Get(minPropertiesMarker)[0], fieldName, "field %s has a greater than zero minimum number of properties without omitempty. The minimum number of properties should be removed.")
+	case isStarExpr:
+		// The field is a pointer and should not be a pointer, so we need to remove the pointer.
+		reportShouldRemovePointer(pass, field, a.pointerPolicy, fieldName, "field %s is an optional struct without omitempty. It should not be a pointer")
+	}
+}
+
 func (a *analyzer) checkFieldPointersPreferenceWhenRequiredString(pass *analysis.Pass, field *ast.Field, fieldName string, isStarExpr bool, markersAccess markers.Markers, jsonTags extractjsontags.FieldTagInfo) {
 	fieldMarkers := markersAccess.FieldMarkers(field)
 
@@ -627,3 +661,24 @@ func getMarkerIntegerValue(marker markers.Marker) (int, error) {
 
 	return value, nil
 }
+
+func structHasGreaterThanZeroMinProperties(structType *ast.StructType, structMarkers markers.MarkerSet) (bool, error) {
+	if structType == nil {
+		return false, nil
+	}
+
+	if structMarkers.Has(minPropertiesMarker) {
+		for _, marker := range structMarkers.Get(minPropertiesMarker) {
+			markerValue, err := getMarkerIntegerValue(marker)
+			if err != nil {
+				return false, fmt.Errorf("error getting marker value: %w", err)
+			}
+
+			if markerValue > 0 {
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}

pkg/analysis/optionalfields/testdata/src/a/a.go

@@ -41,6 +41,15 @@ type A struct {
 	// +optional
 	NonOmittedStruct B `json:"nonOmittedStruct"` // want "field NonOmittedStruct is optional and should be a pointer" "field NonOmittedStruct is optional and should be omitempty"
 
+	// structWithMinProperties is a struct field with a minimum number of properties.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinProperties B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties is optional and should be a pointer"
+
+	// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
+	// +optional
+	StructWithMinPropertiesOnStruct D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct is optional and should be a pointer"
+
 	// slice is a slice field.
 	// +optional
 	Slice []string `json:"slice,omitempty"`
@@ -67,3 +76,15 @@ type B struct {
 	// +optional
 	PointerString *string `json:"pointerString,omitempty"`
 }
+
+// +kubebuilder:validation:MinProperties=1
+type D struct {
+	// string is a string field.
+	// +optional
+	String *string `json:"string,omitempty"`
+
+	// stringWithMinLength1 with minimum length is a string field.
+	// +kubebuilder:validation:MinLength=1
+	// +optional
+	StringWithMinLength1 *string `json:"stringWithMinLength1,omitempty"`
+}

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

@@ -41,6 +41,15 @@ type A struct {
 	// +optional
 	NonOmittedStruct *B `json:"nonOmittedStruct,omitempty"` // want "field NonOmittedStruct is optional and should be a pointer" "field NonOmittedStruct is optional and should be omitempty"
 
+    // structWithMinProperties is a struct field with a minimum number of properties.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinProperties *B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties is optional and should be a pointer"
+
+	// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
+	// +optional
+	StructWithMinPropertiesOnStruct *D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct is optional and should be a pointer"
+
 	// slice is a slice field.
 	// +optional
 	Slice []string `json:"slice,omitempty"`
@@ -67,3 +76,15 @@ type B struct {
 	// +optional
 	PointerString *string `json:"pointerString,omitempty"`
 }
+
+// +kubebuilder:validation:MinProperties=1
+type D struct {
+	// string is a string field.
+	// +optional
+	String *string `json:"string,omitempty"`
+
+	// stringWithMinLength1 with minimum length is a string field.
+	// +kubebuilder:validation:MinLength=1
+	// +optional
+	StringWithMinLength1 *string `json:"stringWithMinLength1,omitempty"`
+}

pkg/analysis/optionalfields/testdata/src/b/a.go

@@ -166,6 +166,15 @@ type A struct {
 	// +optional
 	StructWithOptionalFields B `json:"structWithOptionalFields,omitempty"`
 
+	// structWithMinProperties is a struct field with a minimum number of properties.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinProperties B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties has a greater than zero minimum number of properties and should be a pointer"
+
+	// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
+	// +optional
+	StructWithMinPropertiesOnStruct D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct has a greater than zero minimum number of properties and should be a pointer"
+
 	// structWithRequiredFields is a struct field.
 	// +optional
 	StructWithRequiredFields C `json:"structWithRequiredFields,omitempty"` // want "field StructWithRequiredFields is optional, but contains required field\\(s\\) and should be a pointer"
@@ -219,3 +228,15 @@ type C struct {
 	// +required
 	String string `json:"string"`
 }
+
+// +kubebuilder:validation:MinProperties=1
+type D struct {
+	// string is a string field.
+	// +optional
+	String string `json:"string,omitempty"` // want "field String is an optional string and does not have a minimum length. Either set a minimum length or make String a pointer where the difference between omitted and the empty string is significant"
+
+	// stringWithMinLength1 with minimum length is a string field.
+	// +kubebuilder:validation:MinLength=1
+	// +optional
+	StringWithMinLength1 string `json:"stringWithMinLength1,omitempty"`
+}

pkg/analysis/optionalfields/testdata/src/b/a.go.golden

@@ -48,7 +48,6 @@ type A struct {
 	// +optional
 	PointerIntWithPositiveMaximumValue *int `json:"pointerIntWithPositiveMaximumValue,omitempty"` // want "field PointerIntWithPositiveMaximumValue has a positive maximum value and does not have a minimum value. A minimum value should be set"
 
-
 	// pointerIntWithRange is a pointer int field with a range of values including 0.
 	// +kubebuilder:validation:Minimum=-10
 	// +kubebuilder:validation:Maximum=10
@@ -168,6 +167,15 @@ type A struct {
 	// +optional
 	StructWithOptionalFields B `json:"structWithOptionalFields,omitempty"`
 
+    // structWithMinProperties is a struct field with a minimum number of properties.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinProperties *B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties has a greater than zero minimum number of properties and should be a pointer"
+
+	// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
+	// +optional
+	StructWithMinPropertiesOnStruct *D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct has a greater than zero minimum number of properties and should be a pointer"
+
 	// structWithRequiredFields is a struct field.
 	// +optional
 	StructWithRequiredFields *C `json:"structWithRequiredFields,omitempty"` // want "field StructWithRequiredFields is optional, but contains required field\\(s\\) and should be a pointer"
@@ -221,3 +229,15 @@ type C struct {
 	// +required
 	String string `json:"string"`
 }
+
+// +kubebuilder:validation:MinProperties=1
+type D struct {
+	// string is a string field.
+	// +optional
+	String string `json:"string,omitempty"` // want "field String is an optional string and does not have a minimum length. Either set a minimum length or make String a pointer where the difference between omitted and the empty string is significant"
+
+	// stringWithMinLength1 with minimum length is a string field.
+	// +kubebuilder:validation:MinLength=1
+	// +optional
+	StringWithMinLength1 string `json:"stringWithMinLength1,omitempty"`
+}

pkg/analysis/optionalfields/testdata/src/c/a.go

@@ -298,6 +298,24 @@ type A struct {
 	// +optional
 	StructWithOptionalFieldsWithoutOmitEmpty B `json:"structWithOptionalFieldsWithoutOmitEmpty"`
 
+	// structWithMinProperties is a struct field with a minimum number of properties.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinProperties B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties has a greater than zero minimum number of properties and should be a pointer"
+
+	// structWithMinPropertiesWithoutOmitEmpty is a struct field with a minimum number of properties without omitempty.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinPropertiesWithoutOmitEmpty B `json:"structWithMinPropertiesWithoutOmitEmpty"` // want "field StructWithMinPropertiesWithoutOmitEmpty has a greater than zero minimum number of properties without omitempty. The minimum number of properties should be removed."
+
+	// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
+	// +optional
+	StructWithMinPropertiesOnStruct D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct has a greater than zero minimum number of properties and should be a pointer"
+
+	// structWithMinPropertiesOnStructWithoutOmitEmpty is a struct field with a minimum number of properties on the struct without omitempty.
+	// +optional
+	StructWithMinPropertiesOnStructWithoutOmitEmpty D `json:"structWithMinPropertiesOnStructWithoutOmitEmpty"` // want "field StructWithMinPropertiesOnStructWithoutOmitEmpty has a greater than zero minimum number of properties and should be a pointer"
+
 	// structWithRequiredFields is a struct field.
 	// +optional
 	StructWithRequiredFields C `json:"structWithRequiredFields,omitempty"` // want "field StructWithRequiredFields is optional, but contains required field\\(s\\) and should be a pointer"
@@ -387,7 +405,19 @@ type B struct {
 }
 
 type C struct {
-	// tsring is a string field.
+	// string is a string field.
 	// +required
 	String string `json:"string"`
 }
+
+// +kubebuilder:validation:MinProperties=1
+type D struct {
+	// string is a string field.
+	// +optional
+	String string `json:"string"`
+
+	// stringWithMinLength1 with minimum length is a string field.
+	// +kubebuilder:validation:MinLength=1
+	// +optional
+	StringWithMinLength1 string `json:"stringWithMinLength1,omitempty"`
+}

pkg/analysis/optionalfields/testdata/src/c/a.go.golden

@@ -290,6 +290,23 @@ type A struct {
 	// +optional
 	StructWithOptionalFieldsWithoutOmitEmpty B `json:"structWithOptionalFieldsWithoutOmitEmpty"`
 
+    // structWithMinProperties is a struct field with a minimum number of properties.
+	// +kubebuilder:validation:MinProperties=1
+	// +optional
+	StructWithMinProperties *B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties has a greater than zero minimum number of properties and should be a pointer"
+
+	// structWithMinPropertiesWithoutOmitEmpty is a struct field with a minimum number of properties without omitempty.
+	// +optional
+	StructWithMinPropertiesWithoutOmitEmpty B `json:"structWithMinPropertiesWithoutOmitEmpty"` // want "field StructWithMinPropertiesWithoutOmitEmpty has a greater than zero minimum number of properties without omitempty. The minimum number of properties should be removed."
+
+	// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
+	// +optional
+	StructWithMinPropertiesOnStruct *D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct has a greater than zero minimum number of properties and should be a pointer"
+
+	// structWithMinPropertiesOnStructWithoutOmitEmpty is a struct field with a minimum number of properties on the struct without omitempty.
+	// +optional
+	StructWithMinPropertiesOnStructWithoutOmitEmpty *D `json:"structWithMinPropertiesOnStructWithoutOmitEmpty"` // want "field StructWithMinPropertiesOnStructWithoutOmitEmpty has a greater than zero minimum number of properties and should be a pointer"
+
 	// structWithRequiredFields is a struct field.
 	// +optional
 	StructWithRequiredFields *C `json:"structWithRequiredFields,omitempty"` // want "field StructWithRequiredFields is optional, but contains required field\\(s\\) and should be a pointer"
@@ -377,7 +394,19 @@ type B struct {
 }
 
 type C struct {
-	// tsring is a string field.
+	// string is a string field.
 	// +required
 	String string `json:"string"`
 }
+
+// +kubebuilder:validation:MinProperties=1
+type D struct {
+	// string is a string field.
+	// +optional
+	String string `json:"string"`
+
+	// stringWithMinLength1 with minimum length is a string field.
+	// +kubebuilder:validation:MinLength=1
+	// +optional
+	StringWithMinLength1 string `json:"stringWithMinLength1,omitempty"`
+}