Skip to content

Commit e462791

Browse files
committed
Address reveiw comments
1 parent 257922b commit e462791

File tree

7 files changed

+116
-55
lines changed

7 files changed

+116
-55
lines changed

docs/linters.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
- [NoFloats](#nofloats) - Prevents usage of floating-point types
1111
- [Nomaps](#nomaps) - Restricts usage of map types
1212
- [Nophase](#nophase) - Prevents usage of 'Phase' fields
13-
- [Notimestamp](#Notimestamp) - Prevents usage of `TimeStamp` fields
13+
- [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields
1414
- [OptionalFields](#optionalfields) - Validates optional field conventions
1515
- [OptionalOrRequired](#optionalorrequired) - Ensures fields are explicitly marked as optional or required
1616
- [RequiredFields](#requiredfields) - Validates required field conventions

go.mod

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module sigs.k8s.io/kube-api-linter
33
go 1.24.0
44

55
require (
6+
github.com/golangci/golangci-lint/v2 v2.1.6
67
github.com/golangci/plugin-module-register v0.1.1
78
github.com/onsi/ginkgo/v2 v2.23.3
89
github.com/onsi/gomega v1.36.3
@@ -12,19 +13,24 @@ require (
1213
)
1314

1415
require (
16+
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
17+
github.com/fatih/color v1.18.0 // indirect
1518
github.com/go-logr/logr v1.4.2 // indirect
1619
github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
1720
github.com/google/go-cmp v0.7.0 // indirect
1821
github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad // indirect
1922
github.com/kr/pretty v0.3.1 // indirect
20-
github.com/rogpeppe/go-internal v1.14.1 // indirect
23+
github.com/mattn/go-colorable v0.1.14 // indirect
24+
github.com/mattn/go-isatty v0.0.20 // indirect
25+
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
26+
github.com/sirupsen/logrus v1.9.3 // indirect
27+
github.com/stretchr/objx v0.5.2 // indirect
2128
github.com/stretchr/testify v1.10.0 // indirect
2229
golang.org/x/mod v0.24.0 // indirect
2330
golang.org/x/net v0.39.0 // indirect
2431
golang.org/x/sync v0.13.0 // indirect
2532
golang.org/x/sys v0.32.0 // indirect
2633
golang.org/x/text v0.24.0 // indirect
27-
google.golang.org/protobuf v1.36.6 // indirect
2834
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
2935
gopkg.in/yaml.v3 v3.0.1 // indirect
3036
)

go.sum

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
2+
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
3+
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
24
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
35
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
6+
github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
7+
github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
48
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
59
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
610
github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI=
711
github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8=
12+
github.com/golangci/golangci-lint/v2 v2.1.6 h1:LXqShFfAGM5BDzEOWD2SL1IzJAgUOqES/HRBsfKjI+w=
13+
github.com/golangci/golangci-lint/v2 v2.1.6/go.mod h1:EPj+fgv4TeeBq3TcqaKZb3vkiV5dP4hHHKhXhEhzci8=
814
github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c=
915
github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc=
1016
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
@@ -18,16 +24,27 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
1824
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
1925
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
2026
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
27+
github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
28+
github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
29+
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
30+
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
2131
github.com/onsi/ginkgo/v2 v2.23.3 h1:edHxnszytJ4lD9D5Jjc4tiDkPBZ3siDeJJkUZJJVkp0=
2232
github.com/onsi/ginkgo/v2 v2.23.3/go.mod h1:zXTP6xIp3U8aVuXN8ENK9IXRaTjFnpVB9mGmaSRvxnM=
2333
github.com/onsi/gomega v1.36.3 h1:hID7cr8t3Wp26+cYnfcjR6HpJ00fdogN6dqZ1t6IylU=
2434
github.com/onsi/gomega v1.36.3/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0=
2535
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
36+
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
2637
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
2738
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
2839
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
2940
github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
3041
github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc=
42+
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
43+
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
44+
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
45+
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
46+
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
47+
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
3148
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
3249
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
3350
golang.org/x/mod v0.24.0 h1:ZfthKaKaT4NrhGVZHO1/WDTwGES4De8KtWO0SIbNJMU=
@@ -36,6 +53,8 @@ golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY=
3653
golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E=
3754
golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610=
3855
golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
56+
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
57+
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
3958
golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20=
4059
golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
4160
golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0=
@@ -47,6 +66,7 @@ google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/
4766
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
4867
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
4968
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
69+
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
5070
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
5171
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
5272
k8s.io/apimachinery v0.32.3 h1:JmDuDarhDmA/Li7j3aPrwhpNBA94Nvk5zLeOge9HH1U=

pkg/analysis/notimestamp/analyzer.go

Lines changed: 63 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@ limitations under the License.
1717
package notimestamp
1818

1919
import (
20+
"fmt"
2021
"go/ast"
22+
"go/token"
23+
"regexp"
2124
"strings"
2225

2326
"golang.org/x/tools/go/analysis"
24-
"golang.org/x/tools/go/analysis/passes/inspect"
25-
"golang.org/x/tools/go/ast/inspector"
2627
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
2728
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
29+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
30+
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
2831
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
2932
)
3033

@@ -35,63 +38,75 @@ const name = "notimestamp"
3538
// substring are present.
3639
var Analyzer = &analysis.Analyzer{
3740
Name: name,
38-
Doc: "check that non of the struct field named timestamp or contain timestamp as a substring",
41+
Doc: "check that none of the struct field named timestamp or contain timestamp as a substring",
3942
Run: run,
40-
Requires: []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer},
43+
Requires: []*analysis.Analyzer{inspector.Analyzer},
4144
}
4245

46+
// case insensitive regular expression to match 'timestamp' string in field or json tag.
47+
var timeStampRegEx = regexp.MustCompile("(?i)timestamp")
48+
4349
func run(pass *analysis.Pass) (any, error) {
44-
inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
50+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
4551
if !ok {
4652
return nil, kalerrors.ErrCouldNotGetInspector
4753
}
4854

49-
jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
50-
if !ok {
51-
return nil, kalerrors.ErrCouldNotGetJSONTags
55+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) {
56+
checkFieldsAndTags(pass, field, jsonTagInfo)
57+
})
58+
59+
return nil, nil //nolint:nilnil
60+
}
61+
62+
func checkFieldsAndTags(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.FieldTagInfo) {
63+
fieldName := utils.FieldName(field)
64+
if fieldName == "" {
65+
return
5266
}
5367

54-
// Filter to fields so that we can iterate over fields in a struct.
55-
nodeFilter := []ast.Node{
56-
(*ast.Field)(nil),
68+
var suggestedFixes []analysis.SuggestedFix
69+
70+
// check if filed name contains timestamp in it.
71+
fieldReplacementName := timeStampRegEx.ReplaceAllString(fieldName, "Time")
72+
if fieldReplacementName != fieldName {
73+
suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{
74+
Message: "replace field with Time",
75+
TextEdits: []analysis.TextEdit{
76+
{
77+
Pos: field.Pos(),
78+
NewText: []byte(fieldReplacementName),
79+
End: field.Pos() + token.Pos(len(fieldName)),
80+
},
81+
},
82+
})
5783
}
5884

59-
// Preorder visits all the nodes of the AST in depth-first order. It calls
60-
// f(n) for each node n before it visits n's children.
61-
//
62-
// We use the filter defined above, ensuring we only look at struct fields.
63-
inspect.Preorder(nodeFilter, func(n ast.Node) {
64-
field, ok := n.(*ast.Field)
65-
if !ok {
66-
return
67-
}
68-
69-
if field == nil || len(field.Names) == 0 {
70-
return
71-
}
72-
73-
fieldName := utils.FieldName(field)
74-
75-
// First check if the struct field name contains 'timestamp'
76-
if strings.Contains(strings.ToLower(fieldName), "timestamp") {
77-
pass.Reportf(field.Pos(),
78-
"field %s: fields with timestamp substring should be avoided",
79-
fieldName,
80-
)
81-
82-
return
83-
}
84-
85-
// Then check if the json serialization of the field contains 'timestamp'
86-
tagInfo := jsonTags.FieldTags(field)
87-
88-
if strings.Contains(strings.ToLower(tagInfo.Name), "timestamp") {
89-
pass.Reportf(field.Pos(),
90-
"field %s: fields with timestamp substring should be avoided",
91-
fieldName,
92-
)
93-
}
94-
})
85+
// check if the tag contains timestamp in it.
86+
tagReplacementName := timeStampRegEx.ReplaceAllString(tagInfo.Name, "Time")
87+
if strings.HasPrefix(strings.ToLower(tagInfo.Name), "time") {
88+
// If tag is starts with 'timeStamp', the replacement should be 'time' not 'Time'.
89+
tagReplacementName = timeStampRegEx.ReplaceAllString(tagInfo.Name, "time")
90+
}
9591

96-
return nil, nil //nolint:nilnil
92+
if tagReplacementName != tagInfo.Name {
93+
suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{
94+
Message: "replace json tag with Time",
95+
TextEdits: []analysis.TextEdit{
96+
{
97+
Pos: tagInfo.Pos,
98+
NewText: []byte(tagReplacementName),
99+
End: tagInfo.Pos + token.Pos(len(tagInfo.Name)),
100+
},
101+
},
102+
})
103+
}
104+
105+
if len(suggestedFixes) > 0 {
106+
pass.Report(analysis.Diagnostic{
107+
Pos: field.Pos(),
108+
Message: fmt.Sprintf("field %s: prefer use of the term time over timestamp", fieldName),
109+
SuggestedFixes: suggestedFixes,
110+
})
111+
}
97112
}

pkg/analysis/notimestamp/analyzer_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ limitations under the License.
1717
package notimestamp_test
1818

1919
import (
20-
"sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp"
2120
"testing"
2221

2322
"golang.org/x/tools/go/analysis/analysistest"
23+
"sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp"
2424
)
2525

2626
func Test(t *testing.T) {

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,15 @@ import "time"
44

55
type NoTimeStampTestStruct struct {
66
// +optional
7-
TimeStamp *time.Time `json:"timeStamp,omitempty"` // want "field TimeStamp: fields with timestamp substring should be avoided"
7+
TimeStamp *time.Time `json:"timeStamp,omitempty"` // want "field TimeStamp: prefer use of the term time over timestamp"
88
}
99

1010
// DoNothing is used to check that the analyser doesn't report on methods.
1111
func (NoTimeStampTestStruct) DoNothing() {}
1212

1313
type NoSubTimeStampTestStruct struct {
1414
// +optional
15-
FooTimeStamp *time.Time `json:"fooTimeStamp,omitempty"` // want "field FooTimeStamp: fields with timestamp substring should be avoided"
16-
15+
FooTimeStamp *time.Time `json:"fooTimeStamp,omitempty"` // want "field FooTimeStamp: prefer use of the term time over timestamp"
1716
}
1817

1918
type SerializedTimeStampTestStruct struct {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package a
2+
3+
import "time"
4+
5+
type NoTimeStampTestStruct struct {
6+
// +optional
7+
Time *time.Time `json:"time,omitempty"` // want "field TimeStamp: prefer use of the term time over timestamp"
8+
}
9+
10+
// DoNothing is used to check that the analyser doesn't report on methods.
11+
func (NoTimeStampTestStruct) DoNothing() {}
12+
13+
type NoSubTimeStampTestStruct struct {
14+
// +optional
15+
FooTime *time.Time `json:"fooTime,omitempty"` // want "field FooTimeStamp: prefer use of the term time over timestamp"
16+
}
17+
18+
type SerializedTimeStampTestStruct struct {
19+
// +optional
20+
FooTime *time.Time `json:"fooTime,omitempty"`
21+
}

0 commit comments

Comments
 (0)