Fixup staticcheck and stylecheck, and violations thereof (letsencrypt…

…#5897)

Add `stylecheck` to our list of lints, since it got separated out from
`staticcheck`. Fix the way we configure both to be clearer and not
rely on regexes.

Additionally fix a number of easy-to-change `staticcheck` and
`stylecheck` violations, allowing us to reduce our number of ignored
checks.

Part of letsencrypt#5681

.golangci.yml

@@ -4,13 +4,27 @@ linters:
     - errcheck
     - gofmt
     - gosec
+    - gosimple
     - govet
     - ineffassign
     - misspell
     - staticcheck
+    - stylecheck
+    - unused
 linters-settings:
   errcheck:
     ignore: fmt:[FS]?[Pp]rint*,io:Write,os:Remove,net/http:Write,github.com/miekg/dns:WriteMsg,net:Write,encoding/binary:Write
+  gosimple:
+    # S1029: Range over the string directly
+    checks: ["all", "-S1029"]
+  staticcheck:
+    # SA1019: Using a deprecated function, variable, constant or field
+    # SA6003: Converting a string to a slice of runes before ranging over it
+    checks: ["all", "-SA1019", "-SA6003"]
+  stylecheck:
+    # ST1003: Poorly chosen identifier
+    # ST1005: Incorrectly formatted error string
+    checks: ["all", "-ST1003", "-ST1005"]
 issues:
   exclude-rules:
     - linters:
@@ -29,6 +43,3 @@ issues:
       # G501: Blacklisted import `crypto/md5`: weak cryptographic primitive
       # G505: Blacklisted import `crypto/sha1`: weak cryptographic primitive
       text: "G(101|102|107|201|202|306|401|402|403|404|501|505)"
-    - linters:
-      - staticcheck
-      text: "(SA1019|ST1005|ST1013|SA6003|SA5011|S1029|SA2002):"

akamai/cache-client.go

@@ -96,9 +96,7 @@ func NewCachePurgeClient(
 	}, []string{"type"})
 	stats.MustRegister(purges)
 
