Skip to content

Commit b095879

Browse files
saschagrunertsaschagrunert
committed
Fix JSON tag linting for list types
The linter was incorrectly skipping JSON tag validation for list types (structs with TypeMeta, ListMeta, and Items fields). This allowed list types with missing or incorrect JSON tags to pass validation, which could cause serialization/deserialization issues. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
1 parent 9992248 commit b095879

File tree

4 files changed

+131
-5
lines changed

4 files changed

+131
-5
lines changed

‎pkg/analysis/helpers/inspector/inspector.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ type Inspector interface {
3232
// InspectFields is a function that iterates over fields in structs.
3333
InspectFields(func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers))
3434

35+
// InspectFieldsIncludingListTypes is a function that iterates over fields in structs, including list types.
36+
InspectFieldsIncludingListTypes(func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers))
37+
3538
// InspectTypeSpec is a function that inspects the type spec and calls the provided inspectTypeSpec function.
3639
InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers))
3740
}
@@ -56,6 +59,18 @@ func newInspector(astinspector *astinspector.Inspector, jsonTags extractjsontags
5659
// therefore would not be included in the CRD spec.
5760
// For the remaining fields, it calls the provided inspectField function to apply analysis logic.
5861
func (i *inspector) InspectFields(inspectField func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers)) {
62+
i.inspectFields(inspectField, true)
63+
}
64+
65+
// InspectFieldsIncludingListTypes iterates over fields in structs, including list types.
66+
// Unlike InspectFields, this method does not skip fields in list type structs.
67+
func (i *inspector) InspectFieldsIncludingListTypes(inspectField func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers)) {
68+
i.inspectFields(inspectField, false)
69+
}
70+
71+
// inspectFields is a shared implementation for field iteration.
72+
// The skipListTypes parameter controls whether list type structs should be skipped.
73+
func (i *inspector) inspectFields(inspectField func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers), skipListTypes bool) {
5974
// Filter to fields so that we can iterate over fields in a struct.
6075
nodeFilter := []ast.Node{
6176
(*ast.Field)(nil),
@@ -67,7 +82,7 @@ func (i *inspector) InspectFields(inspectField func(field *ast.Field, jsonTagInf
6782
}
6883

6984
field, ok := n.(*ast.Field)
70-
if !ok || !i.shouldProcessField(stack) {
85+
if !ok || !i.shouldProcessField(stack, skipListTypes) {
7186
return ok
7287
}
7388

@@ -82,7 +97,8 @@ func (i *inspector) InspectFields(inspectField func(field *ast.Field, jsonTagInf
8297
}
8398

8499
// shouldProcessField checks if the field should be processed.
85-
func (i *inspector) shouldProcessField(stack []ast.Node) bool {
100+
// The skipListTypes parameter controls whether list type structs should be skipped.
101+
func (i *inspector) shouldProcessField(stack []ast.Node, skipListTypes bool) bool {
86102
if len(stack) < 3 {
87103
return false
88104
}
@@ -96,8 +112,13 @@ func (i *inspector) shouldProcessField(stack []ast.Node) bool {
96112
}
97113

98114
structType, ok := stack[len(stack)-3].(*ast.StructType)
99-
if !ok || isItemsType(structType) {
100-
// Not in a struct or belongs to an items type.
115+
if !ok {
116+
// Not in a struct.
117+
return false
118+
}
119+
120+
if skipListTypes && isItemsType(structType) {
121+
// Skip list types if requested.
101122
return false
102123
}
103124

‎pkg/analysis/jsontags/analyzer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
7171
return nil, kalerrors.ErrCouldNotGetInspector
7272
}
7373

74-
inspect.InspectFields(func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, _ markers.Markers) {
74+
inspect.InspectFieldsIncludingListTypes(func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, _ markers.Markers) {
7575
a.checkField(pass, field, jsonTagInfo)
7676
})
7777

‎pkg/analysis/jsontags/testdata/src/a/a.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,106 @@ type E struct{}
5050
type Interface interface {
5151
InaccessibleFunction() string
5252
}
53+
54+
// ValidList is a properly tagged list type
55+
type ValidList struct {
56+
metav1.TypeMeta `json:",inline"`
57+
metav1.ListMeta `json:"metadata"`
58+
Items []A `json:"items"`
59+
}
60+
61+
// InvalidList is a list type with missing JSON tag on Items field
62+
type InvalidList struct {
63+
metav1.TypeMeta `json:",inline"`
64+
metav1.ListMeta `json:"metadata"`
65+
Items []A // want "field Items is missing json tag"
66+
}
67+
68+
// InvalidListEmptyTag is a list type with empty JSON tag on Items field
69+
type InvalidListEmptyTag struct {
70+
metav1.TypeMeta `json:",inline"`
71+
metav1.ListMeta `json:"metadata"`
72+
Items []A `json:""` // want "field Items has empty json tag"
73+
}
74+
75+
// InvalidListSnakeCase is a list type with snake_case JSON tag on Items field
76+
type InvalidListSnakeCase struct {
77+
metav1.TypeMeta `json:",inline"`
78+
metav1.ListMeta `json:"metadata"`
79+
Items []A `json:"item_list"` // want "field Items json tag does not match pattern \"\\^\\[a-z\\]\\[a-z0-9\\]\\*\\(\\?:\\[A-Z\\]\\[a-z0-9\\]\\*\\)\\*\\$\": item_list"
80+
}
81+
82+
// InvalidListKebabCase is a list type with kebab-case JSON tag on Items field
83+
type InvalidListKebabCase struct {
84+
metav1.TypeMeta `json:",inline"`
85+
metav1.ListMeta `json:"metadata"`
86+
Items []A `json:"item-list"` // want "field Items json tag does not match pattern \"\\^\\[a-z\\]\\[a-z0-9\\]\\*\\(\\?:\\[A-Z\\]\\[a-z0-9\\]\\*\\)\\*\\$\": item-list"
87+
}
88+
89+
// InvalidListMissingMetadataTag is a list type with missing JSON tag on ListMeta
90+
type InvalidListMissingMetadataTag struct {
91+
metav1.TypeMeta `json:",inline"`
92+
metav1.ListMeta // want "embedded field is missing json tag"
93+
Items []A `json:"items"`
94+
}
95+
96+
// InvalidListWrongMetadataTag is a list type with invalid JSON tag on ListMeta
97+
type InvalidListWrongMetadataTag struct {
98+
metav1.TypeMeta `json:",inline"`
99+
metav1.ListMeta `json:"meta_data"` // want "embedded field json tag does not match pattern \"\\^\\[a-z\\]\\[a-z0-9\\]\\*\\(\\?:\\[A-Z\\]\\[a-z0-9\\]\\*\\)\\*\\$\": meta_data"
100+
Items []A `json:"items"`
101+
}
102+
103+
// ListWithAdditionalFields is a list type with extra fields beyond the standard 3
104+
type ListWithAdditionalFields struct {
105+
metav1.TypeMeta `json:",inline"`
106+
metav1.ListMeta `json:"metadata"`
107+
Items []A `json:"items"`
108+
ExtraField string `json:"extraField"`
109+
MissingTagField string // want "field MissingTagField is missing json tag"
110+
InvalidTagField string `json:"invalid_tag"` // want "field InvalidTagField json tag does not match pattern \"\\^\\[a-z\\]\\[a-z0-9\\]\\*\\(\\?:\\[A-Z\\]\\[a-z0-9\\]\\*\\)\\*\\$\": invalid_tag"
111+
}
112+
113+
// ListWithIgnoredField is a list type with an ignored field
114+
type ListWithIgnoredField struct {
115+
metav1.TypeMeta `json:",inline"`
116+
metav1.ListMeta `json:"metadata"`
117+
Items []A `json:"items"`
118+
IgnoredField string `json:"-"`
119+
}
120+
121+
// Resource is a test resource type
122+
type Resource struct {
123+
Field string `json:"field"`
124+
}
125+
126+
// ResourceList is the exact scenario from issue #147
127+
// This would serialize incorrectly without proper JSON tags
128+
type ResourceList struct {
129+
metav1.TypeMeta `json:",inline"`
130+
metav1.ListMeta `json:"metadata"`
131+
Items []Resource // want "field Items is missing json tag"
132+
}
133+
134+
// ValidResourceList is the corrected version
135+
type ValidResourceList struct {
136+
metav1.TypeMeta `json:",inline"`
137+
metav1.ListMeta `json:"metadata"`
138+
Items []Resource `json:"items"`
139+
}
140+
141+
// ListWithPascalCaseTag tests that PascalCase tags are caught
142+
type ListWithPascalCaseTag struct {
143+
metav1.TypeMeta `json:",inline"`
144+
metav1.ListMeta `json:"metadata"`
145+
Items []A `json:"Items"` // want "field Items json tag does not match pattern \"\\^\\[a-z\\]\\[a-z0-9\\]\\*\\(\\?:\\[A-Z\\]\\[a-z0-9\\]\\*\\)\\*\\$\": Items"
146+
}
147+
148+
// NotActuallyAList should still be linted even though it has TypeMeta and ListMeta
149+
// but is not a proper 3-field list type
150+
type NotActuallyAList struct {
151+
metav1.TypeMeta `json:",inline"`
152+
metav1.ListMeta `json:"metadata"`
153+
Items A // want "field Items is missing json tag"
154+
SomeOtherField string `json:"otherField"`
155+
}

‎pkg/analysis/jsontags/testdata/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,5 @@ package v1
66
type TypeMeta struct{}
77

88
type ObjectMeta struct{}
9+
10+
type ListMeta struct{}

0 commit comments

Comments
 (0)