vet: various cleanups (grpc#6780)

balancer/rls/internal/adaptive/adaptive_test.go

@@ -25,13 +25,13 @@ import (
 )
 
 // stats returns a tuple with accepts, throttles for the current time.
-func (th *Throttler) stats() (int64, int64) {
+func (t *Throttler) stats() (int64, int64) {
 	now := timeNowFunc()
 
-	th.mu.Lock()
-	a, t := th.accepts.sum(now), th.throttles.sum(now)
-	th.mu.Unlock()
-	return a, t
+	t.mu.Lock()
+	a, th := t.accepts.sum(now), t.throttles.sum(now)
+	t.mu.Unlock()
+	return a, th
 }
 
 // Enums for responses.

benchmark/benchmain/main.go

@@ -61,7 +61,6 @@ import (
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/benchmark"
-	bm "google.golang.org/grpc/benchmark"
 	"google.golang.org/grpc/benchmark/flags"
 	"google.golang.org/grpc/benchmark/latency"
 	"google.golang.org/grpc/benchmark/stats"
@@ -378,11 +377,11 @@ func makeClients(bf stats.Features) ([]testgrpc.BenchmarkServiceClient, func())
 		}))
 	}
 	lis = nw.Listener(lis)
-	stopper := bm.StartServer(bm.ServerInfo{Type: "protobuf", Listener: lis}, sopts...)
+	stopper := benchmark.StartServer(benchmark.ServerInfo{Type: "protobuf", Listener: lis}, sopts...)
 	conns := make([]*grpc.ClientConn, bf.Connections)
 	clients := make([]testgrpc.BenchmarkServiceClient, bf.Connections)
 	for cn := 0; cn < bf.Connections; cn++ {
-		conns[cn] = bm.NewClientConn("" /* target not used */, opts...)
+		conns[cn] = benchmark.NewClientConn("" /* target not used */, opts...)
 		clients[cn] = testgrpc.NewBenchmarkServiceClient(conns[cn])
 	}
 
