Skip to content

Commit a8038e6

Browse files
JoelSpeedJoelSpeed
committed
Add serialization checker util for optional/required fields
1 parent 6469492 commit a8038e6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+4471
-0
lines changed

‎pkg/analysis/utils/serialization/config.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
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 serialization
17+
18+
// PointersPolicy is the policy for pointers.
19+
// SuggestFix will suggest a fix for the field.
20+
// Warn will warn about the field.
21+
// Ignore will ignore the field.
22+
type PointersPolicy string
23+
24+
const (
25+
// PointersPolicySuggestFix will suggest a fix for the field.
26+
PointersPolicySuggestFix PointersPolicy = "SuggestFix"
27+
28+
// PointersPolicyWarn will warn about the field.
29+
PointersPolicyWarn PointersPolicy = "Warn"
30+
)
31+
32+
// PointersPreference is the preference for pointers.
33+
// Always will always suggest a fix for the field.
34+
// WhenRequired will only suggest a fix for the field when it is required.
35+
type PointersPreference string
36+
37+
const (
38+
// PointersPreferenceAlways will always suggest a pointer.
39+
PointersPreferenceAlways PointersPreference = "Always"
40+
41+
// PointersPreferenceWhenRequired will only suggest a pointer for the field when it is required.
42+
PointersPreferenceWhenRequired PointersPreference = "WhenRequired"
43+
)
44+
45+
// OmitEmptyPolicy is the policy for omitempty.
46+
// SuggestFix will suggest a fix for the field to add omitempty.
47+
// Warn will warn about the field to add omitempty.
48+
// Ignore will ignore the the absence of omitempty.
49+
type OmitEmptyPolicy string
50+
51+
const (
52+
// OmitEmptyPolicySuggestFix will suggest a fix for the field.
53+
OmitEmptyPolicySuggestFix OmitEmptyPolicy = "SuggestFix"
54+
55+
// OmitEmptyPolicyWarn will warn about the field.
56+
OmitEmptyPolicyWarn OmitEmptyPolicy = "Warn"
57+
58+
// OmitEmptyPolicyIgnore will ignore the field.
59+
OmitEmptyPolicyIgnore OmitEmptyPolicy = "Ignore"
60+
)
61+
62+
// OmitZeroPolicy is the policy for omitzero.
63+
// SuggestFix will suggest a fix for the field to add omitzero.
64+
// Warn will warn about the field to add omitzero.
65+
// Forbid will forbid the field to have omitzero.
66+
type OmitZeroPolicy string
67+
68+
const (
69+
// OmitZeroPolicySuggestFix will suggest a fix for the field.
70+
OmitZeroPolicySuggestFix OmitZeroPolicy = "SuggestFix"
71+
72+
// OmitZeroPolicyWarn will warn about the field.
73+
OmitZeroPolicyWarn OmitZeroPolicy = "Warn"
74+
75+
// OmitZeroPolicyForbid will forbid the field.
76+
OmitZeroPolicyForbid OmitZeroPolicy = "Forbid"
77+
)
78+
79+
// Config is the configuration for the serialization check.
80+
type Config struct {
81+
// Pointers is the configuration for pointers.
82+
Pointers PointersConfig
83+
84+
// OmitEmpty is the configuration for omitempty.
85+
OmitEmpty OmitEmptyConfig
86+
87+
// OmitZero is the configuration for omitzero.
88+
OmitZero OmitZeroConfig
89+
}
90+
91+
// PointersConfig is the configuration for pointers.
92+
type PointersConfig struct {
93+
// Policy is the policy for pointers.
94+
Policy PointersPolicy
95+
96+
// Preference is the preference for pointers.
97+
Preference PointersPreference
98+
}
99+
100+
// OmitEmptyConfig is the configuration for omitempty.
101+
type OmitEmptyConfig struct {
102+
// Policy is the policy for omitempty.
103+
Policy OmitEmptyPolicy
104+
}
105+
106+
// OmitZeroConfig is the configuration for omitzero.
107+
type OmitZeroConfig struct {
108+
// Policy is the policy for omitzero.
109+
Policy OmitZeroPolicy
110+
}

‎pkg/analysis/utils/serialization/serialization_check.go‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
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 serialization
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
22+
"golang.org/x/tools/go/analysis"
23+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
24+
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
26+
)
27+
28+
// SerializationCheck is an interface for checking serialization of fields.
29+
type SerializationCheck interface {
30+
Check(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, jsonTags extractjsontags.FieldTagInfo)
31+
}
32+
33+
// New creates a new SerializationCheck with the given configuration.
34+
func New(cfg *Config) SerializationCheck {
35+
validateConfig(cfg)
36+
37+
return &serializationCheck{
38+
pointerPolicy: cfg.Pointers.Policy,
39+
pointerPreference: cfg.Pointers.Preference,
40+
omitEmptyPolicy: cfg.OmitEmpty.Policy,
41+
omitZeroPolicy: cfg.OmitZero.Policy,
42+
}
43+
}
44+
45+
// validateConfig validates the configuration.
46+
// We panic if the configuration is invalid as this checker is intended to be
47+
// used as an implementation detail of the kube-api-linter.
48+
// Linters implementing this checker should validate the configuration themselves.
49+
func validateConfig(cfg *Config) {
50+
if cfg == nil {
51+
panic("configuration must be provided")
52+
}
53+
54+
switch cfg.Pointers.Policy {
55+
case PointersPolicySuggestFix, PointersPolicyWarn:
56+
default:
57+
panic(fmt.Sprintf("pointers.policy is required and must be one of %q or %q", PointersPolicySuggestFix, PointersPolicyWarn))
58+
}
59+
60+
switch cfg.Pointers.Preference {
61+
case PointersPreferenceAlways, PointersPreferenceWhenRequired:
62+
default:
63+
panic(fmt.Sprintf("pointers.preference is required and must be one of %q or %q", PointersPreferenceAlways, PointersPreferenceWhenRequired))
64+
}
65+
66+
switch cfg.OmitEmpty.Policy {
67+
case OmitEmptyPolicySuggestFix, OmitEmptyPolicyWarn, OmitEmptyPolicyIgnore:
68+
default:
69+
panic(fmt.Sprintf("omitempty.policy is required and must be one of %q, %q or %q", OmitEmptyPolicySuggestFix, OmitEmptyPolicyWarn, OmitEmptyPolicyIgnore))
70+
}
71+
72+
switch cfg.OmitZero.Policy {
73+
case OmitZeroPolicySuggestFix, OmitZeroPolicyWarn, OmitZeroPolicyForbid:
74+
default:
75+
panic(fmt.Sprintf("omitzero.policy is required and must be one of %q, %q or %q", OmitZeroPolicySuggestFix, OmitZeroPolicyWarn, OmitZeroPolicyForbid))
76+
}
77+
}
78+
79+
// serializationCheck is the implementation of the SerializationCheck interface.
80+
type serializationCheck struct {
81+
pointerPolicy PointersPolicy
82+
pointerPreference PointersPreference
83+
omitEmptyPolicy OmitEmptyPolicy
84+
omitZeroPolicy OmitZeroPolicy
85+
}
86+
87+
// Check checks the serialization of the field.
88+
// It will check if the zero value of the field is valid, and whether the field should be a pointer or not.
89+
func (s *serializationCheck) Check(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, jsonTags extractjsontags.FieldTagInfo) {
90+
fieldName := utils.FieldName(field)
91+
92+
hasValidZeroValue, completeValidation := utils.IsZeroValueValid(pass, field, field.Type, markersAccess, s.omitZeroPolicy != OmitZeroPolicyForbid)
93+
hasOmitEmpty := jsonTags.OmitEmpty
94+
hasOmitZero := jsonTags.OmitZero
95+
isPointer, underlying := utils.IsStarExpr(field.Type)
96+
isStruct := utils.IsStructType(pass, field.Type)
97+
98+
switch s.pointerPreference {
99+
case PointersPreferenceAlways:
100+
// The field must always be a pointer, pointers require omitempty, so enforce that too.
101+
s.handleFieldShouldBePointer(pass, field, fieldName, isPointer, underlying, "should be a pointer.")
102+
s.handleFieldShouldHaveOmitEmpty(pass, field, fieldName, hasOmitEmpty, jsonTags)
103+
case PointersPreferenceWhenRequired:
104+
s.handleFieldOmitZero(pass, field, fieldName, jsonTags, hasOmitZero, hasValidZeroValue, isPointer, isStruct)
105+
106+
if s.omitEmptyPolicy != OmitEmptyPolicyIgnore || hasOmitEmpty {
107+
// If we require omitempty, or the field has omitempty, we can check the field properties based on it being an omitempty field.
108+
s.checkFieldPropertiesWithOmitEmptyRequired(pass, field, fieldName, jsonTags, underlying, hasOmitEmpty, hasValidZeroValue, completeValidation, isPointer, isStruct)
109+
} else {
110+
// The field does not have omitempty, and does not require it.
111+
s.checkFieldPropertiesWithoutOmitEmpty(pass, field, fieldName, jsonTags, underlying, hasValidZeroValue, completeValidation, isPointer, isStruct)
112+
}
113+
default:
114+
panic(fmt.Sprintf("unknown pointer preference: %s", s.pointerPreference))
115+
}
116+
}
117+
118+
func (s *serializationCheck) handleFieldOmitZero(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, hasOmitZero, hasValidZeroValue, isPointer, isStruct bool) {
119+
switch s.omitZeroPolicy {
120+
case OmitZeroPolicyForbid:
121+
// when the omitzero policy is set to forbid, we need to report removing omitzero if set on the struct fields.
122+
s.checkFieldPropertiesWithOmitZeroForbidPolicy(pass, field, fieldName, isStruct, hasOmitZero, jsonTags)
123+
case OmitZeroPolicyWarn, OmitZeroPolicySuggestFix:
124+
// If we require omitzero, or the field has omitzero, we can check the field properties based on it being an omitzero field.
125+
s.checkFieldPropertiesWithOmitZeroRequired(pass, field, fieldName, jsonTags, hasOmitZero, isPointer, isStruct, hasValidZeroValue)
126+
default:
127+
panic(fmt.Sprintf("unknown omit zero policy: %s", s.omitZeroPolicy))
128+
}
129+
}
130+
131+
func (s *serializationCheck) handleFieldShouldHaveOmitEmpty(pass *analysis.Pass, field *ast.Field, fieldName string, hasOmitEmpty bool, jsonTags extractjsontags.FieldTagInfo) {
132+
if hasOmitEmpty {
133+
return
134+
}
135+
136+
reportShouldAddOmitEmpty(pass, field, s.omitEmptyPolicy, fieldName, "field %s should have the omitempty tag.", jsonTags)
137+
}
138+
139+
func (s *serializationCheck) checkFieldPropertiesWithOmitEmptyRequired(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, underlying ast.Expr, hasOmitEmpty, hasValidZeroValue, completeValidation, isPointer, isStruct bool) {
140+
switch {
141+
case isStruct && !hasValidZeroValue && s.omitZeroPolicy != OmitZeroPolicyForbid:
142+
// The struct field need not be pointer if it does not have a valid zero value.
143+
return
144+
case hasValidZeroValue && !completeValidation:
145+
zeroValue := utils.GetTypedZeroValue(pass, underlying)
146+
validationHint := utils.GetTypedValidationHint(pass, underlying)
147+
148+
s.handleFieldShouldBePointer(pass, field, fieldName, isPointer, underlying, fmt.Sprintf("has a valid zero value (%s), but the validation is not complete (e.g. %s). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer.", zeroValue, validationHint))
149+
case hasValidZeroValue, isStruct:
150+
// The field validation infers that the zero value is valid, the field needs to be a pointer.
151+
// Structs with omitempty should always be pointers, else they won't actually be omitted.
152+
zeroValue := utils.GetTypedZeroValue(pass, underlying)
153+
154+
s.handleFieldShouldBePointer(pass, field, fieldName, isPointer, underlying, fmt.Sprintf("has a valid zero value (%s) and should be a pointer.", zeroValue))
155+
case !hasValidZeroValue && completeValidation && !isStruct:
156+
// The validation is fully complete, and the zero value is not valid, so we don't need a pointer.
157+
s.handleFieldShouldNotBePointer(pass, field, fieldName, isPointer, "field %s does not allow the zero value. The field does not need to be a pointer.")
158+
}
159+
160+
// In this case, we should always add the omitempty if it isn't present.
161+
s.handleFieldShouldHaveOmitEmpty(pass, field, fieldName, hasOmitEmpty, jsonTags)
162+
}
163+
164+
func (s *serializationCheck) checkFieldPropertiesWithoutOmitEmpty(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, underlying ast.Expr, hasValidZeroValue, completeValidation, isPointer, isStruct bool) {
165+
switch {
166+
case hasValidZeroValue:
167+
// The field is not omitempty, and the zero value is valid, the field does not need to be a pointer.
168+
s.handleFieldShouldNotBePointer(pass, field, fieldName, isPointer, "field %s does not have omitempty and allows the zero value. The field does not need to be a pointer.")
169+
case !hasValidZeroValue:
170+
// The zero value would not be accepted, so the field needs to have omitempty.
171+
// Force the omitempty policy to suggest a fix. We can only get to this function when the policy is configured to Ignore.
172+
// Since we absolutely have to add the omitempty tag, we can report it as a suggestion.
173+
reportShouldAddOmitEmpty(pass, field, OmitEmptyPolicySuggestFix, fieldName, "field %s does not allow the zero value. It must have the omitempty tag.", jsonTags)
174+
// Once it has the omitempty tag, it will also need to be a pointer in some cases.
175+
// Now handle it as if it had the omitempty already.
176+
// We already handle the omitempty tag above, so force the `hasOmitEmpty` to true.
177+
s.checkFieldPropertiesWithOmitEmptyRequired(pass, field, fieldName, jsonTags, underlying, true, hasValidZeroValue, completeValidation, isPointer, isStruct)
178+
}
179+
}
180+
181+
func (s *serializationCheck) checkFieldPropertiesWithOmitZeroRequired(pass *analysis.Pass, field *ast.Field, fieldName string, jsonTags extractjsontags.FieldTagInfo, hasOmitZero, isPointer, isStruct, hasValidZeroValue bool) {
182+
if !isStruct || hasValidZeroValue {
183+
return
184+
}
185+
186+
s.handleFieldShouldHaveOmitZero(pass, field, fieldName, hasOmitZero, jsonTags)
187+
s.handleFieldShouldNotBePointer(pass, field, fieldName, isPointer, "field %s does not allow the zero value. The field does not need to be a pointer.")
188+
}
189+
190+
func (s *serializationCheck) checkFieldPropertiesWithOmitZeroForbidPolicy(pass *analysis.Pass, field *ast.Field, fieldName string, isStruct, hasOmitZero bool, jsonTags extractjsontags.FieldTagInfo) {
191+
if !isStruct || !hasOmitZero {
192+
// Handle omitzero only for struct field having omitZero tag.
193+
return
194+
}
195+
196+
reportShouldRemoveOmitZero(pass, field, fieldName, jsonTags)
197+
}
198+
199+
func (s *serializationCheck) handleFieldShouldHaveOmitZero(pass *analysis.Pass, field *ast.Field, fieldName string, hasOmitZero bool, jsonTags extractjsontags.FieldTagInfo) {
200+
if hasOmitZero {
201+
return
202+
}
203+
204+
// Currently, add omitzero tags to only struct fields.
205+
reportShouldAddOmitZero(pass, field, s.omitZeroPolicy, fieldName, "field %s does not allow the zero value. It must have the omitzero tag.", jsonTags)
206+
}
207+
208+
func (s *serializationCheck) handleFieldShouldBePointer(pass *analysis.Pass, field *ast.Field, fieldName string, isPointer bool, underlying ast.Expr, reason string) {
209+
if utils.IsPointerType(pass, underlying) {
210+
if isPointer {
211+
switch s.pointerPolicy {
212+
case PointersPolicySuggestFix:
213+
reportShouldRemovePointer(pass, field, PointersPolicySuggestFix, fieldName, "field %s underlying type does not need to be a pointer. The pointer should be removed.", fieldName)
214+
case PointersPolicyWarn:
215+
pass.Reportf(field.Pos(), "field %s underlying type does not need to be a pointer. The pointer should be removed.", fieldName)
216+
}
217+
}
218+
219+
return
220+
}
221+
222+
if isPointer {
223+
return
224+
}
225+
226+
switch s.pointerPolicy {
227+
case PointersPolicySuggestFix:
228+
reportShouldAddPointer(pass, field, PointersPolicySuggestFix, fieldName, "field %s %s", fieldName, reason)
229+
case PointersPolicyWarn:
230+
pass.Reportf(field.Pos(), "field %s %s", fieldName, reason)
231+
}
232+
}
233+
234+
func (s *serializationCheck) handleFieldShouldNotBePointer(pass *analysis.Pass, field *ast.Field, fieldName string, isPointer bool, message string) {
235+
if !isPointer {
236+
return
237+
}
238+
239+
reportShouldRemovePointer(pass, field, s.pointerPolicy, fieldName, message, fieldName)
240+
}

0 commit comments

Comments
 (0)