Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create 10-cautious #38

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
264 changes: 264 additions & 0 deletions 10-cautious/.golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
---
# golangci-lint configuration file made by @ccoVeille
# Source: https://github.com/ccoVeille/golangci-lint-config-examples/
# Author: @ccoVeille
# License: MIT
# Variant: 10-cautious
# Version: v1.2.0
#
linters:
# some linters are enabled by default
# https://golangci-lint.run/usage/linters/
#
# enable some extra linters
enable:
# Errcheck is a program for checking for unchecked errors in Go code.
- errcheck

# Linter for Go source code that specializes in simplifying code.
- gosimple

# Vet examines Go source code and reports suspicious constructs.
- govet

# Detects when assignments to existing variables are not used.
- ineffassign

# It's a set of rules from staticcheck. See https://staticcheck.io/
- staticcheck

# Fast, configurable, extensible, flexible, and beautiful linter for Go.
# Drop-in replacement of golint.
- revive

# check imports order and makes it always deterministic.
- gci

# make sure to use t.Helper() when needed
- thelper

# mirror suggests rewrites to avoid unnecessary []byte/string conversion
- mirror

# detect the possibility to use variables/constants from the Go standard library.
- usestdlibvars

# Finds commonly misspelled English words.
- misspell

# Checks for duplicate words in the source code.
- dupword

# linter to detect errors invalid key values count
- loggercheck

# detects nested contexts in loops or function literals
- fatcontext

# checks if package imports are in a list of acceptable packages.
- depguard

linters-settings:
gci: # define the section orders for imports
sections:
# Standard section: captures all standard packages.
- standard
# Default section: catchall that is not standard or custom
- default
# linters that related to local tool, so they should be separated
- localmodule

revive:
rules:
# Check for commonly mistaken usages of the sync/atomic package
- name: atomic

# Blank import should be only in a main or test package, or have a comment justifying it.
- name: blank-imports

# Spots comments not starting with a space
- name: comment-spacings

# context.Context() should be the first parameter of a function when provided as argument.
- name: context-as-argument
arguments:
- allowTypesBefore: "*testing.T"

# Basic types should not be used as a key in `context.WithValue`
- name: context-keys-type

# warns on some common mistakes when using defer statement.
- name: defer

# Importing with `.` makes the programs much harder to understand
- name: dot-imports

# suggest to simplify if-then-else constructions when possible
- name: early-return

# Empty blocks make code less readable and could be a symptom of a bug or unfinished refactoring.
- name: empty-block

# for better readability, variables of type `error` must be named with the prefix `err`.
- name: error-naming

# for better readability, the errors should be last in the list of returned values by a function.
- name: error-return

# for better readability, error messages should not be capitalized or end with punctuation or a newline.
- name: error-strings

# report when replacing `errors.New(fmt.Sprintf())` with `fmt.Errorf()` is possible
- name: errorf

# Checking if an error is nil to just after return the error or nil is redundant.
- name: if-return

# incrementing an integer variable by 1 is recommended to be done using the `++` operator
- name: increment-decrement

# highlights redundant else-blocks that can be eliminated from the code
- name: indent-error-flow

# This rule suggests a shorter way of writing ranges that do not use the second value.
- name: range

# receiver names in a method should reflect the struct name (p for Person, for example)
- name: receiver-naming

# redefining built in names (true, false, append, make) can lead to bugs very difficult to detect.
- name: redefines-builtin-id

# redundant else-blocks that can be eliminated from the code.
- name: superfluous-else

# prevent confusing name for variables when using `time` package
- name: time-naming

# warns when an exported function or method returns a value of an un-exported type.
- name: unexported-return

# spots and proposes to remove unreachable code. also helps to spot errors
- name: unreachable-code

# Functions or methods with unused parameters can be a symptom of an unfinished refactoring or a bug.
- name: unused-parameter

# warns on useless break statements in case clauses of switch and select statements
- name: useless-break

# report when a variable declaration can be simplified
- name: var-declaration

# warns when initialism, variable or package naming conventions are not followed.
- name: var-naming

depguard:
rules:
obsolete:
ccoVeille marked this conversation as resolved.
Show resolved Hide resolved
deny:
- pkg: "golang.org/x/net/context"
desc: "Should be replaced by standard lib context package (Go 1.7+)"
Comment on lines +160 to +161
Copy link

@nobe4 nobe4 Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if it would be clearer to do

Suggested change
- 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+)"

Copy link

@nobe4 nobe4 Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't know that. Then it's probably good to have a separate check :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@nobe4 nobe4 Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol, indeed.

This whole document is quite fascinating. I have many feelings about it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you seen exptostd for this particular one ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


- 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+)"
Comment on lines +156 to +228
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also @nobe4, I'm curious about your possible feedback on this

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


forbidigo:
forbid:
# this one could be moved to ldez/exptostd
- p: "constraints\\.Ordered"
msg: "Use standard lib cmp.Ordered instead (Go 1.21+)"

# doesn't work ?
- pkg: "^golang.org/x/exp/constraints$"
p: "^.*$"
msg: "WIP"

- p: ^atomic\.(Add|CompareAndSwap|Load|Store|Swap).
msg: Go 1.19 atomic types should be used instead.
dupword:
# Keywords used to ignore detection.
# Default: []
ignore: []
# - "blah" # this will accept "blah blah …" as a valid duplicate word

misspell:
# Correct spellings using locale preferences for US or UK.
# Setting locale to US will correct the British spelling of 'colour' to 'color'.
# Default ("") is to use a neutral variety of English.
locale: US

# List of words to ignore
# among the one defined in https://github.com/golangci/misspell/blob/master/words.go
ignore-words: []
# - valor
# - and

# Extra word corrections.
extra-words: []
# - typo: "whattever"
# correction: "whatever"
Loading
Loading