golangci-lint: Upgrade, fix issues (#397)

The golangci-lint being used was quite dated.
This change upgrades to the latest version.
Adds and updates exclusions based on new failures.

Note on revive:
I've included an opt-out for unused parameters for revive
because turning `newThing(required bool)` to `newThing(_ bool)`
is a loss of useful information.

The changes to the Go files are to fix the following issues:

```
camelcase.go:16: File is not `gofmt`-ed with `-s` (gofmt)
config_test.go:50:18: directive `//nolint: gosec` is unused for linter "gosec" (nolintlint)
defaults_test.go:28:25: G601: Implicit memory aliasing in for loop. (gosec)
doc.go:5: File is not `gofmt`-ed with `-s` (gofmt)
kong.go:446:18: directive `//nolint: gosec` is unused for linter "gosec" (nolintlint)
kong_test.go:503:20: G601: Implicit memory aliasing in for loop. (gosec)
model.go:493:10: composites: reflect.ValueError struct literal uses unkeyed fields (govet)
scanner.go:112: File is not `gofmt`-ed with `-s` (gofmt)
```

And to address broken nolint directives reported as follows by
golangci-lint.

```
[.. skipped .. ]
tag.go:65:1: directive `// nolint:gocyclo` should be written without leading space as `//nolint:gocyclo` (nolintlint)
tag.go:206:51: directive `// nolint: gocyclo` should be written without leading space as `//nolint: gocyclo` (nolintlint)
util_test.go:23:43: directive `// nolint: errcheck` should be written without leading space as `//nolint: errcheck` (nolintlint)
util_test.go:51:22: directive `// nolint: errcheck` should be written without leading space as `//nolint: errcheck` (nolintlint)
```

.golangci.yml

@@ -41,10 +41,21 @@ linters:
     - exhaustruct
     - nonamedreturns
     - nilnil
+    - nosnakecase # deprecated since v1.48.1
+    - structcheck # deprecated since v1.49.0
+    - deadcode    # deprecated since v1.49.0
+    - varcheck    # deprecated since v1.49.0
+    - depguard    # nothing to guard against yet
+    - tagalign    # hurts readability of kong tags
 
 linters-settings:
   govet:
     check-shadowing: true
+    # These govet checks are disabled by default, but they're useful.
+    enable:
+      - niliness
+      - sortslice
+      - unusedwrite
   dupl:
     threshold: 100
   gocyclo:
@@ -66,3 +77,14 @@ issues:
     - 'bad syntax for struct tag pair'
     - 'result .* \(error\) is always nil'
     - 'package io/ioutil is deprecated'
+
+  exclude-rules:
+    # Don't warn on unused parameters.
+    # Parameter names are useful for documentation.
+    # Replacing them with '_' hides useful information.
+    - linters: [revive]
+      text: 'unused-parameter: parameter \S+ seems to be unused, consider removing or renaming it as _'
+
+    # Duplicate words are okay in tests.
+    - linters: [dupword]
+      path: _test\.go

bin/golangci-lint

@@ -1 +1 @@
-.golangci-lint-1.46.2.pkg
+.golangci-lint-1.55.2.pkg

camelcase.go

@@ -33,13 +33,13 @@ import (
 //
 // Splitting rules
 //
-//  1) If string is not valid UTF-8, return it without splitting as
+//  1. If string is not valid UTF-8, return it without splitting as
 //     single item array.
-//  2) Assign all unicode characters into one of 4 sets: lower case
+//  2. Assign all unicode characters into one of 4 sets: lower case
 //     letters, upper case letters, numbers, and all other characters.
-//  3) Iterate through characters of string, introducing splits
+//  3. Iterate through characters of string, introducing splits
 //     between adjacent characters that belong to different sets.
-//  4) Iterate through array of split strings, and if a given string
+//  4. Iterate through array of split strings, and if a given string
 //     is upper case:
 //     if subsequent string is lower case:
 //     move last character of upper case string to beginning of

config_test.go

@@ -47,7 +47,7 @@ func makeConfig(t *testing.T, config interface{}) (path string, cleanup func())
 	t.Helper()
 	w, err := ioutil.TempFile("", "")
 	assert.NoError(t, err)
-	defer w.Close() // nolint: gosec
+	defer w.Close()
 	err = json.NewEncoder(w).Encode(config)
 	assert.NoError(t, err)
 	return w.Name(), func() { os.Remove(w.Name()) }

defaults_test.go

@@ -24,6 +24,7 @@ func TestApplyDefaults(t *testing.T) {
 			expected: CLI{Str: "str", Duration: time.Second}},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			err := ApplyDefaults(&tt.target)
 			assert.NoError(t, err)

guesswidth.go

@@ -1,3 +1,4 @@
+//go:build appengine || (!linux && !freebsd && !darwin && !dragonfly && !netbsd && !openbsd)
 // +build appengine !linux,!freebsd,!darwin,!dragonfly,!netbsd,!openbsd
 
 package kong

kong.go

@@ -443,7 +443,7 @@ func (k *Kong) LoadConfig(path string) (Resolver, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer r.Close() // nolint: gosec
+	defer r.Close()
 
 	return k.loader(r)
 }

kong_test.go

@@ -495,6 +495,7 @@ func TestHooks(t *testing.T) {
 	p := mustNew(t, &cli, kong.Bind(ctx))
 
 	for _, test := range tests {
+		test := test
 		*ctx = hookContext{}
 		cli.One = hookCmd{}
 		t.Run(test.name, func(t *testing.T) {
@@ -1352,16 +1353,16 @@ func TestHydratePointerCommandsAndEmbeds(t *testing.T) {
 	assert.Equal(t, &embed{Embed: true}, cli.Embed)
 }
 
-// nolint
+//nolint:revive
 type testIgnoreFields struct {
 	Foo struct {
 		Bar bool
 		Sub struct {
 			SubFlag1     bool `kong:"name=subflag1"`
-			XXX_SubFlag2 bool `kong:"name=subflag2"`
+			XXX_SubFlag2 bool `kong:"name=subflag2"` //nolint:stylecheck
 		} `kong:"cmd"`
 	} `kong:"cmd"`
-	XXX_Baz struct {
+	XXX_Baz struct { //nolint:stylecheck
 		Boo bool
 	} `kong:"cmd,name=baz"`
 }

mapper_test.go

@@ -356,14 +356,14 @@ func TestNumbers(t *testing.T) {
 		_, err := p.Parse([]string{
 			"--f-32", fmt.Sprintf("%v", math.MaxFloat32),
 			"--f-64", fmt.Sprintf("%v", math.MaxFloat64),
-			"--i-8", fmt.Sprintf("%v", int8(math.MaxInt8)),
+			"--i-8", fmt.Sprintf("%v", int8(math.MaxInt8)), //nolint:perfsprint // want int8
-			"--i-16", fmt.Sprintf("%v", int16(math.MaxInt16)),
+			"--i-16", fmt.Sprintf("%v", int16(math.MaxInt16)), //nolint:perfsprint // want int16
-			"--i-32", fmt.Sprintf("%v", int32(math.MaxInt32)),
+			"--i-32", fmt.Sprintf("%v", int32(math.MaxInt32)), //nolint:perfsprint // want int32
-			"--i-64", fmt.Sprintf("%v", int64(math.MaxInt64)),
+			"--i-64", fmt.Sprintf("%v", int64(math.MaxInt64)), //nolint:perfsprint // want int64
-			"--u-8", fmt.Sprintf("%v", uint8(math.MaxUint8)),
+			"--u-8", fmt.Sprintf("%v", uint8(math.MaxUint8)), //nolint:perfsprint // want uint8
-			"--u-16", fmt.Sprintf("%v", uint16(math.MaxUint16)),
+			"--u-16", fmt.Sprintf("%v", uint16(math.MaxUint16)), //nolint:perfsprint // want uint16
-			"--u-32", fmt.Sprintf("%v", uint32(math.MaxUint32)),
+			"--u-32", fmt.Sprintf("%v", uint32(math.MaxUint32)), //nolint:perfsprint // want uint32
-			"--u-64", fmt.Sprintf("%v", uint64(math.MaxUint64)),
+			"--u-64", fmt.Sprintf("%v", uint64(math.MaxUint64)), //nolint:perfsprint // want uint64
 		})
 		assert.NoError(t, err)
 		assert.Equal(t, CLI{

model.go

@@ -490,6 +490,9 @@ func reflectValueIsZero(v reflect.Value) bool {
 	default:
 		// This should never happens, but will act as a safeguard for
 		// later, as a default value doesn't makes sense here.
-		panic(&reflect.ValueError{"reflect.Value.IsZero", v.Kind()})
+		panic(&reflect.ValueError{
+			Method: "reflect.Value.IsZero",
+			Kind:   v.Kind(),
+		})
 	}
 }