Skip to content

Commit 0304415

Browse files
committed
Fix requiredfields linter to respect OmitEmpty Ignore policy
When the requiredfields linter is configured with omitempty policy set to 'Ignore', violations for missing omitempty tags were still reported because checkFieldPropertiesWithoutOmitEmpty was hardcoded to use OmitEmptyPolicySuggestFix instead of respecting the configured policy. Use s.omitEmptyPolicy and skip subsequent pointer checks when policy is Ignore, since those checks assume omitempty will be added. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
1 parent d3015c9 commit 0304415

File tree

17 files changed

+250
-200
lines changed

17 files changed

+250
-200
lines changed

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

Lines changed: 29 additions & 29 deletions
Large diffs are not rendered by default.

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

Lines changed: 56 additions & 56 deletions
Large diffs are not rendered by default.

pkg/analysis/requiredfields/analyzer_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,18 @@ func TestDefaultConfiguration(t *testing.T) {
3232

3333
analysistest.RunWithSuggestedFixes(t, testdata, a, "a")
3434
}
35+
36+
func TestOmitEmptyIgnorePolicy(t *testing.T) {
37+
testdata := analysistest.TestData()
38+
39+
a, err := requiredfields.Initializer().Init(&requiredfields.RequiredFieldsConfig{
40+
OmitEmpty: requiredfields.RequiredFieldsOmitEmpty{
41+
Policy: requiredfields.RequiredFieldsOmitEmptyPolicyIgnore,
42+
},
43+
})
44+
if err != nil {
45+
t.Fatal(err)
46+
}
47+
48+
analysistest.RunWithSuggestedFixes(t, testdata, a, "omitempty_ignore")
49+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package omitempty_ignore
17+
18+
// TestOmitEmptyIgnore tests that when omitempty policy is set to Ignore,
19+
// no diagnostics are reported for required fields missing omitempty tags.
20+
type TestOmitEmptyIgnore struct {
21+
// +kubebuilder:validation:Required
22+
// +kubebuilder:validation:MinLength=1
23+
// This field does not allow the zero value (empty string) and does not have omitempty.
24+
// With Ignore policy, no diagnostic should be reported.
25+
RequiredFieldWithoutOmitEmpty string `json:"requiredField"`
26+
27+
// +kubebuilder:validation:Required
28+
// +kubebuilder:validation:MinLength=1
29+
// This field has omitempty, so it should work normally.
30+
RequiredFieldWithOmitEmpty string `json:"requiredFieldWithOmitEmpty,omitempty"`
31+
}

pkg/analysis/utils/serialization/serialization_check.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,15 +176,19 @@ func (s *serializationCheck) checkFieldPropertiesWithOmitEmptyRequired(pass *ana
176176
}
177177

178178
func (s *serializationCheck) checkFieldPropertiesWithoutOmitEmpty(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, underlying ast.Expr, hasValidZeroValue, completeValidation, isPointer, isStruct bool, markersAccess markershelper.Markers, qualifiedFieldName string) {
179+
// When policy is Ignore, skip all checks for fields without omitempty.
180+
// We respect the user's choice to not enforce omitempty requirements.
181+
if s.omitEmptyPolicy == OmitEmptyPolicyIgnore {
182+
return
183+
}
184+
179185
switch {
180186
case hasValidZeroValue:
181187
// The field is not omitempty, and the zero value is valid, the field does not need to be a pointer.
182188
s.handleFieldShouldNotBePointer(pass, field, fieldName, isPointer, underlying, markersAccess, "field %s does not have omitempty and allows the zero value. The field does not need to be a pointer.", qualifiedFieldName)
183189
case !hasValidZeroValue:
184190
// The zero value would not be accepted, so the field needs to have omitempty.
185-
// Force the omitempty policy to suggest a fix. We can only get to this function when the policy is configured to Ignore.
186-
// Since we absolutely have to add the omitempty tag, we can report it as a suggestion.
187-
reportShouldAddOmitEmpty(pass, field, OmitEmptyPolicySuggestFix, qualifiedFieldName, "field %s does not allow the zero value. It must have the omitempty tag.", jsonTags)
191+
reportShouldAddOmitEmpty(pass, field, s.omitEmptyPolicy, qualifiedFieldName, "field %s does not allow the zero value. It must have the omitempty tag.", jsonTags)
188192
// Once it has the omitempty tag, it will also need to be a pointer in some cases.
189193
// Now handle it as if it had the omitempty already.
190194
// We already handle the omitempty tag above, so force the `hasOmitEmpty` to true.

pkg/analysis/utils/serialization/testdata/src/pointers_when_required_omit_empty_ignore/arrays.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type TestArrays struct {
1010
ArrayWithOmitEmptyPtr []*string `json:"arrayWithOmitEmptyPtr,omitempty"`
1111

1212
// +kubebuilder:validation:MinItems=1
13-
ArrayWithPositiveMinItems []string `json:"arrayWithPositiveMinItems"` // want "field TestArrays.ArrayWithPositiveMinItems does not allow the zero value. It must have the omitempty tag."
13+
ArrayWithPositiveMinItems []string `json:"arrayWithPositiveMinItems"`
1414

1515
// +kubebuilder:validation:MinItems=1
1616
ArrayWithPositiveMinItemsWithOmitEmpty []string `json:"arrayWithPositiveMinItemsWithOmitEmpty,omitempty"`
@@ -26,7 +26,7 @@ type TestArrays struct {
2626
ByteArrayWithOmitEmpty []byte `json:"byteArrayWithOmitEmpty,omitempty"`
2727

2828
// +kubebuilder:validation:MinLength=1
29-
ByteArrayWithMinLength []byte `json:"byteArrayWithMinLength"` // want "field TestArrays.ByteArrayWithMinLength does not allow the zero value. It must have the omitempty tag."
29+
ByteArrayWithMinLength []byte `json:"byteArrayWithMinLength"`
3030

3131
// +kubebuilder:validation:MinLength=1
3232
ByteArrayWithMinLengthWithOmitEmpty []byte `json:"byteArrayWithMinLengthWithOmitEmpty,omitempty"`
@@ -37,7 +37,7 @@ type TestArrays struct {
3737
// +kubebuilder:validation:MinLength=0
3838
ByteArrayWithMinLength0WithOmitEmpty []byte `json:"byteArrayWithMinLength0WithOmitEmpty,omitempty"`
3939

40-
PtrArray *[]string `json:"ptrArray"` // want "field TestArrays.PtrArray does not have omitempty and allows the zero value. The field does not need to be a pointer."
40+
PtrArray *[]string `json:"ptrArray"`
4141

4242
PtrArrayWithOmitEmpty *[]string `json:"ptrArrayWithOmitEmpty,omitempty"` // want "field TestArrays.PtrArrayWithOmitEmpty underlying type does not need to be a pointer. The pointer should be removed."
4343
}

pkg/analysis/utils/serialization/testdata/src/pointers_when_required_omit_empty_ignore/arrays.go.golden

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type TestArrays struct {
1010
ArrayWithOmitEmptyPtr []*string `json:"arrayWithOmitEmptyPtr,omitempty"`
1111

1212
// +kubebuilder:validation:MinItems=1
13-
ArrayWithPositiveMinItems []string `json:"arrayWithPositiveMinItems,omitempty"` // want "field TestArrays.ArrayWithPositiveMinItems does not allow the zero value. It must have the omitempty tag."
13+
ArrayWithPositiveMinItems []string `json:"arrayWithPositiveMinItems"`
1414

1515
// +kubebuilder:validation:MinItems=1
1616
ArrayWithPositiveMinItemsWithOmitEmpty []string `json:"arrayWithPositiveMinItemsWithOmitEmpty,omitempty"`
@@ -26,7 +26,7 @@ type TestArrays struct {
2626
ByteArrayWithOmitEmpty []byte `json:"byteArrayWithOmitEmpty,omitempty"`
2727

2828
// +kubebuilder:validation:MinLength=1
29-
ByteArrayWithMinLength []byte `json:"byteArrayWithMinLength,omitempty"` // want "field TestArrays.ByteArrayWithMinLength does not allow the zero value. It must have the omitempty tag."
29+
ByteArrayWithMinLength []byte `json:"byteArrayWithMinLength"`
3030

3131
// +kubebuilder:validation:MinLength=1
3232
ByteArrayWithMinLengthWithOmitEmpty []byte `json:"byteArrayWithMinLengthWithOmitEmpty,omitempty"`
@@ -37,7 +37,7 @@ type TestArrays struct {
3737
// +kubebuilder:validation:MinLength=0
3838
ByteArrayWithMinLength0WithOmitEmpty []byte `json:"byteArrayWithMinLength0WithOmitEmpty,omitempty"`
3939

40-
PtrArray []string `json:"ptrArray"` // want "field TestArrays.PtrArray does not have omitempty and allows the zero value. The field does not need to be a pointer."
40+
PtrArray *[]string `json:"ptrArray"`
4141

42-
PtrArrayWithOmitEmpty []string `json:"ptrArrayWithOmitEmpty,omitempty"` // want "field TestArrays.PtrArrayWithOmitEmpty underlying type does not need to be a pointer. The pointer should be removed."
42+
PtrArrayWithOmitEmpty *[]string `json:"ptrArrayWithOmitEmpty,omitempty"` // want "field TestArrays.PtrArrayWithOmitEmpty underlying type does not need to be a pointer. The pointer should be removed."
4343
}

pkg/analysis/utils/serialization/testdata/src/pointers_when_required_omit_empty_ignore/bool.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ type TestBools struct {
55

66
BoolWithOmitEmpty bool `json:"boolWithOmitEmpty,omitempty"` // want "field TestBools.BoolWithOmitEmpty has a valid zero value \\(false\\) and should be a pointer."
77

8-
BoolPtr *bool `json:"boolPtr"` // want "field TestBools.BoolPtr does not have omitempty and allows the zero value. The field does not need to be a pointer."
8+
BoolPtr *bool `json:"boolPtr"`
99

1010
BoolPtrWithOmitEmpty *bool `json:"boolPtrWithOmitEmpty,omitempty"`
1111
}

pkg/analysis/utils/serialization/testdata/src/pointers_when_required_omit_empty_ignore/bool.go.golden

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package a
33
type TestBools struct {
44
Bool bool `json:"bool"`
55

6-
BoolWithOmitEmpty *bool `json:"boolWithOmitEmpty,omitempty"` // want "field TestBools.BoolWithOmitEmpty has a valid zero value \\(false\\) and should be a pointer."
6+
BoolWithOmitEmpty bool `json:"boolWithOmitEmpty,omitempty"` // want "field TestBools.BoolWithOmitEmpty has a valid zero value \\(false\\) and should be a pointer."
77

8-
BoolPtr bool `json:"boolPtr"` // want "field TestBools.BoolPtr does not have omitempty and allows the zero value. The field does not need to be a pointer."
8+
BoolPtr *bool `json:"boolPtr"`
99

1010
BoolPtrWithOmitEmpty *bool `json:"boolPtrWithOmitEmpty,omitempty"`
1111
}

pkg/analysis/utils/serialization/testdata/src/pointers_when_required_omit_empty_ignore/maps.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ type TestMaps struct {
55

66
MapWithOmitEmpty map[string]string `json:"mapWithOmitEmpty,omitempty"`
77

8-
MapPtr *map[string]string `json:"mapPtr"` // want "field TestMaps.MapPtr does not have omitempty and allows the zero value. The field does not need to be a pointer."
8+
MapPtr *map[string]string `json:"mapPtr"`
99

1010
MapPtrWithOmitEmpty *map[string]string `json:"mapPtrWithOmitEmpty,omitempty"` // want "field TestMaps.MapPtrWithOmitEmpty underlying type does not need to be a pointer. The pointer should be removed."
1111

1212
// +kubebuilder:validation:MinProperties=1
13-
MapWithPositiveMinProperties map[string]string `json:"mapWithPositiveMinProperties"` // want "field TestMaps.MapWithPositiveMinProperties does not allow the zero value. It must have the omitempty tag."
13+
MapWithPositiveMinProperties map[string]string `json:"mapWithPositiveMinProperties"`
1414

1515
// +kubebuilder:validation:MinProperties=1
1616
MapWithPositiveMinPropertiesWithOmitEmpty map[string]string `json:"mapWithPositiveMinPropertiesWithOmitEmpty,omitempty"`

0 commit comments

Comments
 (0)