*: fix staticcheck lint

Changed TraceKey/StartTimeKey/TokenFieldNameGRPCKey to struct{} to
follow the correct usage of context. Similar patch to etcd-io#8901.

Signed-off-by: Wei Fu <fuweid89@gmail.com>

api/v3rpc/rpctypes/metadatafields.go

@@ -18,3 +18,6 @@ var (
 	TokenFieldNameGRPC    = "token"
 	TokenFieldNameSwagger = "authorization"
 )
+
+// TokenFieldNameGRPCKey is used as a key of context to store token.
+type TokenFieldNameGRPCKey struct{}

client/v3/experimental/recipes/double_barrier.go

@@ -71,10 +71,9 @@ func (b *DoubleBarrier) Enter() error {
 			// delete itself now, otherwise other processes may need to wait
 			// until these keys are automatically deleted when the related
 			// lease expires.
-			if err = b.myKey.Delete(); err != nil {
-				// Nothing to do here. We have to wait for the key to be
-				// deleted when the lease expires.
-			}
+			b.myKey.Delete()
+			// Nothing to do here even if we run into error.
+			// We have to wait for the key to be deleted when the lease expires.
 			return ErrTooManyClients
 		}
 

client/v3/lease.go

@@ -198,12 +198,12 @@ func NewLeaseFromLeaseClient(remote pb.LeaseClient, c *Client, keepAliveTimeout
 		keepAlives:            make(map[LeaseID]*keepAlive),
 		remote:                remote,
 		firstKeepAliveTimeout: keepAliveTimeout,
-		lg:                    c.lg,
 	}
 	if l.firstKeepAliveTimeout == time.Second {
 		l.firstKeepAliveTimeout = defaultTTL
 	}
 	if c != nil {
+		l.lg = c.lg
 		l.callOpts = c.callOpts
 	}
 	reqLeaderCtx := WithRequireLeader(context.Background())

client/v3/maintenance.go

