Skip to content

Commit ef33eac

Browse files
authored
Merge pull request #111 from everettraven/linter/forbiddenmarkers
linters: add forbiddenmarkers and nonullable linters
2 parents 0660253 + c532ff6 commit ef33eac

File tree

17 files changed

+1404
-1
lines changed

17 files changed

+1404
-1
lines changed

docs/linters.md

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
- [CommentStart](#commentstart) - Ensures comments start with the serialized form of the type
55
- [ConflictingMarkers](#conflictingmarkers) - Detects mutually exclusive markers on the same field
66
- [DuplicateMarkers](#duplicatemarkers) - Checks for exact duplicates of markers
7+
- [ForbiddenMarkers](#forbiddenmarkers) - Checks that no forbidden markers are present on types/fields.
78
- [Integers](#integers) - Validates usage of supported integer types
89
- [JSONTags](#jsontags) - Ensures proper JSON tag formatting
910
- [MaxLength](#maxlength) - Checks for maximum length constraints on strings and arrays
1011
- [NoBools](#nobools) - Prevents usage of boolean types
1112
- [NoFloats](#nofloats) - Prevents usage of floating-point types
1213
- [Nomaps](#nomaps) - Restricts usage of map types
14+
- [NoNullable](#nonullable) - Prevents usage of the nullable marker
1315
- [Nophase](#nophase) - Prevents usage of 'Phase' fields
1416
- [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields
1517
- [OptionalFields](#optionalfields) - Validates optional field conventions
@@ -137,6 +139,134 @@ will not.
137139
The `duplicatemarkers` linter can automatically fix all markers that are exact match to another markers.
138140
If there are duplicates across fields and their underlying type, the marker on the type will be preferred and the marker on the field will be removed.
139141

142+
## ForbiddenMarkers
143+
144+
The `forbiddenmarkers` linter ensures that types and fields do not contain any markers that are forbidden.
145+
146+
By default, `forbiddenmarkers` is not enabled.
147+
148+
### Configuation
149+
150+
It can be configured with a list of marker identifiers and optionally their attributes and values that are forbidden.
151+
152+
Some examples configurations explained:
153+
154+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker`
155+
156+
```yaml
157+
linterConfig:
158+
forbiddenmarkers:
159+
markers:
160+
- identifier: "forbidden:marker"
161+
```
162+
163+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` containing the attribute `fruit`
164+
165+
```yaml
166+
linterConfig:
167+
forbiddenmarkers:
168+
markers:
169+
- identifier: "forbidden:marker"
170+
ruleSets:
171+
- attributes:
172+
- name: "fruit"
173+
```
174+
175+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` containing the `fruit` AND `color` attributes
176+
177+
```yaml
178+
linterConfig:
179+
forbiddenmarkers:
180+
markers:
181+
- identifier: "forbidden:marker"
182+
ruleSets:
183+
- attributes:
184+
- name: "fruit"
185+
- name: "color"
186+
```
187+
188+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` where the `fruit` attribute is set to one of `apple`, `banana`, or `orange`
189+
190+
```yaml
191+
linterConfig:
192+
forbiddenmarkers:
193+
markers:
194+
- identifier: "forbidden:marker"
195+
ruleSets:
196+
- attributes:
197+
- name: "fruit"
198+
values:
199+
- "apple"
200+
- "banana"
201+
- "orange"
202+
```
203+
204+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` where the `fruit` attribute is set to one of `apple`, `banana`, or `orange` AND the `color` attribute is set to one of `red`, `green`, or `blue`
205+
206+
```yaml
207+
linterConfig:
208+
forbiddenmarkers:
209+
markers:
210+
- identifier: "forbidden:marker"
211+
ruleSets:
212+
- attributes:
213+
- name: "fruit"
214+
values:
215+
- "apple"
216+
- "banana"
217+
- "orange"
218+
- name: "color"
219+
values:
220+
- "red"
221+
- "blue"
222+
- "green"
223+
```
224+
225+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` where:
226+
227+
- The `fruit` attribute is set to `apple` and the `color` attribute is set to one of `blue` or `orange` (allow any other color apple)
228+
229+
_OR_
230+
231+
- The `fruit` attribute is set to `orange` and the `color` attribute is set to one of `blue`, `red`, or `green` (allow any other color orange)
232+
233+
_OR_
234+
235+
- The `fruit` attribute is set to `banana` (no bananas allowed)
236+
237+
```yaml
238+
linterConfig:
239+
forbiddenmarkers:
240+
markers:
241+
- identifier: "forbidden:marker"
242+
ruleSets:
243+
- attributes:
244+
- name: "fruit"
245+
values:
246+
- "apple"
247+
- name: "color"
248+
values:
249+
- "blue"
250+
- "orange"
251+
- attributes:
252+
- name: "fruit"
253+
values:
254+
- "orange"
255+
- name: "color"
256+
values:
257+
- "blue"
258+
- "red"
259+
- "green"
260+
- attributes:
261+
- name: "fruit"
262+
values:
263+
- "banana"
264+
```
265+
266+
### Fixes
267+
268+
Fixes are suggested to remove all markers that are forbidden.
269+
140270
## Integers
141271

142272
The `integers` linter checks for usage of unsupported integer types.
@@ -200,6 +330,14 @@ lintersConfig:
200330
policy: Enforce | AllowStringToStringMaps | Ignore # Determines how the linter should handle maps of simple types. Defaults to AllowStringToStringMaps.
201331
```
202332

333+
## NoNullable
334+
335+
The `nonullable` linter ensures that types and fields do not have the `nullable` marker.
336+
337+
### Fixes
338+
339+
Fixes are suggested to remove the `nullable` marker.
340+
203341
## Notimestamp
204342

205343
The `notimestamp` linter checks that the fields in the API are not named with the word 'Timestamp'.
@@ -380,10 +518,12 @@ linter will suggest adding the comment `// +kubebuilder:subresource:status` abov
380518

381519
The `uniquemarkers` linter ensures that types and fields do not contain more than a single definition of a marker that should only be present once.
382520

383-
Because this linter has no way of determining which marker definition was intended it does not suggest any fixes
521+
Because this linter has no way of determining which marker definition was intended it does not suggest any fixes
384522

385523
### Configuration
524+
386525
It can configured to include a set of custom markers in the analysis by setting:
526+
387527
```yaml
388528
lintersConfig:
389529
uniquemarkers:
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
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 forbiddenmarkers
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
"slices"
22+
23+
"golang.org/x/tools/go/analysis"
24+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
28+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
29+
)
30+
31+
const name = "forbiddenmarkers"
32+
33+
type analyzer struct {
34+
forbiddenMarkers []Marker
35+
}
36+
37+
// NewAnalyzer creates a new analysis.Analyzer for the forbiddenmarkers
38+
// linter based on the provided config.ForbiddenMarkersConfig.
39+
func newAnalyzer(cfg *Config) *analysis.Analyzer {
40+
a := &analyzer{
41+
forbiddenMarkers: cfg.Markers,
42+
}
43+
44+
analyzer := &analysis.Analyzer{
45+
Name: name,
46+
Doc: "Check that no forbidden markers are present on types and fields.",
47+
Run: a.run,
48+
Requires: []*analysis.Analyzer{inspector.Analyzer},
49+
}
50+
51+
for _, marker := range a.forbiddenMarkers {
52+
markers.DefaultRegistry().Register(marker.Identifier)
53+
}
54+
55+
return analyzer
56+
}
57+
58+
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
59+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
60+
if !ok {
61+
return nil, kalerrors.ErrCouldNotGetInspector
62+
}
63+
64+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
65+
checkField(pass, field, markersAccess, a.forbiddenMarkers)
66+
})
67+
68+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
69+
checkType(pass, typeSpec, markersAccess, a.forbiddenMarkers)
70+
})
71+
72+
return nil, nil //nolint:nilnil
73+
}
74+
75+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, forbiddenMarkers []Marker) {
76+
if field == nil || len(field.Names) == 0 {
77+
return
78+
}
79+
80+
markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
81+
check(markers, forbiddenMarkers, reportField(pass, field))
82+
}
83+
84+
func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers, forbiddenMarkers []Marker) {
85+
if typeSpec == nil {
86+
return
87+
}
88+
89+
markers := markersAccess.TypeMarkers(typeSpec)
90+
check(markers, forbiddenMarkers, reportType(pass, typeSpec))
91+
}
92+
93+
func check(markerSet markers.MarkerSet, forbiddenMarkers []Marker, reportFunc func(marker markers.Marker)) {
94+
for _, marker := range forbiddenMarkers {
95+
marks := markerSet.Get(marker.Identifier)
96+
for _, mark := range marks {
97+
if len(marker.RuleSets) == 0 {
98+
reportFunc(mark)
99+
continue
100+
}
101+
102+
for _, ruleSet := range marker.RuleSets {
103+
if markerMatchesAttributeRules(mark, ruleSet.Attributes...) {
104+
reportFunc(mark)
105+
}
106+
}
107+
}
108+
}
109+
}
110+
111+
func markerMatchesAttributeRules(marker markers.Marker, attrRules ...MarkerAttribute) bool {
112+
for _, attrRule := range attrRules {
113+
// if the marker doesn't contain the attribute for a specified rule it fails the AND
114+
// operation.
115+
val, ok := marker.Expressions[attrRule.Name]
116+
if !ok {
117+
return false
118+
}
119+
120+
// if the value doesn't match one of the forbidden ones, this marker is not forbidden
121+
if len(attrRule.Values) > 0 && !slices.Contains(attrRule.Values, val) {
122+
return false
123+
}
124+
}
125+
126+
return true
127+
}
128+
129+
func reportField(pass *analysis.Pass, field *ast.Field) func(marker markers.Marker) {
130+
return func(marker markers.Marker) {
131+
pass.Report(analysis.Diagnostic{
132+
Pos: field.Pos(),
133+
Message: fmt.Sprintf("field %s has forbidden marker %q", field.Names[0].Name, marker.String()),
134+
SuggestedFixes: []analysis.SuggestedFix{
135+
{
136+
Message: fmt.Sprintf("remove forbidden marker %q", marker.String()),
137+
TextEdits: []analysis.TextEdit{
138+
{
139+
Pos: marker.Pos,
140+
End: marker.End,
141+
},
142+
},
143+
},
144+
},
145+
})
146+
}
147+
}
148+
149+
func reportType(pass *analysis.Pass, typeSpec *ast.TypeSpec) func(marker markers.Marker) {
150+
return func(marker markers.Marker) {
151+
pass.Report(analysis.Diagnostic{
152+
Pos: typeSpec.Pos(),
153+
Message: fmt.Sprintf("type %s has forbidden marker %q", typeSpec.Name, marker.String()),
154+
SuggestedFixes: []analysis.SuggestedFix{
155+
{
156+
Message: fmt.Sprintf("remove forbidden marker %q", marker.String()),
157+
TextEdits: []analysis.TextEdit{
158+
{
159+
Pos: marker.Pos,
160+
End: marker.End,
161+
},
162+
},
163+
},
164+
},
165+
})
166+
}
167+
}

0 commit comments

Comments
 (0)