Skip to content

Commit c82645e

Browse files
authored
Merge pull request #166 from saschagrunert/arrayofstruct-followup
arrayofstruct: follow-up on review
2 parents dda830a + 76c0c5c commit c82645e

File tree

2 files changed

+32
-15
lines changed

2 files changed

+32
-15
lines changed

pkg/analysis/arrayofstruct/analyzer.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
2626
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
2727
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
28-
"sigs.k8s.io/kube-api-linter/pkg/markers"
2928
)
3029

3130
const name = "arrayofstruct"
@@ -119,7 +118,7 @@ func reportArrayOfStructIssue(pass *analysis.Pass, field *ast.Field) {
119118
prefix = fieldName
120119
}
121120

122-
message := fmt.Sprintf("%s is an array of structs, but the struct has no required fields. At least one field should be marked as %s to prevent ambiguous YAML configurations", prefix, markers.RequiredMarker)
121+
message := fmt.Sprintf("%s is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations", prefix)
123122

124123
pass.Report(analysis.Diagnostic{
125124
Pos: field.Pos(),
@@ -166,6 +165,11 @@ func getStructType(pass *analysis.Pass, expr ast.Expr) *ast.StructType {
166165
// Inline struct definition
167166
return et
168167
case *ast.Ident:
168+
// Check if it's a basic type - exit condition for recursion
169+
if utils.IsBasicType(pass, et) {
170+
return nil
171+
}
172+
169173
// Named struct type or type alias
170174
typeSpec, ok := utils.LookupTypeSpec(pass, et)
171175
if !ok {
@@ -197,12 +201,7 @@ func hasRequiredField(structType *ast.StructType, markersAccess markershelper.Ma
197201
}
198202

199203
for _, field := range structType.Fields.List {
200-
fieldMarkers := markersAccess.FieldMarkers(field)
201-
202-
// Check for any of the required markers
203-
if fieldMarkers.Has(markers.RequiredMarker) ||
204-
fieldMarkers.Has(markers.KubebuilderRequiredMarker) ||
205-
fieldMarkers.Has(markers.K8sRequiredMarker) {
204+
if utils.IsFieldRequired(field, markersAccess) {
206205
return true
207206
}
208207
}

pkg/analysis/arrayofstruct/testdata/src/a/a.go

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type ValidPrimitiveArray struct {
5757
// Invalid cases - arrays of structs without any required fields
5858

5959
type InvalidStruct struct {
60-
Items []InvalidItem // want "InvalidStruct.Items is an array of structs, but the struct has no required fields"
60+
Items []InvalidItem // want "InvalidStruct.Items is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
6161
}
6262

6363
type InvalidItem struct {
@@ -69,7 +69,7 @@ type InvalidItem struct {
6969
}
7070

7171
type InvalidStructWithPointer struct {
72-
Items []*InvalidPointerItem // want "InvalidStructWithPointer.Items is an array of structs, but the struct has no required fields"
72+
Items []*InvalidPointerItem // want "InvalidStructWithPointer.Items is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
7373
}
7474

7575
type InvalidPointerItem struct {
@@ -79,7 +79,7 @@ type InvalidPointerItem struct {
7979

8080
type InvalidStructWithInlineStruct struct {
8181
// Inline struct definitions should also be checked
82-
Items []struct { // want "InvalidStructWithInlineStruct.Items is an array of structs, but the struct has no required fields"
82+
Items []struct { // want "InvalidStructWithInlineStruct.Items is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
8383
// +optional
8484
Name string
8585
}
@@ -88,7 +88,7 @@ type InvalidStructWithInlineStruct struct {
8888
// Invalid case - struct with no markers at all
8989

9090
type InvalidStructNoMarkers struct {
91-
Items []InvalidItemNoMarkers // want "InvalidStructNoMarkers.Items is an array of structs, but the struct has no required fields"
91+
Items []InvalidItemNoMarkers // want "InvalidStructNoMarkers.Items is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
9292
}
9393

9494
type InvalidItemNoMarkers struct {
@@ -115,20 +115,38 @@ type AllRequiredItem struct {
115115
type ItemAlias = InvalidItem
116116

117117
type InvalidStructWithAlias struct {
118-
Items []ItemAlias // want "InvalidStructWithAlias.Items is an array of structs, but the struct has no required fields"
118+
Items []ItemAlias // want "InvalidStructWithAlias.Items is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
119119
}
120120

121121
// Test with array type alias
122122

123123
type ArrayAlias = []InvalidItem
124124

125125
type InvalidStructWithArrayAlias struct {
126-
Items ArrayAlias // want "InvalidStructWithArrayAlias.Items is an array of structs, but the struct has no required fields"
126+
Items ArrayAlias // want "InvalidStructWithArrayAlias.Items is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
127127
}
128128

129129
// Valid case with multiple array fields
130130

131131
type ValidMultipleArrays struct {
132132
ValidItems []ValidItem
133-
InvalidItems []InvalidItem // want "ValidMultipleArrays.InvalidItems is an array of structs, but the struct has no required fields"
133+
InvalidItems []InvalidItem // want "ValidMultipleArrays.InvalidItems is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations"
134+
}
135+
136+
// Valid case - type alias to basic type (should not trigger linter)
137+
138+
type StringAlias = string
139+
140+
type ValidStructWithBasicTypeAlias struct {
141+
// This should not trigger the linter because StringAlias resolves to string, a basic type
142+
Items []StringAlias
143+
}
144+
145+
// Valid case - custom type wrapping basic type (should not trigger linter)
146+
147+
type CustomString string
148+
149+
type ValidStructWithCustomBasicType struct {
150+
// This should not trigger the linter because CustomString is based on string, a basic type
151+
Items []CustomString
134152
}

0 commit comments

Comments
 (0)