Skip to content

Commit 0723046

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #61 from sivchari/add-duplicatemarkers-linter
add duplicatemarkers linter
2 parents 00a5c0f + c9f0d68 commit 0723046

File tree

11 files changed

+557
-0
lines changed

11 files changed

+557
-0
lines changed

‎README.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,24 @@ The `commentstart` linter can automatically fix comments that do not start with
194194

195195
When the `json` tag is present, and matches the first word of the field comment in all but casing, the linter will suggest that the comment be updated to match the `json` tag.
196196

197+
## DuplicateMarkers
198+
199+
The duplicatemarkers linter checks for exact duplicates of markers for types and fields.
200+
This means that something like:
201+
202+
// +kubebuilder:validation:MaxLength=10
203+
// +kubebuilder:validation:MaxLength=10
204+
Will be flagged by this linter, while something like:
205+
206+
// +kubebuilder:validation:MaxLength=10
207+
// +kubebuilder:validation:MaxLength=11
208+
will not.
209+
210+
### Fixes
211+
212+
The `duplicatemarkers` linter can automatically fix all markers that are exact match to another markers.
213+
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.
214+
197215
## Integers
198216

199217
The `integers` linter checks for usage of unsupported integer types.

‎pkg/analysis/duplicatemarkers/analyzer.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
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 duplicatemarkers
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
"go/token"
22+
"go/types"
23+
24+
"golang.org/x/tools/go/analysis"
25+
26+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
28+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
29+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
30+
)
31+
32+
const (
33+
name = "duplicatemarkers"
34+
)
35+
36+
// Analyzer is the analyzer for the duplicatemarkers package.
37+
// It checks for duplicate markers on struct fields.
38+
var Analyzer = &analysis.Analyzer{
39+
Name: name,
40+
Doc: "Check for duplicate markers on defined types and struct fields.",
41+
Run: run,
42+
Requires: []*analysis.Analyzer{inspector.Analyzer},
43+
}
44+
45+
func run(pass *analysis.Pass) (any, error) {
46+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
47+
if !ok {
48+
return nil, kalerrors.ErrCouldNotGetInspector
49+
}
50+
51+
inspect.InspectFields(func(field *ast.Field, _ []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
52+
checkField(pass, field, markersAccess)
53+
})
54+
55+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
56+
checkTypeSpec(pass, typeSpec, markersAccess)
57+
})
58+
59+
return nil, nil //nolint:nilnil
60+
}
61+
62+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers) {
63+
if field == nil || len(field.Names) == 0 {
64+
return
65+
}
66+
67+
markerSet := collectMarkers(pass, markersAccess, field)
68+
69+
seen := markers.NewMarkerSet()
70+
71+
for _, marker := range markerSet.UnsortedList() {
72+
if !seen.HasWithValue(marker.String()) {
73+
seen.Insert(marker)
74+
continue
75+
}
76+
77+
report(pass, field.Pos(), field.Names[0].Name, marker)
78+
}
79+
}
80+
81+
func collectMarkers(pass *analysis.Pass, markersAccess markers.Markers, field *ast.Field) markers.MarkerSet {
82+
markers := markersAccess.FieldMarkers(field)
83+
84+
if _, ok := pass.TypesInfo.TypeOf(field.Type).(*types.Basic); ok {
85+
return markers
86+
}
87+
88+
ident, ok := field.Type.(*ast.Ident)
89+
if !ok {
90+
return markers
91+
}
92+
93+
typeSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)
94+
if !ok {
95+
return markers
96+
}
97+
98+
typeMarkers := markersAccess.TypeMarkers(typeSpec)
99+
100+
// duplicatemarkers removes duplicate markers without the first line.
101+
// By inserting the field markers after the type markers, it allows the markers to be removed from the field.
102+
typeMarkers.Insert(markers.UnsortedList()...)
103+
104+
return typeMarkers
105+
}
106+
107+
func checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
108+
if typeSpec == nil {
109+
return
110+
}
111+
112+
typeMarkers := markersAccess.TypeMarkers(typeSpec)
113+
114+
markerSet := markers.NewMarkerSet()
115+
116+
for _, marker := range typeMarkers.UnsortedList() {
117+
if !markerSet.HasWithValue(marker.String()) {
118+
markerSet.Insert(marker)
119+
continue
120+
}
121+
122+
report(pass, typeSpec.Pos(), typeSpec.Name.Name, marker)
123+
}
124+
}
125+
126+
func report(pass *analysis.Pass, pos token.Pos, fieldName string, marker markers.Marker) {
127+
pass.Report(analysis.Diagnostic{
128+
Pos: pos,
129+
Message: fmt.Sprintf("%s has duplicated markers %s", fieldName, marker),
130+
SuggestedFixes: []analysis.SuggestedFix{
131+
{
132+
Message: fmt.Sprintf("Remove duplicated marker %s", marker),
133+
TextEdits: []analysis.TextEdit{
134+
{
135+
Pos: marker.Pos,
136+
// To remove the duplicated marker, we need to remove the whole line.
137+
End: marker.End + 1,
138+
},
139+
},
140+
},
141+
},
142+
})
143+
}

