Skip to content

Commit

Permalink
Add recommendations for many other golangci-lint linters
Browse files Browse the repository at this point in the history
  • Loading branch information
autarch committed Feb 19, 2025
1 parent 08057ef commit 07ff61d
Show file tree
Hide file tree
Showing 4 changed files with 397 additions and 2 deletions.
183 changes: 183 additions & 0 deletions .golangci-maximal.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
linters:
disable-all: true
enable:
- copyloopvar
- cyclop
- depguard
- dupword
- errcheck
- errname
- errorlint
- exhaustruct
- exptostd
- funlen
- gci
- gocheckcompilerdirectives
- gocognit
- gocritic
- gocyclo
- godot
- goimports
- govet
- iface
- importas
- ineffassign
- intrange
- maintidx
- misspell
- nestif
- nilnesserr
- nolintlint
- predeclared
- recvcheck
- revive
- staticcheck
- testifylint
- unconvert
- unparam
- unused
- usestdlibvars
- usetesting
fast: false

linters-settings:
depguard:
rules:
# This is just an example of a rule you could create, though
forbid-some-pacakge:
list-mode: Lax
files:
- "**/*.go"
deny:
pkg: "github.com/some/package"
desc: "Don't use some/package. It's got issues."

errcheck:
check-type-assertions: true

gci:
sections:
- standard
- default

# This configure gocritic so that it is _only_ used to run ruleguard. It's default checks include
# some that can be a bit annoying, like insisting on using `switch` instead of if/else all over
# the place.
gocritic:
disable-all: true
enabled-checks:
- ruleguard
settings:
ruleguard:
rules: "${configDir}/gorules/*.go"
# This causes a warning when rules cannot be compiled.
failOn: dsl


govet:
# This is not part of the default checks, but it's useful for finding bugs and generally
# confusing code.
shadow: true

importas:
no-extra-aliases: true
alias:
- pkg: github.com/google/uuid
alias: guuid

misspell:
locale: US
extra-words:
- typo: "cancelation"
correction: "cancellation"
- typo: "cancelations"
correction: "cancellations"
- typo: "cancelling"
correction: "canceling"
- typo: "cancelled"
correction: "canceled"

recvcheck:
exclusions:
# It's normal for `Marshal*` methods to not take a pointer.
- "*.MarshalBSON"
# We have some structs that are passed around as non-pointers, but `UnmarshalBSON` always
# takes a pointer since it's populating the receiver directly.
- "*.UnmarshalBSON"

revive:
enable-all-rules: false
# revive has a lot of possible rules. This is a bare minimum config. Check out the revive docs
# at https://github.com/mgechev/revive for more options.
rules:
- name: argument-limit
arguments: [5]
- name: function-result-limit
arguments: [3]
- name: import-shadowing
- name: unused-parameter
- name: unused-receiver

testifylint:
enable:
- bool-compare
- compares
- empty
- error-is-as
- error-nil
- expected-actual
- float-compare
- len
- require-error
- suite-extra-assert-call
require-error:
# The default is to insist on require not only for "no error" checks but also for "error is X"
# checks, which is weird.
fn-pattern: ^NoErrorf?$

unused:
# If a field is written but never read, it's almost certainly not being used. The one exception
# (for which we'll add a `nolint` comment) is with fields that exist solely for serialization
# (to YAML, BSON, etc.).
field-writes-are-uses: false
# We want to actually check that exported fields are used.
exported-fields-are-used: false
# Setting this to false caught some places where we created and wrote to local variables but
# never read from them.
local-variables-are-used: false

usetesting:
# We _do_ want to use the testing package's versions of these methods. They ensure that we clean
# up after ourselves automatically.
os-setenv: true
os-temp-dir: true

issues:
max-same-issues: 0

# This allows you to write bson.D literals without spelling out each `bson.E` it contains:
#
# doc := bson.D{{"_id", 42}, {"color", "green"}}
#
# As opposed to:
#
# doc := bson.D{bson.E{"_id", 42}, bson.E{"color", "green"}}
#
# This allows allows the same shorter version for the `bson.Binary` and `bson.Timestamp` types.
#
# The "(bson|primitive)" bit accounts for the package name changes between v1 and v2 of the Golang
# driver. You should probably change this to the appropriate package for whichever driver version
# your project uses, if you just use one version.
exclude:
- "composites: .+((bson|primitive).(Binary|E|Timestamp)) struct literal uses unkeyed fields"

