golangci-lint: enable more linters

thaJeztah · thaJeztah · commit b28554ac9e85 · 2023-11-20T15:00:17.000+01:00
fix some nolintlint false positives

For some reason, nolintlint doesn't consider these used, but they seem to be
legitimate cases where deprecated fields are used.

    templates/templates.go:27:29: directive `//nolint:staticcheck // strings.Title is deprecated, but we only use it for ASCII, so replacing with golang.org/x/text is out of scope` is unused for linter "staticcheck" (nolintlint)
        "title":    strings.Title, //nolint:staticcheck // strings.Title is deprecated, but we only use it for ASCII, so replacing with golang.org/x/text is out of scope
                                   ^
    cli/command/formatter/image_test.go:75:31: directive `//nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API &lt; v1.44.` is unused for linter "staticcheck" (nolintlint)
                call:     ctx.VirtualSize, //nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API &lt; v1.44.
                                           ^
    cli/command/registry/formatter_search.go:100:39: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
        return c.formatBool(c.s.IsAutomated) //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
                                             ^
    cli/command/registry/formatter_search_test.go:50:55: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
                    s: registrytypes.SearchResult{IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
                                                                      ^
    cli/command/registry/formatter_search_test.go:53:31: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
                call:     ctx.IsAutomated, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
                                           ^
    cli/command/registry/formatter_search_test.go:59:27: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
                call: ctx.IsAutomated, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
                                       ^
    cli/command/registry/formatter_search_test.go:202:84: directive `//nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).` is unused for linter "staticcheck" (nolintlint)
            {Name: "result2", Description: "Not official", StarCount: 5, IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
                                                                                             ^

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/.golangci.yml b/.golangci.yml
@@ -4,26 +4,38 @@ linters:
     - depguard
     - dogsled
     - dupword       # Detects duplicate words.
+    - durationcheck
+    - exportloopref # Detects pointers to enclosing loop variables.
+    - gocritic      # Metalinter; detects bugs, performance, and styling issues.
     - gocyclo
-    - gofumpt
+    - gofumpt       # Detects whether code was gofumpt-ed.
     - goimports
-    - gosec
+    - gosec         # Detects security problems.
     - gosimple
     - govet
     - ineffassign
     - lll
     - megacheck
-    - misspell
+    - misspell      # Detects commonly misspelled English words in comments.
     - nakedret
     - nilerr        # Detects code that returns nil even if it checks that the error is not nil.
-    - predeclared
-    - revive
+    - nolintlint    # Detects ill-formed or insufficient nolint directives.
+    - prealloc      # Detects slice declarations that could potentially be pre-allocated.
+    - predeclared   # Detects code that shadows one of Go's predeclared identifiers
+    - reassign
+    - revive        # Metalinter; drop-in replacement for golint.
     - staticcheck
-    - thelper
+    - stylecheck    # Replacement for golint
+    - tenv          # Detects using os.Setenv instead of t.Setenv.
+    - thelper       # Detects test helpers without t.Helper().
+    - tparallel     # Detects inappropriate usage of t.Parallel().
     - typecheck
-    - unconvert
+    - unconvert     # Detects unnecessary type conversions.
     - unparam
     - unused
+    - usestdlibvars
+    - vet
+    - wastedassign
 
   disable:
     - errcheck
@@ -90,7 +102,7 @@ issues:
         - gosec
     # EXC0008
     # TODO: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close" (gosec)
-    - text: "(G104|G307)"
+    - text: "G307"
       linters:
         - gosec
     # EXC0009
@@ -104,10 +116,13 @@ issues:
 
     # G113 Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772)
     # only affects gp < 1.16.14. and go < 1.17.7
-    - text: "(G113)"
+    - text: "G113"
+      linters:
+        - gosec
+    # TODO: G104: Errors unhandled. (gosec)
+    - text: "G104"
       linters:
         - gosec
-
     # Looks like the match in "EXC0007" above doesn't catch this one
     # TODO: consider upstreaming this to golangci-lint's default exclusion rules
     - text: "G204: Subprocess launched with a potential tainted input or cmd arguments"
@@ -132,6 +147,9 @@ issues:
       linters:
         - errcheck
         - gosec
+    - text: "ST1000: at least one file in a package should have a package comment"
+      linters:
+        - stylecheck
 
     # Allow "err" and "ok" vars to shadow existing declarations, otherwise we get too many false positives.
     - text: '^shadow: declaration of "(err|ok)" shadows declaration'
diff --git a/cli/command/formatter/image_test.go b/cli/command/formatter/image_test.go
@@ -72,7 +72,7 @@ func TestImageContext(t *testing.T) {
 		{
 			imageCtx: imageContext{i: image.Summary{Size: 10000}},
 			expValue: "10kB",
-			call:     ctx.VirtualSize, //nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.44.
+			call:     ctx.VirtualSize, //nolint:nolintlint,staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.44.
 		},
 		{
 			imageCtx: imageContext{i: image.Summary{SharedSize: 10000}},
diff --git a/cli/command/registry/formatter_search.go b/cli/command/registry/formatter_search.go
@@ -97,5 +97,5 @@ func (c *searchContext) IsOfficial() string {
 //
 // Deprecated: the "is_automated" field is deprecated and will always be "false" in the future.
 func (c *searchContext) IsAutomated() string {
-	return c.formatBool(c.s.IsAutomated) //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
+	return c.formatBool(c.s.IsAutomated) //nolint:nolintlint,staticcheck // ignore SA1019 (IsAutomated is deprecated).
 }
diff --git a/cli/command/registry/formatter_search_test.go b/cli/command/registry/formatter_search_test.go
@@ -47,16 +47,16 @@ func TestSearchContext(t *testing.T) {
 		},
 		{
 			searchCtx: searchContext{
-				s: registrytypes.SearchResult{IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
+				s: registrytypes.SearchResult{IsAutomated: true}, //nolint:nolintlint,staticcheck // ignore SA1019 (IsAutomated is deprecated).
 			},
 			expValue: "[OK]",
-			call:     ctx.IsAutomated, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
+			call:     ctx.IsAutomated, //nolint:nolintlint,staticcheck // ignore SA1019 (IsAutomated is deprecated).
 		},
 		{
 			searchCtx: searchContext{
 				s: registrytypes.SearchResult{},
 			},
-			call: ctx.IsAutomated, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
+			call: ctx.IsAutomated, //nolint:nolintlint,staticcheck // ignore SA1019 (IsAutomated is deprecated).
 		},
 	}
 
@@ -199,7 +199,7 @@ result2 5
 
 	results := []registrytypes.SearchResult{
 		{Name: "result1", Description: "Official build", StarCount: 5000, IsOfficial: true},
-		{Name: "result2", Description: "Not official", StarCount: 5, IsAutomated: true}, //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).
+		{Name: "result2", Description: "Not official", StarCount: 5, IsAutomated: true},
 	}
 
 	for _, tc := range cases {
diff --git a/cli/connhelper/commandconn/pdeathsig_nolinux.go b/cli/connhelper/commandconn/pdeathsig_nolinux.go
@@ -6,5 +6,4 @@ import (
 	"os/exec"
 )
 
-func setPdeathsig(cmd *exec.Cmd) {
-}
+func setPdeathsig(*exec.Cmd) {}
diff --git a/templates/templates.go b/templates/templates.go
@@ -20,7 +20,7 @@ var basicFunctions = template.FuncMap{
 	},
 	"split":    strings.Split,
 	"join":     strings.Join,
-	"title":    strings.Title, //nolint:staticcheck // strings.Title is deprecated, but we only use it for ASCII, so replacing with golang.org/x/text is out of scope
+	"title":    strings.Title, //nolint:nolintlint,staticcheck // strings.Title is deprecated, but we only use it for ASCII, so replacing with golang.org/x/text is out of scope
 	"lower":    strings.ToLower,
 	"upper":    strings.ToUpper,
 	"pad":      padWithSpace,

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ func TestImageContext(t *testing.T) {`
`72`	`72`	`{`
`73`	`73`	`imageCtx: imageContext{i: image.Summary{Size: 10000}},`
`74`	`74`	`expValue: "10kB",`
`75`		`- call: ctx.VirtualSize, //nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.44.`
	`75`	`+ call: ctx.VirtualSize, //nolint:nolintlint,staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.44.`
`76`	`76`	`},`
`77`	`77`	`{`
`78`	`78`	`imageCtx: imageContext{i: image.Summary{SharedSize: 10000}},`
Original file line number	Diff line number	Diff line change
`@@ -97,5 +97,5 @@ func (c *searchContext) IsOfficial() string {`
`97`	`97`	`//`
`98`	`98`	`// Deprecated: the "is_automated" field is deprecated and will always be "false" in the future.`
`99`	`99`	`func (c *searchContext) IsAutomated() string {`
`100`		`- return c.formatBool(c.s.IsAutomated) //nolint:staticcheck // ignore SA1019 (IsAutomated is deprecated).`
	`100`	`+ return c.formatBool(c.s.IsAutomated) //nolint:nolintlint,staticcheck // ignore SA1019 (IsAutomated is deprecated).`
`101`	`101`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,5 +6,4 @@ import (`
`6`	`6`	`"os/exec"`
`7`	`7`	`)`
`8`	`8`
`9`		`-func setPdeathsig(cmd *exec.Cmd) {`
`10`		`-}`
	`9`	`+func setPdeathsig(*exec.Cmd) {}`