‎pkg/analysis/duplicatemarkers/analyzer_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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 duplicatemarkers_test
17+
18+
import (
19+
"testing"
20+
21+
"golang.org/x/tools/go/analysis/analysistest"
22+
"sigs.k8s.io/kube-api-linter/pkg/analysis/duplicatemarkers"
23+
)
24+
25+
func Test(t *testing.T) {
26+
testdata := analysistest.TestData()
27+
analysistest.RunWithSuggestedFixes(t, testdata, duplicatemarkers.Analyzer, "a")
28+
}

‎pkg/analysis/duplicatemarkers/doc.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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+
duplicatemarkers is an analyzer that checks for duplicate markers in the API types.
19+
It reports exact matches for marker definitions.
20+
21+
For example, something like:
22+
23+
type Foo struct {
24+
// +kubebuilder:validation:MaxLength=10
25+
// +kubebuilder:validation:MaxLength=11
26+
type Bar string
27+
}
28+
29+
would not be reported while something like:
30+
31+
type Foo struct {
32+
// +kubebuilder:validation:MaxLength=10
33+
// +kubebuilder:validation:MaxLength=10
34+
type Bar string
35+
}
36+
37+
would be reported.
38+
39+
This linter also be able to automatically fix all markers that are exact match to another markers.
40+
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.
41+
*/
42+
package duplicatemarkers

