From a8832aa284ea8c89bac0751be8cfe95884c82b9c Mon Sep 17 00:00:00 2001 From: Brandon Sprague Date: Mon, 8 Jan 2024 17:30:00 -0800 Subject: [PATCH] Enable nogo, fix assorted small issues (#126) --- BUILD.bazel | 8 +++++++ WORKSPACE | 5 +++- azure/azevents/azevents.go | 4 ++-- cmd/server/pactasrv/conv/conv_test.go | 6 ++--- db/sqldb/analysis_artifact_test.go | 2 +- db/sqldb/analysis_test.go | 2 +- db/sqldb/incomplete_upload_test.go | 4 ++-- db/sqldb/pacta_version_test.go | 2 +- db/sqldb/portfolio_group_test.go | 6 ++--- db/sqldb/portfolio_initiative_test.go | 14 ++++++------ db/sqldb/portfolio_test.go | 8 +++---- db/sqldb/user_test.go | 10 ++++---- nogo.json | 33 +++++++++++++++++++++++++++ pacta/enum_test.go | 2 +- 14 files changed, 75 insertions(+), 31 deletions(-) create mode 100644 nogo.json diff --git a/BUILD.bazel b/BUILD.bazel index a2f72ab..4f80e0a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,4 +1,5 @@ load("@bazel_gazelle//:def.bzl", "gazelle") +load("@io_bazel_rules_go//go:def.bzl", "TOOLS_NOGO", "nogo") # gazelle:resolve go github.com/RMI/pacta/openapi/pacta //openapi:pacta_generated # gazelle:prefix github.com/RMI/pacta @@ -15,3 +16,10 @@ gazelle( ], command = "update-repos", ) + +nogo( + name = "nogo", + config = ":nogo.json", + visibility = ["//visibility:public"], + deps = TOOLS_NOGO, +) diff --git a/WORKSPACE b/WORKSPACE index 8935a09..2cc6429 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -37,7 +37,10 @@ go_dependencies() oapi_dependencies() go_rules_dependencies() -go_register_toolchains(version = "1.20.6") +go_register_toolchains( + nogo = "@//:nogo", + version = "1.20.6" +) gazelle_dependencies() http_archive( diff --git a/azure/azevents/azevents.go b/azure/azevents/azevents.go index 87f99ee..5bcc75d 100644 --- a/azure/azevents/azevents.go +++ b/azure/azevents/azevents.go @@ -293,13 +293,13 @@ func (s *Server) RegisterHandlers(r chi.Router) { } portfolioIDs = append(portfolioIDs, portfolioID) } - for i, iu := range incompleteUploads { + for id, iu := range incompleteUploads { err := s.db.UpdateIncompleteUpload( tx, iu.ID, db.SetIncompleteUploadCompletedAt(now)) if err != nil { - return fmt.Errorf("updating incomplete upload %d: %w", i, err) + return fmt.Errorf("updating incomplete upload %q: %w", id, err) } } return nil diff --git a/cmd/server/pactasrv/conv/conv_test.go b/cmd/server/pactasrv/conv/conv_test.go index aa7f16d..a06d113 100644 --- a/cmd/server/pactasrv/conv/conv_test.go +++ b/cmd/server/pactasrv/conv/conv_test.go @@ -26,14 +26,14 @@ func testEnumConvertability[A comparable, B any](t *testing.T, as []A, aToB func for _, a := range as { b, err := aToB(a) if err != nil { - t.Fatalf("converting from %T %q: %w", a, a, err) + t.Fatalf("converting from %T %v: %v", a, a, err) } a2, err := bToA(b) if err != nil { - t.Fatalf("converting from %T %q: %w", b, b, err) + t.Fatalf("converting from %T %v: %v", b, b, err) } if a != a2 { - t.Errorf("conversion from %T %q to %T %q and back failed, returned %q", a, a, b, b, a2) + t.Errorf("conversion from %T %v to %T %v and back failed, returned %v", a, a, b, b, a2) } } } diff --git a/db/sqldb/analysis_artifact_test.go b/db/sqldb/analysis_artifact_test.go index 91e16b2..a4270cb 100644 --- a/db/sqldb/analysis_artifact_test.go +++ b/db/sqldb/analysis_artifact_test.go @@ -136,7 +136,7 @@ func TestAnalysisArtifacts(t *testing.T) { _, err = tdb.BlobContexts(tx, []pacta.BlobID{b1.ID, b2.ID, b3.ID}) if err == nil { - t.Fatalf("reading blob owners should have failed but was fine", err) + t.Fatal("reading blob owners should have failed but was fine") } } diff --git a/db/sqldb/analysis_test.go b/db/sqldb/analysis_test.go index ec6f74c..b40ac0f 100644 --- a/db/sqldb/analysis_test.go +++ b/db/sqldb/analysis_test.go @@ -53,7 +53,7 @@ func TestAnalysisCRUD(t *testing.T) { ius, err := tdb.Analyses(tx, []pacta.AnalysisID{iu.ID, iu.ID, "nonsense"}) if err != nil { - t.Fatalf("reading analysiss: %w", err) + t.Fatalf("reading analysiss: %v", err) } if diff := cmp.Diff(map[pacta.AnalysisID]*pacta.Analysis{iu.ID: iu}, ius, cmpOpts); diff != "" { t.Fatalf("analysis mismatch (-want +got):\n%s", diff) diff --git a/db/sqldb/incomplete_upload_test.go b/db/sqldb/incomplete_upload_test.go index 0003f4c..5298087 100644 --- a/db/sqldb/incomplete_upload_test.go +++ b/db/sqldb/incomplete_upload_test.go @@ -46,7 +46,7 @@ func TestIncompleteUploadCRUD(t *testing.T) { ius, err := tdb.IncompleteUploads(tx, []pacta.IncompleteUploadID{iu.ID, iu.ID, "nonsense"}) if err != nil { - t.Fatalf("reading incomplete_uploads: %w", err) + t.Fatalf("reading incomplete_uploads: %v", err) } if diff := cmp.Diff(map[pacta.IncompleteUploadID]*pacta.IncompleteUpload{iu.ID: iu}, ius, cmpOpts); diff != "" { t.Fatalf("incomplete_upload mismatch (-want +got):\n%s", diff) @@ -131,7 +131,7 @@ func TestIncompleteUploadCRUD(t *testing.T) { _, err = tdb.BlobContexts(tx, []pacta.BlobID{b.ID}) if err == nil { - t.Fatalf("reading blob owners should have failed but was fine", err) + t.Fatal("reading blob owners should have failed but was fine") } } diff --git a/db/sqldb/pacta_version_test.go b/db/sqldb/pacta_version_test.go index fdcc3a3..9a9f276 100644 --- a/db/sqldb/pacta_version_test.go +++ b/db/sqldb/pacta_version_test.go @@ -196,7 +196,7 @@ func TestPACTAVersionDeletion(t *testing.T) { // Read by id list m, err := tdb.PACTAVersions(tx) if err != nil { - t.Fatalf("getting pacta_versions: %w", err) + t.Fatalf("getting pacta_versions: %v", err) } if diff := cmp.Diff(m, []*pacta.PACTAVersion{}, pactaVersionCmpOpts()); diff != "" { t.Fatalf("unexpected diff (-want +got)\n%s", diff) diff --git a/db/sqldb/portfolio_group_test.go b/db/sqldb/portfolio_group_test.go index 2ab7ab3..2c778d3 100644 --- a/db/sqldb/portfolio_group_test.go +++ b/db/sqldb/portfolio_group_test.go @@ -28,7 +28,7 @@ func TestPortfolioGroupCRUD(t *testing.T) { } pgID1, err := tdb.CreatePortfolioGroup(tx, pg1) if err != nil { - t.Fatalf("creating portfolio group: %w", err) + t.Fatalf("creating portfolio group: %v", err) } pg1.CreatedAt = time.Now() pg1.ID = pgID1 @@ -39,7 +39,7 @@ func TestPortfolioGroupCRUD(t *testing.T) { } pgID2, err := tdb.CreatePortfolioGroup(tx, pg2) if err != nil { - t.Fatalf("creating portfolio group: %w", err) + t.Fatalf("creating portfolio group: %v", err) } pg2.CreatedAt = time.Now() pg2.ID = pgID2 @@ -249,7 +249,7 @@ func portfolioGroupForTesting(t *testing.T, tdb *DB, owner *pacta.Owner) *pacta. } pgID, err := tdb.CreatePortfolioGroup(tx, pg) if err != nil { - t.Fatalf("creating portfolio group: %w", err) + t.Fatalf("creating portfolio group: %v", err) } pg.ID = pgID pg.CreatedAt = time.Now() diff --git a/db/sqldb/portfolio_initiative_test.go b/db/sqldb/portfolio_initiative_test.go index 5416669..82807f7 100644 --- a/db/sqldb/portfolio_initiative_test.go +++ b/db/sqldb/portfolio_initiative_test.go @@ -29,7 +29,7 @@ func TestCreatePortfolioInitiativeMembership(t *testing.T) { } err := tdb.CreatePortfolioInitiativeMembership(tx, pimA1) if err != nil { - t.Fatalf("creating pim: %w", err) + t.Fatalf("creating pim: %v", err) } pimA1.CreatedAt = time.Now() @@ -40,7 +40,7 @@ func TestCreatePortfolioInitiativeMembership(t *testing.T) { } err = tdb.CreatePortfolioInitiativeMembership(tx, pimA2) if err != nil { - t.Fatalf("creating pim: %w", err) + t.Fatalf("creating pim: %v", err) } pimA2.CreatedAt = time.Now() @@ -51,13 +51,13 @@ func TestCreatePortfolioInitiativeMembership(t *testing.T) { } err = tdb.CreatePortfolioInitiativeMembership(tx, pimB2) if err != nil { - t.Fatalf("creating pim: %w", err) + t.Fatalf("creating pim: %v", err) } pimB2.CreatedAt = time.Now() actual, err := tdb.PortfolioInitiativeMembershipsByPortfolio(tx, p2.ID) if err != nil { - t.Fatalf("getting pims: %w", err) + t.Fatalf("getting pims: %v", err) } if diff := cmp.Diff([]*pacta.PortfolioInitiativeMembership{pimA2, pimB2}, actual, cmpOpts); diff != "" { t.Errorf("unexpected diff (-want +got)\n%s", diff) @@ -65,7 +65,7 @@ func TestCreatePortfolioInitiativeMembership(t *testing.T) { actual, err = tdb.PortfolioInitiativeMembershipsByInitiative(tx, iA.ID) if err != nil { - t.Fatalf("getting pims: %w", err) + t.Fatalf("getting pims: %v", err) } if diff := cmp.Diff([]*pacta.PortfolioInitiativeMembership{pimA1, pimA2}, actual, cmpOpts); diff != "" { t.Errorf("unexpected diff (-want +got)\n%s", diff) @@ -73,12 +73,12 @@ func TestCreatePortfolioInitiativeMembership(t *testing.T) { err = tdb.DeletePortfolioInitiativeMembership(tx, p1.ID, iA.ID) if err != nil { - t.Fatalf("deleting pim: %w", err) + t.Fatalf("deleting pim: %v", err) } actual, err = tdb.PortfolioInitiativeMembershipsByPortfolio(tx, p1.ID) if err != nil { - t.Fatalf("getting pims: %w", err) + t.Fatalf("getting pims: %v", err) } if diff := cmp.Diff([]*pacta.PortfolioInitiativeMembership{}, actual, cmpOpts); diff != "" { t.Errorf("unexpected diff (-want +got)\n%s", diff) diff --git a/db/sqldb/portfolio_test.go b/db/sqldb/portfolio_test.go index d595dd3..01f0177 100644 --- a/db/sqldb/portfolio_test.go +++ b/db/sqldb/portfolio_test.go @@ -46,7 +46,7 @@ func TestPortfolioCRUD(t *testing.T) { ps, err := tdb.Portfolios(tx, []pacta.PortfolioID{p.ID, p.ID}) if err != nil { - t.Fatalf("reading portfolios: %w", err) + t.Fatalf("reading portfolios: %v", err) } if diff := cmp.Diff(map[pacta.PortfolioID]*pacta.Portfolio{p.ID: p}, ps, portfolioCmpOpts()); diff != "" { t.Fatalf("portfolio mismatch (-want +got):\n%s", diff) @@ -82,14 +82,14 @@ func TestPortfolioCRUD(t *testing.T) { } pls, err := tdb.PortfoliosByOwner(tx, o2.ID) if err != nil { - t.Fatalf("reading portfolios: %w", err) + t.Fatalf("reading portfolios: %v", err) } if diff := cmp.Diff([]*pacta.Portfolio{p}, pls, portfolioCmpOpts()); diff != "" { t.Fatalf("portfolio mismatch (-want +got):\n%s", diff) } pls, err = tdb.PortfoliosByOwner(tx, o1.ID) if err != nil { - t.Fatalf("reading portfolios: %w", err) + t.Fatalf("reading portfolios: %v", err) } if diff := cmp.Diff([]*pacta.Portfolio{}, pls, portfolioCmpOpts()); diff != "" { t.Fatalf("portfolio mismatch (-want +got):\n%s", diff) @@ -120,7 +120,7 @@ func TestPortfolioCRUD(t *testing.T) { _, err = tdb.BlobContexts(tx, []pacta.BlobID{b.ID}) if err == nil { - t.Fatalf("reading blob owners should have failed but was fine", err) + t.Fatal("reading blob owners should have failed but was fine") } } diff --git a/db/sqldb/user_test.go b/db/sqldb/user_test.go index abb18dd..22054c9 100644 --- a/db/sqldb/user_test.go +++ b/db/sqldb/user_test.go @@ -12,7 +12,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" ) -func TestcreateUser(t *testing.T) { +func TestCreateUser(t *testing.T) { ctx := context.Background() tdb := createDBForTesting(t) tx := tdb.NoTxn(ctx) @@ -42,7 +42,7 @@ func TestcreateUser(t *testing.T) { // Read by Authn actual, err = tdb.UserByAuthn(tx, u.AuthnMechanism, u.AuthnID) if err != nil { - t.Fatalf("getting user by authn: %w", err) + t.Fatalf("getting user by authn: %v", err) } if diff := cmp.Diff(u, actual, userCmpOpts()); diff != "" { t.Fatalf("unexpected diff (-want +got)\n%s", diff) @@ -51,7 +51,7 @@ func TestcreateUser(t *testing.T) { // Read by id list aMap, err := tdb.Users(tx, []pacta.UserID{"somenonsense", userID}) if err != nil { - t.Fatalf("getting users: %w", err) + t.Fatalf("getting users: %v", err) } eMap := map[pacta.UserID]*pacta.User{userID: u} if diff := cmp.Diff(eMap, aMap, userCmpOpts()); diff != "" { @@ -93,7 +93,7 @@ func TestcreateUser(t *testing.T) { u5.CanonicalEmail = "canonical email 5" _, err = tdb.createUser(tx, u5) if err != nil { - t.Fatal("expected success but got: %w", err) + t.Fatalf("expected success but got: %v", err) } } @@ -238,7 +238,7 @@ func TestDeleteUser(t *testing.T) { // Read by id list aMap, err := tdb.Users(tx, []pacta.UserID{"somenonsense", userID, "something else"}) if err != nil { - t.Fatalf("getting users: %w", err) + t.Fatalf("getting users: %v", err) } eMap := map[pacta.UserID]*pacta.User{} if diff := cmp.Diff(eMap, aMap, userCmpOpts()); diff != "" { diff --git a/nogo.json b/nogo.json new file mode 100644 index 0000000..598491a --- /dev/null +++ b/nogo.json @@ -0,0 +1,33 @@ +{ + "composites": { + "exclude_files": { + "external/org_golang_google_protobuf/internal/impl/.*": "allow unkeyed fields on offending third-party code" + } + }, + "copylocks": { + "exclude_files": { + "external/org_golang_google_protobuf/internal/impl/.*": "disable copylocks check on offending third-party code" + } + }, + "nilness": { + "exclude_files": { + "external/.*": "enforce no nilness only on first-party code" + } + }, + "shadow": { + "exclude_files": { + ".*": "disable the shadowing check on all code, because it's fine" + } + }, + "unreachable": { + "exclude_files": { + "external/.*": "enforce no unreachable paths only on first-party code" + } + }, + "unsafeptr": { + "exclude_files": { + "external/org_golang_x_sys/unix/.*": "disable unsafe pointer check on offending third-party code", + "external/com_github_mailru_easyjson/jlexer/.*": "disable unsafe pointer check on offending third-party code" + } + } +} diff --git a/pacta/enum_test.go b/pacta/enum_test.go index 6c4411f..027411f 100644 --- a/pacta/enum_test.go +++ b/pacta/enum_test.go @@ -23,7 +23,7 @@ func TestParseFileType(t *testing.T) { for _, c := range otherCases { ft, err := ParseFileType(filepath.Ext(c)) if err != nil { - t.Errorf("expected successful parse, got %w", err) + t.Errorf("expected successful parse, got %v", err) } if ft != FileType_JSON { t.Errorf("expected JSON, got %v", ft)