Skip to content

Commit b352e0a

Browse files
committed
check only identical marker
Signed-off-by: sivchari <[email protected]>
1 parent e339eae commit b352e0a

File tree

4 files changed

+63
-30
lines changed

4 files changed

+63
-30
lines changed

README.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,10 @@ When the `json` tag is present, and matches the first word of the field comment
165165

166166
## DuplicateMarkers
167167

168-
The `duplicatemarkers` linter checks that all markers in the API types which have duplicated markers related with field and type.
168+
The `duplicatemarkers` linter checks for duplicate markers related with specific type or field.
169+
This linter diagnose _only_ if the marker and value together are completely unique. e.g. the `duplicatemarkers` will not diagnose the field has kubebuilder:validation:MaxLength=10 and kubebuilder:validation:MaxLength=11
169170

170-
### Fixes
171-
172-
The `duplicatemarkers` linter can automatically fix markers that are duplicated.
173-
174-
When the marker is duplicated, the linter will suggest to remove the duplicate marker.
171+
About the duplicated markers which has different value, it requires specific rule for each marker, these are processed by its corresponding linter.
175172

176173
## Integers
177174

pkg/analysis/duplicatemarkers/analyzer.go

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"go/ast"
2121

2222
"golang.org/x/tools/go/analysis"
23+
"k8s.io/apimachinery/pkg/util/sets"
2324

2425
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
2526
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
@@ -47,37 +48,56 @@ func run(pass *analysis.Pass) (any, error) {
4748
}
4849

4950
inspect.InspectFields(func(field *ast.Field, _ []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
50-
if len(field.Names) == 0 {
51-
return
52-
}
5351
checkField(pass, field, markersAccess)
5452
})
5553

54+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
55+
checkTypeSpec(pass, typeSpec, markersAccess)
56+
})
57+
5658
return nil, nil //nolint:nilnil
5759
}
5860

5961
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers) {
60-
set := markersAccess.FieldMarkers(field)
62+
if field == nil || len(field.Names) == 0 {
63+
return
64+
}
65+
66+
fieldMarkers := markersAccess.FieldMarkers(field)
6167

62-
fieldName := field.Names[0].Name
68+
set := sets.New[string]()
69+
70+
for _, marker := range fieldMarkers.UnsortedList() {
71+
if !set.Has(marker.String()) {
72+
set.Insert(marker.String())
73+
continue
74+
}
6375

64-
for _, marker := range set.UnsortedList() {
65-
// TODO: Add check whether the marker is a duuplicate or not.
6676
pass.Report(analysis.Diagnostic{
6777
Pos: field.Pos(),
68-
Message: fmt.Sprintf("%s has duplicated markers %s", fieldName, marker.String()),
69-
SuggestedFixes: []analysis.SuggestedFix{
70-
{
71-
Message: fmt.Sprintf("should remove `// +%s`", marker.String()),
72-
TextEdits: []analysis.TextEdit{
73-
{
74-
Pos: marker.Pos,
75-
End: marker.End,
76-
NewText: nil,
77-
},
78-
},
79-
},
80-
},
78+
Message: fmt.Sprintf("%s has duplicated markers %s", field.Names[0].Name, marker.String()),
79+
})
80+
}
81+
}
82+
83+
func checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
84+
if typeSpec == nil {
85+
return
86+
}
87+
88+
typeMarkers := markersAccess.TypeMarkers(typeSpec)
89+
90+
set := sets.New[string]()
91+
92+
for _, marker := range typeMarkers.UnsortedList() {
93+
if !set.Has(marker.String()) {
94+
set.Insert(marker.String())
95+
continue
96+
}
97+
98+
pass.Report(analysis.Diagnostic{
99+
Pos: typeSpec.Pos(),
100+
Message: fmt.Sprintf("%s has duplicated markers %s", typeSpec.Name.Name, marker.String()),
81101
})
82102
}
83103
}

pkg/analysis/duplicatemarkers/doc.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ limitations under the License.
1515
*/
1616

1717
/*
18-
duplicatemarkers is an analyzer that checks for duplicated markers in the code.
18+
duplicatemarkers is an analyzer that checks for duplicate markers in the API types.
19+
20+
It reports only if the marker and value together are completely unique now. For example, the `duplicatemarkers` will not diagnose the field has kubebuilder:validation:MaxLength=10 and kubebuilder:validation:MaxLength=11
21+
22+
Example:
23+
24+
type Foo struct {
25+
// +kubebuilder:validation:MaxLength=10
26+
// +kubebuilder:validation:MaxLength=11
27+
Field string `json:"field"`
28+
}
29+
30+
// +kubebuilder:validation:MaxLength=10
31+
// +kubebuilder:validation:MaxLength=11
32+
type Bar string
33+
34+
About the duplicated markers which has different value, it requires specific rule for each marker, these are processed by its corresponding linter.
1935
*/
2036
package duplicatemarkers

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type Enum string // want "Enum has duplicated markers kubebuilder:validation:Enu
1010

1111
// +kubebuilder:validation:MaxLength=10
1212
// +kubebuilder:validation:MaxLength=11
13-
type MaxLength int // want "MaxLength has duplicated markers kubebuilder:validation:MaxLength"
13+
type MaxLength int
1414

1515
// +kubebuilder:object:root=true
1616
// +kubebuilder:subresource:status
@@ -43,14 +43,14 @@ type DuplicateMarkerSpec struct { // want "DuplicateMarkerSpec has duplicated ma
4343

4444
// +kubebuilder:validation:MaxLength=11
4545
// +kubebuilder:validation:MaxLength=10
46-
DuplicatedMaxlength int `json:"maxlength"` // want "DuplicatedMaxlength has duplicated markers kubebuilder:validation:MaxLength"
46+
DuplicatedMaxlength int `json:"maxlength"`
4747

4848
// +listType=map
4949
// +listMapKey=primaryKey
5050
// +listMapKey=secondaryKey
5151
// +listType=map
5252
// +required
53-
DuplicatedListTypeMap Map `json:"duplicatedListTypeMap"` // want "DuplicatedListTypeMap has duplicated markers listType=map, listMapKey=primaryKey, listMapKey=secondaryKey"
53+
DuplicatedListTypeMap Map `json:"duplicatedListTypeMap"` // want "DuplicatedListTypeMap has duplicated markers listType=map"
5454

5555
// +optional
5656
// +kubebuilder:validation:XValidation:rule="self >= 1 && self <= 3",message="must be 1 to 5"

0 commit comments

Comments
 (0)