Skip to content

Commit 12af279

Browse files
committed
nogo: add missing go vet checks
Remove usage of `vet = True` in nogo definition. It's simply a shortcut attribute to appends some additional analyzers to the nogo binary. Instead, run `go tool vet help` and copy all listed analyzers in the doc to our analyzer. This list is more up-to-date than the one in current rules_go. Fix 2 vet issues found with new analyzers added: - Misuse of unbuffered os.Signal channel as argument to signal.Notify The channel should be buffered with minimal size of 1. - time.Since should not be used in defer statement See golang/go#60048 for more information
1 parent 3b29edc commit 12af279

File tree

4 files changed

+31
-9
lines changed

4 files changed

+31
-9
lines changed

BUILD

+24-4
Original file line numberDiff line numberDiff line change
@@ -35,32 +35,42 @@ write_file(
3535
},
3636
}
3737
for analyzer in ANALYZERS + [
38+
"appends",
3839
"asmdecl",
3940
"assign",
4041
"atomicalign",
42+
"bools",
4143
"buildtag",
42-
"cgocall",
44+
# "cgocall",
4345
"composites",
4446
"copylocks",
4547
"deepequalerrors",
48+
"defers",
49+
"directive",
4650
"errorsas",
51+
# Noisy and is not part of 'go vet'
4752
# "fieldalignment",
4853
"framepointer",
4954
"httpresponse",
5055
"ifaceassert",
5156
"loopclosure",
5257
"lostcancel",
58+
"nilfunc",
5359
# template methods currently cause this analyzer to panic
5460
# "nilness",
61+
"printf",
5562
# Everyone shadows `err`
5663
# "shadow",
5764
"shift",
65+
"sigchanyzer",
66+
"slog",
5867
"sortslice",
5968
"stdmethods",
6069
"stringintconv",
6170
"structtag",
62-
"tests",
6371
"testinggoroutine",
72+
"tests",
73+
"timeformat",
6474
"unmarshal",
6575
"unreachable",
6676
"unsafeptr",
@@ -74,34 +84,44 @@ write_file(
7484
nogo(
7585
name = "vet",
7686
config = ":nogo_config.json",
77-
vet = True,
7887
visibility = ["//visibility:public"],
7988
deps = [
89+
"@org_golang_x_tools//go/analysis/passes/appends",
8090
"@org_golang_x_tools//go/analysis/passes/asmdecl",
8191
"@org_golang_x_tools//go/analysis/passes/assign",
92+
"@org_golang_x_tools//go/analysis/passes/atomic",
8293
"@org_golang_x_tools//go/analysis/passes/atomicalign",
94+
"@org_golang_x_tools//go/analysis/passes/bools",
95+
"@org_golang_x_tools//go/analysis/passes/buildtag",
8396
# "@org_golang_x_tools//go/analysis/passes/cgocall",
8497
"@org_golang_x_tools//go/analysis/passes/composite",
8598
"@org_golang_x_tools//go/analysis/passes/copylock",
8699
"@org_golang_x_tools//go/analysis/passes/deepequalerrors",
100+
"@org_golang_x_tools//go/analysis/passes/defers",
101+
"@org_golang_x_tools//go/analysis/passes/directive",
87102
"@org_golang_x_tools//go/analysis/passes/errorsas",
88103
# "@org_golang_x_tools//go/analysis/passes/fieldalignment",
89104
"@org_golang_x_tools//go/analysis/passes/framepointer",
90105
"@org_golang_x_tools//go/analysis/passes/httpresponse",
91106
"@org_golang_x_tools//go/analysis/passes/ifaceassert",
92107
"@org_golang_x_tools//go/analysis/passes/loopclosure",
93108
"@org_golang_x_tools//go/analysis/passes/lostcancel",
109+
"@org_golang_x_tools//go/analysis/passes/nilfunc",
94110
# template methods currently cause this analyzer to panic
95111
# "@org_golang_x_tools//go/analysis/passes/nilness",
112+
"@org_golang_x_tools//go/analysis/passes/printf",
96113
# Everyone shadows `err`
97114
# "@org_golang_x_tools//go/analysis/passes/shadow",
98115
"@org_golang_x_tools//go/analysis/passes/shift",
116+
"@org_golang_x_tools//go/analysis/passes/sigchanyzer",
117+
"@org_golang_x_tools//go/analysis/passes/slog",
99118
"@org_golang_x_tools//go/analysis/passes/sortslice",
100119
"@org_golang_x_tools//go/analysis/passes/stdmethods",
101120
"@org_golang_x_tools//go/analysis/passes/stringintconv",
102121
"@org_golang_x_tools//go/analysis/passes/structtag",
103-
"@org_golang_x_tools//go/analysis/passes/tests",
104122
"@org_golang_x_tools//go/analysis/passes/testinggoroutine",
123+
"@org_golang_x_tools//go/analysis/passes/tests",
124+
"@org_golang_x_tools//go/analysis/passes/timeformat",
105125
"@org_golang_x_tools//go/analysis/passes/unmarshal",
106126
"@org_golang_x_tools//go/analysis/passes/unreachable",
107127
"@org_golang_x_tools//go/analysis/passes/unsafeptr",

cli/remotebazel/remotebazel.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ func Run(ctx context.Context, opts RunOpts, repoConfig *RepoConfig) (int, error)
679679
log.Debugf("Invocation ID: %s", iid)
680680

681681
// If the remote bazel process is canceled or killed, cancel the remote run
682-
sigChan := make(chan os.Signal)
682+
sigChan := make(chan os.Signal, 1)
683683
go func() {
684684
<-sigChan
685685
_, err = bbClient.CancelExecutions(ctx, &inpb.CancelExecutionsRequest{

enterprise/server/remote_execution/soci_store/soci_store_linux.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,11 @@ func streamingStoreArg(path string) string {
278278

279279
func getArtifacts(ctx context.Context, client socipb.SociArtifactStoreClient, env environment.Env, image string, credentials oci.Credentials) error {
280280
startTime := time.Now()
281-
defer metrics.PodmanGetSociArtifactsLatencyUsec.
282-
With(prometheus.Labels{metrics.ContainerImageTag: image}).
283-
Observe(float64(time.Since(startTime).Microseconds()))
281+
defer func() {
282+
metrics.PodmanGetSociArtifactsLatencyUsec.
283+
With(prometheus.Labels{metrics.ContainerImageTag: image}).
284+
Observe(float64(time.Since(startTime).Microseconds()))
285+
}()
284286

285287
ctx, err := prefix.AttachUserPrefixToContext(ctx, env)
286288
if err != nil {

server/util/healthcheck/healthcheck.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func NewHealthChecker(serverType string) *HealthChecker {
6666
checkers: make(map[string]interfaces.Checker, 0),
6767
lastStatus: make([]*serviceStatus, 0),
6868
}
69-
sigTerm := make(chan os.Signal)
69+
sigTerm := make(chan os.Signal, 1)
7070
go func() {
7171
<-sigTerm
7272
hc.Shutdown()

0 commit comments

Comments
 (0)