@@ -430,7 +429,7 @@ func makeFuncStream(bf stats.Features) (rpcCallFunc, rpcCleanupFunc) {
 		if bf.EnablePreloader {
 			req = preparedMsg[cn][pos]
 		} else {
-			pl := bm.NewPayload(testpb.PayloadType_COMPRESSABLE, reqSizeBytes)
+			pl := benchmark.NewPayload(testpb.PayloadType_COMPRESSABLE, reqSizeBytes)
 			req = &testpb.SimpleRequest{
 				ResponseType: pl.Type,
 				ResponseSize: int32(respSizeBytes),
@@ -488,7 +487,7 @@ func setupStream(bf stats.Features, unconstrained bool) ([][]testgrpc.BenchmarkS
 		}
 	}
 
-	pl := bm.NewPayload(testpb.PayloadType_COMPRESSABLE, bf.ReqSizeBytes)
+	pl := benchmark.NewPayload(testpb.PayloadType_COMPRESSABLE, bf.ReqSizeBytes)
 	req := &testpb.SimpleRequest{
 		ResponseType: pl.Type,
 		ResponseSize: int32(bf.RespSizeBytes),
@@ -515,13 +514,13 @@ func prepareMessages(streams [][]testgrpc.BenchmarkService_StreamingCallClient,
 // Makes a UnaryCall gRPC request using the given BenchmarkServiceClient and
 // request and response sizes.
 func unaryCaller(client testgrpc.BenchmarkServiceClient, reqSize, respSize int) {
-	if err := bm.DoUnaryCall(client, reqSize, respSize); err != nil {
+	if err := benchmark.DoUnaryCall(client, reqSize, respSize); err != nil {
 		logger.Fatalf("DoUnaryCall failed: %v", err)
 	}
 }
 
 func streamCaller(stream testgrpc.BenchmarkService_StreamingCallClient, req any) {
-	if err := bm.DoStreamingRoundTripPreloaded(stream, req); err != nil {
+	if err := benchmark.DoStreamingRoundTripPreloaded(stream, req); err != nil {
 		logger.Fatalf("DoStreamingRoundTrip failed: %v", err)
 	}
 }

test/end2end_test.go

@@ -735,8 +735,8 @@ type nopCompressor struct {
 	grpc.Compressor
 }
 
-// NewNopCompressor creates a compressor to test the case that type is not supported.
-func NewNopCompressor() grpc.Compressor {
+// newNopCompressor creates a compressor to test the case that type is not supported.
+func newNopCompressor() grpc.Compressor {
 	return &nopCompressor{grpc.NewGZIPCompressor()}
 }
 
@@ -748,8 +748,8 @@ type nopDecompressor struct {
 	grpc.Decompressor
 }
 
-// NewNopDecompressor creates a decompressor to test the case that type is not supported.
-func NewNopDecompressor() grpc.Decompressor {
+// newNopDecompressor creates a decompressor to test the case that type is not supported.
+func newNopDecompressor() grpc.Decompressor {
 	return &nopDecompressor{grpc.NewGZIPDecompressor()}
 }
 
@@ -775,8 +775,8 @@ func (te *test) configDial(opts ...grpc.DialOption) ([]grpc.DialOption, string)
 	}
 	if te.clientNopCompression {
 		opts = append(opts,
-			grpc.WithCompressor(NewNopCompressor()),
-			grpc.WithDecompressor(NewNopDecompressor()),
+			grpc.WithCompressor(newNopCompressor()),
+			grpc.WithDecompressor(newNopDecompressor()),
 		)
 	}
 	if te.unaryClientInt != nil {

test/tools/go.mod

@@ -5,7 +5,6 @@ go 1.19
 require (
 	github.com/client9/misspell v0.3.4
 	github.com/golang/protobuf v1.5.3
-	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616
 	golang.org/x/tools v0.14.0
 	honnef.co/go/tools v0.4.6
 )

test/tools/go.sum

@@ -7,28 +7,15 @@ github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg
 github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
 github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/exp/typeparams v0.0.0-20231006140011-7918f672742d h1:NRn/Afz91uVUyEsxMp4lGGxpr5y1qz+Iko60dbkfvLQ=
 golang.org/x/exp/typeparams v0.0.0-20231006140011-7918f672742d/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
-golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug=
-golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY=
-golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY=
 golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
-golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
-golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
-golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
-golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
 golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc=
 golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg=
-golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=

test/tools/tools.go

@@ -28,7 +28,6 @@ package tools
 import (
 	_ "github.com/client9/misspell/cmd/misspell"
 	_ "github.com/golang/protobuf/protoc-gen-go"
-	_ "golang.org/x/lint/golint"
 	_ "golang.org/x/tools/cmd/goimports"
 	_ "honnef.co/go/tools/cmd/staticcheck"
 )

vet.sh

@@ -35,7 +35,6 @@ if [[ "$1" = "-install" ]]; then
   # Install the pinned versions as defined in module tools.
   pushd ./test/tools
   go install \
-    golang.org/x/lint/golint \
     golang.org/x/tools/cmd/goimports \
     honnef.co/go/tools/cmd/staticcheck \
     github.com/client9/misspell/cmd/misspell
@@ -77,6 +76,10 @@ fi
 not grep 'func Test[^(]' *_test.go
 not grep 'func Test[^(]' test/*.go
 
+# - Check for typos in test function names
+git grep 'func (s) ' -- "*_test.go" | not grep -v 'func (s) Test'
+git grep 'func [A-Z]' -- "*_test.go" | not grep -v 'func Test\|Benchmark\|Example'
+
 # - Do not import x/net/context.
 not git grep -l 'x/net/context' -- "*.go"
 
@@ -94,23 +97,21 @@ git grep -l -e 'grpclog.I' --or -e 'grpclog.W' --or -e 'grpclog.E' --or -e 'grpc
 not git grep "\(import \|^\s*\)\"github.com/golang/protobuf/ptypes/" -- "*.go"
 
 # - Ensure all usages of grpc_testing package are renamed when importing.
-not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go" 
+not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go"
 
 # - Ensure all xds proto imports are renamed to *pb or *grpc.
 git grep '"github.com/envoyproxy/go-control-plane/envoy' -- '*.go' ':(exclude)*.pb.go' | not grep -v 'pb "\|grpc "'
 
 misspell -error .
 
-# - gofmt, goimports, golint (with exceptions for generated code), go vet,
-# go mod tidy.
+# - gofmt, goimports, go vet, go mod tidy.
 # Perform these checks on each module inside gRPC.
 for MOD_FILE in $(find . -name 'go.mod'); do
   MOD_DIR=$(dirname ${MOD_FILE})
   pushd ${MOD_DIR}
   go vet -all ./... | fail_on_output
   gofmt -s -d -l . 2>&1 | fail_on_output
   goimports -l . 2>&1 | not grep -vE "\.pb\.go"
-  golint ./... 2>&1 | not grep -vE "/grpc_testing_not_regenerate/.*\.pb\.go:"
 
   go mod tidy -compat=1.19
   git status --porcelain 2>&1 | fail_on_output || \
@@ -119,94 +120,73 @@ for MOD_FILE in $(find . -name 'go.mod'); do
 done
 
 # - Collection of static analysis checks
-#
-# TODO(dfawley): don't use deprecated functions in examples or first-party
-# plugins.
-# TODO(dfawley): enable ST1019 (duplicate imports) but allow for protobufs.
 SC_OUT="$(mktemp)"
-staticcheck -go 1.19 -checks 'inherit,-ST1015,-ST1019,-SA1019' ./... > "${SC_OUT}" || true
-# Error if anything other than deprecation warnings are printed.
-not grep -v "is deprecated:.*SA1019" "${SC_OUT}"
-# Only ignore the following deprecated types/fields/functions.
-not grep -Fv '.CredsBundle
-.HeaderMap
-.Metadata is deprecated: use Attributes
-.NewAddress
-.NewServiceConfig
-.Type is deprecated: use Attributes
-BuildVersion is deprecated
-balancer.ErrTransientFailure
-balancer.Picker
-extDesc.Filename is deprecated
-github.com/golang/protobuf/jsonpb is deprecated
-grpc.CallCustomCodec
-grpc.Code
-grpc.Compressor
-grpc.CustomCodec
-grpc.Decompressor
-grpc.MaxMsgSize
-grpc.MethodConfig
-grpc.NewGZIPCompressor
-grpc.NewGZIPDecompressor
-grpc.RPCCompressor
-grpc.RPCDecompressor
-grpc.ServiceConfig
-grpc.WithCompressor
-grpc.WithDecompressor
-grpc.WithDialer
-grpc.WithMaxMsgSize
-grpc.WithServiceConfig
-grpc.WithTimeout
-http.CloseNotifier
-info.SecurityVersion
-proto is deprecated
-proto.InternalMessageInfo is deprecated
-proto.EnumName is deprecated
-proto.ErrInternalBadWireType is deprecated
-proto.FileDescriptor is deprecated
-proto.Marshaler is deprecated
-proto.MessageType is deprecated
-proto.RegisterEnum is deprecated
-proto.RegisterFile is deprecated
-proto.RegisterType is deprecated
-proto.RegisterExtension is deprecated
-proto.RegisteredExtension is deprecated
-proto.RegisteredExtensions is deprecated
-proto.RegisterMapType is deprecated
-proto.Unmarshaler is deprecated
+staticcheck -go 1.19 -checks 'all' ./... > "${SC_OUT}" || true
+
+# Error for anything other than checks that need exclusions.
+grep -v "(ST1000)" "${SC_OUT}" | grep -v "(SA1019)" | grep -v "(ST1003)" | not grep -v "(ST1019)\|\(other import of\)"
+
+# Exclude underscore checks for generated code.
+grep "(ST1003)" "${SC_OUT}" | not grep -v '\(.pb.go:\)\|\(code_string_test.go:\)'
+
+# Error for duplicate imports not including grpc protos.
+grep "(ST1019)\|\(other import of\)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused
+channelz/grpc_channelz_v1"
+go-control-plane/envoy
+grpclb/grpc_lb_v1"
+health/grpc_health_v1"
+interop/grpc_testing"
+orca/v3"
+proto/grpc_gcp"
+proto/grpc_lookup_v1"
+reflection/grpc_reflection_v1"
+reflection/grpc_reflection_v1alpha"
+XXXXX PleaseIgnoreUnused'
+
+# Error for any package comments not in generated code.
+grep "(ST1000)" "${SC_OUT}" | not grep -v "\.pb\.go:"
+
+# Only ignore the following deprecated types/fields/functions and exclude
+# generated code.
+grep "(SA1019)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused
+XXXXX Protobuf related deprecation errors:
+"github.com/golang/protobuf
+.pb.go:
+: ptypes.
+proto.RegisterType
+XXXXX gRPC internal usage deprecation errors:
+"google.golang.org/grpc
+: grpc.
+: v1alpha.
+: v1alphareflectionpb.
+BalancerAttributes is deprecated:
+CredsBundle is deprecated:
+Metadata is deprecated: use Attributes instead.
+NewSubConn is deprecated:
+OverrideServerName is deprecated:
+RemoveSubConn is deprecated:
+SecurityVersion is deprecated:
 Target is deprecated: Use the Target field in the BuildOptions instead.
-xxx_messageInfo_
-' "${SC_OUT}"
-
-# - special golint on package comments.
-lint_package_comment_per_package() {
-  # Number of files in this go package.
-  fileCount=$(go list -f '{{len .GoFiles}}' $1)
-  if [ ${fileCount} -eq 0 ]; then
-    return 0
-  fi
-  # Number of package errors generated by golint.
-  lintPackageCommentErrorsCount=$(golint --min_confidence 0 $1 | grep -c "should have a package comment")
-  # golint complains about every file that's missing the package comment. If the
-  # number of files for this package is greater than the number of errors, there's
-  # at least one file with package comment, good. Otherwise, fail.
-  if [ ${fileCount} -le ${lintPackageCommentErrorsCount} ]; then
-    echo "Package $1 (with ${fileCount} files) is missing package comment"
-    return 1
-  fi
-}
-lint_package_comment() {
-  set +ex
-
-  count=0
-  for i in $(go list ./...); do
-    lint_package_comment_per_package "$i"
-    ((count += $?))
-  done
-
-  set -ex
-  return $count
-}
-lint_package_comment
+UpdateAddresses is deprecated:
+UpdateSubConnState is deprecated:
+balancer.ErrTransientFailure is deprecated:
+grpc/reflection/v1alpha/reflection.proto
+XXXXX xDS deprecated fields we support
+.ExactMatch
+.PrefixMatch
+.SafeRegexMatch
+.SuffixMatch
+GetContainsMatch
+GetExactMatch
+GetMatchSubjectAltNames
+GetPrefixMatch
+GetSafeRegexMatch
+GetSuffixMatch
+GetTlsCertificateCertificateProviderInstance
+GetValidationContextCertificateProviderInstance
+XXXXX TODO: Remove the below deprecation usages:
+CloseNotifier
+Roots.Subjects
+XXXXX PleaseIgnoreUnused'
 
 echo SUCCESS

xds/internal/balancer/outlierdetection/callcounter_test.go

@@ -38,19 +38,19 @@ func (b1 *bucket) Equal(b2 *bucket) bool {
 	return b1.numFailures == b2.numFailures
 }
 
-func (cc1 *callCounter) Equal(cc2 *callCounter) bool {
-	if cc1 == nil && cc2 == nil {
+func (cc *callCounter) Equal(cc2 *callCounter) bool {
+	if cc == nil && cc2 == nil {
 		return true
 	}
-	if (cc1 != nil) != (cc2 != nil) {
+	if (cc != nil) != (cc2 != nil) {
 		return false
 	}
-	ab1 := (*bucket)(atomic.LoadPointer(&cc1.activeBucket))
+	ab1 := (*bucket)(atomic.LoadPointer(&cc.activeBucket))
 	ab2 := (*bucket)(atomic.LoadPointer(&cc2.activeBucket))
 	if !ab1.Equal(ab2) {
 		return false
 	}
-	return cc1.inactiveBucket.Equal(cc2.inactiveBucket)
+	return cc.inactiveBucket.Equal(cc2.inactiveBucket)
 }
 
 // TestClear tests that clear on the call counter clears (everything set to 0)

xds/internal/resolver/xds_resolver_test.go

@@ -67,7 +67,6 @@ import (
 	v3discoverypb "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
 
 	_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the cds LB policy
-	_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // To parse LB config
 	_ "google.golang.org/grpc/xds/internal/httpfilter/router"    // Register the router filter
 )