Skip to content

Commit 8b17a4d

Browse files
authored
vet: various cleanups (#6780)
1 parent 591c481 commit 8b17a4d

File tree

11 files changed

+97
-134
lines changed

11 files changed

+97
-134
lines changed

balancer/rls/internal/adaptive/adaptive_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ import (
2525
)
2626

2727
// stats returns a tuple with accepts, throttles for the current time.
28-
func (th *Throttler) stats() (int64, int64) {
28+
func (t *Throttler) stats() (int64, int64) {
2929
now := timeNowFunc()
3030

31-
th.mu.Lock()
32-
a, t := th.accepts.sum(now), th.throttles.sum(now)
33-
th.mu.Unlock()
34-
return a, t
31+
t.mu.Lock()
32+
a, th := t.accepts.sum(now), t.throttles.sum(now)
33+
t.mu.Unlock()
34+
return a, th
3535
}
3636

3737
// Enums for responses.

benchmark/benchmain/main.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ import (
6161

6262
"google.golang.org/grpc"
6363
"google.golang.org/grpc/benchmark"
64-
bm "google.golang.org/grpc/benchmark"
6564
"google.golang.org/grpc/benchmark/flags"
6665
"google.golang.org/grpc/benchmark/latency"
6766
"google.golang.org/grpc/benchmark/stats"
@@ -378,11 +377,11 @@ func makeClients(bf stats.Features) ([]testgrpc.BenchmarkServiceClient, func())
378377
}))
379378
}
380379
lis = nw.Listener(lis)
381-
stopper := bm.StartServer(bm.ServerInfo{Type: "protobuf", Listener: lis}, sopts...)
380+
stopper := benchmark.StartServer(benchmark.ServerInfo{Type: "protobuf", Listener: lis}, sopts...)
382381
conns := make([]*grpc.ClientConn, bf.Connections)
383382
clients := make([]testgrpc.BenchmarkServiceClient, bf.Connections)
384383
for cn := 0; cn < bf.Connections; cn++ {
385-
conns[cn] = bm.NewClientConn("" /* target not used */, opts...)
384+
conns[cn] = benchmark.NewClientConn("" /* target not used */, opts...)
386385
clients[cn] = testgrpc.NewBenchmarkServiceClient(conns[cn])
387386
}
388387

