Bump golangci-lint run to 1.55, update defaults (#2677)

The plan is to enable linters first, then fix issues types one by one.

.github/workflows/go-tests-windows.yml

@@ -60,7 +60,7 @@ jobs:
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v3
       with:
-        version: v1.54
+        version: v1.55
         args: --issues-exit-code=1 --timeout 10m
         only-new-issues: false
         # the cache is already managed above, enabling it here

.github/workflows/go-tests.yml

@@ -157,7 +157,7 @@ jobs:
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v3
       with:
-        version: v1.54
+        version: v1.55
         args: --issues-exit-code=1 --timeout 10m
         only-new-issues: false
         # the cache is already managed above, enabling it here

.golangci.yml

@@ -31,15 +31,24 @@ linters-settings:
   misspell:
     locale: US
 
+  nlreturn:
+    block-size: 4
+
   nolintlint:
-    allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
     allow-unused: false # report any unused nolint directives
     require-explanation: false # don't require an explanation for nolint directives
     require-specific: false # don't require nolint directives to be specific about which linter is being skipped
 
   interfacebloat:
     max: 12
 
+  depguard:
+    rules:
+      main:
+        deny:
+          - pkg: "github.com/pkg/errors"
+            desc: "errors.New() is deprecated in favor of fmt.Errorf()"
+
 linters:
   enable-all: true
   disable:
@@ -64,10 +73,13 @@ linters:
     # - asasalint           # check for pass []any as any in variadic func(...any)
     # - asciicheck          # Simple linter to check that your code does not contain non-ASCII identifiers
     # - bidichk             # Checks for dangerous unicode character sequences
+    # - bodyclose           # checks whether HTTP response body is closed successfully
     # - decorder            # check declaration order and count of types, constants, variables and functions
+    # - depguard            # Go linter that checks if package imports are in a list of acceptable packages
     # - dupword             # checks for duplicate words in the source code
     # - durationcheck       # check for two durations multiplied together
     # - errcheck            # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
+    # - errorlint           # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
     # - exportloopref       # checks for pointers to enclosing loop variables
     # - funlen              # Tool for detection of long functions
     # - ginkgolinter        # enforces standards of using ginkgo and gomega
@@ -86,8 +98,11 @@ linters:
     # - logrlint            # Check logr arguments.
     # - makezero            # Finds slice declarations with non-zero initial length
     # - misspell            # Finds commonly misspelled English words in comments
+    # - nakedret            # Finds naked returns in functions greater than a specified function length
     # - nilerr              # Finds the code that returns nil even if it checks that the error is not nil.
     # - nolintlint          # Reports ill-formed or insufficient nolint directives
+    # - nonamedreturns      # Reports all named returns
+    # - nosprintfhostport   # Checks for misuse of Sprintf to construct a host with port in a URL.
     # - predeclared         # find code that shadows one of Go's predeclared identifiers
     # - reassign            # Checks that package variables are not reassigned
     # - rowserrcheck        # checks whether Err of rows is checked successfully
@@ -100,38 +115,34 @@ linters:
     # - unconvert           # Remove unnecessary type conversions
     # - unused              # (megacheck): Checks Go code for unused constants, variables, functions and types
     # - usestdlibvars       # A linter that detect the possibility to use variables/constants from the Go standard library.
+    # - wastedassign        # wastedassign finds wasted assignment statements.
 
     #
     # Recommended? (easy)
     #
 
-    - depguard              # Go linter that checks if package imports are in a list of acceptable packages
     - dogsled               # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())
     - errchkjson            # Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occations, where the check for the returned error can be omitted.
-    - errorlint             # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
     - exhaustive            # check exhaustiveness of enum switch statements
     - gci                   # Gci control golang package import order and make it always deterministic.
     - godot                 # Check if comments end in a period
     - gofmt                 # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
     - goimports             # In addition to fixing imports, goimports also formats your code in the same style as gofmt.
     - gosec                 # (gas): Inspects source code for security problems
+    - inamedparam           # reports interfaces with unnamed method parameters
     - lll                   # Reports long lines
     - musttag               # enforce field tags in (un)marshaled structs
-    - nakedret              # Finds naked returns in functions greater than a specified function length
-    - nonamedreturns        # Reports all named returns
-    - nosprintfhostport     # Checks for misuse of Sprintf to construct a host with port in a URL.
     - promlinter            # Check Prometheus metrics naming via promlint
+    - protogetter           # Reports direct reads from proto message fields when getters should be used
     - revive                # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint.
-    - tagalign              # check that struct tags are well aligned [fast: true, auto-fix: true]
+    - tagalign              # check that struct tags are well aligned
     - thelper               # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers
-    - wastedassign          # wastedassign finds wasted assignment statements.
     - wrapcheck             # Checks that errors returned from external packages are wrapped
 
     #
     # Recommended? (requires some work)
     #
 
-    - bodyclose             # checks whether HTTP response body is closed successfully
     - containedctx          # containedctx is a linter that detects struct contained context.Context field
     - contextcheck          # check the function whether use a non-inherited context
     - errname               # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`.
@@ -189,8 +200,11 @@ issues:
   # break ‘em.” ― Terry Pratchett
 
   max-issues-per-linter: 0
-  max-same-issues: 10
+  max-same-issues: 0
   exclude-rules:
+
+    # Won't fix:
+
     - path: go.mod
       text: "replacement are not allowed: golang.org/x/time/rate"
 
@@ -199,30 +213,10 @@ issues:
         - govet
       text: "shadow: declaration of \"err\" shadows declaration"
 
-    #
-    # typecheck
-    #
-
-    - linters:
-        - typecheck
-      text: "undefined: min"
-
-    - linters:
-        - typecheck
-      text: "undefined: max"
-
-    #
-    # errcheck
-    #
-
     - linters:
         - errcheck
       text: "Error return value of `.*` is not checked"
 
-    #
-    # gocritic
-    #
-
     - linters:
         - gocritic
       text: "ifElseChain: rewrite if-else to switch statement"
@@ -239,6 +233,113 @@ issues:
         - gocritic
       text: "commentFormatting: put a space between `//` and comment text"
 
+    # Will fix, trivial - just beware of merge conflicts
+
+    - linters:
+        - testifylint
+      text: "expected-actual: need to reverse actual and expected values"
+
+    - linters:
+        - testifylint
+      text: "bool-compare: use assert.False"
+
+    - linters:
+        - testifylint
+      text: "len: use assert.Len"
+
+    - linters:
+        - testifylint
+      text: "bool-compare: use assert.True"
+
+    - linters:
+        - testifylint
+      text: "bool-compare: use require.True"
+
+    - linters:
+        - testifylint
+      text: "require-error: for error assertions use require"
+
+    - linters:
+        - testifylint
+      text: "error-nil: use assert.NoError"
+
+    - linters:
+        - testifylint
+      text: "error-nil: use assert.Error"
+
+    - linters:
+        - testifylint
+      text: "empty: use assert.Empty"
+
+    - linters:
+        - perfsprint
+      text: "fmt.Sprintf can be replaced .*"
+
+    #
+    # Will fix, easy but some neurons required
+    #
+
+    - linters:
+        - testifylint
+      text: "float-compare: use assert.InEpsilon .*or InDelta.*"
+
+    - linters:
+        - errorlint
+      text: "non-wrapping format verb for fmt.Errorf. Use `%w` to format errors"
+
+    - linters:
+        - errorlint
+      text: "type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors"
+
+    - linters:
+        - errorlint
+      text: "type switch on error will fail on wrapped errors. Use errors.As to check for specific errors"
+
+    - linters:
+        - errorlint
+      text: "type assertion on error will fail on wrapped errors. Use errors.Is to check for specific errors"
+
+    - linters:
+        - errorlint
+      text: "comparing with .* will fail on wrapped errors. Use errors.Is to check for a specific error"
+
+    - linters:
+        - errorlint
+      text: "switch on an error will fail on wrapped errors. Use errors.Is to check for specific errors"
+
+    - linters:
+        - nosprintfhostport
+      text: "host:port in url should be constructed with net.JoinHostPort and not directly with fmt.Sprintf"
+
+    - linters:
+        - wastedassign
+      text: "assigned to .*, but reassigned without using the value"
+
+    # https://github.com/timakin/bodyclose
+    - linters:
+        - bodyclose
+      text: "response body must be closed"
+
+    # named/naked returns are evil
+    - linters:
+        - nonamedreturns
+      text: "named return .* with type .* found"
+
+    # https://github.com/alexkohler/nakedret#purpose
+    - linters:
+        - nakedret
+      text: "naked return in func .* with .* lines of code"
+
+    #
+    # Will fix,  might be trickier
+    #
+
     - linters:
         - staticcheck
       text: "x509.ParseCRL has been deprecated since Go 1.19: Use ParseRevocationList instead"
+
+    # https://github.com/pkg/errors/issues/245
+    - linters:
+        - depguard
+      text: "import 'github.com/pkg/errors' is not allowed .*"
+