Skip to content

Commit 6b4c58f

Browse files
committed
Add linter for nonpointerstruct optionality
1 parent 8e86c46 commit 6b4c58f

File tree

9 files changed

+313
-7
lines changed

9 files changed

+313
-7
lines changed

docs/linters.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- [NoDurations](#nodurations) - Prevents usage of duration types
1717
- [NoFloats](#nofloats) - Prevents usage of floating-point types
1818
- [Nomaps](#nomaps) - Restricts usage of map types
19+
- [NonPointerStructs](#nonpointerstructs) - Ensures non-pointer structs are marked correctly with required/optional markers
1920
- [NoNullable](#nonullable) - Prevents usage of the nullable marker
2021
- [Nophase](#nophase) - Prevents usage of 'Phase' fields
2122
- [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields
@@ -514,6 +515,20 @@ lintersConfig:
514515
policy: Enforce | AllowStringToStringMaps | Ignore # Determines how the linter should handle maps of simple types. Defaults to AllowStringToStringMaps.
515516
```
516517

518+
## NonPointerStructs
519+
520+
The `nonpointerstructs` linter checks that non-pointer structs that contain required fields are marked as required.
521+
Non-pointer structs that contain no required fields are marked as optional.
522+
523+
This linter is important for types validated in Go as there is no way to validate the optionality of the fields at runtime,
524+
aside from checking the fields within them.
525+
526+
If a struct is marked required, this can only be validated by having a required field within it.
527+
If there are no required fields, the struct is implicitly optional and must be marked as so.
528+
529+
To have an optional struct field that includes required fields, the struct must be a pointer.
530+
To have a required struct field that includes no required fields, the struct must be a pointer.
531+
517532
## NoNullable
518533

519534
The `nonullable` linter ensures that types and fields do not have the `nullable` marker.
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
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 nonpointerstructs
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
22+
"golang.org/x/tools/go/analysis"
23+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
24+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
26+
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
28+
"sigs.k8s.io/kube-api-linter/pkg/markers"
29+
)
30+
31+
const name = "nonpointerstructs"
32+
33+
func newAnalyzer() *analysis.Analyzer {
34+
return &analysis.Analyzer{
35+
Name: name,
36+
Doc: "Checks that non-pointer structs that contain required fields are marked as required. Non-pointer structs that contain no required fields are marked as optional.",
37+
Run: run,
38+
Requires: []*analysis.Analyzer{inspector.Analyzer},
39+
}
40+
}
41+
42+
func run(pass *analysis.Pass) (any, error) {
43+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
44+
if !ok {
45+
return nil, kalerrors.ErrCouldNotGetInspector
46+
}
47+
48+
inspect.InspectFields(func(field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, qualifiedFieldName string) {
49+
checkField(pass, field, markersAccess, jsonTagInfo, qualifiedFieldName)
50+
})
51+
52+
return nil, nil //nolint:nilnil
53+
}
54+
55+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers, jsonTagInfo extractjsontags.FieldTagInfo, qualifiedFieldName string) {
56+
if field.Type == nil {
57+
return
58+
}
59+
60+
if jsonTagInfo.Inline {
61+
return
62+
}
63+
64+
structType, ok := asNonPointerStruct(pass, field.Type)
65+
if !ok {
66+
return
67+
}
68+
69+
hasRequiredField := hasRequiredField(structType, markersAccess)
70+
isOptional := utils.IsFieldOptional(field, markersAccess)
71+
isRequired := utils.IsFieldRequired(field, markersAccess)
72+
73+
switch {
74+
case hasRequiredField && isRequired, !hasRequiredField && isOptional:
75+
// This is the desired case.
76+
case hasRequiredField:
77+
pass.Reportf(field.Pos(), "field %s is a non-pointer struct with required fields. It must be marked as required.", qualifiedFieldName)
78+
case !hasRequiredField:
79+
pass.Reportf(field.Pos(), "field %s is a non-pointer struct with no required fields. It must be marked as optional.", qualifiedFieldName)
80+
}
81+
}
82+
83+
func asNonPointerStruct(pass *analysis.Pass, field ast.Expr) (*ast.StructType, bool) {
84+
switch typ := field.(type) {
85+
case *ast.StructType:
86+
return typ, true
87+
case *ast.Ident:
88+
typeSpec, ok := utils.LookupTypeSpec(pass, typ)
89+
if !ok {
90+
return nil, false
91+
}
92+
93+
return asNonPointerStruct(pass, typeSpec.Type)
94+
default:
95+
return nil, false
96+
}
97+
}
98+
99+
func hasRequiredField(structType *ast.StructType, markersAccess markershelper.Markers) bool {
100+
for _, field := range structType.Fields.List {
101+
if utils.IsFieldRequired(field, markersAccess) {
102+
return true
103+
}
104+
}
105+
106+
structMarkers := markersAccess.StructMarkers(structType)
107+
108+
if structMarkers.Has(markers.KubebuilderMinPropertiesMarker) && !structMarkers.HasWithValue(fmt.Sprintf("%s=0", markers.KubebuilderMinPropertiesMarker)) {
109+
// A non-zero min properties marker means that the struct is validated to have at least one field.
110+
// This means it can be treated the same as having a required field.
111+
return true
112+
}
113+
114+
return false
115+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
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 nonpointerstructs_test
17+
18+
import (
19+
"testing"
20+
21+
"golang.org/x/tools/go/analysis/analysistest"
22+
"sigs.k8s.io/kube-api-linter/pkg/analysis/nonpointerstructs"
23+
)
24+
25+
func Test(t *testing.T) {
26+
testdata := analysistest.TestData()
27+
28+
analyzer, err := nonpointerstructs.Initializer().Init(nil)
29+
if err != nil {
30+
t.Fatalf("initializing nonpointerstructs linter: %v", err)
31+
}
32+
33+
analysistest.Run(t, testdata, analyzer, "a")
34+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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+
17+
/*
18+
nonpointerstructs is a linter that checks that non-pointer structs that contain required fields are marked as required.
19+
Non-pointer structs that contain no required fields are marked as optional.
20+
21+
This linter is important for types validated in Go as there is no way to validate the optionality of the fields at runtime,
22+
aside from checking the fields within them.
23+
24+
If a struct is marked required, this can only be validated by having a required field within it.
25+
If there are no required fields, the struct is implicitly optional and must be marked as so.
26+
27+
To have an optional struct field that includes required fields, the struct must be a pointer.
28+
To have a required struct field that includes no required fields, the struct must be a pointer.
29+
*/
30+
package nonpointerstructs
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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 nonpointerstructs
17+
18+
import (
19+
"sigs.k8s.io/kube-api-linter/pkg/analysis/initializer"
20+
"sigs.k8s.io/kube-api-linter/pkg/analysis/registry"
21+
)
22+
23+
func init() {
24+
registry.DefaultRegistry().RegisterLinter(Initializer())
25+
}
26+
27+
// Initializer returns the AnalyzerInitializer for this
28+
// Analyzer so that it can be added to the registry.
29+
func Initializer() initializer.AnalyzerInitializer {
30+
return initializer.NewInitializer(
31+
name,
32+
newAnalyzer(),
33+
true,
34+
)
35+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package a
2+
3+
type A struct {
4+
WithRequired WithRequiredField `json:"withRequired"` // want "field A.WithRequired is a non-pointer struct with required fields. It must be marked as required."
5+
WithOptional WithOptionalField `json:"withOptional"` // want "field A.WithOptional is a non-pointer struct with no required fields. It must be marked as optional."
6+
WithRequiredAndOptional WithRequiredAndOptionalField `json:"withRequiredAndOptional"` // want "field A.WithRequiredAndOptional is a non-pointer struct with required fields. It must be marked as required."
7+
WithOptionalAndMinProperties WithOptionalFieldsAndMinProperties `json:"withOptionalAndMinProperties"` // want "field A.WithOptionalAndMinProperties is a non-pointer struct with required fields. It must be marked as required."
8+
9+
// No diagnostic with correct markers
10+
11+
// +required
12+
WithRequiredFieldCorrect WithRequiredField `json:"withRequiredFieldCorrect"`
13+
14+
// +optional
15+
WithOptionalFieldCorrect WithOptionalField `json:"withOptionalFieldCorrect"`
16+
17+
// +kubebuilder:validation:Required
18+
WithRequiredAndOptionalFieldCorrect WithRequiredAndOptionalField `json:"withRequiredAndOptionalFieldCorrect"`
19+
20+
// +k8s:required
21+
WithOptionalFieldsAndMinPropertiesCorrect WithOptionalFieldsAndMinProperties `json:"withOptionalFieldsAndMinPropertiesCorrect"`
22+
23+
// Diagnostic with incorrect markers
24+
25+
// +kubebuilder:validation:Optional
26+
WithRequiredFieldIncorrect WithRequiredField `json:"withRequiredFieldIncorrect"` // want "field A.WithRequiredFieldIncorrect is a non-pointer struct with required fields. It must be marked as required."
27+
28+
// +kubebuilder:validation:Required
29+
WithOptionalFieldIncorrect WithOptionalField `json:"withOptionalFieldIncorrect"` // want "field A.WithOptionalFieldIncorrect is a non-pointer struct with no required fields. It must be marked as optional."
30+
31+
// +kubebuilder:validation:Optional
32+
WithRequiredAndOptionalFieldIncorrect WithRequiredAndOptionalField `json:"withRequiredAndOptionalFieldIncorrect"` // want "field A.WithRequiredAndOptionalFieldIncorrect is a non-pointer struct with required fields. It must be marked as required."
33+
34+
// +kubebuilder:validation:Optional
35+
WithOptionalFieldsAndMinPropertiesIncorrect WithOptionalFieldsAndMinProperties `json:"withOptionalFieldsAndMinPropertiesIncorrect"` // want "field A.WithOptionalFieldsAndMinPropertiesIncorrect is a non-pointer struct with required fields. It must be marked as required."
36+
37+
// Pointers are ignored in this linter.
38+
WithRequiredFieldAndPointer *WithRequiredField `json:"withRequiredFieldAndPointer"`
39+
WithOptionalFieldAndPointer *WithOptionalField `json:"withOptionalFieldAndPointer"`
40+
WithRequiredAndOptionalFieldAndPointer *WithRequiredAndOptionalField `json:"withRequiredAndOptionalFieldAndPointer"`
41+
WithOptionalFieldsAndMinPropertiesAndPointer *WithOptionalFieldsAndMinProperties `json:"withOptionalFieldsAndMinPropertiesAndPointer"`
42+
43+
// Inline structs are ignored in this linter.
44+
WithRequiredField `json:",inline"`
45+
WithOptionalField `json:",inline"`
46+
WithRequiredAndOptionalField `json:",inline"`
47+
WithOptionalFieldsAndMinProperties `json:",inline"`
48+
}
49+
50+
type WithRequiredField struct {
51+
// +required
52+
RequiredField string `json:"requiredField"`
53+
}
54+
55+
type WithOptionalField struct {
56+
// +optional
57+
OptionalField string `json:"optionalField"`
58+
}
59+
60+
type WithRequiredAndOptionalField struct {
61+
// +k8s:required
62+
RequiredField string `json:"requiredField"`
63+
// +k8s:optional
64+
OptionalField string `json:"optionalField"`
65+
}
66+
67+
// +kubebuilder:validation:MinProperties=1
68+
type WithOptionalFieldsAndMinProperties struct {
69+
// +k8s:optional
70+
OptionalField string `json:"optionalField"`
71+
}

pkg/analysis/optionalfields/analyzer.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
2424
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
2525
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
2627
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils/serialization"
2728
"sigs.k8s.io/kube-api-linter/pkg/markers"
2829
)
@@ -105,8 +106,7 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, markersAcce
105106
return
106107
}
107108

108-
fieldMarkers := markersAccess.FieldMarkers(field)
109-
if !isFieldOptional(fieldMarkers) {
109+
if !utils.IsFieldOptional(field, markersAccess) {
110110
// The field is not marked optional, so we don't need to check it.
111111
return
112112
}
@@ -136,8 +136,3 @@ func defaultConfig(cfg *OptionalFieldsConfig) {
136136
cfg.OmitZero.Policy = OptionalFieldsOmitZeroPolicySuggestFix
137137
}
138138
}
139-
140-
// isFieldOptional checks if a field has an optional marker.
141-
func isFieldOptional(fieldMarkers markershelper.MarkerSet) bool {
142-
return fieldMarkers.Has(markers.OptionalMarker) || fieldMarkers.Has(markers.KubebuilderOptionalMarker)
143-
}

pkg/analysis/utils/zero_value.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,3 +449,13 @@ func IsFieldRequired(field *ast.Field, markersAccess markershelper.Markers) bool
449449
fieldMarkers.Has(markers.KubebuilderRequiredMarker) ||
450450
fieldMarkers.Has(markers.K8sRequiredMarker)
451451
}
452+
453+
// IsFieldOptional checks if the field is optional.
454+
// It checks for the presence of the optional marker, the kubebuilder optional marker, or the k8s optional marker.
455+
func IsFieldOptional(field *ast.Field, markersAccess markershelper.Markers) bool {
456+
fieldMarkers := markersAccess.FieldMarkers(field)
457+
458+
return fieldMarkers.Has(markers.OptionalMarker) ||
459+
fieldMarkers.Has(markers.KubebuilderOptionalMarker) ||
460+
fieldMarkers.Has(markers.K8sOptionalMarker)
461+
}

pkg/registration/doc.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/nodurations"
3939
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/nofloats"
4040
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/nomaps"
41+
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/nonpointerstructs"
4142
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/nonullable"
4243
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/nophase"
4344
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/noreferences"

0 commit comments

Comments
 (0)