@@ -430,7 +429,7 @@ func makeFuncStream(bf stats.Features) (rpcCallFunc, rpcCleanupFunc) {
430429
if bf.EnablePreloader {
431430
req = preparedMsg[cn][pos]
432431
} else {
433-
pl := bm.NewPayload(testpb.PayloadType_COMPRESSABLE, reqSizeBytes)
432+
pl := benchmark.NewPayload(testpb.PayloadType_COMPRESSABLE, reqSizeBytes)
434433
req = &testpb.SimpleRequest{
435434
ResponseType: pl.Type,
436435
ResponseSize: int32(respSizeBytes),
@@ -488,7 +487,7 @@ func setupStream(bf stats.Features, unconstrained bool) ([][]testgrpc.BenchmarkS
488487
}
489488
}
490489

491-
pl := bm.NewPayload(testpb.PayloadType_COMPRESSABLE, bf.ReqSizeBytes)
490+
pl := benchmark.NewPayload(testpb.PayloadType_COMPRESSABLE, bf.ReqSizeBytes)
492491
req := &testpb.SimpleRequest{
493492
ResponseType: pl.Type,
494493
ResponseSize: int32(bf.RespSizeBytes),
@@ -515,13 +514,13 @@ func prepareMessages(streams [][]testgrpc.BenchmarkService_StreamingCallClient,
515514
// Makes a UnaryCall gRPC request using the given BenchmarkServiceClient and
516515
// request and response sizes.
517516
func unaryCaller(client testgrpc.BenchmarkServiceClient, reqSize, respSize int) {
518-
if err := bm.DoUnaryCall(client, reqSize, respSize); err != nil {
517+
if err := benchmark.DoUnaryCall(client, reqSize, respSize); err != nil {
519518
logger.Fatalf("DoUnaryCall failed: %v", err)
520519
}
521520
}
522521

523522
func streamCaller(stream testgrpc.BenchmarkService_StreamingCallClient, req any) {
524-
if err := bm.DoStreamingRoundTripPreloaded(stream, req); err != nil {
523+
if err := benchmark.DoStreamingRoundTripPreloaded(stream, req); err != nil {
525524
logger.Fatalf("DoStreamingRoundTrip failed: %v", err)
526525
}
527526
}

test/end2end_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -735,8 +735,8 @@ type nopCompressor struct {
735735
grpc.Compressor
736736
}
737737

738-
// NewNopCompressor creates a compressor to test the case that type is not supported.
739-
func NewNopCompressor() grpc.Compressor {
738+
// newNopCompressor creates a compressor to test the case that type is not supported.
739+
func newNopCompressor() grpc.Compressor {
740740
return &nopCompressor{grpc.NewGZIPCompressor()}
741741
}
742742

@@ -748,8 +748,8 @@ type nopDecompressor struct {
748748
grpc.Decompressor
749749
}
750750

751-
// NewNopDecompressor creates a decompressor to test the case that type is not supported.
752-
func NewNopDecompressor() grpc.Decompressor {
751+
// newNopDecompressor creates a decompressor to test the case that type is not supported.
752+
func newNopDecompressor() grpc.Decompressor {
753753
return &nopDecompressor{grpc.NewGZIPDecompressor()}
754754
}
755755

@@ -775,8 +775,8 @@ func (te *test) configDial(opts ...grpc.DialOption) ([]grpc.DialOption, string)
775775
}
776776
if te.clientNopCompression {
777777
opts = append(opts,
778-
grpc.WithCompressor(NewNopCompressor()),
779-
grpc.WithDecompressor(NewNopDecompressor()),
778+
grpc.WithCompressor(newNopCompressor()),
779+
grpc.WithDecompressor(newNopDecompressor()),
780780
)
781781
}
782782
if te.unaryClientInt != nil {

test/tools/go.mod

-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ go 1.19
55
require (
66
github.com/client9/misspell v0.3.4
77
github.com/golang/protobuf v1.5.3
8-
golang.org/x/lint v0.0.0-20210508222113-6edffad5e616
98
golang.org/x/tools v0.14.0
109
honnef.co/go/tools v0.4.6
1110
)

test/tools/go.sum

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

test/tools/tools.go

-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ package tools
2828
import (
2929
_ "github.com/client9/misspell/cmd/misspell"
3030
_ "github.com/golang/protobuf/protoc-gen-go"
31-
_ "golang.org/x/lint/golint"
3231
_ "golang.org/x/tools/cmd/goimports"
3332
_ "honnef.co/go/tools/cmd/staticcheck"
3433
)

vet.sh

+72-92
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ if [[ "$1" = "-install" ]]; then
3535
# Install the pinned versions as defined in module tools.
3636
pushd ./test/tools
3737
go install \
38-
golang.org/x/lint/golint \
3938
golang.org/x/tools/cmd/goimports \
4039
honnef.co/go/tools/cmd/staticcheck \
4140
github.com/client9/misspell/cmd/misspell
@@ -77,6 +76,10 @@ fi
7776
not grep 'func Test[^(]' *_test.go
7877
not grep 'func Test[^(]' test/*.go
7978

79+
# - Check for typos in test function names
80+
git grep 'func (s) ' -- "*_test.go" | not grep -v 'func (s) Test'
81+
git grep 'func [A-Z]' -- "*_test.go" | not grep -v 'func Test\|Benchmark\|Example'
82+
8083
# - Do not import x/net/context.
8184
not git grep -l 'x/net/context' -- "*.go"
8285

@@ -94,23 +97,21 @@ git grep -l -e 'grpclog.I' --or -e 'grpclog.W' --or -e 'grpclog.E' --or -e 'grpc
9497
not git grep "\(import \|^\s*\)\"github.com/golang/protobuf/ptypes/" -- "*.go"
9598

9699
# - Ensure all usages of grpc_testing package are renamed when importing.
97-
not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go"
100+
not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go"
98101

99102
# - Ensure all xds proto imports are renamed to *pb or *grpc.
100103
git grep '"github.com/envoyproxy/go-control-plane/envoy' -- '*.go' ':(exclude)*.pb.go' | not grep -v 'pb "\|grpc "'
101104

102105
misspell -error .
103106

104-
# - gofmt, goimports, golint (with exceptions for generated code), go vet,
105-
# go mod tidy.
107+
# - gofmt, goimports, go vet, go mod tidy.
106108
# Perform these checks on each module inside gRPC.
107109
for MOD_FILE in $(find . -name 'go.mod'); do
108110
MOD_DIR=$(dirname ${MOD_FILE})
109111
pushd ${MOD_DIR}
110112
go vet -all ./... | fail_on_output
111113
gofmt -s -d -l . 2>&1 | fail_on_output
112114
goimports -l . 2>&1 | not grep -vE "\.pb\.go"
113-
golint ./... 2>&1 | not grep -vE "/grpc_testing_not_regenerate/.*\.pb\.go:"
114115

115116
go mod tidy -compat=1.19
116117
git status --porcelain 2>&1 | fail_on_output || \
@@ -119,94 +120,73 @@ for MOD_FILE in $(find . -name 'go.mod'); do
119120
done
120121

121122
# - Collection of static analysis checks
122-
#
123-
# TODO(dfawley): don't use deprecated functions in examples or first-party
124-
# plugins.
125-
# TODO(dfawley): enable ST1019 (duplicate imports) but allow for protobufs.
126123
SC_OUT="$(mktemp)"
127-
staticcheck -go 1.19 -checks 'inherit,-ST1015,-ST1019,-SA1019' ./... > "${SC_OUT}" || true
128-
# Error if anything other than deprecation warnings are printed.
129-
not grep -v "is deprecated:.*SA1019" "${SC_OUT}"
130-
# Only ignore the following deprecated types/fields/functions.
131-
not grep -Fv '.CredsBundle
132-
.HeaderMap
133-
.Metadata is deprecated: use Attributes
134-
.NewAddress
135-
.NewServiceConfig
136-
.Type is deprecated: use Attributes
137-
BuildVersion is deprecated
138-
balancer.ErrTransientFailure
139-
balancer.Picker
140-
extDesc.Filename is deprecated
141-
github.com/golang/protobuf/jsonpb is deprecated
142-
grpc.CallCustomCodec
143-
grpc.Code
144-
grpc.Compressor
145-
grpc.CustomCodec
146-
grpc.Decompressor
147-
grpc.MaxMsgSize
148-
grpc.MethodConfig
149-
grpc.NewGZIPCompressor
150-
grpc.NewGZIPDecompressor
151-
grpc.RPCCompressor
152-
grpc.RPCDecompressor
153-
grpc.ServiceConfig
154-
grpc.WithCompressor
155-
grpc.WithDecompressor
156-
grpc.WithDialer
157-
grpc.WithMaxMsgSize
158-
grpc.WithServiceConfig
159-
grpc.WithTimeout
160-
http.CloseNotifier
161-
info.SecurityVersion
162-
proto is deprecated
163-
proto.InternalMessageInfo is deprecated
164-
proto.EnumName is deprecated
165-
proto.ErrInternalBadWireType is deprecated
166-
proto.FileDescriptor is deprecated
167-
proto.Marshaler is deprecated
168-
proto.MessageType is deprecated
169-
proto.RegisterEnum is deprecated
170-
proto.RegisterFile is deprecated
171-
proto.RegisterType is deprecated
172-
proto.RegisterExtension is deprecated
173-
proto.RegisteredExtension is deprecated
174-
proto.RegisteredExtensions is deprecated
175-
proto.RegisterMapType is deprecated
176-
proto.Unmarshaler is deprecated
124+
staticcheck -go 1.19 -checks 'all' ./... > "${SC_OUT}" || true
125+
126+
# Error for anything other than checks that need exclusions.
127+
grep -v "(ST1000)" "${SC_OUT}" | grep -v "(SA1019)" | grep -v "(ST1003)" | not grep -v "(ST1019)\|\(other import of\)"
128+
129+
# Exclude underscore checks for generated code.
130+
grep "(ST1003)" "${SC_OUT}" | not grep -v '\(.pb.go:\)\|\(code_string_test.go:\)'
131+
132+
# Error for duplicate imports not including grpc protos.
133+
grep "(ST1019)\|\(other import of\)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused
134+
channelz/grpc_channelz_v1"
135+
go-control-plane/envoy
136+
grpclb/grpc_lb_v1"
137+
health/grpc_health_v1"
138+
interop/grpc_testing"
139+
orca/v3"
140+
proto/grpc_gcp"
141+
proto/grpc_lookup_v1"
142+
reflection/grpc_reflection_v1"
143+
reflection/grpc_reflection_v1alpha"
144+
XXXXX PleaseIgnoreUnused'
145+
146+
# Error for any package comments not in generated code.
147+
grep "(ST1000)" "${SC_OUT}" | not grep -v "\.pb\.go:"
148+
149+
# Only ignore the following deprecated types/fields/functions and exclude
150+
# generated code.
151+
grep "(SA1019)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused
152+
XXXXX Protobuf related deprecation errors:
153+
"github.com/golang/protobuf
154+
.pb.go:
155+
: ptypes.
156+
proto.RegisterType
157+
XXXXX gRPC internal usage deprecation errors:
158+
"google.golang.org/grpc
159+
: grpc.
160+
: v1alpha.
161+
: v1alphareflectionpb.
162+
BalancerAttributes is deprecated:
163+
CredsBundle is deprecated:
164+
Metadata is deprecated: use Attributes instead.
165+
NewSubConn is deprecated:
166+
OverrideServerName is deprecated:
167+
RemoveSubConn is deprecated:
168+
SecurityVersion is deprecated:
177169
Target is deprecated: Use the Target field in the BuildOptions instead.
178-
xxx_messageInfo_
179-
' "${SC_OUT}"
180-
181-
# - special golint on package comments.
182-
lint_package_comment_per_package() {
183-
# Number of files in this go package.
184-
fileCount=$(go list -f '{{len .GoFiles}}' $1)
185-
if [ ${fileCount} -eq 0 ]; then
186-
return 0
187-
fi
188-
# Number of package errors generated by golint.
189-
lintPackageCommentErrorsCount=$(golint --min_confidence 0 $1 | grep -c "should have a package comment")
190-
# golint complains about every file that's missing the package comment. If the
191-
# number of files for this package is greater than the number of errors, there's
192-
# at least one file with package comment, good. Otherwise, fail.
193-
if [ ${fileCount} -le ${lintPackageCommentErrorsCount} ]; then
194-
echo "Package $1 (with ${fileCount} files) is missing package comment"
195-
return 1
196-
fi
197-
}
198-
lint_package_comment() {
199-
set +ex
200-
201-
count=0
202-
for i in $(go list ./...); do
203-
lint_package_comment_per_package "$i"
204-
((count += $?))
205-
done
206-
207-
set -ex
208-
return $count
209-
}
210-
lint_package_comment
170+
UpdateAddresses is deprecated:
171+
UpdateSubConnState is deprecated:
172+
balancer.ErrTransientFailure is deprecated:
173+
grpc/reflection/v1alpha/reflection.proto
174+
XXXXX xDS deprecated fields we support
175+
.ExactMatch
176+
.PrefixMatch
177+
.SafeRegexMatch
178+
.SuffixMatch
179+
GetContainsMatch
180+
GetExactMatch
181+
GetMatchSubjectAltNames
182+
GetPrefixMatch
183+
GetSafeRegexMatch
184+
GetSuffixMatch
185+
GetTlsCertificateCertificateProviderInstance
186+
GetValidationContextCertificateProviderInstance
187+
XXXXX TODO: Remove the below deprecation usages:
188+
CloseNotifier
189+
Roots.Subjects
190+
XXXXX PleaseIgnoreUnused'
211191

212192
echo SUCCESS

xds/internal/balancer/outlierdetection/callcounter_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,19 @@ func (b1 *bucket) Equal(b2 *bucket) bool {
3838
return b1.numFailures == b2.numFailures
3939
}
4040

41-
func (cc1 *callCounter) Equal(cc2 *callCounter) bool {
42-
if cc1 == nil && cc2 == nil {
41+
func (cc *callCounter) Equal(cc2 *callCounter) bool {
42+
if cc == nil && cc2 == nil {
4343
return true
4444
}
45-
if (cc1 != nil) != (cc2 != nil) {
45+
if (cc != nil) != (cc2 != nil) {
4646
return false
4747
}
48-
ab1 := (*bucket)(atomic.LoadPointer(&cc1.activeBucket))
48+
ab1 := (*bucket)(atomic.LoadPointer(&cc.activeBucket))
4949
ab2 := (*bucket)(atomic.LoadPointer(&cc2.activeBucket))
5050
if !ab1.Equal(ab2) {
5151
return false
5252
}
53-
return cc1.inactiveBucket.Equal(cc2.inactiveBucket)
53+
return cc.inactiveBucket.Equal(cc2.inactiveBucket)
5454
}
5555

5656
// TestClear tests that clear on the call counter clears (everything set to 0)

xds/internal/resolver/xds_resolver_test.go

-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ import (
6767
v3discoverypb "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
6868

6969
_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the cds LB policy
70-
_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // To parse LB config
7170
_ "google.golang.org/grpc/xds/internal/httpfilter/router" // Register the router filter
7271
)
7372

0 commit comments

Comments
 (0)