exclude-rules:
# We're okay with type assertions panicking in test code and dev tools.
- path: _test\.go|_test_suite\.go|cmd/
linters:
- errcheck
# This is the output we see from errcheck for failures to check type assertion results. We are
# okay with not checking these in tests, since if the type assertion fails we'll just get a
# panic. Failing to check an error from a _function_ gives us output like "Error return value
# of `os.Mkdir` is not checked".
text: "Error return value is not checked"
1 change: 1 addition & 0 deletions CHANGELOG-MongoDB.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@
- Added a recommendation to use `gopls` in general, and specifically to configure your editor to use
it to update imports.
- Added recommendations to use `golangci-lint` or `nogo` for linting orchestration.
- Added recommendations for many more linters on top of the recommended minimal linting config.
127 changes: 126 additions & 1 deletion src/lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,129 @@ quality without being unnecessarily prescriptive:
[revive]: https://github.com/mgechev/revive
[staticcheck]: https://staticcheck.dev

## Additional Recommended Linters

We recommend consider some or all the following linters in addition to those listed above. Note that
many of these linters have a variety of configuration options. We encourage you to read each
linter's documentation and consider how you'd like to configure them when you adopt them.

### Code Complexity and Quality

- [cyclop] to check code's cyclomatic complexity, which is a heuristic evaluation of "how hard is
this code to understand" based on things like how many statements a function has, how many
conditional it uses, etc.
- [funlen] to check a function's length in lines and statements.
- [gocognit] to check code's cognitive complexity. This is similar to cognitive complexity but uses
a different heuristic.
- [gocritic] is a style checker along the lines of `govet` and `revive`. Notably, this linter allows
you to use [ruleguard]. See the [Ruleguard] section for more details.
- [maintidx] measures the maintainability index of functions. Again, this is imilar to the
cyclomatic and cognitive complexity heuristics.
- [nestif] detects `if` nesting past a given level.

[cyclop]: https://github.com/bkielbasa/cyclop
[funlen]: https://github.com/ultraware/funlen
[gocognit]: https://github.com/uudashr/gocognit
[gocritic]: https://go-critic.com/
[maintidx]: https://github.com/uudashr/gocognit
[nestif]: https://github.com/nakabonne/nestif
[ruleguard]: https://github.com/quasilyte/go-ruleguard

### Error Types, Messages, and Error Handling

- [errname] checks that error types and names follow Golang standards.
- [errorlint] checks for code that embeds errors as strings instead of wrapping them, as well as
code that uses type assertions instead of `errors.As` or `errors.Is`.
- [nilnesserr] checks for incorrect error handling corner cases, for example checking if `err2 !=
nil` and then returning `err` instead of `err2`.

[errname]: https://github.com/Antonboom/errname
[errorlint]: https://github.com/polyfloyd/go-errorlint
[nilnesserr]: https://github.com/alingse/nilnesserr

### Imports

- [depguard] allows you to forbid imports of specific packages, either entirely, or only in certain
files.
- [gci] enforces import order and general formatting of the `imports` section. This goes a bit
beyond what `gopls` does in terms of formatting imports.
- [importas] enforces the use of the same alias everywhere a given package is aliased. In other
words, it requires to always alias `github.com/google/uuid` as something like `guuid`, rather than
using `guuid` in one package and `u` in another.

[depguard]: https://github.com/OpenPeeDeeP/depguard
[gci]: https://github.com/daixiang0/gci
[importas]: https://github.com/julz/importas

## Spelling and Grammar

- [dupword] checks for duplicate words in comments or strings like "failed because the the server
returned an error".
- [godot] requires all comments to end in a period.
- [misspell] checks for commonly misspelled English words.