-	if strings.HasSuffix(endpoint, "/") {
-		endpoint = endpoint[:len(endpoint)-1]
-	}
+	endpoint = strings.TrimSuffix(endpoint, "/")
 	apiURL, err := url.Parse(endpoint)
 	if err != nil {
 		return nil, err

ca/ocsp.go

@@ -206,7 +206,7 @@ func newOCSPLogQueue(
 		wg:        sync.WaitGroup{},
 		depth:     depth,
 		logger:    logger,
-		clk:       clock.Default(),
+		clk:       clock.New(),
 	}
 	olq.wg.Add(1)
 	return &olq

canceled/canceled.go

@@ -3,14 +3,14 @@ package canceled
 import (
 	"context"
 
-	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // Is returns true if err is non-nil and is either context.Canceled, or has a
 // grpc code of Canceled. This is useful because cancellations propagate through
 // gRPC boundaries, and if we choose to treat in-process cancellations a certain
 // way, we usually want to treat cross-process cancellations the same way.
 func Is(err error) bool {
-	return err == context.Canceled || grpc.Code(err) == codes.Canceled
+	return err == context.Canceled || status.Code(err) == codes.Canceled
 }

canceled/canceled_test.go

@@ -5,15 +5,15 @@ import (
 	"errors"
 	"testing"
 
-	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestCanceled(t *testing.T) {
 	if !Is(context.Canceled) {
 		t.Errorf("Expected context.Canceled to be canceled, but wasn't.")
 	}
-	if !Is(grpc.Errorf(codes.Canceled, "hi")) {
+	if !Is(status.Errorf(codes.Canceled, "hi")) {
 		t.Errorf("Expected gRPC cancellation to be cancelled, but wasn't.")
 	}
 	if Is(errors.New("hi")) {

cmd/boulder-wfe2/main.go

@@ -18,7 +18,6 @@ import (
 	"github.com/letsencrypt/boulder/cmd"
 	"github.com/letsencrypt/boulder/features"
 	"github.com/letsencrypt/boulder/goodkey"
-	"github.com/letsencrypt/boulder/grpc"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
 	"github.com/letsencrypt/boulder/issuance"
 	blog "github.com/letsencrypt/boulder/log"
@@ -286,23 +285,23 @@ func setupWFE(c Config, logger blog.Logger, stats prometheus.Registerer, clk clo
 	tlsConfig, err := c.WFE.TLS.Load()
 	cmd.FailOnError(err, "TLS config")
 	clientMetrics := bgrpc.NewClientMetrics(stats)
-	raConn, err := bgrpc.ClientSetup(c.WFE.RAService, tlsConfig, clientMetrics, clk, grpc.CancelTo408Interceptor)
+	raConn, err := bgrpc.ClientSetup(c.WFE.RAService, tlsConfig, clientMetrics, clk, bgrpc.CancelTo408Interceptor)
 	cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to RA")
 	rac := rapb.NewRegistrationAuthorityClient(raConn)
 
-	saConn, err := bgrpc.ClientSetup(c.WFE.SAService, tlsConfig, clientMetrics, clk, grpc.CancelTo408Interceptor)
+	saConn, err := bgrpc.ClientSetup(c.WFE.SAService, tlsConfig, clientMetrics, clk, bgrpc.CancelTo408Interceptor)
 	cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to SA")
 	sac := sapb.NewStorageAuthorityClient(saConn)
 
 	var rns noncepb.NonceServiceClient
 	npm := map[string]noncepb.NonceServiceClient{}
 	if c.WFE.GetNonceService != nil {
-		rnsConn, err := bgrpc.ClientSetup(c.WFE.GetNonceService, tlsConfig, clientMetrics, clk, grpc.CancelTo408Interceptor)
+		rnsConn, err := bgrpc.ClientSetup(c.WFE.GetNonceService, tlsConfig, clientMetrics, clk, bgrpc.CancelTo408Interceptor)
 		cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to get nonce service")
 		rns = noncepb.NewNonceServiceClient(rnsConn)
 		for prefix, serviceConfig := range c.WFE.RedeemNonceServices {
 			serviceConfig := serviceConfig
-			conn, err := bgrpc.ClientSetup(&serviceConfig, tlsConfig, clientMetrics, clk, grpc.CancelTo408Interceptor)
+			conn, err := bgrpc.ClientSetup(&serviceConfig, tlsConfig, clientMetrics, clk, bgrpc.CancelTo408Interceptor)
 			cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to redeem nonce service")
 			npm[prefix] = noncepb.NewNonceServiceClient(conn)
 		}

cmd/contact-auditor/main_test.go

@@ -130,7 +130,7 @@ type testCtx struct {
 	cleanUp func()
 }
 
-func (c testCtx) addRegistrations(t *testing.T) {
+func (tc testCtx) addRegistrations(t *testing.T) {
 	emailA := "mailto:" + emailARaw
 	emailB := "mailto:" + emailBRaw
 	emailC := "mailto:" + emailCRaw
@@ -189,17 +189,17 @@ func (c testCtx) addRegistrations(t *testing.T) {
 
 	// Add the four test registrations
 	ctx := context.Background()
-	regA, err = c.ssa.NewRegistration(ctx, regA)
+	regA, err = tc.ssa.NewRegistration(ctx, regA)
 	test.AssertNotError(t, err, "Couldn't store regA")
-	regB, err = c.ssa.NewRegistration(ctx, regB)
+	regB, err = tc.ssa.NewRegistration(ctx, regB)
 	test.AssertNotError(t, err, "Couldn't store regB")
-	regC, err = c.ssa.NewRegistration(ctx, regC)
+	regC, err = tc.ssa.NewRegistration(ctx, regC)
 	test.AssertNotError(t, err, "Couldn't store regC")
-	regD, err = c.ssa.NewRegistration(ctx, regD)
+	regD, err = tc.ssa.NewRegistration(ctx, regD)
 	test.AssertNotError(t, err, "Couldn't store regD")
 }
 
-func (ctx testCtx) addCertificates(t *testing.T) {
+func (tc testCtx) addCertificates(t *testing.T) {
 	serial1 := big.NewInt(1336)
 	serial1String := core.SerialToString(serial1)
 	serial2 := big.NewInt(1337)
@@ -238,9 +238,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertA.NotAfter,
 		DER:            certDerA,
 	}
-	err := ctx.dbMap.Insert(certA)
+	err := tc.dbMap.Insert(certA)
 	test.AssertNotError(t, err, "Couldn't add certA")
-	_, err = ctx.dbMap.Exec(
+	_, err = tc.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-a",
 		serial1String,
@@ -263,9 +263,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertB.NotAfter,
 		DER:            certDerB,
 	}
-	err = ctx.dbMap.Insert(certB)
+	err = tc.dbMap.Insert(certB)
 	test.AssertNotError(t, err, "Couldn't add certB")
-	_, err = ctx.dbMap.Exec(
+	_, err = tc.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-b",
 		serial2String,
@@ -288,9 +288,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertC.NotAfter,
 		DER:            certDerC,
 	}
-	err = ctx.dbMap.Insert(certC)
+	err = tc.dbMap.Insert(certC)
 	test.AssertNotError(t, err, "Couldn't add certC")
-	_, err = ctx.dbMap.Exec(
+	_, err = tc.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-c",
 		serial3String,
@@ -313,9 +313,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertD.NotAfter,
 		DER:            certDerD,
 	}
-	err = ctx.dbMap.Insert(certD)
+	err = tc.dbMap.Insert(certD)
 	test.AssertNotError(t, err, "Couldn't add certD")
-	_, err = ctx.dbMap.Exec(
+	_, err = tc.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-d",
 		serial4String,

cmd/id-exporter/main_test.go

@@ -244,7 +244,7 @@ type testCtx struct {
 	cleanUp func()
 }
 
-func (c testCtx) addRegistrations(t *testing.T) {
+func (tc testCtx) addRegistrations(t *testing.T) {
 	emailA := "mailto:" + emailARaw
 	emailB := "mailto:" + emailBRaw
 	emailC := "mailto:" + emailCRaw
@@ -304,17 +304,17 @@ func (c testCtx) addRegistrations(t *testing.T) {
 
 	// Add the four test registrations
 	ctx := context.Background()
-	regA, err = c.ssa.NewRegistration(ctx, regA)
+	regA, err = tc.ssa.NewRegistration(ctx, regA)
 	test.AssertNotError(t, err, "Couldn't store regA")
-	regB, err = c.ssa.NewRegistration(ctx, regB)
+	regB, err = tc.ssa.NewRegistration(ctx, regB)
 	test.AssertNotError(t, err, "Couldn't store regB")
-	regC, err = c.ssa.NewRegistration(ctx, regC)
+	regC, err = tc.ssa.NewRegistration(ctx, regC)
 	test.AssertNotError(t, err, "Couldn't store regC")
-	regD, err = c.ssa.NewRegistration(ctx, regD)
+	regD, err = tc.ssa.NewRegistration(ctx, regD)
 	test.AssertNotError(t, err, "Couldn't store regD")
 }
 
-func (ctx testCtx) addCertificates(t *testing.T) {
+func (tc testCtx) addCertificates(t *testing.T) {
 	serial1 := big.NewInt(1336)
 	serial1String := core.SerialToString(serial1)
 	serial2 := big.NewInt(1337)
@@ -353,9 +353,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertA.NotAfter,
 		DER:            certDerA,
 	}
-	err := ctx.c.dbMap.Insert(certA)
+	err := tc.c.dbMap.Insert(certA)
 	test.AssertNotError(t, err, "Couldn't add certA")
-	_, err = ctx.c.dbMap.Exec(
+	_, err = tc.c.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-a",
 		serial1String,
@@ -378,9 +378,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertB.NotAfter,
 		DER:            certDerB,
 	}
-	err = ctx.c.dbMap.Insert(certB)
+	err = tc.c.dbMap.Insert(certB)
 	test.AssertNotError(t, err, "Couldn't add certB")
-	_, err = ctx.c.dbMap.Exec(
+	_, err = tc.c.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-b",
 		serial2String,
@@ -403,9 +403,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertC.NotAfter,
 		DER:            certDerC,
 	}
-	err = ctx.c.dbMap.Insert(certC)
+	err = tc.c.dbMap.Insert(certC)
 	test.AssertNotError(t, err, "Couldn't add certC")
-	_, err = ctx.c.dbMap.Exec(
+	_, err = tc.c.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-c",
 		serial3String,
@@ -428,9 +428,9 @@ func (ctx testCtx) addCertificates(t *testing.T) {
 		Expires:        rawCertD.NotAfter,
 		DER:            certDerD,
 	}
-	err = ctx.c.dbMap.Insert(certD)
+	err = tc.c.dbMap.Insert(certD)
 	test.AssertNotError(t, err, "Couldn't add certD")
-	_, err = ctx.c.dbMap.Exec(
+	_, err = tc.c.dbMap.Exec(
 		"INSERT INTO issuedNames (reversedName, serial, notBefore) VALUES (?,?,0)",
 		"com.example-d",
 		serial4String,

cmd/log-validator/main.go

@@ -19,7 +19,7 @@ import (
 	blog "github.com/letsencrypt/boulder/log"
 )
 
-var invalidChecksumErr = errors.New("invalid checksum length")
+var errInvalidChecksum = errors.New("invalid checksum length")
 
 func lineValid(text string) error {
 	// Line format should match the following rsyslog omfile template:
@@ -53,7 +53,7 @@ func lineValid(text string) error {
 			"%s expected a 7 character base64 raw URL decodable string, got %q: %w",
 			errorPrefix,
 			checksum,
-			invalidChecksumErr,
+			errInvalidChecksum,
 		)
 	}
 
@@ -186,7 +186,7 @@ func main() {
 					continue
 				}
 				if err := lineValid(line.Text); err != nil {
-					if errors.Is(err, invalidChecksumErr) {
+					if errors.Is(err, errInvalidChecksum) {
 						lineCounter.WithLabelValues(t.Filename, "invalid checksum length").Inc()
 					} else {
 						lineCounter.WithLabelValues(t.Filename, "bad").Inc()

cmd/log-validator/main_test.go

@@ -19,7 +19,7 @@ func TestLineValidRejects(t *testing.T) {
 func TestLineValidRejectsNotAChecksum(t *testing.T) {
 	err := lineValid("2020-07-06T18:07:43.109389+00:00 70877f679c72 datacenter 6 boulder-wfe[1595]: xxxx Caught SIGTERM")
 	test.AssertError(t, err, "didn't error on invalid checksum")
-	test.AssertErrorIs(t, err, invalidChecksumErr)
+	test.AssertErrorIs(t, err, errInvalidChecksum)
 }
 
 func TestLineValidNonOurobouros(t *testing.T) {

cmd/ocsp-responder/main.go

@@ -55,8 +55,8 @@ func newFilter(issuerCerts []string, serialPrefixes []string) (*ocspFilter, erro
 	if len(issuerCerts) < 1 {
 		return nil, errors.New("Filter must include at least 1 issuer cert")
 	}
-	issuerKeyHashes := make(map[issuance.IssuerID][]byte, 0)
-	issuerNameKeyHashes := make(map[issuance.IssuerNameID][]byte, 0)
+	issuerKeyHashes := make(map[issuance.IssuerID][]byte)
+	issuerNameKeyHashes := make(map[issuance.IssuerNameID][]byte)
 	for _, issuerCert := range issuerCerts {
 		// Load the certificate from the file path.
 		cert, err := core.LoadCert(issuerCert)

core/util.go

@@ -148,7 +148,7 @@ func PublicKeysEqual(a, b interface{}) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	return bytes.Compare(aBytes, bBytes) == 0, nil
+	return bytes.Equal(aBytes, bBytes), nil
 }
 
 // SerialToString converts a certificate serial number (big.Int) to a String