Recommend more linters

.golangci-maximal.yml

@@ -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"

CHANGELOG-MongoDB.md

@@ -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.

src/lint.md

@@ -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]
@@ -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