‎pkg/analysis/duplicatemarkers/initializer.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
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 duplicatemarkers
17+
18+
import (
19+
"golang.org/x/tools/go/analysis"
20+
"sigs.k8s.io/kube-api-linter/pkg/config"
21+
)
22+
23+
// Initializer returns the AnalyzerInitializer for this
24+
// Analyzer so that it can be added to the registry.
25+
func Initializer() initializer {
26+
return initializer{}
27+
}
28+
29+
// intializer implements the AnalyzerInitializer interface.
30+
type initializer struct{}
31+
32+
// Name returns the name of the Analyzer.
33+
func (initializer) Name() string {
34+
return name
35+
}
36+
37+
// Init returns the intialized Analyzer.
38+
func (initializer) Init(cfg config.LintersConfig) (*analysis.Analyzer, error) {
39+
return Analyzer, nil
40+
}
41+
42+
// Default determines whether this Analyzer is on by default, or not.
43+
func (initializer) Default() bool {
44+
// Duplicated markers are a sign of bad code, and should be avoided.
45+
// This is a good rule to have on by default.
46+
return true
47+
}

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package a
2+
3+
// It must be ignored since it is not a type
4+
// +kubebuilder:validation:Enum=foo;bar;baz
5+
// +kubebuilder:validation:Enum=foo;bar;baz
6+
var Variable string
7+
8+
// +kubebuilder:validation:Enum=foo;bar;baz
9+
// +kubebuilder:validation:Enum=foo;bar;baz
10+
type Enum string // want "Enum has duplicated markers kubebuilder:validation:Enum"
11+
12+
// +kubebuilder:validation:MaxLength=10
13+
// +kubebuilder:validation:MaxLength=11
14+
type MaxLength int
15+
16+
// +kubebuilder:validation:MaxLength=10
17+
// +kubebuilder:validation:MaxLength=10
18+
type DuplicatedMaxLength int // want "DuplicatedMaxLength has duplicated markers kubebuilder:validation:MaxLength=10"
19+
20+
// +kubebuilder:object:root=true
21+
// +kubebuilder:subresource:status
22+
// +kubebuilder:object:root=true
23+
type DuplicateMarkerSpec struct { // want "DuplicateMarkerSpec has duplicated markers kubebuilder:object:root"
24+
// +kubebuilder:validation:Required
25+
// should be ignored since it only has single marker
26+
Required string `json:"required"`
27+
28+
// +listType=map
29+
// +listMapKey=primaryKey
30+
// +listMapKey=secondaryKey
31+
// +required
32+
// should be ignored since listMapKey is allowed to have different values
33+
Map Map `json:"map"`
34+
35+
// +optional
36+
// +kubebuilder:validation:XValidation:rule="self >= 1 && self <= 3",message="must be 1 to 5"
37+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="replicas must be immutable"
38+
// should be ignored since XValidation is allowed to have different values
39+
Replicas *int `json:"replicas"`
40+
41+
// +kubebuilder:validation:MaxLength=11
42+
// +kubebuilder:validation:MaxLength=10
43+
// should be ignored since MaxLength is allowed to have different values
44+
Maxlength int `json:"maxlength"`
45+
46+
// +kubebuilder:validation:Required
47+
// +kubebuilder:validation:Required
48+
DuplicatedRequired string `json:"duplicatedRequired"` // want "DuplicatedRequired has duplicated markers kubebuilder:validation:Required"
49+
50+
// +kubebuilder:validation:Enum=foo;bar;baz
51+
// +kubebuilder:validation:Enum=foo;bar;baz
52+
DuplicatedEnum string `json:"duplicatedEnum"` // want "DuplicatedEnum has duplicated markers kubebuilder:validation:Enum"
53+
54+
// +kubebuilder:validation:MaxLength=10
55+
// +kubebuilder:validation:MaxLength=10
56+
DuplicatedMaxLength int `json:"duplicatedMaxLength"` // want "DuplicatedMaxLength has duplicated markers kubebuilder:validation:MaxLength=10"
57+
58+
// +kubebuilder:validation:MaxLength=10
59+
DuplicatedMaxLengthIncludingTypeMarker MaxLength `json:"duplicatedMaxLengthIncludingTypeMarker"` // want "DuplicatedMaxLengthIncludingTypeMarker has duplicated markers kubebuilder:validation:MaxLength=10"
60+
61+
// +listType=map
62+
// +listMapKey=primaryKey
63+
// +listMapKey=secondaryKey
64+
// +listType=map
65+
// +required
66+
DuplicatedListTypeMap Map `json:"duplicatedListTypeMap"` // want "DuplicatedListTypeMap has duplicated markers listType=map"
67+
68+
// +optional
69+
// +kubebuilder:validation:XValidation:rule="self >= 1 && self <= 3",message="must be 1 to 5"
70+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="replicas must be immutable"
71+
// +kubebuilder:validation:XValidation:rule="self >= 1 && self <= 3",message="must be 1 to 5"
72+
DuplicatedReplicas *int `json:"duplicatedReplicas"` // want "DuplicatedReplicas has duplicated markers kubebuilder:validation:XValidation:rule=\"self >= 1 && self <= 3\",message=\"must be 1 to 5\""
73+
74+
// +optional
75+
// +kubebuilder:validation:XValidation:rule="self >= 1 && self <= 3",message="must be 1 to 5"
76+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="replicas must be immutable"
77+
// +kubebuilder:validation:XValidation:message="must be 1 to 5",rule="self >= 1 && self <= 3"
78+
DuplicatedUnorderedValidationReplicas *int `json:"duplicatedUnorderedValidationReplicas"` // want "DuplicatedUnorderedValidationReplicas has duplicated markers kubebuilder:validation:XValidation:message=\"must be 1 to 5\",rule=\"self >= 1 && self <= 3\""
79+
}
80+
81+
type Map struct {
82+
// +required
83+
PrimaryKey string `json:"primaryKey"`
84+
// +required
85+
SecondaryKey string `json:"secondaryKey"`
86+
// +required
87+
Value string `json:"value"`
88+
}

0 commit comments

Comments
 (0)