From 67cbf774f58da072c484639afa8c410e86c29fae Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Sun, 21 Apr 2019 10:14:34 -0400 Subject: [PATCH] Add nogo to introduce built time linting (#2317) * Add nogo and fix lint issues * Run gazelle * better gazelle * ignore external struct tags --- BUILD.bazel | 38 ++++++++++++++ WORKSPACE | 4 +- .../backend/simulated_backend_test.go | 2 +- beacon-chain/powchain/service.go | 1 + nogo_config.json | 50 +++++++++++++++++++ shared/p2p/monitoring.go | 3 +- shared/p2p/service.go | 3 +- shared/sliceutil/slice_generic_test.go | 22 ++++---- tools/cluster-pk-manager/server/db.go | 2 - validator/client/runner.go | 4 +- validator/client/service.go | 1 + 11 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 nogo_config.json diff --git a/BUILD.bazel b/BUILD.bazel index 83ac23794..1dd28f288 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -2,6 +2,7 @@ load("@bazel_gazelle//:def.bzl", "gazelle") load("@com_github_atlassian_bazel_tools//gometalinter:def.bzl", "gometalinter") load("@com_github_atlassian_bazel_tools//goimports:def.bzl", "goimports") load("@io_kubernetes_build//defs:run_in_workspace.bzl", "workspace_binary") +load("@io_bazel_rules_go//go:def.bzl", "nogo") prefix = "github.com/prysmaticlabs/prysm" @@ -51,3 +52,40 @@ workspace_binary( name = "golint", cmd = "@com_github_golang_lint//golint", ) + +nogo( + name = "nogo", + deps = [ + "@org_golang_x_tools//go/analysis/passes/unsafeptr:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/unreachable:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/unmarshal:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/tests:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/structtag:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/stdmethods:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/shift:go_tool_library", + # "@org_golang_x_tools//go/analysis/passes/shadow:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/printf:go_tool_library", + "@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", + "@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", + "@org_golang_x_tools//go/analysis/passes/bools:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/atomicalign:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/atomic:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/assign:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/asmdecl:go_tool_library", + ], + visibility = ["//visibility:public"], + config = "nogo_config.json", +) diff --git a/WORKSPACE b/WORKSPACE index 47e68085f..18530ee25 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -92,7 +92,7 @@ load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_to go_rules_dependencies() -go_register_toolchains() +go_register_toolchains(nogo = "@//:nogo") load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") @@ -562,7 +562,7 @@ go_repository( go_repository( name = "com_github_jackpal_go_nat_pmp", - commit = "b977c5efcadd604e306a97fdf06ff544f785df56", # v1.0.1 + commit = "d89d09f6f3329bc3c2479aa3cafd76a5aa93a35c", importpath = "github.com/jackpal/go-nat-pmp", ) diff --git a/beacon-chain/chaintest/backend/simulated_backend_test.go b/beacon-chain/chaintest/backend/simulated_backend_test.go index 24359a4d3..13c05c57c 100644 --- a/beacon-chain/chaintest/backend/simulated_backend_test.go +++ b/beacon-chain/chaintest/backend/simulated_backend_test.go @@ -47,7 +47,7 @@ func TestGenerateBlockAndAdvanceChain_IncreasesSlot(t *testing.T) { t.Fatalf("Could not generate block and transition state successfully %v for slot %d", err, backend.state.Slot+1) } if backend.inMemoryBlocks[len(backend.inMemoryBlocks)-1].Slot != backend.state.Slot { - t.Errorf("In memory Blocks do not have the same last slot as the state, expected %d but got %d", + t.Errorf("In memory Blocks do not have the same last slot as the state, expected %d but got %v", backend.state.Slot, backend.inMemoryBlocks[len(backend.inMemoryBlocks)-1]) } } diff --git a/beacon-chain/powchain/service.go b/beacon-chain/powchain/service.go index d03c74c18..26611876a 100644 --- a/beacon-chain/powchain/service.go +++ b/beacon-chain/powchain/service.go @@ -129,6 +129,7 @@ func NewWeb3Service(ctx context.Context, config *Web3ServiceConfig) (*Web3Servic ctx, cancel := context.WithCancel(ctx) depositTrie, err := trieutil.GenerateTrieFromItems([][]byte{{}}, int(params.BeaconConfig().DepositContractTreeDepth)) if err != nil { + cancel() return nil, fmt.Errorf("could not setup deposit trie: %v", err) } return &Web3Service{ diff --git a/nogo_config.json b/nogo_config.json new file mode 100644 index 000000000..032b0f408 --- /dev/null +++ b/nogo_config.json @@ -0,0 +1,50 @@ +{ + "unsafeptr": { + "exclude_files": { + "external/com_github_gxed_hashland/murmur3/*": "Unsafe third party code", + "external/io_k8s_apimachinery/vendor/github.com/modern-go/reflect2/reflect/*": "Unsafe third party code" + } + }, + "unreachable": { + "exclude_files": { + "shared/messagehandler/messagehandler_test.go": "Necessary panic before return for test", + "external/*": "Unreachable third party code" + } + }, + "nilness": { + "exclude_files": { + "external/*": "Third party code" + } + }, + "stdmethods": { + "exclude_files": { + "external/*": "Third party code" + } + }, + "copylocks": { + "exclude_files": { + "external/*": "Third party code" + } + }, + "composites": { + "exclude_files": { + "external/*": "Third party code" + } + }, + "cgocall": { + "exclude_files": { + "external/*": "Third party code" + } + }, + "assign": { + "exclude_files": { + "external/*": "Third party code" + } + }, + "structtag": { + "exclude_files": { + "external/*": "Third party code" + } + + } +} diff --git a/shared/p2p/monitoring.go b/shared/p2p/monitoring.go index cf3a6ddf5..2251bab32 100644 --- a/shared/p2p/monitoring.go +++ b/shared/p2p/monitoring.go @@ -62,7 +62,8 @@ func ensurePeerConnections(ctx context.Context, h host.Host, peers ...string) { c := h.Network().ConnsToPeer(peer.ID) if len(c) == 0 { log.WithField("peer", peer.ID).Debug("No connections to peer, reconnecting") - ctx, _ := context.WithTimeout(ctx, 30*time.Second) + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() if err := h.Connect(ctx, *peer); err != nil { log.Errorf("Failed to reconnect to peer %v", err) continue diff --git a/shared/p2p/service.go b/shared/p2p/service.go index 321607528..08ffbe024 100644 --- a/shared/p2p/service.go +++ b/shared/p2p/service.go @@ -343,7 +343,8 @@ func (s *Server) Send(ctx context.Context, msg proto.Message, peerID peer.ID) er ctx, span := trace.StartSpan(ctx, "p2p.Send") defer span.End() - ctx, _ = context.WithTimeout(ctx, 30*time.Second) + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() topic := s.topicMapping[messageType(msg)] pid := protocol.ID(prysmProtocolPrefix + "/" + topic) diff --git a/shared/sliceutil/slice_generic_test.go b/shared/sliceutil/slice_generic_test.go index 8da18584c..586232248 100644 --- a/shared/sliceutil/slice_generic_test.go +++ b/shared/sliceutil/slice_generic_test.go @@ -27,7 +27,7 @@ func TestGenericIntersection(t *testing.T) { result, err := GenericIntersection(tt.setA, tt.setB) if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %d", result, tt.out) } } @@ -61,7 +61,7 @@ func TestGenericIntersectionWithSSZ(t *testing.T) { result, err := GenericIntersection(b1.Bytes(), b2.Bytes()) if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %d", result, tt.out) } } } @@ -90,7 +90,7 @@ func TestFloatGenericIntersection(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %v", result, tt.out) } } @@ -112,7 +112,7 @@ func TestStringGenericIntersection(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %v", result, tt.out) } } @@ -140,7 +140,7 @@ func TestIntGenericIntersection(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %d", result, tt.out) } } @@ -167,7 +167,7 @@ func TestGenericNot(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %d", result, tt.out) } } @@ -193,7 +193,7 @@ func TestFloatGenericNot(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %v", result, tt.out) } } @@ -214,7 +214,7 @@ func TestStringGenericNot(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %v", result, tt.out) } } @@ -240,7 +240,7 @@ func TestIntGenericNot(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %d", result, tt.out) } } @@ -266,7 +266,7 @@ func TestGenericUnion(t *testing.T) { if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %d", result, tt.out) } } @@ -292,7 +292,7 @@ func TestFloatGenericUnion(t *testing.T) { result, err := GenericUnion(tt.setA, tt.setB) if err != nil { if !reflect.DeepEqual(result, tt.out) { - t.Errorf("got %d, want %d", result, tt.out) + t.Errorf("got %v, want %v", result, tt.out) } } diff --git a/tools/cluster-pk-manager/server/db.go b/tools/cluster-pk-manager/server/db.go index 25b075e5c..48905c03d 100644 --- a/tools/cluster-pk-manager/server/db.go +++ b/tools/cluster-pk-manager/server/db.go @@ -181,8 +181,6 @@ func (d *db) AssignExistingPKs(_ context.Context, pks *pb.PrivateKeys, podName s } return tx.Bucket(assignedPkBucket).Put([]byte(podName), data) }) - - return nil } // AllocatedPodNames returns the string list of pod names with current private diff --git a/validator/client/runner.go b/validator/client/runner.go index c419cabde..83a804984 100644 --- a/validator/client/runner.go +++ b/validator/client/runner.go @@ -62,7 +62,7 @@ func run(ctx context.Context, v Validator) { return // Exit if context is canceled. case slot := <-v.NextSlot(): span.AddAttributes(trace.Int64Attribute("slot", int64(slot))) - slotCtx, _ := context.WithDeadline(ctx, v.SlotDeadline(slot)) + slotCtx, cancel := context.WithDeadline(ctx, v.SlotDeadline(slot)) // Report this validator client's rewards and penalties throughout its lifecycle. if err := v.LogValidatorGainsAndLosses(slotCtx, slot); err != nil { log.Errorf("Could not report validator's rewards/penalties for slot %d: %v", @@ -73,6 +73,7 @@ func run(ctx context.Context, v Validator) { // epoch transition in the beacon node's state. if err := v.UpdateAssignments(slotCtx, slot); err != nil { handleAssignmentError(err, slot) + cancel() continue } for id, role := range v.RolesAt(slot) { @@ -99,6 +100,7 @@ func run(ctx context.Context, v Validator) { }(role, id) } + cancel() } } } diff --git a/validator/client/service.go b/validator/client/service.go index 2c9a92917..5d467763c 100644 --- a/validator/client/service.go +++ b/validator/client/service.go @@ -46,6 +46,7 @@ func NewValidatorService(ctx context.Context, cfg *Config) (*ValidatorService, e ks := keystore.NewKeystore(cfg.KeystorePath) keys, err := ks.GetKeys(validatorFolder, validatorPrefix, cfg.Password) if err != nil { + cancel() return nil, fmt.Errorf("could not get private key: %v", err) } var key *keystore.Key