Skip to content

Commit fe408eb

Browse files
committed
Update zero value utils to handle omitted required fields
1 parent 1b29e82 commit fe408eb

File tree

2 files changed

+32
-7
lines changed

2 files changed

+32
-7
lines changed

pkg/analysis/utils/testdata/src/b/structs.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ type ZeroValueTestStructs struct {
88
StructWithNonOmittedFieldsAndMinProperties StructWithNonOmittedFieldsAndMinProperties `json:"structWithOneNonOmittedFieldAndMinProperties,omitempty"` // want "zero value is valid" "validation is complete"
99

1010
StructWithOneNonOmittedFieldAndMinProperties StructWithOneNonOmittedFieldAndMinProperties `json:"structWithOneNonOmittedFieldAndMinPropertiesAndOmitEmpty,omitempty"` // want "zero value is not valid" "validation is complete"
11+
12+
StructWithOmittedRequiredField StructWithOmittedRequiredField `json:"structWithOmittedRequiredField,omitempty"` // want "zero value is not valid" "validation is complete"
1113
}
1214

1315
type StructWithAllOptionalFields struct {
@@ -61,3 +63,10 @@ type StructWithOneNonOmittedFieldAndMinProperties struct {
6163
// +optional
6264
Int int32 `json:"int,omitempty"` // want "zero value is valid" "validation is not complete"
6365
}
66+
67+
// Struct with an omitted required field.
68+
// The zero value of the struct is `{}` which is not valid because it does not satisfy the required marker on the string field.
69+
type StructWithOmittedRequiredField struct {
70+
// +required
71+
String string `json:"string,omitempty"` // want "zero value is valid" "validation is not complete"
72+
}

pkg/analysis/utils/zero_value.go

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,17 +117,23 @@ func areStructFieldZeroValuesValid(pass *analysis.Pass, structType *ast.StructTy
117117
nonOmittedFields := 0
118118

119119
for _, field := range structType.Fields.List {
120+
fieldRequired := isFieldRequired(field, markersAccess)
120121
fieldTagInfo := jsonTagInfo.FieldTags(field)
121122

122-
if fieldTagInfo.OmitEmpty {
123-
// If the field is omitted, we can use a zero value.
124-
// For structs, if they aren't a pointer another error will be raised.
125-
continue
126-
}
123+
// Assume the field has omitempty.
124+
// Then the zero value (omitted) for a required field is not valid, and for an optional field it is valid.
125+
validValue := !fieldRequired
127126

128-
nonOmittedFields++
127+
// Count the required fields as non-omitted even if they have omitempty.
128+
// This allows us to count them towards the min-properties count in the parent function.
129+
if !fieldTagInfo.OmitEmpty || fieldRequired {
130+
nonOmittedFields++
131+
}
129132

130-
validValue, _ := IsZeroValueValid(pass, field, field.Type, markersAccess)
133+
// When the field is not omitted, we need to check if the zero value is valid (required or not).
134+
if !fieldTagInfo.OmitEmpty {
135+
validValue, _ = IsZeroValueValid(pass, field, field.Type, markersAccess)
136+
}
131137

132138
// If either value is false then the collected values will be false.
133139
zeroValueValid = zeroValueValid && validValue
@@ -417,3 +423,13 @@ func isBoolIdent(ident *ast.Ident) bool {
417423
func isFloatIdent(ident *ast.Ident) bool {
418424
return ident.Name == "float32" || ident.Name == "float64"
419425
}
426+
427+
// isFieldRequired checks if the field is required.
428+
// It checks for the presence of the required marker, the kubebuilder required marker, or the k8s required marker.
429+
func isFieldRequired(field *ast.Field, markersAccess markershelper.Markers) bool {
430+
fieldMarkers := markersAccess.FieldMarkers(field)
431+
432+
return fieldMarkers.Has(markers.RequiredMarker) ||
433+
fieldMarkers.Has(markers.KubebuilderRequiredMarker) ||
434+
fieldMarkers.Has(markers.K8sRequiredMarker)
435+
}

0 commit comments

Comments
 (0)