CI: Run staticcheck standalone (#7055)

Run staticcheck as a standalone binary rather than as a library via
golangci-lint. From the golangci-lint help out,
> staticcheck (megacheck): It's a set of rules from staticcheck. It's
not the same thing as the staticcheck binary. The author of staticcheck
doesn't support or approve the use of staticcheck as a library inside
golangci-lint.

We decided to disable ST1000 which warns about incorrect or missing
package comments.

For SA4011, I chose to change the semantics[1] of the for loop rather
than ignoring the SA4011 lint for that line.

Fixes #6988

1. https://go.dev/ref/spec#Continue_statements

.github/workflows/boulder-ci.yml

@@ -36,8 +36,8 @@ jobs:
       matrix:
         # Add additional docker image tags here and all tests will be run with the additional image.
         BOULDER_TOOLS_TAG:
-          - go1.20.7_2023-08-02
-          - go1.21rc4_2023-08-02
+          - go1.20.7_2023-08-28
+          - go1.21rc4_2023-08-28
         # Tests command definitions. Use the entire "docker compose" command you want to run.
         tests:
           # Run ./test.sh --help for a description of each of the flags.
@@ -113,8 +113,8 @@ jobs:
       matrix:
         # Add additional docker image tags here and all tests will be run with the additional image.
         BOULDER_TOOLS_TAG:
-          - go1.20.7_2023-08-02
-          - go1.21rc4_2023-08-02
+          - go1.20.7_2023-08-28
+          - go1.21rc4_2023-08-28
 
     env:
       # This sets the docker image tag for the boulder-tools repository to

.golangci.yml

@@ -8,8 +8,6 @@ linters:
     - govet
     - ineffassign
     - misspell
-    - staticcheck
-    - stylecheck
     - typecheck
     - unconvert
     - unparam
@@ -40,14 +38,6 @@ linters-settings:
           - (github.com/letsencrypt/boulder/log.Logger).AuditErrf
           - (github.com/letsencrypt/boulder/ocsp/responder).SampledError
           - (github.com/letsencrypt/boulder/web.RequestEvent).AddError
-  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"]
   gosec:
     excludes:
       # TODO: Identify, fix, and remove violations of most of these rules

bdns/servers.go

@@ -15,7 +15,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-// serverProvider represents a type which can provide a list of addresses for
+// ServerProvider represents a type which can provide a list of addresses for
 // the bdns to use as DNS resolvers. Different implementations may provide
 // different strategies for providing addresses, and may provide different kinds
 // of addresses (e.g. host:port combos vs IP addresses).

cmd/expiration-mailer/main.go

@@ -934,7 +934,7 @@ func main() {
 					cmd.FailOnError(err, "expiration-mailer has failed")
 				}
 			case <-ctx.Done():
-				break
+				return
 			}
 		}
 	} else {

db/interfaces.go

@@ -101,7 +101,7 @@ type Rows[T any] interface {
 	Close() error
 }
 
-// MockSqlExecuter implement SqlExecutor by returning errors from every call.
+// MockSqlExecutor implement SqlExecutor by returning errors from every call.
 //
 // TODO: To mock out WithContext, we needed to be able to return objects that satisfy
 // borp.SqlExecutor. That's a pretty big interface, so we specify one no-op mock

docker-compose.yml

@@ -2,7 +2,7 @@ version: '3'
 services:
   boulder:
     # Should match one of the GO_DEV_VERSIONS in test/boulder-tools/tag_and_upload.sh.
-    image: &boulder_image letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.20.7_2023-08-02}
+    image: &boulder_image letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.20.7_2023-08-28}
     environment:
       # To solve HTTP-01 and TLS-ALPN-01 challenges, change the IP in FAKE_DNS
       # to the IP address where your ACME client's solver is listening.

mail/mailer.go

@@ -171,8 +171,8 @@ func New(
 	}
 }
 
