Merge pull request #201 from keybase/joshblum/revive

Add revive linter to golangci-lint

.golangci.yml

@@ -7,5 +7,7 @@ linters-settings:
 linters:
   enable:
     - gofmt
+    - govet
     - gocritic
     - unconvert
+    - revive

go.mod

@@ -5,11 +5,11 @@ go 1.21
 require (
 	github.com/keybase/backoff v1.0.1-0.20160517061000-726b63b835ec
 	github.com/keybase/go-codec v0.0.0-20180928230036-164397562123
-	github.com/keybase/msgpackzip v0.0.0-20250106182721-6f0080476008
+	github.com/keybase/msgpackzip v0.0.0-20250106200500-93bf3a4c34cf
 	github.com/reiver/go-telnet v0.0.0-20180421082511-9ff0b2ab096e
 	github.com/stretchr/testify v1.10.0
-	golang.org/x/net v0.19.0
-	golang.org/x/sync v0.5.0
+	golang.org/x/net v0.34.0
+	golang.org/x/sync v0.10.0
 )
 
 require (

go.sum

@@ -4,8 +4,8 @@ github.com/keybase/backoff v1.0.1-0.20160517061000-726b63b835ec h1:D6qL2WCnAuxuc
 github.com/keybase/backoff v1.0.1-0.20160517061000-726b63b835ec/go.mod h1:jeBKj+20GIDry3doFsAMYH9n7Y3l7ajE3xJrKvVB23s=
 github.com/keybase/go-codec v0.0.0-20180928230036-164397562123 h1:yg56lYPqh9suJepqxOMd/liFgU/x+maRPiB30JNYykM=
 github.com/keybase/go-codec v0.0.0-20180928230036-164397562123/go.mod h1:r/eVVWCngg6TsFV/3HuS9sWhDkAzGG8mXhiuYA+Z/20=
-github.com/keybase/msgpackzip v0.0.0-20250106182721-6f0080476008 h1:eGLsSh7V10/j3WBZAW7k2kUzvz88M2qdmpMY4L7wyXk=
-github.com/keybase/msgpackzip v0.0.0-20250106182721-6f0080476008/go.mod h1:XGERKRnPD1bFQJrhQp5XHw4JtZ+u3nlgdx/xrwF13ow=
+github.com/keybase/msgpackzip v0.0.0-20250106200500-93bf3a4c34cf h1:wG5lhAbfl5Gir35gAdJywwf7tEjPW9oI0jBjzQZysxQ=
+github.com/keybase/msgpackzip v0.0.0-20250106200500-93bf3a4c34cf/go.mod h1:XGERKRnPD1bFQJrhQp5XHw4JtZ+u3nlgdx/xrwF13ow=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/reiver/go-oi v1.0.0 h1:nvECWD7LF+vOs8leNGV/ww+F2iZKf3EYjYZ527turzM=
@@ -14,10 +14,10 @@ github.com/reiver/go-telnet v0.0.0-20180421082511-9ff0b2ab096e h1:quuzZLi72kkJjl
 github.com/reiver/go-telnet v0.0.0-20180421082511-9ff0b2ab096e/go.mod h1:+5vNVvEWwEIx86DB9Ke/+a5wBI464eDRo3eF0LcfpWg=
 github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
 github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
-golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
-golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U=
-golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE=
-golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/net v0.34.0 h1:Mb7Mrk043xzHgnRM88suvJFwzVrRfHEHJEl5/71CKw0=
+golang.org/x/net v0.34.0/go.mod h1:di0qlW3YNM5oh6GqDGQr92MyTozJPmybPK4Ev/Gm31k=
+golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=

rpc/client.go

@@ -70,13 +70,13 @@ func (c *Client) call(ctx context.Context, method string,
 	if c.tagsFunc != nil {
 		tags, ok := c.tagsFunc(ctx)
 		if ok {
-			rpcTags := make(CtxRpcTags)
+			rpcTags := make(CtxRPCTags)
 			for key, tagName := range tags {
 				if v := ctx.Value(key); v != nil {
 					rpcTags[tagName] = v
 				}
 			}
-			ctx = AddRpcTagsToContext(ctx, rpcTags)
+			ctx = AddRPCTagsToContext(ctx, rpcTags)
 		}
 	}
 

rpc/connection.go

@@ -119,7 +119,7 @@ func (t *connTransport) Dial(ctx context.Context) (Transporter, error) {
 		// https://github.com/rust-lang/rust/blob/028569ab1b/src/libstd/sys_common/net.rs#L186-L190
 		// Note that we still propagate the error here, and we expect callers
 		// to retry.
-		resinit.ResInitIfDNSError(err)
+		resinit.IfDNSError(err)
 		return nil, err
 	}
 
@@ -274,7 +274,7 @@ func (ct *ConnectionTransportTLS) Dial(ctx context.Context) (
 		// https://github.com/rust-lang/rust/blob/028569ab1b/src/libstd/sys_common/net.rs#L186-L190
 		// Note that we still propagate the error here, and we expect
 		// callers to retry.
-		resinit.ResInitIfDNSError(err)
+		resinit.IfDNSError(err)
 		return nil, err
 	}
 	ct.log.Debug("baseConn: %s; Calling %s",

rpc/connection_test.go

@@ -49,20 +49,20 @@ func (ut *unitTester) OnDisconnected(context.Context, DisconnectStatus) {
 }
 
 // ShouldRetry implements the ConnectionHandler interface.
-func (ut *unitTester) ShouldRetry(name string, err error) bool {
+func (ut *unitTester) ShouldRetry(_ string, err error) bool {
 	_, isThrottle := err.(throttleError)
 	return isThrottle
 }
 
-var errCanceled = errors.New("Canceled!")
+var errCanceled = errors.New("canceled")
 
 // ShouldRetryOnConnect implements the ConnectionHandler interface.
 func (ut *unitTester) ShouldRetryOnConnect(err error) bool {
 	return err != errCanceled
 }
 
 // Dial implements the ConnectionTransport interface.
-func (ut *unitTester) Dial(ctx context.Context) (
+func (ut *unitTester) Dial(_ context.Context) (
 	Transporter, error) {
 	if ut.alwaysFail || ut.numConnectErrors == 0 {
 		return nil, ut.errToThrow
@@ -338,13 +338,13 @@ type mockedDialable struct {
 	setoptsWasCalled bool
 }
 
-func (md *mockedDialable) SetOpts(timeout time.Duration, keepAlive time.Duration) {
+func (md *mockedDialable) SetOpts(_ time.Duration, _ time.Duration) {
 	md.mutex.Lock()
 	md.setoptsWasCalled = true
 	md.mutex.Unlock()
 }
 
-func (md *mockedDialable) Dial(ctx context.Context, network string, addr string) (net.Conn, error) {
+func (md *mockedDialable) Dial(_ context.Context, _ string, _ string) (net.Conn, error) {
 	md.mutex.Lock()
 	md.dialWasCalled = true
 	md.mutex.Unlock()
@@ -357,8 +357,9 @@ func TestDialableTransport(t *testing.T) {
 	}
 	output := testLogOutput{t: t}
 	opts := ConnectionOpts{
-		WrapErrorFunc: testWrapError,
-		TagsFunc:      testLogTags,
+		WrapErrorFunc:  testWrapError,
+		TagsFunc:       testLogTags,
+		DontConnectNow: true,
 	}
 
 	uriStr := "fmprpc://localhost:8080"
@@ -400,8 +401,9 @@ func TestDialableTLSConn(t *testing.T) {
 	}
 	output := testLogOutput{t: t}
 	opts := ConnectionOpts{
-		WrapErrorFunc: testWrapError,
-		TagsFunc:      testLogTags,
+		WrapErrorFunc:  testWrapError,
+		TagsFunc:       testLogTags,
+		DontConnectNow: true,
 	}
 
 	uriStr := "fmprpc+tls://localhost:8080"

rpc/connection_test_util.go

@@ -17,20 +17,20 @@ func (testConnectionHandler) OnConnect(context.Context, *Connection, GenericClie
 	return nil
 }
 
-func (testConnectionHandler) OnConnectError(err error, reconnectThrottleDuration time.Duration) {
+func (testConnectionHandler) OnConnectError(_ error, _ time.Duration) {
 }
 
-func (testConnectionHandler) OnDoCommandError(err error, nextTime time.Duration) {
+func (testConnectionHandler) OnDoCommandError(_ error, _ time.Duration) {
 }
 
-func (testConnectionHandler) OnDisconnected(ctx context.Context, status DisconnectStatus) {
+func (testConnectionHandler) OnDisconnected(_ context.Context, _ DisconnectStatus) {
 }
 
-func (testConnectionHandler) ShouldRetry(name string, err error) bool {
+func (testConnectionHandler) ShouldRetry(_ string, _ error) bool {
 	return false
 }
 
-func (testConnectionHandler) ShouldRetryOnConnect(err error) bool {
+func (testConnectionHandler) ShouldRetryOnConnect(_ error) bool {
 	return false
 }
 
@@ -45,7 +45,7 @@ type singleTransport struct {
 var _ ConnectionTransport = singleTransport{}
 
 // Dial is an implementation of the ConnectionTransport interface.
-func (st singleTransport) Dial(ctx context.Context) (Transporter, error) {
+func (st singleTransport) Dial(_ context.Context) (Transporter, error) {
 	if !st.t.IsConnected() {
 		return nil, io.EOF
 	}
@@ -67,11 +67,11 @@ type testStatus struct {
 	Code int
 }
 
-func testWrapError(err error) interface{} {
+func testWrapError(_ error) interface{} {
 	return &testStatus{}
 }
 
-func testLogTags(ctx context.Context) (map[interface{}]string, bool) {
+func testLogTags(_ context.Context) (map[interface{}]string, bool) {
 	return nil, false
 }
 

rpc/context.go

@@ -2,36 +2,36 @@ package rpc
 
 import "golang.org/x/net/context"
 
-// CtxRpcKey is a type defining the context key for the RPC context
-type CtxRpcKey int
+// CtxRPCKey is a type defining the context key for the RPC context
+type CtxRPCKey int
 
 const (
-	// CtxRpcTagsKey defines a context key that can hold a slice of context keys
-	CtxRpcTagsKey CtxRpcKey = iota
+	// CtxRPCTagsKey defines a context key that can hold a slice of context keys
+	CtxRPCTagsKey CtxRPCKey = iota
 )
 
-type CtxRpcTags map[string]interface{}
+type CtxRPCTags map[string]interface{}
 
-// AddRpcTagsToContext adds the given log tag mappings (logTagsToAdd) to the
+// AddRPCTagsToContext adds the given log tag mappings (logTagsToAdd) to the
 // given context, creating a new one if necessary. Returns the resulting
 // context with the new log tag mappings.
-func AddRpcTagsToContext(ctx context.Context, logTagsToAdd CtxRpcTags) context.Context {
-	currTags, ok := RpcTagsFromContext(ctx)
+func AddRPCTagsToContext(ctx context.Context, logTagsToAdd CtxRPCTags) context.Context {
+	currTags, ok := TagsFromContext(ctx)
 	if !ok {
-		currTags = make(CtxRpcTags)
+		currTags = make(CtxRPCTags)
 	}
 	for key, tag := range logTagsToAdd {
 		currTags[key] = tag
 	}
 
-	return context.WithValue(ctx, CtxRpcTagsKey, currTags)
+	return context.WithValue(ctx, CtxRPCTagsKey, currTags)
 }
 
-// RpcTagsFromContext returns the tags being passed along with the given context.
-func RpcTagsFromContext(ctx context.Context) (CtxRpcTags, bool) {
-	logTags, ok := ctx.Value(CtxRpcTagsKey).(CtxRpcTags)
+// TagsFromContext returns the tags being passed along with the given context.
+func TagsFromContext(ctx context.Context) (CtxRPCTags, bool) {
+	logTags, ok := ctx.Value(CtxRPCTagsKey).(CtxRPCTags)
 	if ok {
-		ret := make(CtxRpcTags)
+		ret := make(CtxRPCTags)
 		for k, v := range logTags {
 			ret[k] = v
 		}

rpc/context_test.go

@@ -8,21 +8,21 @@ import (
 )
 
 func TestRpcTags(t *testing.T) {
-	logTags := make(CtxRpcTags)
+	logTags := make(CtxRPCTags)
 
 	logTags["hello"] = "world"
 	logTags["foo"] = "bar"
-	ctx := AddRpcTagsToContext(context.Background(), logTags)
+	ctx := AddRPCTagsToContext(context.Background(), logTags)
 
-	logTags2 := make(CtxRpcTags)
+	logTags2 := make(CtxRPCTags)
 	logTags2["hello"] = "world2"
-	ctx = AddRpcTagsToContext(ctx, logTags2)
+	ctx = AddRPCTagsToContext(ctx, logTags2)
 
-	logTags, _ = RpcTagsFromContext(ctx)
+	logTags, _ = TagsFromContext(ctx)
 	require.Equal(t, "world2", logTags["hello"])
 	require.Equal(t, "bar", logTags["foo"])
 
-	outTags, ok := RpcTagsFromContext(ctx)
+	outTags, ok := TagsFromContext(ctx)
 
 	require.Equal(t, true, ok)
 	require.Equal(t, logTags, outTags)

rpc/dispatch.go

@@ -62,7 +62,7 @@ func (d *dispatch) Call(ctx context.Context, name string, arg interface{}, res i
 		method = MethodCallCompressed
 	}
 
-	record := NewNetworkInstrumenter(d.instrumenterStorage, RPCInstrumentTag(method, name))
+	record := NewNetworkInstrumenter(d.instrumenterStorage, InstrumentTag(method, name))
 	c := d.calls.NewCall(ctx, name, arg, res, ctype, u, record)
 
 	// Have to add call before encoding otherwise we'll race the response
@@ -84,7 +84,7 @@ func (d *dispatch) Call(ctx context.Context, name string, arg interface{}, res i
 		logCall = func() { d.log.ClientCallCompressed(c.seqid, c.method, c.arg, c.ctype) }
 	}
 
-	rpcTags, _ := RpcTagsFromContext(ctx)
+	rpcTags, _ := TagsFromContext(ctx)
 	if len(rpcTags) > 0 {
 		v = append(v, rpcTags)
 	}
@@ -118,14 +118,14 @@ func (d *dispatch) Call(ctx context.Context, name string, arg interface{}, res i
 }
 
 func (d *dispatch) Notify(ctx context.Context, name string, arg interface{}, sendNotifier SendNotifier) error {
-	rpcTags, _ := RpcTagsFromContext(ctx)
+	rpcTags, _ := TagsFromContext(ctx)
 	v := []interface{}{MethodNotify, name, arg}
 	if len(rpcTags) > 0 {
 		v = append(v, rpcTags)
 	}
 
 	size, errCh := d.writer.EncodeAndWrite(ctx, v, currySendNotifier(sendNotifier, SeqNumber(-1)))
-	record := NewNetworkInstrumenter(d.instrumenterStorage, RPCInstrumentTag(MethodNotify, name))
+	record := NewNetworkInstrumenter(d.instrumenterStorage, InstrumentTag(MethodNotify, name))
 	defer func() { _ = record.RecordAndFinish(ctx, size) }()
 
 	select {
@@ -149,7 +149,7 @@ func (d *dispatch) Close() {
 func (d *dispatch) handleCancel(ctx context.Context, c *call) error {
 	d.log.ClientCancel(c.seqid, c.method, nil)
 	size, errCh := d.writer.EncodeAndWriteAsync([]interface{}{MethodCancel, c.seqid, c.method})
-	record := NewNetworkInstrumenter(d.instrumenterStorage, RPCInstrumentTag(MethodCancel, c.method))
+	record := NewNetworkInstrumenter(d.instrumenterStorage, InstrumentTag(MethodCancel, c.method))
 	defer func() { _ = record.RecordAndFinish(ctx, size) }()
 	select {
 	case err := <-errCh:

rpc/dispatch_test.go

@@ -9,7 +9,7 @@ import (
 	"golang.org/x/net/context"
 )
 
-func dispatchTestCallWithContextAndCompressionType(t *testing.T, ctx context.Context, ctype CompressionType) (dispatcher, *callContainer, chan error) {
+func dispatchTestCallWithContextAndCompressionType(ctx context.Context, t *testing.T, ctype CompressionType) (dispatcher, *callContainer, chan error) {
 	log := newTestLog(t)
 	instrumenterStorage := NewMemoryInstrumentationStorage()
 
@@ -33,12 +33,12 @@ func dispatchTestCallWithContextAndCompressionType(t *testing.T, ctx context.Con
 	return d, calls, done
 }
 
-func dispatchTestCallWithContext(t *testing.T, ctx context.Context) (dispatcher, *callContainer, chan error) {
-	return dispatchTestCallWithContextAndCompressionType(t, ctx, CompressionNone)
+func dispatchTestCallWithContext(ctx context.Context, t *testing.T) (dispatcher, *callContainer, chan error) {
+	return dispatchTestCallWithContextAndCompressionType(ctx, t, CompressionNone)
 }
 
 func dispatchTestCall(t *testing.T) (dispatcher, *callContainer, chan error) {
-	return dispatchTestCallWithContext(t, context.Background())
+	return dispatchTestCallWithContext(context.Background(), t)
 }
 
 func sendResponse(c *call, err error) {
@@ -63,7 +63,7 @@ func TestDispatchSuccessfulCall(t *testing.T) {
 
 func TestDispatchSuccessfulCallCompressed(t *testing.T) {
 	doWithAllCompressionTypes(func(ctype CompressionType) {
-		d, calls, done := dispatchTestCallWithContextAndCompressionType(t, context.Background(), ctype)
+		d, calls, done := dispatchTestCallWithContextAndCompressionType(context.Background(), t, ctype)
 
 		c := calls.RetrieveCall(0)
 		require.NotNil(t, c, "Expected c not to be nil")
@@ -79,7 +79,7 @@ func TestDispatchSuccessfulCallCompressed(t *testing.T) {
 
 func TestDispatchCanceledBeforeResult(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
-	d, calls, done := dispatchTestCallWithContext(t, ctx)
+	d, calls, done := dispatchTestCallWithContext(ctx, t)
 
 	c := calls.RetrieveCall(0)
 	require.NotNil(t, c, "Expected c not to be nil")
@@ -102,7 +102,7 @@ func TestDispatchCanceledBeforeResult(t *testing.T) {
 
 func TestDispatchCanceledAfterResult(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
-	d, calls, done := dispatchTestCallWithContext(t, ctx)
+	d, calls, done := dispatchTestCallWithContext(ctx, t)
 
 	c := calls.RetrieveCall(0)
 	require.NotNil(t, c, "Expected c not to be nil")