Fix default golangci-lint warnings, add it to GHA #308
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kolyshkin
force-pushed
the
golangci-lint
branch
from
8feb653 to
18c6a99
Compare
kolyshkin
commented
Feb 7, 2022
kolyshkin
commented
Feb 8, 2022
kolyshkin
force-pushed
the
golangci-lint
branch
from
18c6a99 to
da0875d
Compare
guelfey
reviewed
Feb 13, 2022
kolyshkin
force-pushed
the
golangci-lint
branch
2 times, most recently
from
c5b996d to
292251c
Compare
|
@kolyshkin anything left here after #313 is merged? Otherwise let's merge this as well |
This is redundant since the file name ends with _windows. Signed-off-by: Kir Kolyshkin <[email protected]>
Generated by /home/kir/sdk/go1.17.3/bin/gofmt -s -w ./ ./_examples/ ./prop/ ./introspect/ A linter will be added later to ensure that the sources stay formatted. Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following warnings:
call.go:8:5: `errSignature` is unused (deadcode)
var errSignature = errors.New("dbus: mismatched signature")
^
dbus.go:13:2: `uint8Type` is unused (deadcode)
uint8Type = reflect.TypeOf(uint8(0))
^
dbus.go:16:2: `intType` is unused (deadcode)
intType = reflect.TypeOf(int(0))
^
dbus.go:17:2: `uintType` is unused (deadcode)
uintType = reflect.TypeOf(uint(0))
^
Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following warnings:
export_test.go:12:31: func `lowerCaseExport.foo` is unused (unused)
func (export lowerCaseExport) foo() (string, *Error) {
^
conn.go:935:29: func `(*callTracker).finalize` is unused (unused)
func (tracker *callTracker) finalize(sn uint32) {
^
variant_parser.go:420:2: field `val` is unused (unused)
val interface{}
^
default_handler.go:218:25: func `(*exportedObj).isFallbackInterface` is unused (unused)
func (obj *exportedObj) isFallbackInterface() bool {
^
variant_parser.go:577:2: field `val` is unused (unused)
val interface{}
^
Signed-off-by: Kir Kolyshkin <[email protected]>
Remove the select, following the recommendation of this warning:
examples_test.go:44:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
select {
^
Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following warning: conn_test.go:130:2: ineffectual assignment to signals (ineffassign) signals := bus.signalHandler.(*defaultSignalHandler).signals ^ Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following warnings:
decoder_test.go:68:2: SA4006: this value of `msg` is never used (staticcheck)
msg, err = DecodeMessage(buf)
^
conn_test.go:363:2: SA5001: should check returned error before deferring srv.Close() (staticcheck)
defer srv.Close()
^
exec_command_test.go:33:2: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
fmt.Fprintf(os.Stdout, os.Getenv("STDOUT"))
^
Signed-off-by: Kir Kolyshkin <[email protected]>
This fixes warnings reported by govet and staticcheck linters:
conn_test.go:659:2: SA2002: the goroutine calls T.Fatal, which must be called in the same goroutine as the test (staticcheck)
go func() {
^
conn_test.go:663:5: SA2002(related information): call to T.Fatal (staticcheck)
b.Fatal(v.Err)
^
server_interfaces_test.go:473:2: SA2002: the goroutine calls T.Fatal, which must be called in the same goroutine as the test (staticcheck)
go func() {
^
server_interfaces_test.go:476:4: SA2002(related information): call to T.Fatal (staticcheck)
t.Fatal(err)
^
[kir@kir-rhat dbus]$ golangci-lint run --disable-all -E govet
conn_test.go:663:5: testinggoroutine: call to (*B).Fatal from a non-test goroutine (govet)
b.Fatal(v.Err)
^
server_interfaces_test.go:476:4: testinggoroutine: call to (*T).Fatal from a non-test goroutine (govet)
t.Fatal(err)
^
Signed-off-by: Kir Kolyshkin <[email protected]>
Fix a bunch of warnings like
prop/prop_test.go:21:12: Error return value of `dbus.Store` is not checked (errcheck)
dbus.Store([]interface{}{r.Value()}, haveValue)
^
prop/prop_test.go:81:17: Error return value of `obj.SetProperty` is not checked (errcheck)
obj.SetProperty("org.guelfey.DBus.Test.FooStruct", dbus.MakeVariant(yoo))
^
Signed-off-by: Kir Kolyshkin <[email protected]>
First, we already have fds to return, so there's no need to make another one. Second, it's perfectly fine to return nil slice on error. Signed-off-by: Kir Kolyshkin <[email protected]>
Fix a few "return value of ... is not checked" warnings. Signed-off-by: Kir Kolyshkin <[email protected]>
Fix a bunch of warnings like Error return value of `cmd.Process.Kill` is not checked (errcheck) Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
force-pushed
the
golangci-lint
branch
3 times, most recently
from
86d09e7 to
71751f6
Compare
Signed-off-by: Kir Kolyshkin <[email protected]>
This is golangci-lint run with default linters and configuration. Install golang since this is no longer done by golangci-lint v3.x. We use 1.x for golang version to ease the maintenance burden, since this should always point to latest stable golang release (at the time of writing this, using 1.x results in go1.17.7 being installed). Signed-off-by: Kir Kolyshkin <[email protected]>
Quite a lot. This fixes all the warnings from the default golangci-lint set, and adds golangci-lint into CI. Once this PR is merged, we can enable more [non-default] linters. In particular, I like gofumpt, errorlint, unconvert, and unparam. |
|
@guelfey PTAL |
guelfey
approved these changes
Mar 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables golangci-lint to be run in GHA CI, and fixes the warnings found.
Lots of changes, although mostly trivial, but require a thorough review. See individual commits for details.
Once merged, more linters can be enabled in addition to the default set.
Currently based on and includes #313 -- will rebase and move out of draft once it's merged.