feat: create 10-cautious #38
Conversation
This ruleset is a cautious approach to the dependencies and the code quality. Compared to 03-safe: - add depguard to maintain the dependencies - add revive rules - atomic - comment-spacings - defer - early-return - if-return - useless-break
|
@nobe4 following this discussion I wrote all these rules, including the syscall ones we talked about Could you please review? |
| depguard: | ||
| rules: | ||
| obsolete: | ||
| deny: | ||
| - pkg: "golang.org/x/net/context" | ||
| desc: "Should be replaced by standard lib context package (Go 1.7+)" | ||
|
|
||
| - pkg: "github.com/pkg/errors" | ||
| desc: "Should be replaced by standard lib errors package (Go 1.13+)" | ||
|
|
||
| - pkg: "golang.org/x/xerrors" | ||
| desc: "Should be replaced by standard lib errors package (Go 1.13+)" | ||
|
|
||
| - pkg: github.com/go-errors/errors | ||
| desc: "Should be replaced by standard lib errors package" | ||
|
|
||
| - pkg: "io/ioutil" | ||
| desc: "Should be replaced by standard lib os package (Go 1.16+)" | ||
|
|
||
| - pkg: "golang.org/x/exp/slices" | ||
| desc: "Should be replaced by slices (Go 1.21+)" | ||
|
|
||
| - pkg: "golang.org/x/exp/maps" | ||
| desc: "Should be replaced by standard lib maps package (Go 1.21+)" | ||
|
|
||
| - pkg: "math/rand$" | ||
| desc: "Should be replaced by standard lib math/rand/v2 package (Go 1.23+)" | ||
|
|
||
| - pkg: "golang.org/x/syscall" | ||
| desc: "Should be replaced by golang.org/x/sys or os package according to Go maintainers. More information: https://golang.org/s/go1.4-syscall" | ||
|
|
||
| - pkg: "golang.org/x/crypto/ed25519" | ||
| desc: "Should be replaced by standard lib crypto/ed25519 package" | ||
|
|
||
| - pkg: "github.com/golang/protobuf" | ||
| desc: "Should be replaced by google.golang.org/protobuf package (github.com/golang/protobuf is deprecated)" | ||
|
|
||
| - pkg: "github.com/gogo/protobuf" | ||
| desc: "Should be replaced by google.golang.org/protobuf package (github.com/gogo/protobuf is deprecated)" | ||
|
|
||
| - pkg: "github.com/golang/protobuf/proto" | ||
| desc: "Should be replaced by google.golang.org/protobuf/proto package (github.com/golang/protobuf/proto is deprecated)" | ||
|
|
||
| - pkg: "github.com/gogo/status" | ||
| desc: "Should be replaced by google.golang.org/grpc/status package (github.com/gogo/status is deprecated)" | ||
|
|
||
| logs: | ||
| deny: | ||
| - pkg: "github.com/prometheus/common/log" | ||
| desc: "Could be replaced by standard lib log/slog package" | ||
|
|
||
| - pkg: "github.com/sirupsen/logrus" | ||
| desc: "Should be replaced by standard lib log/slog package" | ||
|
|
||
| - pkg: github.com/siddontang/go-log/log | ||
| desc: "Could be replaced by standard lib log/slog package" | ||
|
|
||
| - pkg: github.com/siddontang/go/log | ||
| desc: "Could be replaced by standard lib log/slog package" | ||
|
|
||
| - pkg: github.com/mailgun/log | ||
| desc: "Could be replaced by standard lib log/slog package" | ||
|
|
||
| - pkg: github.com/saferwall/pe/log | ||
| desc: "Could be replaced by standard lib log/slog package" | ||
|
|
||
| recommended: | ||
| deny: | ||
| - pkg: "go.uber.org/atomic" | ||
| desc: "Could be replaced by standard lib sync/atomic package" | ||
|
|
||
| - pkg: "github.com/hashicorp/go-multierror" | ||
| desc: "Could be replaced by errors.Join (Go 1.20+)" |
There was a problem hiding this comment.
Hi @mmorel-35
Happy new year, good to see you.
I saw your recent PR on gopsutil
I noted the golangci-lint of this project was interesting when dealing with package replacement and suggestions.
gomodguard was used. I barely faced it and it intrigued me. depguard was the one, I'm used to see or use.
It was interesting because I have this PR here, that I worked lately and opened yesterday evening.
I dug deeper and found out that you were the one who set it up here
So here I am. I would like to know if you would recommend using it over depguard, and why.
I will take a look by my own, of course. But I would like to get a feedback from you.
Then I might update this PR by switching to gomodguard
Thanks
There was a problem hiding this comment.
Also @nobe4, I'm curious about your possible feedback on this
There was a problem hiding this comment.
Both of those linters have there interest.
I would say that gomodguard is able to provide a list of possible alternative package as for io/ioutil that shall be replaced with io or os or github.com/pkg/errors with errors or fmt
There was a problem hiding this comment.
Indeed. Thanks
I will also take a look at the way these errors are suggested. I feel like gomodguard might provide a better wording, at least less aggressive than depguard
| - pkg: "golang.org/x/net/context" | ||
| desc: "Should be replaced by standard lib context package (Go 1.7+)" |
There was a problem hiding this comment.
I'm wondering if it would be clearer to do
| - pkg: "golang.org/x/net/context" | |
| desc: "Should be replaced by standard lib context package (Go 1.7+)" | |
| - pkg: "golang.org/x/net/context" | |
| desc: "Should be replaced by standard lib [context] package (Go 1.7+)" |
There was a problem hiding this comment.
staticcheck for example formats it like that:
staticcheck: SA1019: "io/ioutil" has been deprecated since Go 1.19:
As of Go 1.16, the same functionality is now provided by package [io]
or package [os], and those implementations should be preferred in new
code. See the specific function documentation for details.
There was a problem hiding this comment.
This also yield the question of possible redundancy between this and staticcheck. It might be worth enabling the staticcheck rule SA1019 and seeing what's missing. IMHO contributing to this rule instead of maintaining a custom list of deprecated pkg is better.
There was a problem hiding this comment.
Thanks for your valuable feedbacks here.
I will indeed have to validate the std lib ones against staticcheck
The problem I have is staticcheck and its maintainer is that the project is barely maintained and maintainer refuses PR
There was a problem hiding this comment.
Ah, I didn't know that. Then it's probably good to have a separate check :)
There was a problem hiding this comment.
here
https://github.com/dominikh/.github/blob/main/CONTRIBUTING.md#pull-requests
I dislike the "I don't have time to review your bad code. I prefer to code everything by my own" tone
There was a problem hiding this comment.
Lol, indeed.
This whole document is quite fascinating. I have many feelings about it
There was a problem hiding this comment.
Have you seen exptostd for this particular one ?
There was a problem hiding this comment.
Yes. I did. I even filled an issue on the exptostd repository last week. It was about something that led me to write this here.
I had also opened one on usestdlibvar about syscall constants.
This PR needs some reworks. I thought about mentioning exptostd and keep a comment in the file
Maybe it's simply because exptostd was not yet available via golangci-lint when I started working on these changes a few weeks ago
There was a problem hiding this comment.
Yes. I did. I even filled an issue on the exptostd repository last week. It was about something that led me to write this here.
I had also opened one on usestdlibvar about syscall constants.
This PR needs some reworks. I thought about mentioning exptostd and keep a comment in the file
Maybe it's simply because exptostd was not yet available via golangci-lint when I started working on these changes a few weeks ago
This ruleset is a cautious approach to the dependencies and the code quality.
Compared to 03-safe: