Chore upgrade linter (#751)

Upgrades the linter version to the latest fixing small issues. For
readability `nolint` global directives have been brought inline to avoid
misuse. This fixes the issue brought up in #748. Also bumps the
version of buf to the latest.

---------

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

.golangci.yml

@@ -50,6 +50,7 @@ linters:
     - lll               # don't want hard limits for line length
     - maintidx          # covered by gocyclo
     - maligned          # readability trumps efficient struct packing
+    - mnd               # status codes are clearer than constants
     - nlreturn          # generous whitespace violates house style
     - nonamedreturns    # named returns are fine; it's *bare* returns that are bad
     - nosnakecase       # deprecated in https://github.com/golangci/golangci-lint/pull/3065
@@ -64,7 +65,7 @@ issues:
   exclude:
     # Don't ban use of fmt.Errorf to create new errors, but the remaining
     # checks from err113 are useful.
-    - "err113: do not define dynamic errors.*"
+    - "do not define dynamic errors, use wrapped static errors instead: .*"
 
   exclude-rules:
     # If future reflect.Kinds are nil-able, we'll find out when a test fails.
@@ -76,12 +77,6 @@ issues:
     # We need to init a global in-mem HTTP server for testable examples.
     - linters: [gochecknoinits, gochecknoglobals]
       path: example_init_test.go
-    # We need to initialize default grpc User-Agent
-    - linters: [gochecknoglobals]
-      path: protocol_grpc.go
-    # We need to initialize default connect User-Agent
-    - linters: [gochecknoglobals]
-      path: protocol_connect.go
     # We purposefully do an ineffectual assignment for an example.
     - linters: [ineffassign]
       path: client_example_test.go
@@ -132,6 +127,9 @@ issues:
     # We want to show examples with http.Get
     - linters: [noctx]
       path: internal/memhttp/memhttp_test.go
-    # We need to initialize a map of all protocol headers
-    - linters: [gochecknoglobals]
-      path: header.go
+    # Allow fmt.Sprintf for cmd/protoc-gen-connect-go for consistency
+    - linters: [perfsprint]
+      path: cmd/protoc-gen-connect-go/main.go
+    # Allow non-canonical headers in tests
+    - linters: [canonicalheader]
+      path: '.*_test.go'

Makefile

@@ -11,6 +11,7 @@ export PATH := $(BIN):$(PATH)
 export GOBIN := $(abspath $(BIN))
 COPYRIGHT_YEARS := 2021-2024
 LICENSE_IGNORE := --ignore /testdata/
+BUF_VERSION := 1.32.2
 
 .PHONY: help
 help: ## Describe useful make targets
@@ -100,15 +101,15 @@ $(BIN)/protoc-gen-connect-go:
 
 $(BIN)/buf: Makefile
 	@mkdir -p $(@D)
-	go install github.com/bufbuild/buf/cmd/buf@v1.32.0
+	go install github.com/bufbuild/buf/cmd/buf@v${BUF_VERSION}
 
 $(BIN)/license-header: Makefile
 	@mkdir -p $(@D)
-	go install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.32.0
+	go install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v${BUF_VERSION}
 
 $(BIN)/golangci-lint: Makefile
 	@mkdir -p $(@D)
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.59.1
 
 $(BIN)/protoc-gen-go: Makefile go.mod
 	@mkdir -p $(@D)

client_stream_test.go

@@ -95,8 +95,8 @@ func verifyHeaders(t *testing.T, headers http.Header) {
 	assert.Equal(t, headers, http.Header{})
 
 	// Verify set/del don't panic
-	headers.Set("a", "b")
-	headers.Del("a")
+	headers.Set("A", "b")
+	headers.Del("A")
 }
 
 type nopStreamingClientConn struct {

connect_ext_test.go

@@ -2969,7 +2969,7 @@ func (d *deflateReader) Reset(reader io.Reader) error {
 	if resetter, ok := d.r.(flate.Resetter); ok {
 		return resetter.Reset(reader, nil)
 	}
-	return fmt.Errorf("flate reader should implement flate.Resetter")
+	return errors.New("flate reader should implement flate.Resetter")
 }
 
 var _ connect.Decompressor = (*deflateReader)(nil)

duplex_http_call.go

@@ -17,7 +17,6 @@ package connect
 import (
 	"context"
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -131,7 +130,7 @@ func (d *duplexHTTPCall) sendUnary(payload messagePayload) (int64, error) {
 	// Unary messages are sent as a single HTTP request. We don't need to use a
 	// pipe for the request body and we don't need to send headers separately.
 	if !d.requestSent.CompareAndSwap(false, true) {
-		return 0, fmt.Errorf("request already sent")
+		return 0, errors.New("request already sent")
 	}
 	payloadLength := int64(payload.Len())
 	if payloadLength > 0 {
@@ -141,7 +140,7 @@ func (d *duplexHTTPCall) sendUnary(payload messagePayload) (int64, error) {
 		d.request.ContentLength = payloadLength
 		d.request.GetBody = func() (io.ReadCloser, error) {
 			if !payloadBody.Rewind() {
-				return nil, fmt.Errorf("payload cannot be retried")
+				return nil, errors.New("payload cannot be retried")
 			}
 			return payloadBody, nil
 		}

error_test.go

@@ -39,8 +39,8 @@ func TestErrorNilUnderlying(t *testing.T) {
 	err.AddDetail(detail)
 	assert.Equal(t, len(err.Details()), 1)
 	assert.Equal(t, err.Details()[0].Type(), "google.protobuf.Empty")
-	err.Meta().Set("foo", "bar")
-	assert.Equal(t, err.Meta().Get("foo"), "bar")
+	err.Meta().Set("Foo", "bar")
+	assert.Equal(t, err.Meta().Get("Foo"), "bar")
 	assert.Equal(t, CodeOf(err), CodeUnknown)
 }
 
@@ -51,9 +51,9 @@ func TestErrorFormatting(t *testing.T) {
 		NewError(CodeUnavailable, errors.New("")).Error(),
 		CodeUnavailable.String(),
 	)
-	got := NewError(CodeUnavailable, errors.New("foo")).Error()
+	got := NewError(CodeUnavailable, errors.New("Foo")).Error()
 	assert.True(t, strings.Contains(got, CodeUnavailable.String()))
-	assert.True(t, strings.Contains(got, "foo"))
+	assert.True(t, strings.Contains(got, "Foo"))
 }
 
 func TestErrorCode(t *testing.T) {

handler.go

@@ -16,7 +16,6 @@ package connect
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 )
 
@@ -53,7 +52,7 @@ func NewUnaryHandler[Req, Res any](
 		if res == nil && err == nil {
 			// This is going to panic during serialization. Debugging is much easier
 			// if we panic here instead, so we can include the procedure name.
-			panic(fmt.Sprintf("%s returned nil *connect.Response and nil error", procedure)) //nolint: forbidigo
+			panic(procedure + " returned nil *connect.Response and nil error") //nolint: forbidigo
 		}
 		return res, err
 	})
@@ -107,7 +106,7 @@ func NewClientStreamHandler[Req, Res any](
 			if res == nil {
 				// This is going to panic during serialization. Debugging is much easier
 				// if we panic here instead, so we can include the procedure name.
-				panic(fmt.Sprintf("%s returned nil *connect.Response and nil error", procedure)) //nolint: forbidigo
+				panic(procedure + " returned nil *connect.Response and nil error") //nolint: forbidigo
 			}
 			mergeHeaders(conn.ResponseHeader(), res.header)
 			mergeHeaders(conn.ResponseTrailer(), res.trailer)

header.go

@@ -20,6 +20,7 @@ import (
 )
 
 var (
+	//nolint:gochecknoglobals
 	protocolHeaders = map[string]struct{}{
 		// HTTP headers.
 		headerContentType:     {},

protocol_connect.go

@@ -60,6 +60,8 @@ const (
 )
 
 // defaultConnectUserAgent returns a User-Agent string similar to those used in gRPC.
+//
+//nolint:gochecknoglobals
 var defaultConnectUserAgent = fmt.Sprintf("connect-go/%s (%s)", Version, runtime.Version())
 
 type protocolConnect struct{}
@@ -1016,7 +1018,8 @@ func (m *connectUnaryRequestMarshaler) marshalWithGet(message any) *Error {
 	if !isTooBig {
 		url := m.buildGetURL(data, false /* compressed */)
 		if m.getURLMaxBytes <= 0 || len(url.String()) < m.getURLMaxBytes {
-			return m.writeWithGet(url)
+			m.writeWithGet(url)
+			return nil
 		}
 		if m.compressionPool == nil {
 			if m.getUseFallback {
@@ -1042,7 +1045,8 @@ func (m *connectUnaryRequestMarshaler) marshalWithGet(message any) *Error {
 	}
 	url := m.buildGetURL(compressed.Bytes(), true /* compressed */)
 	if m.getURLMaxBytes <= 0 || len(url.String()) < m.getURLMaxBytes {
-		return m.writeWithGet(url)
+		m.writeWithGet(url)
+		return nil
 	}
 	if m.getUseFallback {
 		setHeaderCanonical(m.header, connectUnaryHeaderCompression, m.compressionName)
@@ -1069,14 +1073,13 @@ func (m *connectUnaryRequestMarshaler) buildGetURL(data []byte, compressed bool)
 	return &url
 }
 
-func (m *connectUnaryRequestMarshaler) writeWithGet(url *url.URL) *Error {
+func (m *connectUnaryRequestMarshaler) writeWithGet(url *url.URL) {
 	delHeaderCanonical(m.header, connectHeaderProtocolVersion)
 	delHeaderCanonical(m.header, headerContentType)
 	delHeaderCanonical(m.header, headerContentEncoding)
 	delHeaderCanonical(m.header, headerContentLength)
 	m.duplexCall.SetMethod(http.MethodGet)
 	*m.duplexCall.URL() = *url
-	return nil
 }
 
 type connectUnaryUnmarshaler struct {

protocol_grpc.go

@@ -63,8 +63,11 @@ var (
 	//	in heterogeneous environments. The following structure is recommended to library developers:
 	//
 	//	User-Agent → "grpc-" Language ?("-" Variant) "/" Version ?( " ("  *(AdditionalProperty ";") ")" )
+	//
+	//nolint:gochecknoglobals
 	defaultGrpcUserAgent = fmt.Sprintf("grpc-go-connect/%s (%s)", Version, runtime.Version())
-	grpcAllowedMethods   = map[string]struct{}{
+	//nolint:gochecknoglobals
+	grpcAllowedMethods = map[string]struct{}{
 		http.MethodPost: {},
 	}
 )