From 72fbe0576db7a83c7026dd561a402d3084762e46 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Tue, 3 Oct 2017 02:14:02 -0700 Subject: [PATCH 1/3] test: run ineffassign in fmt pass Signed-off-by: Gyu-Ho Lee --- test | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test b/test index a0470057c..01a8ba7f3 100755 --- a/test +++ b/test @@ -384,6 +384,17 @@ function fmt_pass { echo "Skipping staticcheck..." fi + if which ineffassign >/dev/null; then + echo "Checking ineffassign..." + ineffassignResult=$(ineffassign "${STATIC_ANALYSIS_PATHS[@]}" 2>&1 || true) + if [ -n "${ineffassignResult}" ]; then + echo -e "ineffassign checking failed:\n${ineffassignResult}" + exit 255 + fi + else + echo "Skipping ineffassign..." + fi + echo "Checking for license header..." licRes="" files=$(find . -type f -iname '*.go' ! -path './cmd/*' ! -path './gopath.proto/*') From 0199bdc2665ca96b0cdbab70ed956b24a7f85ffd Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Tue, 3 Oct 2017 02:14:22 -0700 Subject: [PATCH 2/3] *: fix 'ineffassign' issues Signed-off-by: Gyu-Ho Lee --- client/keys.generated.go | 5 ++--- client/main_test.go | 2 +- clientv3/client.go | 2 +- clientv3/integration/watch_test.go | 6 +++--- clientv3/main_test.go | 2 +- clientv3/naming/grpc_test.go | 5 ++++- clientv3/ordering/kv_test.go | 9 +++++++++ clientv3/ordering/util_test.go | 6 ++++++ compactor/revision_test.go | 1 - e2e/ctl_v3_role_test.go | 2 +- e2e/etcd_spawn_cov.go | 2 +- etcdctl/ctlv3/help.go | 2 +- etcdserver/api/v2http/client.go | 2 +- etcdserver/api/v2v3/store.go | 3 +-- proxy/tcpproxy/userspace.go | 2 +- raft/progress.go | 3 ++- store/store_bench_test.go | 1 - store/store_ttl_test.go | 4 ++-- tools/functional-tester/etcd-runner/command/help.go | 2 +- 19 files changed, 38 insertions(+), 23 deletions(-) diff --git a/client/keys.generated.go b/client/keys.generated.go index 6bed584cb..d4ed117ef 100644 --- a/client/keys.generated.go +++ b/client/keys.generated.go @@ -8,10 +8,11 @@ package client import ( "errors" "fmt" - codec1978 "github.com/ugorji/go/codec" "reflect" "runtime" time "time" + + codec1978 "github.com/ugorji/go/codec" ) const ( @@ -77,7 +78,6 @@ func (x *Response) CodecEncodeSelf(e *codec1978.Encoder) { } } r.EncodeMapStart(yynn2) - yynn2 = 0 } if yyr2 || yy2arr2 { z.EncSendContainerState(codecSelfer_containerArrayElem1819) @@ -352,7 +352,6 @@ func (x *Node) CodecEncodeSelf(e *codec1978.Encoder) { } } r.EncodeMapStart(yynn2) - yynn2 = 0 } if yyr2 || yy2arr2 { z.EncSendContainerState(codecSelfer_containerArrayElem1819) diff --git a/client/main_test.go b/client/main_test.go index 747740601..1f2f3f306 100644 --- a/client/main_test.go +++ b/client/main_test.go @@ -49,7 +49,7 @@ func TestMain(m *testing.M) { os.Args = append(os.Args, "-test.run=Test") } - v := 0 + var v int if useCluster { tr, trerr := transport.NewTransport(transport.TLSInfo{}, time.Second) if trerr != nil { diff --git a/clientv3/client.go b/clientv3/client.go index 06e2d7713..317990f7e 100644 --- a/clientv3/client.go +++ b/clientv3/client.go @@ -453,7 +453,7 @@ func (c *Client) checkVersion() (err error) { vs := strings.Split(resp.Version, ".") maj, min := 0, 0 if len(vs) >= 2 { - maj, rerr = strconv.Atoi(vs[0]) + maj, _ = strconv.Atoi(vs[0]) min, rerr = strconv.Atoi(vs[1]) } if maj < 3 || (maj == 3 && min < 2) { diff --git a/clientv3/integration/watch_test.go b/clientv3/integration/watch_test.go index 30d166eff..dfd2e5083 100644 --- a/clientv3/integration/watch_test.go +++ b/clientv3/integration/watch_test.go @@ -311,7 +311,7 @@ func testWatchCancelRunning(t *testing.T, wctx *watchctx) { select { case <-time.After(time.Second): t.Fatalf("took too long to cancel") - case v, ok := <-wctx.ch: + case _, ok := <-wctx.ch: if !ok { // closed before getting put; OK break @@ -320,8 +320,8 @@ func testWatchCancelRunning(t *testing.T, wctx *watchctx) { select { case <-time.After(time.Second): t.Fatalf("took too long to close") - case v, ok = <-wctx.ch: - if ok { + case v, ok2 := <-wctx.ch: + if ok2 { t.Fatalf("expected watcher channel to close, got %v", v) } } diff --git a/clientv3/main_test.go b/clientv3/main_test.go index 6df3f6334..89b30860b 100644 --- a/clientv3/main_test.go +++ b/clientv3/main_test.go @@ -48,7 +48,7 @@ func TestMain(m *testing.M) { os.Args = append(os.Args, "-test.run=Test") } - v := 0 + var v int if useCluster { cfg := integration.ClusterConfig{Size: 3} clus := integration.NewClusterV3(nil, &cfg) diff --git a/clientv3/naming/grpc_test.go b/clientv3/naming/grpc_test.go index ba53666a9..d0157f132 100644 --- a/clientv3/naming/grpc_test.go +++ b/clientv3/naming/grpc_test.go @@ -66,10 +66,13 @@ func TestGRPCResolver(t *testing.T) { delOp := naming.Update{Op: naming.Delete, Addr: "127.0.0.1"} err = r.Update(context.TODO(), "foo", delOp) + if err != nil { + t.Fatalf("failed to udpate %v", err) + } us, err = w.Next() if err != nil { - t.Fatal("failed to get udpate", err) + t.Fatalf("failed to get udpate %v", err) } wu = &naming.Update{ diff --git a/clientv3/ordering/kv_test.go b/clientv3/ordering/kv_test.go index 5c68cc787..5cb9ddc67 100644 --- a/clientv3/ordering/kv_test.go +++ b/clientv3/ordering/kv_test.go @@ -43,6 +43,9 @@ func TestDetectKvOrderViolation(t *testing.T) { }, } cli, err := clientv3.New(cfg) + if err != nil { + t.Fatal(err) + } ctx := context.TODO() if _, err = clus.Client(0).Put(ctx, "foo", "bar"); err != nil { @@ -101,6 +104,9 @@ func TestDetectTxnOrderViolation(t *testing.T) { }, } cli, err := clientv3.New(cfg) + if err != nil { + t.Fatal(err) + } ctx := context.TODO() if _, err = clus.Client(0).Put(ctx, "foo", "bar"); err != nil { @@ -143,6 +149,9 @@ func TestDetectTxnOrderViolation(t *testing.T) { cli.SetEndpoints(clus.Members[2].GRPCAddr()) _, err = orderingKv.Get(ctx, "foo", clientv3.WithSerializable()) + if err != nil { + t.Fatal(err) + } orderingTxn = orderingKv.Txn(ctx) _, err = orderingTxn.If( clientv3.Compare(clientv3.Value("b"), ">", "a"), diff --git a/clientv3/ordering/util_test.go b/clientv3/ordering/util_test.go index 6f86ca478..75d2cd924 100644 --- a/clientv3/ordering/util_test.go +++ b/clientv3/ordering/util_test.go @@ -35,6 +35,9 @@ func TestEndpointSwitchResolvesViolation(t *testing.T) { } cfg := clientv3.Config{Endpoints: []string{clus.Members[0].GRPCAddr()}} cli, err := clientv3.New(cfg) + if err != nil { + t.Fatal(err) + } ctx := context.TODO() @@ -91,6 +94,9 @@ func TestUnresolvableOrderViolation(t *testing.T) { }, } cli, err := clientv3.New(cfg) + if err != nil { + t.Fatal(err) + } eps := cli.Endpoints() ctx := context.TODO() diff --git a/compactor/revision_test.go b/compactor/revision_test.go index 766afaee0..3c52f94c9 100644 --- a/compactor/revision_test.go +++ b/compactor/revision_test.go @@ -56,7 +56,6 @@ func TestRevision(t *testing.T) { // skip the same revision rg.SetRev(99) // will be 100 - expectedRevision = int64(90) rg.Wait(1) // nothing happens diff --git a/e2e/ctl_v3_role_test.go b/e2e/ctl_v3_role_test.go index 340446a10..68b77d0c1 100644 --- a/e2e/ctl_v3_role_test.go +++ b/e2e/ctl_v3_role_test.go @@ -124,7 +124,7 @@ func ctlV3RoleRevokePermission(cx ctlCtx, rolename string, key, rangeEnd string, cmdArgs := append(cx.PrefixArgs(), "role", "revoke-permission") cmdArgs = append(cmdArgs, rolename) cmdArgs = append(cmdArgs, key) - expStr := "" + var expStr string if len(rangeEnd) != 0 { cmdArgs = append(cmdArgs, rangeEnd) expStr = fmt.Sprintf("Permission of range [%s, %s) is revoked from role %s", key, rangeEnd, rolename) diff --git a/e2e/etcd_spawn_cov.go b/e2e/etcd_spawn_cov.go index e3098caa2..cf67d3776 100644 --- a/e2e/etcd_spawn_cov.go +++ b/e2e/etcd_spawn_cov.go @@ -69,7 +69,7 @@ func spawnEtcd(args []string) (*expect.ExpectProcess, error) { return nil, err } - env := []string{} + var env []string if args[1] == "grpc-proxy" { // avoid test flag conflicts in coverage enabled etcd by putting flags in ETCDCOV_ARGS env = append(os.Environ(), "ETCDCOV_ARGS="+strings.Join(args, "\xe7\xcd")) diff --git a/etcdctl/ctlv3/help.go b/etcdctl/ctlv3/help.go index fc2c62ddb..1315a03cb 100644 --- a/etcdctl/ctlv3/help.go +++ b/etcdctl/ctlv3/help.go @@ -110,7 +110,7 @@ func etcdFlagUsages(flagSet *pflag.FlagSet) string { if len(flag.Deprecated) > 0 { return } - format := "" + var format string if len(flag.Shorthand) > 0 { format = " -%s, --%s" } else { diff --git a/etcdserver/api/v2http/client.go b/etcdserver/api/v2http/client.go index d23375297..6aaf3db36 100644 --- a/etcdserver/api/v2http/client.go +++ b/etcdserver/api/v2http/client.go @@ -317,7 +317,7 @@ func (h *statsHandler) serveLeader(w http.ResponseWriter, r *http.Request) { // a server Request, performing validation of supplied fields as appropriate. // If any validation fails, an empty Request and non-nil error is returned. func parseKeyRequest(r *http.Request, clock clockwork.Clock) (etcdserverpb.Request, bool, error) { - noValueOnSuccess := false + var noValueOnSuccess bool emptyReq := etcdserverpb.Request{} err := r.ParseForm() diff --git a/etcdserver/api/v2v3/store.go b/etcdserver/api/v2v3/store.go index 22227f8aa..444f93f3a 100644 --- a/etcdserver/api/v2v3/store.go +++ b/etcdserver/api/v2v3/store.go @@ -382,7 +382,6 @@ func (s *v2v3Store) Delete(nodePath string, dir, recursive bool) (*store.Event, if !dir && !recursive { return s.deleteNode(nodePath) } - dir = true if !recursive { return s.deleteEmptyDir(nodePath) } @@ -516,7 +515,7 @@ func compareFail(nodePath, prevValue string, prevIndex uint64, resp *clientv3.Tx kv := kvs[0] indexMatch := (prevIndex == 0 || kv.ModRevision == int64(prevIndex)) valueMatch := (prevValue == "" || string(kv.Value) == prevValue) - cause := "" + var cause string switch { case indexMatch && !valueMatch: cause = fmt.Sprintf("[%v != %v]", prevValue, string(kv.Value)) diff --git a/proxy/tcpproxy/userspace.go b/proxy/tcpproxy/userspace.go index 807e76a3c..6dc1d1d6d 100644 --- a/proxy/tcpproxy/userspace.go +++ b/proxy/tcpproxy/userspace.go @@ -112,7 +112,7 @@ func (tp *TCPProxy) pick() *remote { case r.srv.Priority < bestPr: bestPr = r.srv.Priority w = 0 - weighted, unweighted = nil, nil + weighted = nil unweighted = []*remote{r} fallthrough case r.srv.Priority == bestPr: diff --git a/raft/progress.go b/raft/progress.go index 77c7b52ef..f321e2175 100644 --- a/raft/progress.go +++ b/raft/progress.go @@ -243,7 +243,8 @@ func (in *inflights) freeTo(to uint64) { return } - i, idx := 0, in.start + idx := in.start + var i int for i = 0; i < in.count; i++ { if to < in.buffer[idx] { // found the first large inflight break diff --git a/store/store_bench_test.go b/store/store_bench_test.go index e96dd4000..f84309cf1 100644 --- a/store/store_bench_test.go +++ b/store/store_bench_test.go @@ -194,7 +194,6 @@ func benchStoreSet(b *testing.B, valueSize int, process func(interface{}) ([]byt } } - kvs = nil b.StopTimer() memStats := new(runtime.MemStats) runtime.GC() diff --git a/store/store_ttl_test.go b/store/store_ttl_test.go index 62c6a95ee..89d76e71c 100644 --- a/store/store_ttl_test.go +++ b/store/store_ttl_test.go @@ -216,7 +216,7 @@ func TestStoreWatchExpireEmptyRefresh(t *testing.T) { fc := newFakeClock() s.clock = fc - var eidx uint64 = 1 + var eidx uint64 s.Create("/foo", false, "bar", false, TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond), Refresh: true}) // Should be no-op fc.Advance(200 * time.Millisecond) @@ -241,7 +241,7 @@ func TestStoreWatchNoRefresh(t *testing.T) { fc := newFakeClock() s.clock = fc - var eidx uint64 = 1 + var eidx uint64 s.Create("/foo", false, "bar", false, TTLOptionSet{ExpireTime: fc.Now().Add(500 * time.Millisecond), Refresh: true}) // Should be no-op fc.Advance(200 * time.Millisecond) diff --git a/tools/functional-tester/etcd-runner/command/help.go b/tools/functional-tester/etcd-runner/command/help.go index e7d7a4e89..448140a3b 100644 --- a/tools/functional-tester/etcd-runner/command/help.go +++ b/tools/functional-tester/etcd-runner/command/help.go @@ -110,7 +110,7 @@ func etcdFlagUsages(flagSet *pflag.FlagSet) string { if len(flag.Deprecated) > 0 { return } - format := "" + var format string if len(flag.Shorthand) > 0 { format = " -%s, --%s" } else { From 207c90c5e787b271a36991ad484ae4f45db03118 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Tue, 3 Oct 2017 02:15:06 -0700 Subject: [PATCH 3/3] travis: install 'ineffassign' Signed-off-by: Gyu-Ho Lee --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 378bb46e9..d600c942e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -56,6 +56,7 @@ before_install: - go get -v -u honnef.co/go/tools/cmd/gosimple - go get -v -u honnef.co/go/tools/cmd/unused - go get -v -u honnef.co/go/tools/cmd/staticcheck + - go get -v -u github.com/gordonklaus/ineffassign - ./scripts/install-marker.sh amd64 - export GOROOT=$(go env GOROOT)