From 257922b6b9023f22d0fed6c375e320fd641a4d3b Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Wed, 25 Jun 2025 23:34:34 +0530 Subject: [PATCH 1/2] Add notimestamp linter --- docs/linters.md | 5 + pkg/analysis/notimestamp/analyzer.go | 97 ++++++++++++++++++++ pkg/analysis/notimestamp/analyzer_test.go | 29 ++++++ pkg/analysis/notimestamp/doc.go | 24 +++++ pkg/analysis/notimestamp/initializer.go | 46 ++++++++++ pkg/analysis/notimestamp/testdata/src/a/a.go | 22 +++++ pkg/analysis/registry.go | 2 + pkg/analysis/registry_test.go | 2 + 8 files changed, 227 insertions(+) create mode 100644 pkg/analysis/notimestamp/analyzer.go create mode 100644 pkg/analysis/notimestamp/analyzer_test.go create mode 100644 pkg/analysis/notimestamp/doc.go create mode 100644 pkg/analysis/notimestamp/initializer.go create mode 100644 pkg/analysis/notimestamp/testdata/src/a/a.go diff --git a/docs/linters.md b/docs/linters.md index 21e003b3..e6c17332 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -10,6 +10,7 @@ - [NoFloats](#nofloats) - Prevents usage of floating-point types - [Nomaps](#nomaps) - Restricts usage of map types - [Nophase](#nophase) - Prevents usage of 'Phase' fields +- [Notimestamp](#Notimestamp) - Prevents usage of `TimeStamp` fields - [OptionalFields](#optionalfields) - Validates optional field conventions - [OptionalOrRequired](#optionalorrequired) - Ensures fields are explicitly marked as optional or required - [RequiredFields](#requiredfields) - Validates required field conventions @@ -151,6 +152,10 @@ The `nomaps` linter checks the usage of map types. Maps are discouraged apart from `map[string]string` which is used for labels and annotations in Kubernetes APIs since it's hard to distinguish between structs and maps in spec. Instead of plain map, lists of named subobjects are preferred. +## Notimestamp + +The `notimestamp` linter checks that the fields in the API types don't contain a 'Timestamp', or any field which contains 'Timestamp' as a substring, e.g CreateTimestamp. + ### Configuration ```yaml diff --git a/pkg/analysis/notimestamp/analyzer.go b/pkg/analysis/notimestamp/analyzer.go new file mode 100644 index 00000000..2951b570 --- /dev/null +++ b/pkg/analysis/notimestamp/analyzer.go @@ -0,0 +1,97 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package notimestamp + +import ( + "go/ast" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" +) + +const name = "notimestamp" + +// Analyzer is the analyzer for the notimestamp package. +// It checks that no struct fields named 'timestamp', or that contain timestamp as a +// substring are present. +var Analyzer = &analysis.Analyzer{ + Name: name, + Doc: "check that non of the struct field named timestamp or contain timestamp as a substring", + Run: run, + Requires: []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer}, +} + +func run(pass *analysis.Pass) (any, error) { + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, kalerrors.ErrCouldNotGetInspector + } + + jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags) + if !ok { + return nil, kalerrors.ErrCouldNotGetJSONTags + } + + // Filter to fields so that we can iterate over fields in a struct. + nodeFilter := []ast.Node{ + (*ast.Field)(nil), + } + + // Preorder visits all the nodes of the AST in depth-first order. It calls + // f(n) for each node n before it visits n's children. + // + // We use the filter defined above, ensuring we only look at struct fields. + inspect.Preorder(nodeFilter, func(n ast.Node) { + field, ok := n.(*ast.Field) + if !ok { + return + } + + if field == nil || len(field.Names) == 0 { + return + } + + fieldName := utils.FieldName(field) + + // First check if the struct field name contains 'timestamp' + if strings.Contains(strings.ToLower(fieldName), "timestamp") { + pass.Reportf(field.Pos(), + "field %s: fields with timestamp substring should be avoided", + fieldName, + ) + + return + } + + // Then check if the json serialization of the field contains 'timestamp' + tagInfo := jsonTags.FieldTags(field) + + if strings.Contains(strings.ToLower(tagInfo.Name), "timestamp") { + pass.Reportf(field.Pos(), + "field %s: fields with timestamp substring should be avoided", + fieldName, + ) + } + }) + + return nil, nil //nolint:nilnil +} diff --git a/pkg/analysis/notimestamp/analyzer_test.go b/pkg/analysis/notimestamp/analyzer_test.go new file mode 100644 index 00000000..3b6cf909 --- /dev/null +++ b/pkg/analysis/notimestamp/analyzer_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package notimestamp_test + +import ( + "sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, notimestamp.Analyzer, "a") +} diff --git a/pkg/analysis/notimestamp/doc.go b/pkg/analysis/notimestamp/doc.go new file mode 100644 index 00000000..1bf63907 --- /dev/null +++ b/pkg/analysis/notimestamp/doc.go @@ -0,0 +1,24 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* +notimestamp provides a linter to ensure that structs do not contain a TimeStamp field. + +The linter will flag any struct field containing the substring 'timestamp'. This means both +TimeStamp and FooTimeStamp will be flagged. +*/ + +package notimestamp diff --git a/pkg/analysis/notimestamp/initializer.go b/pkg/analysis/notimestamp/initializer.go new file mode 100644 index 00000000..53aaecd3 --- /dev/null +++ b/pkg/analysis/notimestamp/initializer.go @@ -0,0 +1,46 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package notimestamp + +import ( + "golang.org/x/tools/go/analysis" + "sigs.k8s.io/kube-api-linter/pkg/config" +) + +// Initializer returns the AnalyzerInitializer for this +// Analyzer so that it can be added to the registry. +func Initializer() initializer { + return initializer{} +} + +// intializer implements the AnalyzerInitializer interface. +type initializer struct{} + +// Name returns the name of the Analyzer. +func (initializer) Name() string { + return name +} + +// Init returns the intialized Analyzer. +func (initializer) Init(cfg config.LintersConfig) (*analysis.Analyzer, error) { + return Analyzer, nil +} + +// Default determines whether this Analyzer is on by default, or not. +func (initializer) Default() bool { + return true +} diff --git a/pkg/analysis/notimestamp/testdata/src/a/a.go b/pkg/analysis/notimestamp/testdata/src/a/a.go new file mode 100644 index 00000000..bfdb88b9 --- /dev/null +++ b/pkg/analysis/notimestamp/testdata/src/a/a.go @@ -0,0 +1,22 @@ +package a + +import "time" + +type NoTimeStampTestStruct struct { + // +optional + TimeStamp *time.Time `json:"timeStamp,omitempty"` // want "field TimeStamp: fields with timestamp substring should be avoided" +} + +// DoNothing is used to check that the analyser doesn't report on methods. +func (NoTimeStampTestStruct) DoNothing() {} + +type NoSubTimeStampTestStruct struct { + // +optional + FooTimeStamp *time.Time `json:"fooTimeStamp,omitempty"` // want "field FooTimeStamp: fields with timestamp substring should be avoided" + +} + +type SerializedTimeStampTestStruct struct { + // +optional + FooTime *time.Time `json:"fooTime,omitempty"` +} diff --git a/pkg/analysis/registry.go b/pkg/analysis/registry.go index 6e8f8fc5..092c6a59 100644 --- a/pkg/analysis/registry.go +++ b/pkg/analysis/registry.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/analysis/nofloats" "sigs.k8s.io/kube-api-linter/pkg/analysis/nomaps" "sigs.k8s.io/kube-api-linter/pkg/analysis/nophase" + "sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp" "sigs.k8s.io/kube-api-linter/pkg/analysis/optionalfields" "sigs.k8s.io/kube-api-linter/pkg/analysis/optionalorrequired" "sigs.k8s.io/kube-api-linter/pkg/analysis/requiredfields" @@ -86,6 +87,7 @@ func NewRegistry() Registry { nofloats.Initializer(), nomaps.Initializer(), nophase.Initializer(), + notimestamp.Initializer(), optionalfields.Initializer(), optionalorrequired.Initializer(), requiredfields.Initializer(), diff --git a/pkg/analysis/registry_test.go b/pkg/analysis/registry_test.go index 42c6fdfe..53b99223 100644 --- a/pkg/analysis/registry_test.go +++ b/pkg/analysis/registry_test.go @@ -39,6 +39,7 @@ var _ = Describe("Registry", func() { "nofloats", "nomaps", "nophase", + "notimestamp", "optionalfields", "optionalorrequired", "requiredfields", @@ -62,6 +63,7 @@ var _ = Describe("Registry", func() { "nofloats", "nomaps", "nophase", + "notimestamp", "optionalfields", "optionalorrequired", "requiredfields", From 627e267a78317def4fe064b11222e0b3c2f8bb74 Mon Sep 17 00:00:00 2001 From: Karthik Bhat Date: Mon, 30 Jun 2025 19:17:51 +0530 Subject: [PATCH 2/2] Address reveiw comments --- docs/linters.md | 13 +- go.mod | 10 +- go.sum | 20 ++++ pkg/analysis/notimestamp/analyzer.go | 111 ++++++++++-------- pkg/analysis/notimestamp/analyzer_test.go | 2 +- pkg/analysis/notimestamp/testdata/src/a/a.go | 5 +- .../notimestamp/testdata/src/a/a.go.golden | 21 ++++ 7 files changed, 126 insertions(+), 56 deletions(-) create mode 100644 pkg/analysis/notimestamp/testdata/src/a/a.go.golden diff --git a/docs/linters.md b/docs/linters.md index e6c17332..8433077f 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -10,7 +10,7 @@ - [NoFloats](#nofloats) - Prevents usage of floating-point types - [Nomaps](#nomaps) - Restricts usage of map types - [Nophase](#nophase) - Prevents usage of 'Phase' fields -- [Notimestamp](#Notimestamp) - Prevents usage of `TimeStamp` fields +- [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields - [OptionalFields](#optionalfields) - Validates optional field conventions - [OptionalOrRequired](#optionalorrequired) - Ensures fields are explicitly marked as optional or required - [RequiredFields](#requiredfields) - Validates required field conventions @@ -154,7 +154,16 @@ Maps are discouraged apart from `map[string]string` which is used for labels and ## Notimestamp -The `notimestamp` linter checks that the fields in the API types don't contain a 'Timestamp', or any field which contains 'Timestamp' as a substring, e.g CreateTimestamp. +The `notimestamp` linter checks that the fields in the API are not named with the word 'Timestamp'. + +The name of a field that specifies the time at which something occurs should be called `somethingTime`.Its recommended not use stamp (e.g., creationTimestamp). + +### Fixes + +The `notimestamp` linter will automatically fix fields and json tags that are named with the word 'TimeStamp'. + +It will automatically replace 'TimeStamp' with 'Time' and update both the field and tag name. +Example: 'FooTimeStamp' will be updated to 'FooTime'. ### Configuration diff --git a/go.mod b/go.mod index d76581a1..3c2c717a 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module sigs.k8s.io/kube-api-linter go 1.24.0 require ( + github.com/golangci/golangci-lint/v2 v2.1.6 github.com/golangci/plugin-module-register v0.1.1 github.com/onsi/ginkgo/v2 v2.23.3 github.com/onsi/gomega v1.36.3 @@ -12,19 +13,24 @@ require ( ) require ( + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect + github.com/fatih/color v1.18.0 // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad // indirect github.com/kr/pretty v0.3.1 // indirect - github.com/rogpeppe/go-internal v1.14.1 // indirect + github.com/mattn/go-colorable v0.1.14 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/sirupsen/logrus v1.9.3 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/stretchr/testify v1.10.0 // indirect golang.org/x/mod v0.24.0 // indirect golang.org/x/net v0.39.0 // indirect golang.org/x/sync v0.13.0 // indirect golang.org/x/sys v0.32.0 // indirect golang.org/x/text v0.24.0 // indirect - google.golang.org/protobuf v1.36.6 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 1bce0f78..1568c2c8 100644 --- a/go.sum +++ b/go.sum @@ -1,10 +1,16 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= +github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= +github.com/golangci/golangci-lint/v2 v2.1.6 h1:LXqShFfAGM5BDzEOWD2SL1IzJAgUOqES/HRBsfKjI+w= +github.com/golangci/golangci-lint/v2 v2.1.6/go.mod h1:EPj+fgv4TeeBq3TcqaKZb3vkiV5dP4hHHKhXhEhzci8= github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c= github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc= 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= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= +github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/onsi/ginkgo/v2 v2.23.3 h1:edHxnszytJ4lD9D5Jjc4tiDkPBZ3siDeJJkUZJJVkp0= github.com/onsi/ginkgo/v2 v2.23.3/go.mod h1:zXTP6xIp3U8aVuXN8ENK9IXRaTjFnpVB9mGmaSRvxnM= github.com/onsi/gomega v1.36.3 h1:hID7cr8t3Wp26+cYnfcjR6HpJ00fdogN6dqZ1t6IylU= github.com/onsi/gomega v1.36.3/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= +github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= +github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= 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= golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= 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/ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/apimachinery v0.32.3 h1:JmDuDarhDmA/Li7j3aPrwhpNBA94Nvk5zLeOge9HH1U= diff --git a/pkg/analysis/notimestamp/analyzer.go b/pkg/analysis/notimestamp/analyzer.go index 2951b570..077663fa 100644 --- a/pkg/analysis/notimestamp/analyzer.go +++ b/pkg/analysis/notimestamp/analyzer.go @@ -17,14 +17,17 @@ limitations under the License. package notimestamp import ( + "fmt" "go/ast" + "go/token" + "regexp" "strings" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" + markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" ) @@ -35,63 +38,75 @@ const name = "notimestamp" // substring are present. var Analyzer = &analysis.Analyzer{ Name: name, - Doc: "check that non of the struct field named timestamp or contain timestamp as a substring", + Doc: "check that none of the struct field named timestamp or contain timestamp as a substring", Run: run, - Requires: []*analysis.Analyzer{inspect.Analyzer, extractjsontags.Analyzer}, + Requires: []*analysis.Analyzer{inspector.Analyzer}, } +// case insensitive regular expression to match 'timestamp' string in field or json tag. +var timeStampRegEx = regexp.MustCompile("(?i)timestamp") + func run(pass *analysis.Pass) (any, error) { - inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector) if !ok { return nil, kalerrors.ErrCouldNotGetInspector } - jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags) - if !ok { - return nil, kalerrors.ErrCouldNotGetJSONTags + inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) { + checkFieldsAndTags(pass, field, jsonTagInfo) + }) + + return nil, nil //nolint:nilnil +} + +func checkFieldsAndTags(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.FieldTagInfo) { + fieldName := utils.FieldName(field) + if fieldName == "" { + return } - // Filter to fields so that we can iterate over fields in a struct. - nodeFilter := []ast.Node{ - (*ast.Field)(nil), + var suggestedFixes []analysis.SuggestedFix + + // check if filed name contains timestamp in it. + fieldReplacementName := timeStampRegEx.ReplaceAllString(fieldName, "Time") + if fieldReplacementName != fieldName { + suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{ + Message: "replace field with Time", + TextEdits: []analysis.TextEdit{ + { + Pos: field.Pos(), + NewText: []byte(fieldReplacementName), + End: field.Pos() + token.Pos(len(fieldName)), + }, + }, + }) } - // Preorder visits all the nodes of the AST in depth-first order. It calls - // f(n) for each node n before it visits n's children. - // - // We use the filter defined above, ensuring we only look at struct fields. - inspect.Preorder(nodeFilter, func(n ast.Node) { - field, ok := n.(*ast.Field) - if !ok { - return - } - - if field == nil || len(field.Names) == 0 { - return - } - - fieldName := utils.FieldName(field) - - // First check if the struct field name contains 'timestamp' - if strings.Contains(strings.ToLower(fieldName), "timestamp") { - pass.Reportf(field.Pos(), - "field %s: fields with timestamp substring should be avoided", - fieldName, - ) - - return - } - - // Then check if the json serialization of the field contains 'timestamp' - tagInfo := jsonTags.FieldTags(field) - - if strings.Contains(strings.ToLower(tagInfo.Name), "timestamp") { - pass.Reportf(field.Pos(), - "field %s: fields with timestamp substring should be avoided", - fieldName, - ) - } - }) + // check if the tag contains timestamp in it. + tagReplacementName := timeStampRegEx.ReplaceAllString(tagInfo.Name, "Time") + if strings.HasPrefix(strings.ToLower(tagInfo.Name), "time") { + // If tag is starts with 'timeStamp', the replacement should be 'time' not 'Time'. + tagReplacementName = timeStampRegEx.ReplaceAllString(tagInfo.Name, "time") + } - return nil, nil //nolint:nilnil + if tagReplacementName != tagInfo.Name { + suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{ + Message: "replace json tag with Time", + TextEdits: []analysis.TextEdit{ + { + Pos: tagInfo.Pos, + NewText: []byte(tagReplacementName), + End: tagInfo.Pos + token.Pos(len(tagInfo.Name)), + }, + }, + }) + } + + if len(suggestedFixes) > 0 { + pass.Report(analysis.Diagnostic{ + Pos: field.Pos(), + Message: fmt.Sprintf("field %s: prefer use of the term time over timestamp", fieldName), + SuggestedFixes: suggestedFixes, + }) + } } diff --git a/pkg/analysis/notimestamp/analyzer_test.go b/pkg/analysis/notimestamp/analyzer_test.go index 3b6cf909..748709fc 100644 --- a/pkg/analysis/notimestamp/analyzer_test.go +++ b/pkg/analysis/notimestamp/analyzer_test.go @@ -17,10 +17,10 @@ limitations under the License. package notimestamp_test import ( - "sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp" "testing" "golang.org/x/tools/go/analysis/analysistest" + "sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp" ) func Test(t *testing.T) { diff --git a/pkg/analysis/notimestamp/testdata/src/a/a.go b/pkg/analysis/notimestamp/testdata/src/a/a.go index bfdb88b9..9eb5afda 100644 --- a/pkg/analysis/notimestamp/testdata/src/a/a.go +++ b/pkg/analysis/notimestamp/testdata/src/a/a.go @@ -4,7 +4,7 @@ import "time" type NoTimeStampTestStruct struct { // +optional - TimeStamp *time.Time `json:"timeStamp,omitempty"` // want "field TimeStamp: fields with timestamp substring should be avoided" + TimeStamp *time.Time `json:"timeStamp,omitempty"` // want "field TimeStamp: prefer use of the term time over timestamp" } // DoNothing is used to check that the analyser doesn't report on methods. @@ -12,8 +12,7 @@ func (NoTimeStampTestStruct) DoNothing() {} type NoSubTimeStampTestStruct struct { // +optional - FooTimeStamp *time.Time `json:"fooTimeStamp,omitempty"` // want "field FooTimeStamp: fields with timestamp substring should be avoided" - + FooTimeStamp *time.Time `json:"fooTimeStamp,omitempty"` // want "field FooTimeStamp: prefer use of the term time over timestamp" } type SerializedTimeStampTestStruct struct { diff --git a/pkg/analysis/notimestamp/testdata/src/a/a.go.golden b/pkg/analysis/notimestamp/testdata/src/a/a.go.golden new file mode 100644 index 00000000..9eeb36d7 --- /dev/null +++ b/pkg/analysis/notimestamp/testdata/src/a/a.go.golden @@ -0,0 +1,21 @@ +package a + +import "time" + +type NoTimeStampTestStruct struct { + // +optional + Time *time.Time `json:"time,omitempty"` // want "field TimeStamp: prefer use of the term time over timestamp" +} + +// DoNothing is used to check that the analyser doesn't report on methods. +func (NoTimeStampTestStruct) DoNothing() {} + +type NoSubTimeStampTestStruct struct { + // +optional + FooTime *time.Time `json:"fooTime,omitempty"` // want "field FooTimeStamp: prefer use of the term time over timestamp" +} + +type SerializedTimeStampTestStruct struct { + // +optional + FooTime *time.Time `json:"fooTime,omitempty"` +}