diff --git a/client/client_test.go b/client/client_test.go index 592987113..d8e14b0bd 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -472,7 +472,7 @@ func TestHTTPClusterClientDoDeadlineExceedContext(t *testing.T) { type fakeCancelContext struct{} -var fakeCancelContextError = errors.New("fake context canceled") +var errFakeCancelContext = errors.New("fake context canceled") func (f fakeCancelContext) Deadline() (time.Time, bool) { return time.Time{}, false } func (f fakeCancelContext) Done() <-chan struct{} { @@ -480,7 +480,7 @@ func (f fakeCancelContext) Done() <-chan struct{} { d <- struct{}{} return d } -func (f fakeCancelContext) Err() error { return fakeCancelContextError } +func (f fakeCancelContext) Err() error { return errFakeCancelContext } func (f fakeCancelContext) Value(key interface{}) interface{} { return 1 } func withTimeout(parent context.Context, timeout time.Duration) ( @@ -512,8 +512,8 @@ func TestHTTPClusterClientDoCanceledContext(t *testing.T) { select { case err := <-errc: - if err != fakeCancelContextError { - t.Errorf("err = %+v, want %+v", err, fakeCancelContextError) + if err != errFakeCancelContext { + t.Errorf("err = %+v, want %+v", err, errFakeCancelContext) } case <-time.After(time.Second): t.Fatalf("unexpected timeout when waiting for request to fake context canceled") diff --git a/client/integration/client_test.go b/client/integration/client_test.go index 5d4d0a05e..19c9ede76 100644 --- a/client/integration/client_test.go +++ b/client/integration/client_test.go @@ -46,7 +46,7 @@ func TestV2NoRetryEOF(t *testing.T) { conn.Close() } }() - eofURL := integration.UrlScheme + "://" + lEOF.Addr().String() + eofURL := integration.URLScheme + "://" + lEOF.Addr().String() cli := integration.MustNewHTTPClient(t, []string{eofURL, eofURL}, nil) kapi := client.NewKeysAPI(cli) for i, f := range noRetryList(kapi) { @@ -64,16 +64,16 @@ func TestV2NoRetryEOF(t *testing.T) { // TestV2NoRetryNoLeader tests destructive api calls won't retry if given an error code. func TestV2NoRetryNoLeader(t *testing.T) { defer testutil.AfterTest(t) - lHttp := integration.NewListenerWithAddr(t, fmt.Sprintf("127.0.0.1:%05d", os.Getpid())) + lHTTP := integration.NewListenerWithAddr(t, fmt.Sprintf("127.0.0.1:%05d", os.Getpid())) eh := &errHandler{errCode: http.StatusServiceUnavailable} srv := httptest.NewUnstartedServer(eh) - defer lHttp.Close() + defer lHTTP.Close() defer srv.Close() - srv.Listener = lHttp + srv.Listener = lHTTP go srv.Start() - lHttpURL := integration.UrlScheme + "://" + lHttp.Addr().String() + lHTTPURL := integration.URLScheme + "://" + lHTTP.Addr().String() - cli := integration.MustNewHTTPClient(t, []string{lHttpURL, lHttpURL}, nil) + cli := integration.MustNewHTTPClient(t, []string{lHTTPURL, lHTTPURL}, nil) kapi := client.NewKeysAPI(cli) // test error code for i, f := range noRetryList(kapi) { @@ -94,7 +94,7 @@ func TestV2RetryRefuse(t *testing.T) { cl.Launch(t) defer cl.Terminate(t) // test connection refused; expect no error failover - cli := integration.MustNewHTTPClient(t, []string{integration.UrlScheme + "://refuseconn:123", cl.URL(0)}, nil) + cli := integration.MustNewHTTPClient(t, []string{integration.URLScheme + "://refuseconn:123", cl.URL(0)}, nil) kapi := client.NewKeysAPI(cli) if _, err := kapi.Set(context.Background(), "/delkey", "def", nil); err != nil { t.Fatal(err) diff --git a/clientv3/balancer/resolver/endpoint/endpoint.go b/clientv3/balancer/resolver/endpoint/endpoint.go index 104ec773c..2f33594ae 100644 --- a/clientv3/balancer/resolver/endpoint/endpoint.go +++ b/clientv3/balancer/resolver/endpoint/endpoint.go @@ -157,7 +157,7 @@ func (b *builder) close(id string) { b.mu.Unlock() } -func (r *builder) Scheme() string { +func (b *builder) Scheme() string { return scheme } diff --git a/clientv3/integration/metrics_test.go b/clientv3/integration/metrics_test.go index cd45500ba..b1e795b76 100644 --- a/clientv3/integration/metrics_test.go +++ b/clientv3/integration/metrics_test.go @@ -39,7 +39,7 @@ func TestV3ClientMetrics(t *testing.T) { defer testutil.AfterTest(t) var ( - addr string = "localhost:27989" + addr = "localhost:27989" ln net.Listener err error ) diff --git a/clientv3/lease.go b/clientv3/lease.go index 3d2e897ee..00e0b918e 100644 --- a/clientv3/lease.go +++ b/clientv3/lease.go @@ -295,7 +295,7 @@ func (l *lessor) KeepAlive(ctx context.Context, id LeaseID) (<-chan *LeaseKeepAl } l.mu.Unlock() - go l.keepAliveCtxCloser(id, ctx, ka.donec) + go l.keepAliveCtxCloser(ctx, id, ka.donec) l.firstKeepAliveOnce.Do(func() { go l.recvKeepAliveLoop() go l.deadlineLoop() @@ -327,7 +327,7 @@ func (l *lessor) Close() error { return nil } -func (l *lessor) keepAliveCtxCloser(id LeaseID, ctx context.Context, donec <-chan struct{}) { +func (l *lessor) keepAliveCtxCloser(ctx context.Context, id LeaseID, donec <-chan struct{}) { select { case <-donec: return diff --git a/clientv3/options.go b/clientv3/options.go index b82b7554d..4660acea0 100644 --- a/clientv3/options.go +++ b/clientv3/options.go @@ -47,7 +47,7 @@ var ( // client-side streaming retry limit, only applied to requests where server responds with // a error code clearly indicating it was unable to process the request such as codes.Unavailable. // If set to 0, retry is disabled. - defaultStreamMaxRetries uint = ^uint(0) // max uint + defaultStreamMaxRetries = uint(^uint(0)) // max uint // client-side retry backoff wait between requests. defaultBackoffWaitBetween = 25 * time.Millisecond diff --git a/clientv3/retry_interceptor.go b/clientv3/retry_interceptor.go index 9fcec4291..af38998af 100644 --- a/clientv3/retry_interceptor.go +++ b/clientv3/retry_interceptor.go @@ -46,7 +46,7 @@ func (c *Client) unaryClientInterceptor(logger *zap.Logger, optFuncs ...retryOpt } var lastErr error for attempt := uint(0); attempt < callOpts.max; attempt++ { - if err := waitRetryBackoff(attempt, ctx, callOpts); err != nil { + if err := waitRetryBackoff(ctx, attempt, callOpts); err != nil { return err } lastErr = invoker(ctx, method, req, reply, cc, grpcOpts...) @@ -173,7 +173,7 @@ func (s *serverStreamingRetryingStream) RecvMsg(m interface{}) error { } // We start off from attempt 1, because zeroth was already made on normal SendMsg(). for attempt := uint(1); attempt < s.callOpts.max; attempt++ { - if err := waitRetryBackoff(attempt, s.ctx, s.callOpts); err != nil { + if err := waitRetryBackoff(s.ctx, attempt, s.callOpts); err != nil { return err } newStream, err := s.reestablishStreamAndResendBuffer(s.ctx) @@ -243,8 +243,8 @@ func (s *serverStreamingRetryingStream) reestablishStreamAndResendBuffer(callCtx return newStream, nil } -func waitRetryBackoff(attempt uint, ctx context.Context, callOpts *options) error { - var waitTime time.Duration = 0 +func waitRetryBackoff(ctx context.Context, attempt uint, callOpts *options) error { + waitTime := time.Duration(0) if attempt > 0 { waitTime = callOpts.backoffFunc(attempt) } diff --git a/contrib/raftexample/raft.go b/contrib/raftexample/raft.go index ff08d556a..a57305746 100644 --- a/contrib/raftexample/raft.go +++ b/contrib/raftexample/raft.go @@ -393,7 +393,7 @@ func (rc *raftNode) serveChannels() { // send proposals over raft go func() { - var confChangeCount uint64 = 0 + confChangeCount := uint64(0) for rc.proposeC != nil && rc.confChangeC != nil { select { @@ -409,7 +409,7 @@ func (rc *raftNode) serveChannels() { if !ok { rc.confChangeC = nil } else { - confChangeCount += 1 + confChangeCount++ cc.ID = confChangeCount rc.node.ProposeConfChange(context.TODO(), cc) } diff --git a/etcdctl/ctlv3/command/printer_simple.go b/etcdctl/ctlv3/command/printer_simple.go index cd9129ff3..5b749cc59 100644 --- a/etcdctl/ctlv3/command/printer_simple.go +++ b/etcdctl/ctlv3/command/printer_simple.go @@ -86,11 +86,11 @@ func (s *simplePrinter) Grant(resp v3.LeaseGrantResponse) { fmt.Printf("lease %016x granted with TTL(%ds)\n", resp.ID, resp.TTL) } -func (p *simplePrinter) Revoke(id v3.LeaseID, r v3.LeaseRevokeResponse) { +func (s *simplePrinter) Revoke(id v3.LeaseID, r v3.LeaseRevokeResponse) { fmt.Printf("lease %016x revoked\n", id) } -func (p *simplePrinter) KeepAlive(resp v3.LeaseKeepAliveResponse) { +func (s *simplePrinter) KeepAlive(resp v3.LeaseKeepAliveResponse) { fmt.Printf("lease %016x keepalived with TTL(%d)\n", resp.ID, resp.TTL) } diff --git a/etcdserver/api/v2auth/auth.go b/etcdserver/api/v2auth/auth.go index 70a74e1b1..c828323c4 100644 --- a/etcdserver/api/v2auth/auth.go +++ b/etcdserver/api/v2auth/auth.go @@ -160,12 +160,12 @@ func NewStore(lg *zap.Logger, server doer, timeout time.Duration) Store { // passwordStore implements PasswordStore using bcrypt to hash user passwords type passwordStore struct{} -func (_ passwordStore) CheckPassword(user User, password string) bool { +func (passwordStore) CheckPassword(user User, password string) bool { err := bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(password)) return err == nil } -func (_ passwordStore) HashPassword(password string) (string, error) { +func (passwordStore) HashPassword(password string) (string, error) { hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) return string(hash), err } diff --git a/etcdserver/api/v2auth/auth_test.go b/etcdserver/api/v2auth/auth_test.go index 2171bda8a..43bce1a9c 100644 --- a/etcdserver/api/v2auth/auth_test.go +++ b/etcdserver/api/v2auth/auth_test.go @@ -30,7 +30,7 @@ import ( type fakeDoer struct{} -func (_ fakeDoer) Do(context.Context, etcdserverpb.Request) (etcdserver.Response, error) { +func (fakeDoer) Do(context.Context, etcdserverpb.Request) (etcdserver.Response, error) { return etcdserver.Response{}, nil } @@ -372,11 +372,11 @@ func TestEnsure(t *testing.T) { type fastPasswordStore struct { } -func (_ fastPasswordStore) CheckPassword(user User, password string) bool { +func (fastPasswordStore) CheckPassword(user User, password string) bool { return user.Password == password } -func (_ fastPasswordStore) HashPassword(password string) (string, error) { return password, nil } +func (fastPasswordStore) HashPassword(password string) (string, error) { return password, nil } func TestCreateAndUpdateUser(t *testing.T) { olduser := `{"user": "cat", "roles" : ["animal"]}` diff --git a/etcdserver/api/v2store/store.go b/etcdserver/api/v2store/store.go index e6d0f32d0..65cc8559c 100644 --- a/etcdserver/api/v2store/store.go +++ b/etcdserver/api/v2store/store.go @@ -216,9 +216,8 @@ func (s *store) Set(nodePath string, dir bool, value string, expireOpts TTLOptio if getErr != nil { err = getErr return nil, err - } else { - value = n.Value } + value = n.Value } // Set new value diff --git a/integration/cluster.go b/integration/cluster.go index e3924e64d..944f94806 100644 --- a/integration/cluster.go +++ b/integration/cluster.go @@ -68,8 +68,8 @@ const ( clusterName = "etcd" basePort = 21000 - UrlScheme = "unix" - UrlSchemeTLS = "unixs" + URLScheme = "unix" + URLSchemeTLS = "unixs" ) var ( @@ -77,7 +77,7 @@ var ( // integration test uses unique ports, counting up, to listen for each // member, ensuring restarted members can listen on the same port again. - localListenCount int64 = 0 + localListenCount = int64(0) testTLSInfo = transport.TLSInfo{ KeyFile: "./fixtures/server.key.insecure", @@ -157,9 +157,9 @@ type cluster struct { func schemeFromTLSInfo(tls *transport.TLSInfo) string { if tls == nil { - return UrlScheme + return URLScheme } - return UrlSchemeTLS + return URLSchemeTLS } func (c *cluster) fillClusterForMembers() error { diff --git a/mvcc/backend/config_default.go b/mvcc/backend/config_default.go index edfed0025..555ea8fa3 100644 --- a/mvcc/backend/config_default.go +++ b/mvcc/backend/config_default.go @@ -18,6 +18,6 @@ package backend import bolt "github.com/coreos/bbolt" -var boltOpenOptions *bolt.Options = nil +var boltOpenOptions *bolt.Options func (bcfg *BackendConfig) mmapSize() int { return int(bcfg.MmapSize) } diff --git a/mvcc/key_index.go b/mvcc/key_index.go index 2b0844e3c..cf77cb438 100644 --- a/mvcc/key_index.go +++ b/mvcc/key_index.go @@ -324,22 +324,22 @@ func (ki *keyIndex) findGeneration(rev int64) *generation { return nil } -func (a *keyIndex) Less(b btree.Item) bool { - return bytes.Compare(a.key, b.(*keyIndex).key) == -1 +func (ki *keyIndex) Less(b btree.Item) bool { + return bytes.Compare(ki.key, b.(*keyIndex).key) == -1 } -func (a *keyIndex) equal(b *keyIndex) bool { - if !bytes.Equal(a.key, b.key) { +func (ki *keyIndex) equal(b *keyIndex) bool { + if !bytes.Equal(ki.key, b.key) { return false } - if a.modified != b.modified { + if ki.modified != b.modified { return false } - if len(a.generations) != len(b.generations) { + if len(ki.generations) != len(b.generations) { return false } - for i := range a.generations { - ag, bg := a.generations[i], b.generations[i] + for i := range ki.generations { + ag, bg := ki.generations[i], b.generations[i] if !ag.equal(bg) { return false } @@ -384,16 +384,16 @@ func (g *generation) String() string { return fmt.Sprintf("g: created[%d] ver[%d], revs %#v\n", g.created, g.ver, g.revs) } -func (a generation) equal(b generation) bool { - if a.ver != b.ver { +func (g generation) equal(b generation) bool { + if g.ver != b.ver { return false } - if len(a.revs) != len(b.revs) { + if len(g.revs) != len(b.revs) { return false } - for i := range a.revs { - ar, br := a.revs[i], b.revs[i] + for i := range g.revs { + ar, br := g.revs[i], b.revs[i] if ar != br { return false } diff --git a/pkg/adt/interval_tree.go b/pkg/adt/interval_tree.go index ec302e4a7..342eab75b 100644 --- a/pkg/adt/interval_tree.go +++ b/pkg/adt/interval_tree.go @@ -81,12 +81,12 @@ func (x *intervalNode) color() rbcolor { return x.c } -func (n *intervalNode) height() int { - if n == nil { +func (x *intervalNode) height() int { + if x == nil { return 0 } - ld := n.left.height() - rd := n.right.height() + ld := x.left.height() + rd := x.right.height() if ld < rd { return rd + 1 } diff --git a/pkg/cpuutil/endian.go b/pkg/cpuutil/endian.go index 6ab898d4b..06c06cd4a 100644 --- a/pkg/cpuutil/endian.go +++ b/pkg/cpuutil/endian.go @@ -27,7 +27,7 @@ var byteOrder binary.ByteOrder func ByteOrder() binary.ByteOrder { return byteOrder } func init() { - var i int = 0x1 + i := int(0x1) if v := (*[intWidth]byte)(unsafe.Pointer(&i)); v[0] == 0 { byteOrder = binary.BigEndian } else { diff --git a/proxy/grpcproxy/util.go b/proxy/grpcproxy/util.go index 45a51d8c5..a6d0b40dc 100644 --- a/proxy/grpcproxy/util.go +++ b/proxy/grpcproxy/util.go @@ -34,7 +34,7 @@ func getAuthTokenFromClient(ctx context.Context) string { return "" } -func withClientAuthToken(ctx context.Context, ctxWithToken context.Context) context.Context { +func withClientAuthToken(ctx, ctxWithToken context.Context) context.Context { token := getAuthTokenFromClient(ctxWithToken) if token != "" { ctx = context.WithValue(ctx, rpctypes.TokenFieldNameGRPC, token) diff --git a/test b/test index 61f8fda65..15c11c952 100755 --- a/test +++ b/test @@ -511,6 +511,18 @@ function staticcheck_pass { fi } +function revive_pass { + if which revive >/dev/null; then + reviveResult=$(revive -config ./tests/revive.toml -exclude "vendor/..." ./... 2>&1 || true) + if [ -n "${reviveResult}" ]; then + echo -e "revive checking failed:\\n${reviveResult}" + exit 255 + fi + else + echo "Skipping revive..." + fi +} + function unconvert_pass { if which unconvert >/dev/null; then unconvertResult=$(unconvert -v "${STATIC_ANALYSIS_PATHS[@]}" 2>&1 || true) @@ -615,6 +627,7 @@ function fmt_pass { unused \ unparam \ staticcheck \ + revive \ unconvert \ ineffassign \ nakedret \ diff --git a/tests/Dockerfile b/tests/Dockerfile index 82651e6d3..666e9760f 100644 --- a/tests/Dockerfile +++ b/tests/Dockerfile @@ -46,6 +46,7 @@ ADD ./scripts/install-marker.sh /tmp/install-marker.sh RUN go get -v -u -tags spell github.com/chzchzchz/goword \ && go get -v -u github.com/coreos/license-bill-of-materials \ + && go get -v -u github.com/mgechev/revive \ && go get -v -u github.com/mdempsky/unconvert \ && go get -v -u mvdan.cc/unparam \ && go get -v -u honnef.co/go/tools/cmd/gosimple \ diff --git a/tests/e2e/ctl_v3_lease_test.go b/tests/e2e/ctl_v3_lease_test.go index 6d0dad855..608b8ca2a 100644 --- a/tests/e2e/ctl_v3_lease_test.go +++ b/tests/e2e/ctl_v3_lease_test.go @@ -148,10 +148,7 @@ func leaseTestGrantLeasesList(cx ctlCtx) error { if err != nil { return fmt.Errorf("lease id not in returned list (%v)", err) } - if err = proc.Close(); err != nil { - return err - } - return nil + return proc.Close() } func leaseTestTimeToLiveExpired(cx ctlCtx) { diff --git a/tests/revive.toml b/tests/revive.toml new file mode 100644 index 000000000..53a58b51a --- /dev/null +++ b/tests/revive.toml @@ -0,0 +1,38 @@ +ignoreGeneratedHeader = false +severity = "warning" +confidence = 0.8 +errorCode = 0 +warningCode = 0 + +[rule.blank-imports] +[rule.context-as-argument] +[rule.dot-imports] +[rule.error-return] +[rule.error-naming] +[rule.if-return] +[rule.increment-decrement] +[rule.var-declaration] +[rule.package-comments] +[rule.range] +[rule.receiver-naming] +[rule.time-naming] +[rule.indent-error-flow] +[rule.errorf] + + +# TODO: enable following + +# grpcproxy context.WithValue(ctx, rpctypes.TokenFieldNameGRPC, token) +# [rule.context-keys-type] + +# punctuation in error value +# [rule.error-strings] + +# underscore variables +# [rule.var-naming] + +# godoc +# [rule.exported] + +# return unexported type +# [rule.unexported-return] diff --git a/wal/repair_test.go b/wal/repair_test.go index 64effeb15..c4b3593dc 100644 --- a/wal/repair_test.go +++ b/wal/repair_test.go @@ -153,10 +153,7 @@ func TestRepairWriteTearLast(t *testing.T) { if terr := f.Truncate(1024); terr != nil { return terr } - if terr := f.Truncate(offset); terr != nil { - return terr - } - return nil + return f.Truncate(offset) } testRepair(t, makeEnts(50), corruptf, 40) } diff --git a/wal/wal_test.go b/wal/wal_test.go index b2c84c6ea..18b4c0cbb 100644 --- a/wal/wal_test.go +++ b/wal/wal_test.go @@ -273,7 +273,7 @@ func TestSaveWithCut(t *testing.T) { const EntrySize int = 500 SegmentSizeBytes = 2 * 1024 defer func() { SegmentSizeBytes = restoreLater }() - var index uint64 = 0 + index := uint64(0) for totalSize := 0; totalSize < int(SegmentSizeBytes); totalSize += EntrySize { ents := []raftpb.Entry{{Index: index, Term: 1, Data: bigData}} if err = w.Save(state, ents); err != nil {