From 15b2874ff3d3de7e936a6b62c6fdbee34bbf4d47 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Mar 2022 13:58:23 -0700 Subject: [PATCH 1/5] ci: add gofumpt linter gofumpt is basically a stricter/better gofmt. It is supported by gopls (and thus all IDEs), and in general results in a better formatted code. This commit - formats all code with gofumpt (gofumpt -w .) - adds gofumpt linter to golangci-lint to ensure future commits Signed-off-by: Kir Kolyshkin --- .golangci.yml | 5 +++++ _examples/monitor.go | 2 +- conn.go | 10 ++++++---- conn_darwin.go | 1 - conn_other.go | 1 - dbus.go | 12 ++++++------ decoder.go | 6 +++--- default_handler.go | 2 +- exec_command_test.go | 12 ++++++++---- message_test.go | 3 +-- prop/prop.go | 2 +- store_test.go | 3 ++- variant_test.go | 13 ++++++++----- 13 files changed, 42 insertions(+), 30 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..e2d02fbd --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,5 @@ +# For documentation, see https://golangci-lint.run/usage/configuration/ + +linters: + enable: + - gofumpt diff --git a/_examples/monitor.go b/_examples/monitor.go index 0750da89..5cfefc27 100644 --- a/_examples/monitor.go +++ b/_examples/monitor.go @@ -15,7 +15,7 @@ func main() { } defer conn.Close() - var rules = []string{ + rules := []string{ "type='signal',member='Notify',path='/org/freedesktop/Notifications',interface='org.freedesktop.Notifications'", "type='method_call',member='Notify',path='/org/freedesktop/Notifications',interface='org.freedesktop.Notifications'", "type='method_return',member='Notify',path='/org/freedesktop/Notifications',interface='org.freedesktop.Notifications'", diff --git a/conn.go b/conn.go index 1a6cf7e6..81e93663 100644 --- a/conn.go +++ b/conn.go @@ -76,7 +76,6 @@ func SessionBus() (conn *Conn, err error) { func getSessionBusAddress(autolaunch bool) (string, error) { if address := os.Getenv("DBUS_SESSION_BUS_ADDRESS"); address != "" && address != "autolaunch:" { return address, nil - } else if address := tryDiscoverDbusSessionBusAddress(); address != "" { os.Setenv("DBUS_SESSION_BUS_ADDRESS", address) return address, nil @@ -740,9 +739,7 @@ type transport interface { SendMessage(*Message) error } -var ( - transports = make(map[string]func(string) (transport, error)) -) +var transports = make(map[string]func(string) (transport, error)) func getTransport(address string) (transport, error) { var err error @@ -853,16 +850,19 @@ type nameTracker struct { func newNameTracker() *nameTracker { return &nameTracker{names: map[string]struct{}{}} } + func (tracker *nameTracker) acquireUniqueConnectionName(name string) { tracker.lck.Lock() defer tracker.lck.Unlock() tracker.unique = name } + func (tracker *nameTracker) acquireName(name string) { tracker.lck.Lock() defer tracker.lck.Unlock() tracker.names[name] = struct{}{} } + func (tracker *nameTracker) loseName(name string) { tracker.lck.Lock() defer tracker.lck.Unlock() @@ -874,12 +874,14 @@ func (tracker *nameTracker) uniqueNameIsKnown() bool { defer tracker.lck.RUnlock() return tracker.unique != "" } + func (tracker *nameTracker) isKnownName(name string) bool { tracker.lck.RLock() defer tracker.lck.RUnlock() _, ok := tracker.names[name] return ok || name == tracker.unique } + func (tracker *nameTracker) listKnownNames() []string { tracker.lck.RLock() defer tracker.lck.RUnlock() diff --git a/conn_darwin.go b/conn_darwin.go index 6e2e4020..cb2325a0 100644 --- a/conn_darwin.go +++ b/conn_darwin.go @@ -12,7 +12,6 @@ const defaultSystemBusAddress = "unix:path=/opt/local/var/run/dbus/system_bus_so func getSessionBusPlatformAddress() (string, error) { cmd := exec.Command("launchctl", "getenv", "DBUS_LAUNCHD_SESSION_BUS_SOCKET") b, err := cmd.CombinedOutput() - if err != nil { return "", err } diff --git a/conn_other.go b/conn_other.go index 8d4e6bc4..4730ee91 100644 --- a/conn_other.go +++ b/conn_other.go @@ -20,7 +20,6 @@ var execCommand = exec.Command func getSessionBusPlatformAddress() (string, error) { cmd := execCommand("dbus-launch") b, err := cmd.CombinedOutput() - if err != nil { return "", err } diff --git a/dbus.go b/dbus.go index b97f0db8..d9822186 100644 --- a/dbus.go +++ b/dbus.go @@ -82,7 +82,7 @@ func storeBase(dest, src reflect.Value) error { func setDest(dest, src reflect.Value) error { if !isVariant(src.Type()) && isVariant(dest.Type()) { - //special conversion for dbus.Variant + // special conversion for dbus.Variant dest.Set(reflect.ValueOf(MakeVariant(src.Interface()))) return nil } @@ -163,8 +163,8 @@ func storeMapIntoVariant(dest, src reflect.Value) error { func storeMapIntoInterface(dest, src reflect.Value) error { var dv reflect.Value if isVariant(src.Type().Elem()) { - //Convert variants to interface{} recursively when converting - //to interface{} + // Convert variants to interface{} recursively when converting + // to interface{} dv = reflect.MakeMap( reflect.MapOf(src.Type().Key(), interfaceType)) } else { @@ -197,7 +197,7 @@ func storeMapIntoMap(dest, src reflect.Value) error { func storeSlice(dest, src reflect.Value) error { switch { case src.Type() == interfacesType && dest.Kind() == reflect.Struct: - //The decoder always decodes structs as slices of interface{} + // The decoder always decodes structs as slices of interface{} return storeStruct(dest, src) case !kindsAreCompatible(dest.Type(), src.Type()): return fmt.Errorf( @@ -257,8 +257,8 @@ func storeSliceIntoVariant(dest, src reflect.Value) error { func storeSliceIntoInterface(dest, src reflect.Value) error { var dv reflect.Value if isVariant(src.Type().Elem()) { - //Convert variants to interface{} recursively when converting - //to interface{} + // Convert variants to interface{} recursively when converting + // to interface{} dv = reflect.MakeSlice(reflect.SliceOf(interfaceType), src.Len(), src.Cap()) } else { diff --git a/decoder.go b/decoder.go index 89bfed9d..1b0f07ea 100644 --- a/decoder.go +++ b/decoder.go @@ -205,9 +205,9 @@ func (dec *decoder) decode(s string, depth int) interface{} { // Even for empty arrays, the correct padding must be included align := alignment(typeFor(s[1:])) if len(s) > 1 && s[1] == '(' { - //Special case for arrays of structs - //structs decode as a slice of interface{} values - //but the dbus alignment does not match this + // Special case for arrays of structs + // structs decode as a slice of interface{} values + // but the dbus alignment does not match this align = 8 } dec.align(align) diff --git a/default_handler.go b/default_handler.go index f48e9e9f..cfe8fb26 100644 --- a/default_handler.go +++ b/default_handler.go @@ -148,7 +148,7 @@ func (m exportedMethod) Call(args ...interface{}) ([]interface{}, error) { out[i] = val.Interface() } if nilErr || err == nil { - //concrete type to interface nil is a special case + // concrete type to interface nil is a special case return out, nil } return out, err diff --git a/exec_command_test.go b/exec_command_test.go index f32b8522..a05036ba 100755 --- a/exec_command_test.go +++ b/exec_command_test.go @@ -10,17 +10,21 @@ import ( // How to mock exec.Command for unit tests // https://stackoverflow.com/q/45789101/10513533 -var mockedExitStatus = 0 -var mockedStdout string +var ( + mockedExitStatus = 0 + mockedStdout string +) func fakeExecCommand(command string, args ...string) *exec.Cmd { cs := []string{"-test.run=TestExecCommandHelper", "--", command} cs = append(cs, args...) cmd := exec.Command(os.Args[0], cs...) es := strconv.Itoa(mockedExitStatus) - cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1", + cmd.Env = []string{ + "GO_WANT_HELPER_PROCESS=1", "STDOUT=" + mockedStdout, - "EXIT_STATUS=" + es} + "EXIT_STATUS=" + es, + } return cmd } diff --git a/message_test.go b/message_test.go index d6c73da0..cb8a5fe4 100644 --- a/message_test.go +++ b/message_test.go @@ -3,7 +3,7 @@ package dbus import "testing" func TestMessage_validateHeader(t *testing.T) { - var tcs = []struct { + tcs := []struct { msg Message err error }{ @@ -85,7 +85,6 @@ func TestMessage_validateHeader(t *testing.T) { err: InvalidMessageError("invalid error name"), }, { - msg: Message{ Type: TypeError, Headers: map[HeaderField]Variant{ diff --git a/prop/prop.go b/prop/prop.go index 2c2a6132..3a80d826 100644 --- a/prop/prop.go +++ b/prop/prop.go @@ -148,7 +148,7 @@ type Prop struct { // Introspection returns the introspection data for p. // The "name" argument is used as the property's name in the resulting data. func (p *Prop) Introspection(name string) introspect.Property { - var result = introspect.Property{Name: name, Type: dbus.SignatureOf(p.Value).String()} + result := introspect.Property{Name: name, Type: dbus.SignatureOf(p.Value).String()} if p.Writable { result.Access = "readwrite" } else { diff --git a/store_test.go b/store_test.go index 732c366a..aab61091 100644 --- a/store_test.go +++ b/store_test.go @@ -77,7 +77,8 @@ func TestStoreSliceVariantToSliceInterfaceMulti(t *testing.T) { func TestStoreNested(t *testing.T) { src := map[string]interface{}{ - "foo": []interface{}{"1", "2", "3", "5", + "foo": []interface{}{ + "1", "2", "3", "5", map[string]interface{}{ "bar": "baz", }, diff --git a/variant_test.go b/variant_test.go index c9dce3e6..23ac645c 100644 --- a/variant_test.go +++ b/variant_test.go @@ -1,7 +1,9 @@ package dbus -import "reflect" -import "testing" +import ( + "reflect" + "testing" +) var variantFormatTests = []struct { v interface{} @@ -54,8 +56,10 @@ var variantParseTests = []struct { {`@a{ss} {}`, map[string]string{}}, {`{"foo": 1}`, map[string]int32{"foo": 1}}, {`[{}, {"foo": "bar"}]`, []map[string]string{{}, {"foo": "bar"}}}, - {`{"a": <1>, "b": <"foo">}`, - map[string]Variant{"a": MakeVariant(int32(1)), "b": MakeVariant("foo")}}, + { + `{"a": <1>, "b": <"foo">}`, + map[string]Variant{"a": MakeVariant(int32(1)), "b": MakeVariant("foo")}, + }, {`b''`, []byte{0}}, {`b"abc"`, []byte{'a', 'b', 'c', 0}}, {`b"\x01\0002\a\b\f\n\r\t"`, []byte{1, 2, 0x7, 0x8, 0xc, '\n', '\r', '\t', 0}}, @@ -88,5 +92,4 @@ func TestVariantStore(t *testing.T) { if result != str { t.Fatalf("expected %s, got %s\n", str, result) } - } From e948f1a92b30e3edd4ef349b32927815aa76e433 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Mar 2022 14:01:11 -0700 Subject: [PATCH 2/5] ci: enable unconvert linter No issues found. Signed-off-by: Kir Kolyshkin --- .golangci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yml b/.golangci.yml index e2d02fbd..e2ecaa95 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,3 +3,4 @@ linters: enable: - gofumpt + - unconvert From 9bfa67e929d19a79edd06cde2b82f6b74e2817d9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Mar 2022 14:11:24 -0700 Subject: [PATCH 3/5] sequential_handler_test.go: fix unparam linter warnings 1. Fix the following warning: sequential_handler_test.go:224:18: `readSignals` - `t` is unused (unparam) func readSignals(t *testing.T, channel <-chan *Signal, count int) error { ^ Instead of removing t, let's use it -- since all the callers of readSignals call t.Error(), we can inline it. 2. Fix this warning: sequential_handler_test.go:212:56: `readSignals` - `count` always receives `1000` (unparam) func readSignals(t *testing.T, channel <-chan *Signal, count int) { ^ by removing the count argument. Signed-off-by: Kir Kolyshkin --- sequential_handler_test.go | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/sequential_handler_test.go b/sequential_handler_test.go index 3a2d36aa..4014b575 100644 --- a/sequential_handler_test.go +++ b/sequential_handler_test.go @@ -2,8 +2,6 @@ package dbus import ( "context" - "errors" - "fmt" "testing" "time" ) @@ -20,9 +18,7 @@ func TestSequentialHandlerNoDrop(t *testing.T) { writeSignals(handler, 1000) - if err := readSignals(t, channel, 1000); err != nil { - t.Error(err) - } + readSignals(t, channel) } // Verifies that signals are written to the destination channel in the @@ -39,9 +35,7 @@ func TestSequentialHandlerSequential(t *testing.T) { // Concurrently read and write signals go func() { - if err := readSignals(t, channel, 1000); err != nil { - t.Error(err) - } + readSignals(t, channel) close(done) }() writeSignals(handler, 1000) @@ -63,9 +57,7 @@ func TestSequentialHandlerMultipleChannel(t *testing.T) { writeSignals(handler, 1000) - if err := readSignals(t, channelTwo, 1000); err != nil { - t.Error(err) - } + readSignals(t, channelTwo) } // Test that removing one channel results in no more messages being @@ -119,14 +111,10 @@ func TestSequentialHandler_RemoveOneChannelOfMany(t *testing.T) { } // Check that closing channel two does not close channel one. - if err := readSignals(t, channelOne, 1000); err != nil { - t.Error(err) - } + readSignals(t, channelOne) // Check that closing channel two does not close channel three. - if err := readSignals(t, channelThree, 1000); err != nil { - t.Error(err) - } + readSignals(t, channelThree) } // Test that Terminate() closes all channels that were attached at the time. @@ -221,21 +209,22 @@ func writeSignals(handler SignalHandler, count int) { } } -func readSignals(t *testing.T, channel <-chan *Signal, count int) error { +func readSignals(t *testing.T, channel <-chan *Signal) { // Overly generous timeout ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - for i := 1; i <= count; i++ { + for i := 1; i <= 1000; i++ { select { case signal := <-channel: if signal.Sequence != Sequence(i) { - return fmt.Errorf("Received signal out of order. Expected %v, got %v", i, signal.Sequence) + t.Errorf("Received signal out of order. Expected %v, got %v", i, signal.Sequence) + return } case <-ctx.Done(): - return errors.New("Timeout occurred before all messages received") + t.Error("Timeout occurred before all messages received") + return } } - return nil } func countSignals(channel <-chan *Signal) (count int, closed bool) { From f754e574d3ca14839a444b3983224a50ef0f8686 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Mar 2022 14:15:06 -0700 Subject: [PATCH 4/5] conn_test: fix unparam warning Fix the following warning: conn_test.go:341:51: `waitSignal` - `timeout` always receives `time.Second` (`1000000000`) (unparam) func waitSignal(sigc <-chan *Signal, name string, timeout time.Duration) *Signal { ^ by removing the timeout argument. Signed-off-by: Kir Kolyshkin --- conn_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/conn_test.go b/conn_test.go index 55a7af76..a97f6ca6 100644 --- a/conn_test.go +++ b/conn_test.go @@ -279,7 +279,7 @@ func TestAddAndRemoveMatchSignalContext(t *testing.T) { if err = conn.Emit("/", "org.test.CtxTest"); err != nil { t.Fatal(err) } - if sig := waitSignal(sigc, "org.test.CtxTest", time.Second); sig == nil { + if sig := waitSignal(sigc, "org.test.CtxTest"); sig == nil { t.Fatal("signal receive timed out") } @@ -294,7 +294,7 @@ func TestAddAndRemoveMatchSignalContext(t *testing.T) { if err = conn.Emit("/", "org.test.CtxTest"); err != nil { t.Fatal(err) } - if sig := waitSignal(sigc, "org.test.CtxTest", time.Second); sig != nil { + if sig := waitSignal(sigc, "org.test.CtxTest"); sig != nil { t.Fatalf("unsubscribed from %q signal, but received %#v", "org.test.CtxTest", sig) } } @@ -319,7 +319,7 @@ func TestAddAndRemoveMatchSignal(t *testing.T) { if err = conn.Emit("/", "org.test.Test"); err != nil { t.Fatal(err) } - if sig := waitSignal(sigc, "org.test.Test", time.Second); sig == nil { + if sig := waitSignal(sigc, "org.test.Test"); sig == nil { t.Fatal("signal receive timed out") } @@ -333,19 +333,19 @@ func TestAddAndRemoveMatchSignal(t *testing.T) { if err = conn.Emit("/", "org.test.Test"); err != nil { t.Fatal(err) } - if sig := waitSignal(sigc, "org.test.Test", time.Second); sig != nil { + if sig := waitSignal(sigc, "org.test.Test"); sig != nil { t.Fatalf("unsubscribed from %q signal, but received %#v", "org.test.Test", sig) } } -func waitSignal(sigc <-chan *Signal, name string, timeout time.Duration) *Signal { +func waitSignal(sigc <-chan *Signal, name string) *Signal { for { select { case sig := <-sigc: if sig.Name == name { return sig } - case <-time.After(timeout): + case <-time.After(time.Second): return nil } } From 972b6b14b70869deebed760aa5ddf7a091607538 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Mar 2022 14:19:57 -0700 Subject: [PATCH 5/5] ci: enable unparam linter And fix the last warning from it: match.go:31:20: `withMatchType` - `typ` always receives `"signal"` (unparam) func withMatchType(typ string) MatchOption { ^ Signed-off-by: Kir Kolyshkin --- .golangci.yml | 1 + conn.go | 4 ++-- match.go | 8 ++++---- match_test.go | 2 +- object.go | 4 ++-- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e2ecaa95..f2d7910d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,3 +4,4 @@ linters: enable: - gofumpt - unconvert + - unparam diff --git a/conn.go b/conn.go index 81e93663..f9cf27b7 100644 --- a/conn.go +++ b/conn.go @@ -629,7 +629,7 @@ func (conn *Conn) AddMatchSignal(options ...MatchOption) error { // AddMatchSignalContext acts like AddMatchSignal but takes a context. func (conn *Conn) AddMatchSignalContext(ctx context.Context, options ...MatchOption) error { - options = append([]MatchOption{withMatchType("signal")}, options...) + options = append([]MatchOption{withMatchTypeSignal()}, options...) return conn.busObj.CallWithContext( ctx, "org.freedesktop.DBus.AddMatch", 0, @@ -644,7 +644,7 @@ func (conn *Conn) RemoveMatchSignal(options ...MatchOption) error { // RemoveMatchSignalContext acts like RemoveMatchSignal but takes a context. func (conn *Conn) RemoveMatchSignalContext(ctx context.Context, options ...MatchOption) error { - options = append([]MatchOption{withMatchType("signal")}, options...) + options = append([]MatchOption{withMatchTypeSignal()}, options...) return conn.busObj.CallWithContext( ctx, "org.freedesktop.DBus.RemoveMatch", 0, diff --git a/match.go b/match.go index 5a607e53..ffb01344 100644 --- a/match.go +++ b/match.go @@ -26,10 +26,10 @@ func WithMatchOption(key, value string) MatchOption { return MatchOption{key, value} } -// doesn't make sense to export this option because clients can only -// subscribe to messages with signal type. -func withMatchType(typ string) MatchOption { - return WithMatchOption("type", typ) +// It does not make sense to have a public WithMatchType function +// because clients can only subscribe to messages with signal type. +func withMatchTypeSignal() MatchOption { + return WithMatchOption("type", "signal") } // WithMatchSender sets sender match option. diff --git a/match_test.go b/match_test.go index 1e56b293..bfebe14e 100644 --- a/match_test.go +++ b/match_test.go @@ -4,7 +4,7 @@ import "testing" func TestFormatMatchOptions(t *testing.T) { opts := []MatchOption{ - withMatchType("signal"), + withMatchTypeSignal(), WithMatchSender("org.bluez"), WithMatchInterface("org.freedesktop.DBus.Properties"), WithMatchMember("PropertiesChanged"), diff --git a/object.go b/object.go index 664abb7f..1a605fd0 100644 --- a/object.go +++ b/object.go @@ -46,7 +46,7 @@ func (o *Object) CallWithContext(ctx context.Context, method string, flags Flags // Deprecated: use (*Conn) AddMatchSignal instead. func (o *Object) AddMatchSignal(iface, member string, options ...MatchOption) *Call { base := []MatchOption{ - withMatchType("signal"), + withMatchTypeSignal(), WithMatchInterface(iface), WithMatchMember(member), } @@ -65,7 +65,7 @@ func (o *Object) AddMatchSignal(iface, member string, options ...MatchOption) *C // Deprecated: use (*Conn) RemoveMatchSignal instead. func (o *Object) RemoveMatchSignal(iface, member string, options ...MatchOption) *Call { base := []MatchOption{ - withMatchType("signal"), + withMatchTypeSignal(), WithMatchInterface(iface), WithMatchMember(member), }