diff --git a/BUILD.bazel b/BUILD.bazel index 88160a63fadc..2b362408bd9a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -169,9 +169,6 @@ nogo( "@org_golang_x_tools//go/analysis/passes/nilness:go_default_library", "@org_golang_x_tools//go/analysis/passes/pkgfact:go_default_library", "@org_golang_x_tools//go/analysis/passes/printf:go_default_library", - # TODO(ricky): we do want to run the shadow check, we just want to exclude - # shadows of the variable "err". Presumably we can hack around this. #73069 - # "@org_golang_x_tools//go/analysis/passes/shadow:go_default_library", "@org_golang_x_tools//go/analysis/passes/shift:go_default_library", "@org_golang_x_tools//go/analysis/passes/sortslice:go_default_library", "@org_golang_x_tools//go/analysis/passes/stdmethods:go_default_library", @@ -196,6 +193,7 @@ nogo( "//pkg/testutils/lint/passes/nocopy", "//pkg/testutils/lint/passes/returncheck", "//pkg/testutils/lint/passes/returnerrcheck", + "//pkg/testutils/lint/passes/shadow", "//pkg/testutils/lint/passes/timer", "//pkg/testutils/lint/passes/unconvert", ] + STATICCHECK_CHECKS, diff --git a/build/bazelutil/nogo_config.json b/build/bazelutil/nogo_config.json index f0da77dcc78e..9cf27c7cc784 100644 --- a/build/bazelutil/nogo_config.json +++ b/build/bazelutil/nogo_config.json @@ -1653,6 +1653,11 @@ "cockroach/pkg/.*$": "first-party code" } }, + "shadow": { + "only_files": { + "cockroach/pkg/.*$": "first-party code" + } + }, "stdmethods": { "only_files": { "cockroach/pkg/.*$": "first-party code" diff --git a/pkg/testutils/lint/passes/shadow/BUILD.bazel b/pkg/testutils/lint/passes/shadow/BUILD.bazel new file mode 100644 index 000000000000..e5c6a58ff65a --- /dev/null +++ b/pkg/testutils/lint/passes/shadow/BUILD.bazel @@ -0,0 +1,12 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "shadow", + srcs = ["shadow.go"], + importpath = "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/shadow", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis", + "@org_golang_x_tools//go/analysis/passes/shadow", + ], +) diff --git a/pkg/testutils/lint/passes/shadow/shadow.go b/pkg/testutils/lint/passes/shadow/shadow.go new file mode 100644 index 000000000000..ff035a476a3b --- /dev/null +++ b/pkg/testutils/lint/passes/shadow/shadow.go @@ -0,0 +1,53 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +// Package shadow defines an Analyzer that is a slightly modified version +// of the shadow Analyzer from upstream (golang.org/x/tools). +// We allow shadows of a few variable names, like err. + +//go:build bazel +// +build bazel + +package shadow + +import ( + "fmt" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/shadow" +) + +var ( + Analyzer = shadow.Analyzer + permittedShadows = []string{ + "ctx", + "err", + "pErr", + } +) + +func init() { + oldRun := Analyzer.Run + Analyzer.Run = func(p *analysis.Pass) (interface{}, error) { + pass := *p + oldReport := p.Report + pass.Report = func(diag analysis.Diagnostic) { + for _, permittedShadow := range permittedShadows { + if strings.HasPrefix(diag.Message, fmt.Sprintf("declaration of %q shadows declaration at line", permittedShadow)) { + // Can throw the failure away. + return + } + } + oldReport(diag) + } + return oldRun(&pass) + } +} diff --git a/pkg/testutils/lint/passes/shadow/shadow_no_bazel.go b/pkg/testutils/lint/passes/shadow/shadow_no_bazel.go new file mode 100644 index 000000000000..e63416b216e2 --- /dev/null +++ b/pkg/testutils/lint/passes/shadow/shadow_no_bazel.go @@ -0,0 +1,18 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +// Package shadow is empty when not built under Bazel. + +//go:build !bazel +// +build !bazel + +package shadow + +// File intentionally empty. diff --git a/pkg/util/log/eventpb/gen.go b/pkg/util/log/eventpb/gen.go index ad9d3f787507..9a42e493c7a4 100644 --- a/pkg/util/log/eventpb/gen.go +++ b/pkg/util/log/eventpb/gen.go @@ -387,10 +387,11 @@ func readInput( // redact:"safeif:" - safe for reporting if the string matches the regexp. safeReName := "" if re := fieldDefRe.ReplaceAllString(line, "$safeif"); re != "" { + var err error // We're reading the regular expression from the .proto source, so we must // take care of string un-escaping ourselves. If this code ever improves // to apply as a protobuf plugin, this step can be removed. - re, err := strconv.Unquote(`"` + re + `"`) + re, err = strconv.Unquote(`"` + re + `"`) if err != nil { return errors.Wrapf(err, "error while unquoting regexp at %q", line) }