Enable nogo, fix assorted small issues (#126)

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,
+)

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(

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

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)
 		}
 	}
 }

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")
 	}
 }
 

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)

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")
 	}
 }
 

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)

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()

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,34 +51,34 @@ 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)
 	}
 
 	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)
 	}
 
 	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)

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")
 	}
 }
 

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 != "" {

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"
+    }
+  }
+}

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)