-// New constructs a Mailer suitable for doing a dry run. It simply logs each
-// command that would have been run, at debug level.
+// NewDryRun constructs a Mailer suitable for doing a dry run. It simply logs
+// each command that would have been run, at debug level.
 func NewDryRun(from mail.Address, logger blog.Logger) *mailerImpl {
 	return &mailerImpl{
 		config: config{

mocks/mocks.go

@@ -572,7 +572,7 @@ func (sa *StorageAuthority) RevokeCertificate(ctx context.Context, req *sapb.Rev
 	return nil, nil
 }
 
-// RevokeCertificate is a mock
+// UpdateRevokedCertificate is a mock
 func (sa *StorageAuthority) UpdateRevokedCertificate(ctx context.Context, req *sapb.RevokeCertificateRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
 	return nil, nil
 }
@@ -602,7 +602,7 @@ func (sa *StorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Update
 	return nil, errors.New("unimplemented")
 }
 
-// Publisher is a mock
+// PublisherClient is a mock
 type PublisherClient struct {
 	// empty
 }

ocsp/responder/responder.go

@@ -163,10 +163,10 @@ func (rs Responder) sampledError(format string, a ...interface{}) {
 	SampledError(rs.log, rs.sampleRate, format, a...)
 }
 
-// A Responder can process both GET and POST requests. The mapping from an OCSP
-// request to an OCSP response is done by the Source; the Responder simply
-// decodes the request, and passes back whatever response is provided by the
-// source.
+// ServeHTTP is a Responder that can process both GET and POST requests. The
+// mapping from an OCSP request to an OCSP response is done by the Source; the
+// Responder simply decodes the request, and passes back whatever response is
+// provided by the source.
 // The Responder will set these headers:
 //
 //	Cache-Control: "max-age=(response.NextUpdate-now), public, no-transform, must-revalidate",

staticcheck.conf

@@ -0,0 +1,8 @@
+# Ignores the following:
+#   SA1019: Using a deprecated function, variable, constant or field
+#   SA6003: Converting a string to a slice of runes before ranging over it
+#   ST1000: Incorrect or missing package comment
+#   ST1003: Poorly chosen identifier
+#   ST1005: Incorrectly formatted error string
+
+checks = ["all", "-SA1019", "-SA6003", "-ST1000", "-ST1003", "-ST1005"]

test.sh

@@ -212,6 +212,8 @@ STAGE="lints"
 if [[ "${RUN[@]}" =~ "$STAGE" ]] ; then
   print_heading "Running Lints"
   golangci-lint run --timeout 9m ./...
+  # Implicitly loads staticcheck.conf from the root of the boulder repository
+  staticcheck ./...
   python3 test/grafana/lint.py
   # Check for common spelling errors using codespell.
   # Update .codespell.ignore.txt if you find false positives (NOTE: ignored

test/boulder-tools/install-go.sh

@@ -19,6 +19,7 @@ go install github.com/rubenv/sql-migrate/[email protected]
 go install golang.org/x/tools/cmd/stringer@latest
 go install github.com/letsencrypt/pebble/cmd/pebble-challtestsrv@master
 go install github.com/golangci/golangci-lint/cmd/[email protected]
+go install honnef.co/go/tools/cmd/[email protected]
 
 go clean -cache
 go clean -modcache

test/ocsp/helper/helper.go

@@ -107,7 +107,7 @@ func (template Config) WithExpectStatus(status int) Config {
 	return ret
 }
 
-// WithExpectStatus returns a new Config with the given expectReason,
+// WithExpectReason returns a new Config with the given expectReason,
 // and all other fields the same as the receiver.
 func (template Config) WithExpectReason(reason int) Config {
 	ret := template
@@ -208,7 +208,7 @@ func parseCMS(body []byte) (*x509.Certificate, error) {
 	return cert, nil
 }
 
-// ReqFle makes an OCSP request using the given config for the PEM-encoded
+// ReqFile makes an OCSP request using the given config for the PEM-encoded
 // certificate in fileName, and returns the response.
 func ReqFile(fileName string, config Config) (*ocsp.Response, error) {
 	contents, err := os.ReadFile(fileName)

web/context.go

@@ -183,10 +183,9 @@ func (th *TopHandler) logEvent(logEvent *RequestEvent) {
 		int(logEvent.Latency*1000), logEvent.RealIP, jsonEvent)
 }
 
-// Comma-separated list of HTTP clients involved in making this
-// request, starting with the original requester and ending with the
-// remote end of our TCP connection (which is typically our own
-// proxy).
+// GetClientAddr returns a comma-separated list of HTTP clients involved in
+// making this request, starting with the original requester and ending with the
+// remote end of our TCP connection (which is typically our own proxy).
 func GetClientAddr(r *http.Request) string {
 	if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
 		return xff + "," + r.RemoteAddr

web/probs.go

@@ -63,7 +63,7 @@ func problemDetailsForBoulderError(err *berrors.BoulderError, msg string) *probs
 	return outProb
 }
 
-// problemDetailsForError turns an error into a ProblemDetails with the special
+// ProblemDetailsForError turns an error into a ProblemDetails with the special
 // case of returning the same error back if its already a ProblemDetails. If the
 // error is of an type unknown to ProblemDetailsForError, it will return a
 // ServerInternal ProblemDetails.