[dupword]: https://github.com/Abirdcfly/dupword
[godot]: https://github.com/tetafro/godot
[misspell]: https://github.com/client9/misspell

### Code Quality and Cleanup

- [copyloopvar] finds places where loop variables are copied, like `i := i`. This is no longer
necessary as of Go 1.22.
- [exhaustruct] requires that all fields of a struct be initialized when a struct is created.
- [exptostd] checks for use of function in `golang.org/x/exp` packages that can be replaced by
functions from the standard library.
- [gocheckcompilerdirectives] checks that Go compiler directive comments, like `//go:build ...`, are
valid.
- [iface] detects unused interfaces, multiple identical interfaces, and functions which return
interfaces but always return the same concrete implementation type.
- [ineffassign] detects when a variable is assigned a value but that value is never used, for
example if you reassign to a variable but never use the first value.
- [intrange] finds loops where you can use the `for i := 10` integer range feature added in Go 1.22.
- [nolintlint] finds `//nolint` comments for `golangci-lint` that are ill-formed, don't have an
exaplanatory comment, or are applied to code that doesn't trigger the disabled linter(s).
- [predeclared] finds code that shadow predeclared Go identifiers like a function named `copy`.
- [recvcheck] checks that all receivers for a type are pointers or non-pointers.
- Note that if you use this you will almost certainly want to exempt `MarshalBSON` and
`UnmarshalBSON` methods from this linter.
- [testifylint] checks for various issues when using the [testify] test packages.
- [unconvert] finds unnecessary type conversions.
- [unparam] finds unused function parameters.
- [unused] finds unused constants, variables, functions, and types.
- [usestdlibvars] finds places where you can use stdlib vars like `http.StatusOK` instead of `200`.
- [usetesting] finds places where you can using `testing` package helpers like `t.TempDir` instead
of `os.TempDir`.

[copyloopvar]: https://github.com/karamaru-alpha/copyloopvar
[exhaustruct]: https://github.com/GaijinEntertainment/go-exhaustruct
[exptostd]: https://github.com/ldez/exptostd
[gocheckcompilerdirectives]: https://github.com/leighmcculloch/gocheckcompilerdirectives
[iface]: https://github.com/uudashr/iface
[ineffassign]: https://github.com/gordonklaus/ineffassign
[intrange]: https://github.com/ckaznocha/intrange
[nolintlint]: https://github.com/golangci/golangci-lint/tree/master/pkg/golinters/nolintlint/internal
[predeclared]: https://github.com/nishanths/predeclared
[recvcheck]: https://github.com/raeperd/recvcheck
[testifylint]: https://github.com/Antonboom/testifylint
[testify]: https://github.com/stretchr/testify
[unconvert]: https://github.com/mdempsky/unconvert
[unparam]: https://github.com/mvdan/unparam
[unused]: https://github.com/dominikh/go-tools/tree/master/unused
[usestdlibvars]: https://github.com/sashamelentyev/usestdlibvars
[usetesting]: https://github.com/ldez/usetesting

### Ruleguard

The [gocritic] linter allows you to use [ruleguard]. Ruleguard is a tool that lets you write
declarative rules that your codebase is checked against. This is a very flexible tool that allows
you to write checks for things that cannot be done with any existing linter.

For example, you could write rules to:

- forbid the use of specific generic types.
- forbid specific functions from certain packages.
- require that all context cancellations, deadlines, and timeouts have a cause, among many .

## Lint Runners

For projects which use Bazel, you should use [nogo] instead of [golangci-lint], as [golangci-lint]
Expand All @@ -27,7 +150,9 @@ does not work well with Bazel's sandboxing.
Otherwise, we recommend [golangci-lint] as the go-to lint runner for Go code, largely due
to its performance in larger codebases and ability to configure and use many
canonical linters at once. This repo has an example [.golangci.yml] config file
with recommended linters and settings.
with recommended minimal linters and settings.

There is also a [.golangci-maximal.yml] file with all the additional linters enabled.

golangci-lint has [various linters] available for use. The above linters are
recommended as a base set, and we encourage teams to add any additional linters
Expand Down
Loading

0 comments on commit 07ff61d

Please sign in to comment.