Skip to content

Commit

Permalink
golangci-lint: Upgrade, fix issues (#397)
Browse files Browse the repository at this point in the history
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)
```
  • Loading branch information
abhinav authored Dec 10, 2023
1 parent 4ca2606 commit a86bda4
Show file tree
Hide file tree
Showing 25 changed files with 129 additions and 101 deletions.
22 changes: 22 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
File renamed without changes.
2 changes: 1 addition & 1 deletion bin/golangci-lint
4 changes: 2 additions & 2 deletions callbacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (b bindings) addProvider(provider interface{}) error {
errv := out[1]
var err error
if !errv.IsNil() {
err = errv.Interface().(error) // nolint
err = errv.Interface().(error) //nolint
}
return out[0], err
}
Expand Down Expand Up @@ -99,7 +99,7 @@ func callFunction(f reflect.Value, bindings bindings) error {
if out[0].IsNil() {
return nil
}
return out[0].Interface().(error) // nolint
return out[0].Interface().(error) //nolint
}

func callAnyFunction(f reflect.Value, bindings bindings) (out []any, err error) {
Expand Down
48 changes: 24 additions & 24 deletions camelcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,37 @@ import (
//
// Examples
//
// "" => [""]
// "lowercase" => ["lowercase"]
// "Class" => ["Class"]
// "MyClass" => ["My", "Class"]
// "MyC" => ["My", "C"]
// "HTML" => ["HTML"]
// "PDFLoader" => ["PDF", "Loader"]
// "AString" => ["A", "String"]
// "SimpleXMLParser" => ["Simple", "XML", "Parser"]
// "vimRPCPlugin" => ["vim", "RPC", "Plugin"]
// "GL11Version" => ["GL", "11", "Version"]
// "99Bottles" => ["99", "Bottles"]
// "May5" => ["May", "5"]
// "BFG9000" => ["BFG", "9000"]
// "BöseÜberraschung" => ["Böse", "Überraschung"]
// "Two spaces" => ["Two", " ", "spaces"]
// "BadUTF8\xe2\xe2\xa1" => ["BadUTF8\xe2\xe2\xa1"]
// "" => [""]
// "lowercase" => ["lowercase"]
// "Class" => ["Class"]
// "MyClass" => ["My", "Class"]
// "MyC" => ["My", "C"]
// "HTML" => ["HTML"]
// "PDFLoader" => ["PDF", "Loader"]
// "AString" => ["A", "String"]
// "SimpleXMLParser" => ["Simple", "XML", "Parser"]
// "vimRPCPlugin" => ["vim", "RPC", "Plugin"]
// "GL11Version" => ["GL", "11", "Version"]
// "99Bottles" => ["99", "Bottles"]
// "May5" => ["May", "5"]
// "BFG9000" => ["BFG", "9000"]
// "BöseÜberraschung" => ["Böse", "Überraschung"]
// "Two spaces" => ["Two", " ", "spaces"]
// "BadUTF8\xe2\xe2\xa1" => ["BadUTF8\xe2\xe2\xa1"]
//
// 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
// lower case string
// if subsequent string is lower case:
// move last character of upper case string to beginning of
// lower case string
func camelCase(src string) (entries []string) {
// don't split invalid utf8
if !utf8.ValidString(src) {
Expand Down
2 changes: 1 addition & 1 deletion config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()) }
Expand Down
6 changes: 3 additions & 3 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (c *Context) Empty() bool {
}

// Validate the current context.
func (c *Context) Validate() error { // nolint: gocyclo
func (c *Context) Validate() error { //nolint: gocyclo
err := Visit(c.Model, func(node Visitable, next Next) error {
switch node := node.(type) {
case *Value:
Expand Down Expand Up @@ -347,7 +347,7 @@ func (c *Context) endParsing() {
}
}

func (c *Context) trace(node *Node) (err error) { // nolint: gocyclo
func (c *Context) trace(node *Node) (err error) { //nolint: gocyclo
positional := 0
node.Active = true

Expand Down Expand Up @@ -377,7 +377,7 @@ func (c *Context) trace(node *Node) (err error) { // nolint: gocyclo
switch {
case v == "-":
fallthrough
default: // nolint
default: //nolint
c.scan.Pop()
c.scan.PushTyped(token.Value, PositionalArgumentToken)

Expand Down
1 change: 1 addition & 0 deletions defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 17 additions & 17 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,31 @@
//
// Here's an example:
//
// shell rm [-f] [-r] <paths> ...
// shell ls [<paths> ...]
// shell rm [-f] [-r] <paths> ...
// shell ls [<paths> ...]
//
// This can be represented by the following command-line structure:
//
// package main
// package main
//
// import "github.com/alecthomas/kong"
// import "github.com/alecthomas/kong"
//
// var CLI struct {
// Rm struct {
// Force bool `short:"f" help:"Force removal."`
// Recursive bool `short:"r" help:"Recursively remove files."`
// var CLI struct {
// Rm struct {
// Force bool `short:"f" help:"Force removal."`
// Recursive bool `short:"r" help:"Recursively remove files."`
//
// Paths []string `arg help:"Paths to remove." type:"path"`
// } `cmd help:"Remove files."`
// Paths []string `arg help:"Paths to remove." type:"path"`
// } `cmd help:"Remove files."`
//
// Ls struct {
// Paths []string `arg optional help:"Paths to list." type:"path"`
// } `cmd help:"List paths."`
// }
// Ls struct {
// Paths []string `arg optional help:"Paths to list." type:"path"`
// } `cmd help:"List paths."`
// }
//
// func main() {
// kong.Parse(&CLI)
// }
// func main() {
// kong.Parse(&CLI)
// }
//
// See https://github.com/alecthomas/kong for details.
package kong
2 changes: 1 addition & 1 deletion global_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestParseHandlingBadBuild(t *testing.T) {

defer func() {
if r := recover(); r != nil {
assert.Equal(t, "fail=' is not quoted properly", r.(error).Error()) // nolint
assert.Equal(t, "fail=' is not quoted properly", r.(error).Error()) //nolint
}
}()

Expand Down
1 change: 1 addition & 0 deletions guesswidth.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build appengine || (!linux && !freebsd && !darwin && !dragonfly && !netbsd && !openbsd)
// +build appengine !linux,!freebsd,!darwin,!dragonfly,!netbsd,!openbsd

package kong
Expand Down
4 changes: 2 additions & 2 deletions guesswidth_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func guessWidth(w io.Writer) int {

if _, _, err := syscall.Syscall6(
syscall.SYS_IOCTL,
uintptr(fd), // nolint: unconvert
uintptr(fd), //nolint: unconvert
uintptr(syscall.TIOCGWINSZ),
uintptr(unsafe.Pointer(&dimensions)), // nolint: gas
uintptr(unsafe.Pointer(&dimensions)), //nolint: gas
0, 0, 0,
); err == 0 {
if dimensions[1] == 0 {
Expand Down
2 changes: 1 addition & 1 deletion help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ func TestAutoGroup(t *testing.T) {
if node, ok := parent.(*kong.Node); ok {
return &kong.Group{
Key: node.Name,
Title: strings.Title(node.Name) + " flags:", // nolint
Title: strings.Title(node.Name) + " flags:", //nolint
}
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions kong.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (k *Kong) FatalIfErrorf(err error, args ...interface{}) {
}
msg := err.Error()
if len(args) > 0 {
msg = fmt.Sprintf(args[0].(string), args[1:]...) + ": " + err.Error() // nolint
msg = fmt.Sprintf(args[0].(string), args[1:]...) + ": " + err.Error() //nolint
}
// Maybe display usage information.
var parseErr *ParseError
Expand All @@ -439,11 +439,11 @@ func (k *Kong) LoadConfig(path string) (Resolver, error) {
if err != nil {
return nil, err
}
r, err := os.Open(path) // nolint: gas
r, err := os.Open(path) //nolint: gas
if err != nil {
return nil, err
}
defer r.Close() // nolint: gosec
defer r.Close()

return k.loader(r)
}
13 changes: 7 additions & 6 deletions kong_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -753,7 +754,7 @@ func TestEmbedInterface(t *testing.T) {
_, err := p.Parse([]string{"--some-flag=foo", "--flag=yes"})
assert.NoError(t, err)
assert.Equal(t, "foo", cli.SomeFlag)
assert.Equal(t, "yes", cli.TestInterface.(*TestImpl).Flag) // nolint
assert.Equal(t, "yes", cli.TestInterface.(*TestImpl).Flag) //nolint
}

func TestExcludedField(t *testing.T) {
Expand Down Expand Up @@ -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"`
}
Expand Down Expand Up @@ -1928,8 +1929,8 @@ func TestBoolPtrNil(t *testing.T) {

func TestUnsupportedPtr(t *testing.T) {
type Foo struct {
x int // nolint
y int // nolint
x int //nolint
y int //nolint
}

var cli struct {
Expand Down
Loading

0 comments on commit a86bda4

Please sign in to comment.