checkers: removed timeCmpSimplify checker #1281
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM. The alternative would be: //doc:summary Detects Before/After call of time.Time that can be simplified
//doc:tags style experimental
//doc:before !t.Before(tt)
//doc:after (t.After(tt) || t.Equal(tt))
func timeCmpSimplify(m dsl.Matcher) {
isTime := func(v dsl.Var) bool {
return v.Type.Is(`time.Time`) || v.Type.Is(`*time.Time`)
}
m.Match(`!$t.Before($tt)`).
Where(isTime(m["t"])).
Suggest("($t.After($tt) || $t.Equal($tt))")
m.Match(`!$t.After($tt)`).
Where(isTime(m["t"])).
Suggest("($t.Before($tt) || $t.Equal($tt))")
} |
|
A year on should this get a move on and merged in? |
|
Indeed, I will merge this today, thanks for the reminder. |
The checker suggested a change that is not equivalent to the code being replaced. The results were different when the time values were identical. This issue has no obvious fix, so the checker is removed completely. It was marked as experimental after all. Fixes #1257
b95390c to
c87bdc7
Compare
akhilerm
added a commit
to akhilerm/publishing-bot
that referenced
this pull request
Sep 19, 2024
Ref: go-critic/go-critic#1281 Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
akhilerm
added a commit
to akhilerm/publishing-bot
that referenced
this pull request
Sep 21, 2024
Ref: go-critic/go-critic#1281 Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
akhilerm
added a commit
to akhilerm/publishing-bot
that referenced
this pull request
Sep 21, 2024
Ref: go-critic/go-critic#1281 Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
akhilerm
added a commit
to akhilerm/publishing-bot
that referenced
this pull request
Sep 26, 2024
Ref: go-critic/go-critic#1281 Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
akhilerm
added a commit
to akhilerm/publishing-bot
that referenced
this pull request
Sep 27, 2024
Ref: go-critic/go-critic#1281 Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
k8s-ci-robot
pushed a commit
to kubernetes/publishing-bot
that referenced
this pull request
Sep 30, 2024
* update to go1.22.7 Signed-off-by: Akhil Mohan <akhilerm@gmail.com> * update golangci-lint to v1.61.0 Signed-off-by: Akhil Mohan <akhilerm@gmail.com> * removed sloppyTestFuncName Ref: go-critic/go-critic#1376 Signed-off-by: Akhil Mohan <akhilerm@gmail.com> * removed timeCmpSimplify Ref: go-critic/go-critic#1281 Signed-off-by: Akhil Mohan <akhilerm@gmail.com> * fix errors reported by emptyStringTest linter Signed-off-by: Akhil Mohan <akhilerm@gmail.com> * removed goerr113 and perfsprint goerr113 and perfsprint linters are removed since the fixes reduces the readability Signed-off-by: Akhil Mohan <akhilerm@gmail.com> --------- Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The checker suggested a change that is not equivalent to the code being replaced. The results were different when the time values were identical.
This issue has no obvious fix, so the checker is removed completely. It was marked as experimental after all.
Fixes #1257