testutils: test files inadvertently excluded from roachvet

[stevendanna]

Describe the problem

In #62243, we added exclusions for various files that was supposed to be targeted just at a single linter within roachvet. Unfortunately, the constructed regular expression ends matching all violations in the targeted files and packages.

Nearly all of the violations are in _test.go files, and not production code.

When we remove this filter, the violations as of 3a60751 are:

Running make with -j16
GOPATH set to /Users/ssd/go
go install -v roachvet
GOFLAGS= bin/prereqs ./pkg/cmd/roachvet > bin/roachvet.d.tmp
mkdir -p lib
rm -f lib/lib{geos,geos_c}.dylib
cp -L /Users/ssd/go/native/x86_64-apple-darwin20.6.0/geos/lib/lib{geos,geos_c}.dylib lib
mv -f bin/roachvet.d.tmp bin/roachvet.d
GOBIN='/Users/ssd/go/src/github.com/cockroachdb/cockroach/bin' GOFLAGS= go install -ldflags '-X github.com/cockroachdb/cockroach/pkg/build.typ=development -extldflags "" -X "github.com/cockroachdb/cockroach/pkg/build.tag=v21.2.0-alpha.00000000-4139-g3a607514f1-dirty" -X "github.com/cockroachdb/cockroach/pkg/build.rev=3a607514f126c44246ff3ede2d051fa99fb04d67" -X "github.com/cockroachdb/cockroach/pkg/build.cgoTargetTriple=x86_64-apple-darwin20.6.0"  ' -v ./pkg/cmd/roachvet
GOFLAGS= go build -i -v  -mod=vendor -tags 'make x86_64_apple_darwin20.6.0 lint' -ldflags '-X github.com/cockroachdb/cockroach/pkg/build.typ=development -extldflags "" -X "github.com/cockroachdb/cockroach/pkg/build.tag=v21.2.0-alpha.00000000-4139-g3a607514f1-dirty" -X "github.com/cockroachdb/cockroach/pkg/build.rev=3a607514f126c44246ff3ede2d051fa99fb04d67" -X "github.com/cockroachdb/cockroach/pkg/build.cgoTargetTriple=x86_64-apple-darwin20.6.0"  ' ./pkg/...
go build: -i flag is deprecated
GOFLAGS= go test  ./pkg/testutils/lint -v  -mod=vendor -tags 'make x86_64_apple_darwin20.6.0 lint' -ldflags '-X github.com/cockroachdb/cockroach/pkg/build.typ=development -extldflags "" -X "github.com/cockroachdb/cockroach/pkg/build.tag=v21.2.0-alpha.00000000-4139-g3a607514f1-dirty" -X "github.com/cockroachdb/cockroach/pkg/build.rev=3a607514f126c44246ff3ede2d051fa99fb04d67" -X "github.com/cockroachdb/cockroach/pkg/build.cgoTargetTriple=x86_64-apple-darwin20.6.0"  ' -timeout 30m -run 'Lint/TestRoachVet'
=== RUN   TestLint
=== RUN   TestLint/TestRoachVet
    lint_test.go:109: 
        pkg/ccl/sqlproxyccl/throttler/local_test.go:110:19: unnecessary conversion
        pkg/ccl/sqlproxyccl/throttler/local_test.go:116:19: unnecessary conversion
        pkg/cmd/roachtest/logger/log.go:260:38: PrintfCtxDepth(): format argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/cmd/roachtest/logger/log.go:275:38: ErrorfCtxDepth(): format argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/kv/kvserver/spanlatch/manager_test.go:556:4: declaration of "g" shadows declaration at line 553
        pkg/settings/settings_test.go:230:3: declaration of "ctx" shadows declaration at line 216
        pkg/cmd/reduce/main.go:152:10: unnecessary conversion
        pkg/kv/kvserver/gc_queue_test.go:1172:27: unnecessary conversion
        pkg/kv/kvserver/intent_resolver_integration_test.go:569:26: invalid direct cast on error object
        Alternatives:
           if _, ok := err.(*T); ok        ->   if errors.HasType(err, (*T)(nil))
           if _, ok := err.(I); ok         ->   if errors.HasInterface(err, (*I)(nil))
           if myErr, ok := err.(*T); ok    ->   if myErr := (*T)(nil); errors.As(err, &myErr)
           if myErr, ok := err.(I); ok     ->   if myErr := (I)(nil); errors.As(err, &myErr)
           switch err.(type) { case *T:... ->   switch { case errors.HasType(err, (*T)(nil)): ...
        
        pkg/kv/kvserver/intent_resolver_integration_test.go:571:35: invalid direct cast on error object
        Alternatives:
           if _, ok := err.(*T); ok        ->   if errors.HasType(err, (*T)(nil))
           if _, ok := err.(I); ok         ->   if errors.HasInterface(err, (*I)(nil))
           if myErr, ok := err.(*T); ok    ->   if myErr := (*T)(nil); errors.As(err, &myErr)
           if myErr, ok := err.(I); ok     ->   if myErr := (I)(nil); errors.As(err, &myErr)
           switch err.(type) { case *T:... ->   switch { case errors.HasType(err, (*T)(nil)): ...
        
        pkg/kv/kvserver/store_rebalancer_test.go:44:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:61:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:78:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:95:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:112:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:129:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:146:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:163:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/store_rebalancer_test.go:180:15: github.com/cockroachdb/cockroach/pkg/roachpb.Locality composite literal uses unkeyed fields
        pkg/kv/kvserver/allocator_test.go:1439:10: declaration of "ok" shadows declaration at line 1424
        pkg/kv/kvserver/allocator_test.go:1491:2: declaration of "replicas" shadows declaration at line 362
        pkg/kv/kvserver/intent_resolver_integration_test.go:426:4: declaration of "ctx" shadows declaration at line 293
        pkg/kv/kvserver/replica_test.go:8459:4: declaration of "curFlushAttempt" shadows declaration at line 8455
        pkg/ccl/changefeedccl/kvevent/blocking_buffer_test.go:98:5: unexpected nil error return after checking for a non-nil error; if this is not a mistake, add a "//nolint:returnerrcheck" comment
        pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go:256:46: github.com/cockroachdb/cockroach/pkg/clusterversion.ClusterVersion composite literal uses unkeyed fields
        pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go:256:52: github.com/cockroachdb/cockroach/pkg/clusterversion.ClusterVersion composite literal uses unkeyed fields
        pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go:284:7: unreachable code
        pkg/ccl/sqlproxyccl/proxy_handler_test.go:897:26: invalid direct cast on error object
        Alternatives:
           if _, ok := err.(*T); ok        ->   if errors.HasType(err, (*T)(nil))
           if _, ok := err.(I); ok         ->   if errors.HasInterface(err, (*I)(nil))
           if myErr, ok := err.(*T); ok    ->   if myErr := (*T)(nil); errors.As(err, &myErr)
           if myErr, ok := err.(I); ok     ->   if myErr := (I)(nil); errors.As(err, &myErr)
           switch err.(type) { case *T:... ->   switch { case errors.HasType(err, (*T)(nil)): ...
        
        pkg/ccl/backupccl/backup_test.go:321:24: invalid direct cast on error object
        Alternatives:
           if _, ok := err.(*T); ok        ->   if errors.HasType(err, (*T)(nil))
           if _, ok := err.(I); ok         ->   if errors.HasInterface(err, (*I)(nil))
           if myErr, ok := err.(*T); ok    ->   if myErr := (*T)(nil); errors.As(err, &myErr)
           if myErr, ok := err.(I); ok     ->   if myErr := (I)(nil); errors.As(err, &myErr)
           switch err.(type) { case *T:... ->   switch { case errors.HasType(err, (*T)(nil)): ...
        
        pkg/ccl/backupccl/backup_test.go:6635:14: github.com/cockroachdb/cockroach/pkg/util/hlc.Timestamp composite literal uses unkeyed fields
        pkg/ccl/backupccl/backup_test.go:6737:46: github.com/cockroachdb/cockroach/pkg/roachpb.Span composite literal uses unkeyed fields
        pkg/ccl/backupccl/backup_test.go:6764:46: github.com/cockroachdb/cockroach/pkg/roachpb.Span composite literal uses unkeyed fields
        pkg/ccl/multitenantccl/tenantcostserver/server_test.go:151:15: unnecessary conversion
        pkg/cli/debug_job_trace_test.go:80:10: the cancel function returned by context.WithCancel should be called, not discarded, to avoid a context leak
        pkg/ccl/serverccl/tenant_grpc_test.go:71:8: Illegal call to ClientConn.Close(), must elide the call to Close() if the ClientConn is from an *rpc.Context. Do `grpcConn.Close() // nolint:grpcconnclose` when the conn does not come from an rpc.Context.
        pkg/ccl/serverccl/tenant_grpc_test.go:138:8: Illegal call to ClientConn.Close(), must elide the call to Close() if the ClientConn is from an *rpc.Context. Do `grpcConn.Close() // nolint:grpcconnclose` when the conn does not come from an rpc.Context.
        pkg/ccl/serverccl/tenant_grpc_test.go:155:8: Illegal call to ClientConn.Close(), must elide the call to Close() if the ClientConn is from an *rpc.Context. Do `grpcConn.Close() // nolint:grpcconnclose` when the conn does not come from an rpc.Context.
        pkg/ccl/changefeedccl/helpers_test.go:192:4: unexpected nil error return after checking for a non-nil error; if this is not a mistake, add a "//nolint:returnerrcheck" comment
        pkg/ccl/changefeedccl/changefeed_test.go:4286:4: declaration of "progress" shadows declaration at line 4261
        pkg/cmd/roachtest/tests/util.go:35:3: the cancel function is not used on all paths (possible context leak)
        pkg/cmd/roachtest/tests/util.go:44:4: this return statement may be reached without using the cancel var defined on line 35
        pkg/cmd/roachtest/tests/gossip.go:94:6: (github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test.Test).Fatalf format %.1f has arg deadNode of wrong type int
        pkg/cmd/roachtest/tests/kvbench.go:199:2: declaration of "loadGroup" shadows declaration at line 15
        pkg/jobs/registry_external_test.go:656:57: TestErrorsPopulatedOnRetry(): message argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/jobs/registry_external_test.go:694:58: TestErrorsPopulatedOnRetry(): message argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/migration/migrations/helpers_test.go:108:4: github.com/stretchr/testify/require.Error call has possible formatting directive %s
        pkg/server/statements_test.go:45:7: Illegal call to ClientConn.Close(), must elide the call to Close() if the ClientConn is from an *rpc.Context. Do `grpcConn.Close() // nolint:grpcconnclose` when the conn does not come from an rpc.Context.
        pkg/server/server_test.go:1246:9: github.com/cockroachdb/cockroach/pkg/storage.MVCCKey composite literal uses unkeyed fields
        pkg/kv/kvserver/client_replica_test.go:3869:3: must set timer.Read = true after reading from timer.C (see timeutil/timer.go)
        pkg/kv/kvserver/client_replica_test.go:3231:14: github.com/cockroachdb/errors.Errorf format %d has arg c.TTL().Seconds() of wrong type float64
        pkg/sql/catalog/nstree/map_test.go:93:23: testMapDataDriven(): message argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/sql/distsql_physical_planner_test.go:1331:14: github.com/cockroachdb/cockroach/pkg/util/log.Fatalf call has arguments but no formatting directives
        pkg/sql/query_sampling_test.go:49:2: declaration of "stubTime" shadows declaration at line 24
        pkg/sql/telemetry_logging_test.go:81:2: declaration of "stubTime" shadows declaration at line 24
        pkg/sql/colexec/parallel_unordered_synchronizer_test.go:121:62: TestParallelUnorderedSynchronizer(): format argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/security/certificate_loader_test.go:287:23: TestNamingScheme(): message argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/security/certificate_loader_test.go:289:23: TestNamingScheme(): message argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/sql/colexec/colexecutils/spilling_queue_test.go:280:17: unnecessary conversion
        pkg/sql/crdb_internal_test.go:607:4: declaration of "ctx" shadows declaration at line 539
        pkg/sql/sqlinstance/instanceprovider/instanceprovider_test.go:84:52: use errors.Is instead of a direct comparison
        For example:
           if errors.Is(err, errMyOwnErrReference) {
             ...
           }
        
        pkg/sql/sqlstats/persistedsqlstats/scheduled_sql_stats_compaction_test.go:206:5: declaration of "sj" shadows declaration at line 173
        pkg/storage/pebble_file_registry_test.go:110:3: declaration of "registry" shadows declaration at line 93
        pkg/testutils/reduce/reduce_test.go:56:30: unnecessary conversion
        pkg/testutils/reduce/reducesql/reducesql_test.go:68:25: unnecessary conversion
        pkg/util/admission/work_queue_test.go:139:6: declaration of "workMap" shadows declaration at line 90
        pkg/util/log/file_log_gc_test.go:43:8: TestGC(): format argument is not a constant expression
        Tip: use YourFuncf("descriptive prefix %s", ...) or list new formatting wrappers in pkg/testutils/lint/passes/fmtsafe/functions.go.
        pkg/util/log/http_sink_test.go:101:11: use errors.Is instead of a direct comparison
        For example:
           if errors.Is(err, errMyOwnErrReference) {
             ...
           }
        
        pkg/workload/tpcc/checks.go:275:35: use errors.Is instead of a direct comparison
        For example:
           if errors.Is(err, errMyOwnErrReference) {
             ...
           }
        
        pkg/workload/tpcc/checks.go:282:36: use errors.Is instead of a direct comparison
        For example:
           if errors.Is(err, errMyOwnErrReference) {
             ...
           }
        
        pkg/workload/tpcc/checks.go:305:35: use errors.Is instead of a direct comparison
        For example:
           if errors.Is(err, errMyOwnErrReference) {
             ...
           }
        
        pkg/workload/tpcc/checks.go:314:36: use errors.Is instead of a direct comparison
        For example:
           if errors.Is(err, errMyOwnErrReference) {
             ...
           }
        
        pkg/workload/bench_test.go:42:10: unnecessary conversion
--- FAIL: TestLint (538.43s)
    --- FAIL: TestLint/TestRoachVet (538.37s)
FAIL
FAIL	github.com/cockroachdb/cockroach/pkg/testutils/lint	540.091s
FAIL
make: *** [Makefile:1145: lint] Error 1