Disable some static analysis checks for coverage builds (#6105)

* Disable some static analysis checks for coverage builds
* Merge branch 'master' into fix-coverage-builds
* disable lostcancel for third_party code
* Merge branch 'fix-coverage-builds' of github.com:prysmaticlabs/prysm into fix-coverage-builds
* Merge refs/heads/master into fix-coverage-builds
* A few lostcancel fixes
* Merge branch 'fix-coverage-builds' of github.com:prysmaticlabs/prysm into fix-coverage-builds
* Merge refs/heads/master into fix-coverage-builds

.bazelrc

@@ -5,6 +5,9 @@ test --test_verbose_timeout_warnings
 test --build_tests_only
 test --test_output=errors
 
+# Clearly indicate that coverage is enabled to disable certain nogo checks.
+coverage --define=coverage_enabled=1
+
 # Fix for rules_docker. See: https://github.com/bazelbuild/rules_docker/issues/842
 build --host_force_python=PY2
 test --host_force_python=PY2

BUILD.bazel

@@ -88,15 +88,12 @@ nogo(
         "@org_golang_x_tools//go/analysis/passes/pkgfact:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/nilness:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/nilfunc:go_tool_library",
-        # lost cancel ignore doesn't seem to work when running with coverage
-        #"@org_golang_x_tools//go/analysis/passes/lostcancel:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/loopclosure:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/httpresponse:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/findcall:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/deepequalerrors:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/ctrlflow:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/copylock:go_tool_library",
-        "@org_golang_x_tools//go/analysis/passes/composite:go_tool_library",
         #  "@org_golang_x_tools//go/analysis/passes/cgocall:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/buildtag:go_tool_library",
         "@org_golang_x_tools//go/analysis/passes/buildssa:go_tool_library",
@@ -110,7 +107,19 @@ nogo(
         "//tools/analyzers/roughtime:go_tool_library",
         "//tools/analyzers/errcheck:go_tool_library",
         "//tools/analyzers/featureconfig:go_tool_library",
-    ],
+    ] + select({
+        # nogo checks that fail with coverage enabled.
+        ":coverage_enabled": [],
+        "//conditions:default": [
+            "@org_golang_x_tools//go/analysis/passes/lostcancel:go_tool_library",
+            "@org_golang_x_tools//go/analysis/passes/composite:go_tool_library",
+        ],
+    }),
+)
+
+config_setting(
+    name = "coverage_enabled",
+    values = {"define": "coverage_enabled=1"},
 )
 
 common_files = {

beacon-chain/p2p/service.go

@@ -94,6 +94,7 @@ type Service struct {
 func NewService(cfg *Config) (*Service, error) {
 	var err error
 	ctx, cancel := context.WithCancel(context.Background())
+	_ = cancel // govet fix for lost cancel. Cancel is handled in service.Stop().
 	cache, err := ristretto.NewCache(&ristretto.Config{
 		NumCounters: 1000,
 		MaxCost:     1000,

beacon-chain/powchain/service.go

@@ -172,6 +172,7 @@ func NewService(ctx context.Context, config *Web3ServiceConfig) (*Service, error
 		)
 	}
 	ctx, cancel := context.WithCancel(ctx)
+	_ = cancel // govet fix for lost cancel. Cancel is handled in service.Stop()
 	depositTrie, err := trieutil.NewTrie(int(params.BeaconConfig().DepositContractTreeDepth))
 	if err != nil {
 		cancel()

beacon-chain/sync/initial-sync/blocks_fetcher_test.go

@@ -436,7 +436,8 @@ func TestBlocksFetcherHandleRequest(t *testing.T) {
 			p2p:         p2p,
 		})
 
-		requestCtx, _ := context.WithTimeout(context.Background(), 2*time.Second)
+		requestCtx, reqCancel := context.WithTimeout(context.Background(), 2*time.Second)
+		defer reqCancel()
 		go func() {
 			response := fetcher.handleRequest(requestCtx, 1 /* start */, blockBatchLimit /* count */)
 			select {
@@ -529,7 +530,8 @@ func TestBlocksFetcherRequestBeaconBlocksByRangeRequest(t *testing.T) {
 
 		// Test request fail over (error).
 		err = fetcher.p2p.Disconnect(peers[1])
-		ctx, _ = context.WithTimeout(context.Background(), time.Second*1)
+		ctx, cancel = context.WithTimeout(context.Background(), time.Second*1)
+		defer cancel()
 		blocks, err = fetcher.requestBeaconBlocksByRange(ctx, peers[1], root, 1, 1, blockBatchLimit)
 		testutil.AssertLogsContain(t, hook, "Request failed, trying to forward request to another peer")
 		if err == nil || err.Error() != "context deadline exceeded" {

beacon-chain/sync/subscriber.go

@@ -114,7 +114,8 @@ func (r *Service) subscribeWithBase(base proto.Message, topic string, validator
 	// Pipeline decodes the incoming subscription data, runs the validation, and handles the
 	// message.
 	pipeline := func(msg *pubsub.Message) {
-		ctx, _ := context.WithTimeout(context.Background(), pubsubMessageTimeout)
+		ctx, cancel := context.WithTimeout(context.Background(), pubsubMessageTimeout)
+		defer cancel()
 		ctx, span := trace.StartSpan(ctx, "sync.pubsub")
 		defer span.End()
 
@@ -169,7 +170,8 @@ func (r *Service) subscribeWithBase(base proto.Message, topic string, validator
 func wrapAndReportValidation(topic string, v pubsub.ValidatorEx) (string, pubsub.ValidatorEx) {
 	return topic, func(ctx context.Context, pid peer.ID, msg *pubsub.Message) pubsub.ValidationResult {
 		defer messagehandler.HandlePanic(ctx, msg)
-		ctx, _ = context.WithTimeout(ctx, pubsubMessageTimeout)
+		ctx, cancel := context.WithTimeout(ctx, pubsubMessageTimeout)
+		defer cancel()
 		messageReceivedCounter.WithLabelValues(topic).Inc()
 		b := v(ctx, pid, msg)
 		if b == pubsub.ValidationReject {

beacon-chain/sync/validate_attester_slashing_test.go

@@ -134,7 +134,8 @@ func TestValidateAttesterSlashing_ContextTimeout(t *testing.T) {
 	slashing, state := setupValidAttesterSlashing(t)
 	slashing.Attestation_1.Data.Target.Epoch = 100000000
 
-	ctx, _ := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
 
 	c, err := lru.New(10)
 	if err != nil {

beacon-chain/sync/validate_proposer_slashing_test.go

@@ -170,7 +170,8 @@ func TestValidateProposerSlashing_ContextTimeout(t *testing.T) {
 	slashing, state := setupValidProposerSlashing(t)
 	slashing.Header_1.Header.Slot = 100000000
 
-	ctx, _ := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
 
 	c, err := lru.New(10)
 	if err != nil {

nogo_config.json

@@ -13,7 +13,8 @@
   },
   "lostcancel": {
     "exclude_files": {
-      "validator/client/runner.go": "No need to cancel right when goroutines begin"
+      "validator/client/runner.go": "No need to cancel right when goroutines begin",
+      "external/.*": "Third party code"
     }
   },
   "nilness": {
@@ -41,12 +42,12 @@
   "composites": {
     "exclude_files": {
       "external/.*": "Third party code"
-    } 
+    }
   }, 
   "cgocall": {
     "exclude_files": {
       "external/.*": "Third party code"
-    } 
+    }
   },
   "assign": {
     "exclude_files": {

slasher/beaconclient/service.go

@@ -75,6 +75,7 @@ type Config struct {
 // NewBeaconClientService instantiation.
 func NewBeaconClientService(ctx context.Context, cfg *Config) (*Service, error) {
 	ctx, cancel := context.WithCancel(ctx)
+	_ = cancel // govet fix for lost cancel. Cancel is handled in service.Stop()
 	publicKeyCache, err := cache.NewPublicKeyCache(0, nil)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not create new cache")