Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golangci-lint: Upgrade, fix issues #397

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Dec 10, 2023

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)

Comment on lines +36 to +42
// 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was gofmt, not me.

@@ -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()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused //nolint

@alecthomas
Copy link
Owner

Thanks, but I prefer to use enable-all and explicitly disable any "problematic" linters when I upgrade. This way I can take advantage of new linters as they appear.

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)
```
@abhinav abhinav changed the title golangci-lint: Upgrade, fix issues, reconfigure usage: git log [<options>] [<revision-range>] [[--] <path>...] or: git show [<options>] <object>... -q, --[no-]quiet suppress diff output --[no-]source show source --[no-]use-mailmap use mail map file --[no-]mailmap alias of --use-mailmap --clear-decorations clear all previously-defined decoration filters --[no-]decorate-refs <pattern> only decorate refs that match <pattern> --[no-]decorate-refs-exclude <pattern> do not decorate refs that match <pattern> --[no-]decorate[=...] decorate options -L <range:file> trace the evolution of line range <start>,<end> or function :<funcname> in <file> Dec 10, 2023
@abhinav abhinav changed the title usage: git log [<options>] [<revision-range>] [[--] <path>...] or: git show [<options>] <object>... -q, --[no-]quiet suppress diff output --[no-]source show source --[no-]use-mailmap use mail map file --[no-]mailmap alias of --use-mailmap --clear-decorations clear all previously-defined decoration filters --[no-]decorate-refs <pattern> only decorate refs that match <pattern> --[no-]decorate-refs-exclude <pattern> do not decorate refs that match <pattern> --[no-]decorate[=...] decorate options -L <range:file> trace the evolution of line range <start>,<end> or function :<funcname> in <file> golangci-lint: Upgrade, fix issues Dec 10, 2023
@abhinav
Copy link
Contributor Author

abhinav commented Dec 10, 2023

Fair enough. Brought the enable-all back.

@alecthomas alecthomas merged commit a86bda4 into alecthomas:master Dec 10, 2023
3 checks passed
@alecthomas
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants