Skip to content

Commit d5f8bf2

Browse files
committed
Support omitzero tags in optionalfields linter
1 parent e719da1 commit d5f8bf2

File tree

5 files changed

+33
-9
lines changed

5 files changed

+33
-9
lines changed

pkg/analysis/helpers/extractjsontags/analyzer.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ import (
2929
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
3030
)
3131

32+
const (
33+
omitEmpty = "omitempty"
34+
omitZero = "omitzero"
35+
)
36+
3237
// StructFieldTags is used to find information about
3338
// json tags on fields within struct.
3439
type StructFieldTags interface {
@@ -140,10 +145,12 @@ func extractTagInfo(tag *ast.BasicLit) FieldTagInfo {
140145

141146
return FieldTagInfo{
142147
Name: tagName,
143-
OmitEmpty: len(tagValues) == 2 && tagValues[1] == "omitempty",
144-
RawValue: tagValue,
145-
Pos: pos,
146-
End: end,
148+
OmitEmpty: (len(tagValues) == 2 && tagValues[1] == omitEmpty) || (len(tagValues) == 3 && (tagValues[1] == omitEmpty || tagValues[2] == omitEmpty)),
149+
// Considering, omitzero will always coexist with omitemtpy and it can be either second or third tag.
150+
OmitZero: len(tagValues) == 3 && (tagValues[1] == omitZero || tagValues[2] == omitZero),
151+
RawValue: tagValue,
152+
Pos: pos,
153+
End: end,
147154
}
148155
}
149156

@@ -159,6 +166,9 @@ type FieldTagInfo struct {
159166
// OmitEmpty is true if the field has the omitempty option in the json tag.
160167
OmitEmpty bool
161168

169+
// OmitZero is true if the field has the omitzero option in the json tag.
170+
OmitZero bool
171+
162172
// Inline is true if the field has the inline option in the json tag.
163173
Inline bool
164174

pkg/analysis/optionalfields/analyzer.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ func defaultConfig(cfg *config.OptionalFieldsConfig) {
134134
func (a *analyzer) checkFieldProperties(pass *analysis.Pass, field *ast.Field, fieldName string, markersAccess markershelper.Markers, jsonTags extractjsontags.FieldTagInfo) {
135135
hasValidZeroValue, completeValidation := isZeroValueValid(pass, field, field.Type, markersAccess)
136136
hasOmitEmpty := jsonTags.OmitEmpty
137+
hasOmitZero := jsonTags.OmitZero
137138
isPointer, underlying := isStarExpr(field.Type)
138139
isStruct := isStructType(pass, field.Type)
139140

@@ -149,18 +150,21 @@ func (a *analyzer) checkFieldProperties(pass *analysis.Pass, field *ast.Field, f
149150

150151
if a.omitEmptyPolicy != config.OptionalFieldsOmitEmptyPolicyIgnore || hasOmitEmpty {
151152
// If we require omitempty, or the field has omitempty, we can check the field properties based on it being an omitempty field.
152-
a.checkFieldPropertiesWithOmitEmptyRequired(pass, field, fieldName, jsonTags, underlying, hasOmitEmpty, hasValidZeroValue, completeValidation, isPointer, isStruct)
153+
a.checkFieldPropertiesWithOmitEmptyRequired(pass, field, fieldName, jsonTags, underlying, hasOmitEmpty, hasOmitZero, hasValidZeroValue, completeValidation, isPointer, isStruct)
153154
} else {
154155
// The field does not have omitempty, and does not require it.
155156
a.checkFieldPropertiesWithoutOmitEmpty(pass, field, fieldName, jsonTags, underlying, hasValidZeroValue, completeValidation, isPointer, isStruct)
156157
}
157158
}
158159

159-
func (a *analyzer) checkFieldPropertiesWithOmitEmptyRequired(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, underlying ast.Expr, hasOmitEmpty, hasValidZeroValue, completeValidation, isPointer, isStruct bool) {
160+
func (a *analyzer) checkFieldPropertiesWithOmitEmptyRequired(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, underlying ast.Expr, hasOmitEmpty, hasOmitZero, hasValidZeroValue, completeValidation, isPointer, isStruct bool) {
160161
// In this case, we should always add the omitempty if it isn't present.
161162
a.handleFieldShouldHaveOmitEmpty(pass, field, fieldName, hasOmitEmpty, jsonTags)
162163

163164
switch {
165+
case !hasValidZeroValue && isStruct && hasOmitZero:
166+
// The struct field need not be pointer if it does not have valid zero value and has omitzero tag.
167+
return
164168
case hasValidZeroValue && !completeValidation:
165169
a.handleIncompleteFieldValidation(pass, field, fieldName, isPointer, underlying)
166170
fallthrough // Since it's a valid zero value, we should still enforce the pointer.
@@ -187,7 +191,7 @@ func (a *analyzer) checkFieldPropertiesWithoutOmitEmpty(pass *analysis.Pass, fie
187191
// Once it has the omitempty tag, it will also need to be a pointer in some cases.
188192
// Now handle it as if it had the omitempty already.
189193
// We already handle the omitempty tag above, so force the `hasOmitEmpty` to true.
190-
a.checkFieldPropertiesWithOmitEmptyRequired(pass, field, fieldName, jsonTags, underlying, true, hasValidZeroValue, completeValidation, isPointer, isStruct)
194+
a.checkFieldPropertiesWithOmitEmptyRequired(pass, field, fieldName, jsonTags, underlying, true, false, hasValidZeroValue, completeValidation, isPointer, isStruct)
191195
}
192196
}
193197

@@ -300,7 +304,7 @@ func getStructZeroValue(pass *analysis.Pass, structType *ast.StructType) string
300304
for _, field := range structType.Fields.List {
301305
fieldTagInfo := jsonTagInfo.FieldTags(field)
302306

303-
if fieldTagInfo.OmitEmpty {
307+
if fieldTagInfo.OmitEmpty || fieldTagInfo.OmitZero {
304308
// If the field is omitted, we can use a zero value.
305309
// For structs, if they aren't a pointer another error will be raised.
306310
continue

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,11 @@ type A struct {
171171
// +optional
172172
StructWithMinProperties B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties is optional and should be a pointer"
173173

174+
// StructWithMinPropertiesAndOmitEmptyTag is a struct field with a minimum number of properties.
175+
// +kubebuilder:validation:MinProperties=1
176+
// +optional
177+
StructWithMinPropertiesAndOmitEmptyTag B `json:"structWithMinPropertiesAndOmitEmptyTag,omitempty,omitzero"`
178+
174179
// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
175180
// +optional
176181
StructWithMinPropertiesOnStruct D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct is optional and should be a pointer"

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,11 @@ type A struct {
171171
// +optional
172172
StructWithMinProperties *B `json:"structWithMinProperties,omitempty"` // want "field StructWithMinProperties is optional and should be a pointer"
173173

174+
// StructWithMinPropertiesAndOmitEmptyTag is a struct field with a minimum number of properties.
175+
// +kubebuilder:validation:MinProperties=1
176+
// +optional
177+
StructWithMinPropertiesAndOmitEmptyTag B `json:"structWithMinPropertiesAndOmitEmptyTag,omitempty,omitzero"`
178+
174179
// structWithMinPropertiesOnStruct is a struct field with a minimum number of properties on the struct.
175180
// +optional
176181
StructWithMinPropertiesOnStruct *D `json:"structWithMinPropertiesOnStruct,omitempty"` // want "field StructWithMinPropertiesOnStruct is optional and should be a pointer"

pkg/analysis/optionalfields/util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func areStructFieldZeroValuesValid(pass *analysis.Pass, structType *ast.StructTy
224224
for _, field := range structType.Fields.List {
225225
fieldTagInfo := jsonTagInfo.FieldTags(field)
226226

227-
if fieldTagInfo.OmitEmpty {
227+
if fieldTagInfo.OmitEmpty || fieldTagInfo.OmitZero {
228228
// If the field is omitted, we can use a zero value.
229229
// For structs, if they aren't a pointer another error will be raised.
230230
continue

0 commit comments

Comments
 (0)