@@ -133,14 +133,14 @@ func NewMaintenance(c *Client) Maintenance {
 
 func NewMaintenanceFromMaintenanceClient(remote pb.MaintenanceClient, c *Client) Maintenance {
 	api := &maintenance{
-		lg: c.lg,
 		dial: func(string) (pb.MaintenanceClient, func(), error) {
 			return remote, func() {}, nil
 		},
 		remote: remote,
 	}
 	if c != nil {
 		api.callOpts = c.callOpts
+		api.lg = c.lg
 	}
 	return api
 }

pkg/traceutil/trace.go

@@ -25,10 +25,11 @@ import (
 	"go.uber.org/zap"
 )
 
-const (
-	TraceKey     = "trace"
-	StartTimeKey = "startTime"
-)
+// TraceKey is used as a key of context for Trace.
+type TraceKey struct{}
+
+// StartTimeKey is used as a key of context for start time of operation.
+type StartTimeKey struct{}
 
 // Field is a kv pair to record additional details of the trace.
 type Field struct {
@@ -81,7 +82,7 @@ func TODO() *Trace {
 }
 
 func Get(ctx context.Context) *Trace {
-	if trace, ok := ctx.Value(TraceKey).(*Trace); ok && trace != nil {
+	if trace, ok := ctx.Value(TraceKey{}).(*Trace); ok && trace != nil {
 		return trace
 	}
 	return TODO()

pkg/traceutil/trace_test.go

@@ -40,7 +40,7 @@ func TestGet(t *testing.T) {
 		},
 		{
 			name:        "When the context has trace",
-			inputCtx:    context.WithValue(context.Background(), TraceKey, traceForTest),
+			inputCtx:    context.WithValue(context.Background(), TraceKey{}, traceForTest),
 			outputTrace: traceForTest,
 		},
 	}
@@ -51,7 +51,7 @@ func TestGet(t *testing.T) {
 			if trace == nil {
 				t.Errorf("Expected %v; Got nil", tt.outputTrace)
 			}
-			if trace.operation != tt.outputTrace.operation {
+			if tt.outputTrace == nil || trace.operation != tt.outputTrace.operation {
 				t.Errorf("Expected %v; Got %v", tt.outputTrace, trace)
 			}
 		})

scripts/test.sh

@@ -426,10 +426,44 @@ function unparam_pass {
 }
 
 function staticcheck_pass {
-  # TODO: we should upgrade pb or ignore the pb package
+  local tmp_lintyaml
+  tmp_lintyaml=$(mktemp -t 'tmp_golangcilint_staticcheckXXX.yaml')
+
+  # shellcheck disable=SC2064
+  trap "rm ${tmp_lintyaml}; trap - RETURN" RETURN
+
+  # TODO: The golangci-lint commandline doesn't support listener-settings. And
+  # staticcheck command[1] not just verifies the staticcheck, but also includes
+  # stylecheck and unused. So copy the tools/.golangci.yaml and just run the
+  # staticcheck rule. It should be removed when closes #16610
   #
-  # versionpb/version.pb.go:69:15: proto.RegisterFile is deprecated: Use protoregistry.GlobalFiles.RegisterFile instead.  (SA1019)
-  run_for_modules generic_checker run_go_tool "honnef.co/go/tools/cmd/staticcheck"
+  # REF:
+  # [1] https://github.com/dominikh/go-tools/blob/v0.4.5/cmd/staticcheck/staticcheck.go#L29
+  cat > "${tmp_lintyaml}" <<EOF
+---
+run:
+  timeout: 30m
+  skip-files: [^zz_generated.*]
+issues:
+  max-same-issues: 0
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    # exclude ineffassing linter for generated files for conversion
+    - path: conversion\.go
+      linters: [ineffassign]
+linters:
+  disable-all: true
+  enable:
+    - staticcheck
+linters-settings:
+  staticcheck:
+    checks:
+      - all
+      - -SA1019 # TODO(fix) Using a deprecated function, variable, constant or field
+      - -SA2002 # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isn't allowed
+EOF
+
+  run_for_modules generic_checker run golangci-lint run --config "${tmp_lintyaml}"
 }
 
 function revive_pass {

server/auth/store_test.go

@@ -1173,10 +1173,10 @@ func testAuthInfoFromCtxWithRoot(t *testing.T, opts string) {
 
 	ai, aerr := as.AuthInfoFromCtx(ctx)
 	if aerr != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 	if ai == nil {
-		t.Error("expected non-nil *AuthInfo")
+		t.Fatal("expected non-nil *AuthInfo")
 	}
 	if ai.Username != "root" {
 		t.Errorf("expected user name 'root', got %+v", ai)

server/etcdserver/txn/txn.go

@@ -40,7 +40,7 @@ func Put(ctx context.Context, lg *zap.Logger, lessor lease.Lessor, kv mvcc.KV, p
 			traceutil.Field{Key: "key", Value: string(p.Key)},
 			traceutil.Field{Key: "req_size", Value: p.Size()},
 		)
-		ctx = context.WithValue(ctx, traceutil.TraceKey, trace)
+		ctx = context.WithValue(ctx, traceutil.TraceKey{}, trace)
 	}
 	leaseID := lease.LeaseID(p.Lease)
 	if leaseID != lease.NoLease {
@@ -102,7 +102,7 @@ func DeleteRange(ctx context.Context, lg *zap.Logger, kv mvcc.KV, dr *pb.DeleteR
 			traceutil.Field{Key: "key", Value: string(dr.Key)},
 			traceutil.Field{Key: "range_end", Value: string(dr.RangeEnd)},
 		)
-		ctx = context.WithValue(ctx, traceutil.TraceKey, trace)
+		ctx = context.WithValue(ctx, traceutil.TraceKey{}, trace)
 	}
 	txnWrite := kv.Write(trace)
 	defer txnWrite.End()
@@ -136,7 +136,7 @@ func Range(ctx context.Context, lg *zap.Logger, kv mvcc.KV, r *pb.RangeRequest)
 	trace = traceutil.Get(ctx)
 	if trace.IsEmpty() {
 		trace = traceutil.New("range", lg)
-		ctx = context.WithValue(ctx, traceutil.TraceKey, trace)
+		ctx = context.WithValue(ctx, traceutil.TraceKey{}, trace)
 	}
 	txnRead := kv.Read(mvcc.ConcurrentReadTxMode, trace)
 	defer txnRead.End()
@@ -248,7 +248,7 @@ func Txn(ctx context.Context, lg *zap.Logger, rt *pb.TxnRequest, txnModeWriteWit
 	trace := traceutil.Get(ctx)
 	if trace.IsEmpty() {
 		trace = traceutil.New("transaction", lg)
-		ctx = context.WithValue(ctx, traceutil.TraceKey, trace)
+		ctx = context.WithValue(ctx, traceutil.TraceKey{}, trace)
 	}
 	isWrite := !IsTxnReadonly(rt)
 	// When the transaction contains write operations, we use ReadTx instead of

server/etcdserver/v3_server.go

@@ -105,7 +105,7 @@ func (s *EtcdServer) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeRe
 		traceutil.Field{Key: "range_begin", Value: string(r.Key)},
 		traceutil.Field{Key: "range_end", Value: string(r.RangeEnd)},
 	)
-	ctx = context.WithValue(ctx, traceutil.TraceKey, trace)
+	ctx = context.WithValue(ctx, traceutil.TraceKey{}, trace)
 
 	var resp *pb.RangeResponse
 	var err error
@@ -140,7 +140,7 @@ func (s *EtcdServer) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeRe
 }
 
 func (s *EtcdServer) Put(ctx context.Context, r *pb.PutRequest) (*pb.PutResponse, error) {
-	ctx = context.WithValue(ctx, traceutil.StartTimeKey, time.Now())
+	ctx = context.WithValue(ctx, traceutil.StartTimeKey{}, time.Now())
 	resp, err := s.raftRequest(ctx, pb.InternalRaftRequest{Put: r})
 	if err != nil {
 		return nil, err
@@ -162,7 +162,7 @@ func (s *EtcdServer) Txn(ctx context.Context, r *pb.TxnRequest) (*pb.TxnResponse
 			s.Logger(),
 			traceutil.Field{Key: "read_only", Value: true},
 		)
-		ctx = context.WithValue(ctx, traceutil.TraceKey, trace)
+		ctx = context.WithValue(ctx, traceutil.TraceKey{}, trace)
 		if !txn.IsTxnSerializable(r) {
 			err := s.linearizableReadNotify(ctx)
 			trace.Step("agreement among raft nodes before linearized reading")
@@ -190,7 +190,7 @@ func (s *EtcdServer) Txn(ctx context.Context, r *pb.TxnRequest) (*pb.TxnResponse
 		return resp, err
 	}
 
-	ctx = context.WithValue(ctx, traceutil.StartTimeKey, time.Now())
+	ctx = context.WithValue(ctx, traceutil.StartTimeKey{}, time.Now())
 	resp, err := s.raftRequest(ctx, pb.InternalRaftRequest{Txn: r})
 	if err != nil {
 		return nil, err
@@ -671,7 +671,7 @@ func (s *EtcdServer) raftRequestOnce(ctx context.Context, r pb.InternalRaftReque
 	if result.Err != nil {
 		return nil, result.Err
 	}
-	if startTime, ok := ctx.Value(traceutil.StartTimeKey).(time.Time); ok && result.Trace != nil {
+	if startTime, ok := ctx.Value(traceutil.StartTimeKey{}).(time.Time); ok && result.Trace != nil {
 		applyStart := result.Trace.GetStartTime()
 		// The trace object is created in toApply. Here reset the start time to trace
 		// the raft request time by the difference between the request start time

server/proxy/grpcproxy/util.go

@@ -37,7 +37,7 @@ func getAuthTokenFromClient(ctx context.Context) string {
 func withClientAuthToken(ctx, ctxWithToken context.Context) context.Context {
 	token := getAuthTokenFromClient(ctxWithToken)
 	if token != "" {
-		ctx = context.WithValue(ctx, rpctypes.TokenFieldNameGRPC, token)
+		ctx = context.WithValue(ctx, rpctypes.TokenFieldNameGRPCKey{}, token)
 	}
 	return ctx
 }
@@ -66,7 +66,7 @@ func AuthUnaryClientInterceptor(ctx context.Context, method string, req, reply a
 }
 
 func AuthStreamClientInterceptor(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
-	tokenif := ctx.Value(rpctypes.TokenFieldNameGRPC)
+	tokenif := ctx.Value(rpctypes.TokenFieldNameGRPCKey{})
 	if tokenif != nil {
 		tokenCred := &proxyTokenCredential{tokenif.(string)}
 		opts = append(opts, grpc.PerRPCCredentials(tokenCred))

tests/framework/e2e/etcd_process.go

@@ -189,10 +189,12 @@ func (ep *EtcdServerProcess) Restart(ctx context.Context) error {
 }
 
 func (ep *EtcdServerProcess) Stop() (err error) {
-	ep.cfg.lg.Info("stopping server...", zap.String("name", ep.cfg.Name))
 	if ep == nil || ep.proc == nil {
 		return nil
 	}
+
+	ep.cfg.lg.Info("stopping server...", zap.String("name", ep.cfg.Name))
+
 	defer func() {
 		ep.proc = nil
 	}()

tests/integration/clientv3/lease/leasing_test.go

@@ -693,7 +693,7 @@ func TestLeasingTxnOwnerGet(t *testing.T) {
 		k := fmt.Sprintf("k-%d", i)
 		rr := tresp.Responses[i].GetResponseRange()
 		if rr == nil {
-			t.Errorf("expected get response, got %+v", tresp.Responses[i])
+			t.Fatalf("expected get response, got %+v", tresp.Responses[i])
 		}
 		if string(rr.Kvs[0].Key) != k || string(rr.Kvs[0].Value) != k+k {
 			t.Errorf(`expected key for %q, got %+v`, k, rr.Kvs)

tests/integration/lazy_cluster.go

@@ -89,7 +89,10 @@ func (lc *lazyCluster) mustLazyInit() {
 }
 
 func (lc *lazyCluster) Terminate() {
-	lc.tb.Logf("Terminating...")
+	if lc != nil && lc.tb != nil {
+		lc.tb.Logf("Terminating...")
+	}
+
 	if lc != nil && lc.cluster != nil {
 		lc.cluster.Terminate(nil)
 		lc.cluster = nil

tests/robustness/report/client.go

@@ -169,7 +169,6 @@ func loadKeyValueOperations(path string) (operations []porcupine.Operation, err
 func persistWatchOperations(t *testing.T, lg *zap.Logger, path string, responses []model.WatchOperation) {
 	lg.Info("Saving watch operations", zap.String("path", path))
 	file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755)
-	defer file.Close()
 	if err != nil {
 		t.Errorf("Failed to save watch operations: %v", err)
 		return