Upgrade min go version to v1.21 (#132)

Bumps the golanglint-ci to 1.60.1 and fixes the lint errors. Updated the
disabled list based on connect-go.

---------

Signed-off-by: Edward McFarlane <[email protected]>

.github/workflows/ci.yaml

@@ -15,7 +15,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go-version: [1.20.x, 1.21.x, 1.22.x]
+        go-version: [1.21.x, 1.22.x, 1.23.x]
     steps:
       - name: Checkout Code
         uses: actions/checkout@v4
@@ -32,5 +32,5 @@ jobs:
         # conflicting guidance, run only on the most recent supported version.
         # For the same reason, only check generated code on the most recent
         # supported version.
-        if: matrix.go-version == '1.22.x'
+        if: matrix.go-version == '1.23.x'
         run: make checkgenerate && make lint

.golangci.yml

@@ -33,38 +33,29 @@ linters:
   enable-all: true
   disable:
     - cyclop            # covered by gocyclo
-    - deadcode          # abandoned
-    - depguard          # may enable later but needs configuration
-    - exhaustive        # noisy in practice: many switches have default, so they don't need to be exhaustive
-    - exhaustivestruct  # replaced by exhaustruct
-    - exhaustruct       # opt-in on a type-by-type basis (no useful for now)
+    - depguard          # unnecessary for small libraries
+    - execinquery       # deprecated since golangci v1.58
     - funlen            # rely on code review to limit function length
     - gocognit          # dubious "cognitive overhead" quantification
     - gofumpt           # prefer standard gofmt
     - goimports         # rely on gci instead
-    - golint            # deprecated by Go team
     - gomnd             # some unnamed constants are okay
-    - ifshort           # deprecated by author
     - inamedparam       # convention is not followed
-    - interfacer        # deprecated by author
     - ireturn           # "accept interfaces, return structs" isn't ironclad
     - 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
-    - nosnakecase       # deprecated in https://github.com/golangci/golangci-lint/pull/3065
-    - nonamedreturns    # named returns are fine; it's bare returns that are bad
-    - scopelint         # deprecated by author
-    - structcheck       # abandoned
+    - nonamedreturns    # named returns are fine; it's *bare* returns that are bad
+    - protogetter       # too many false positives
     - testpackage       # internal tests are fine
-    - varcheck          # abandoned
     - wrapcheck         # don't _always_ need to wrap errors
     - wsl               # generous whitespace violates house style
 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:
     - path: internal/examples/.*/.*\.go
       linters:

Makefile

@@ -9,7 +9,7 @@ MAKEFLAGS += --no-print-directory
 BIN := .tmp/bin
 COPYRIGHT_YEARS := 2023-2024
 LICENSE_IGNORE := -e testdata/
-BUF_VERSION ?= 1.32.1
+BUF_VERSION ?= 1.38.0
 # Set to use a different compiler. For example, `GO=go1.18rc1 make test`.
 GO ?= go
 
@@ -91,4 +91,4 @@ $(BIN)/license-header: Makefile
 
 $(BIN)/golangci-lint: Makefile
 	@mkdir -p $(@D)
-	GOBIN=$(abspath $(@D)) $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.56.2
+	GOBIN=$(abspath $(@D)) $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.1

go.mod

@@ -1,6 +1,6 @@
 module connectrpc.com/vanguard
 
-go 1.20
+go 1.21
 
 require (
 	buf.build/gen/go/connectrpc/eliza/connectrpc/go v1.11.1-20230822171018-8b8b971d6fde.1

params.go

@@ -169,7 +169,7 @@ func unmarshalFieldValue(msg protoreflect.Message, field protoreflect.FieldDescr
 			return protoreflect.Value{}, fmt.Errorf("unknown enum: %s", data)
 		}
 		return protoreflect.ValueOf(enumVal.Number()), nil
-	case protoreflect.MessageKind:
+	case protoreflect.MessageKind, protoreflect.GroupKind:
 		return unmarshalFieldWKT(msg, field, data)
 	default:
 		return protoreflect.Value{}, fmt.Errorf("unsupported type %s", field.Kind())
@@ -300,7 +300,7 @@ func marshalFieldValue(field protoreflect.FieldDescriptor, value protoreflect.Va
 			return nil, fmt.Errorf("unknown enum value %d", value.Enum())
 		}
 		return []byte(enumValue.Name()), nil
-	case protoreflect.MessageKind:
+	case protoreflect.MessageKind, protoreflect.GroupKind:
 		return marshalFieldWKT(field, value)
 	default:
 		return nil, fmt.Errorf("unsupported type %s", field.Kind())

protocol_rest.go

@@ -52,6 +52,8 @@ func (r restClientProtocol) acceptsStreamType(op *operation, streamType connect.
 		return restHTTPBodyRequest(op)
 	case connect.StreamTypeServer:
 		return restHTTPBodyResponse(op)
+	case connect.StreamTypeBidi:
+		return false
 	default:
 		return false
 	}

transcoder.go

@@ -404,7 +404,7 @@ func (o *operation) validate(transcoder *Transcoder) error {
 	// Identify the request encoding and compression.
 	reqMeta, err := clientProtoHandler.extractProtocolRequestHeaders(o, o.request.Header)
 	if err != nil {
-		return newHTTPError(http.StatusBadRequest, err.Error())
+		return newHTTPError(http.StatusBadRequest, "%w", err)
 	}
 	// Remove other headers that might mess up the next leg
 	if enc := o.request.Header.Get("Content-Encoding"); enc != "" && enc != CompressionIdentity {
@@ -603,8 +603,7 @@ func (o *operation) handle() {
 
 func (o *operation) resolveMethod(transcoder *Transcoder) error {
 	uriPath := o.request.URL.Path
-	switch o.client.protocol.protocol() {
-	case ProtocolREST:
+	if o.client.protocol.protocol() == ProtocolREST {
 		var methods routeMethods
 		o.restTarget, o.restVars, methods = transcoder.restRoutes.match(uriPath, o.request.Method)
 		if o.restTarget != nil {
@@ -627,35 +626,34 @@ func (o *operation) resolveMethod(transcoder *Transcoder) error {
 				"Allow": []string{sb.String()},
 			},
 		}
-	default:
-		methodConf := transcoder.methods[uriPath]
-		if methodConf == nil {
-			return errNotFound
-		}
-		o.restTarget = methodConf.httpRule
-		if o.request.Method != http.MethodPost {
-			mayAllowGet, ok := o.client.protocol.(clientProtocolAllowsGet)
-			allowsGet := ok && mayAllowGet.allowsGetRequests(methodConf)
-			if !allowsGet {
-				return &httpError{
-					code: http.StatusMethodNotAllowed,
-					header: http.Header{
-						"Allow": []string{http.MethodPost},
-					},
-				}
+	}
+	methodConf := transcoder.methods[uriPath]
+	if methodConf == nil {
+		return errNotFound
+	}
+	o.restTarget = methodConf.httpRule
+	if o.request.Method != http.MethodPost {
+		mayAllowGet, ok := o.client.protocol.(clientProtocolAllowsGet)
+		allowsGet := ok && mayAllowGet.allowsGetRequests(methodConf)
+		if !allowsGet {
+			return &httpError{
+				code: http.StatusMethodNotAllowed,
+				header: http.Header{
+					"Allow": []string{http.MethodPost},
+				},
 			}
-			if allowsGet && o.request.Method != http.MethodGet {
-				return &httpError{
-					code: http.StatusMethodNotAllowed,
-					header: http.Header{
-						"Allow": []string{http.MethodGet + "," + http.MethodPost},
-					},
-				}
+		}
+		if allowsGet && o.request.Method != http.MethodGet {
+			return &httpError{
+				code: http.StatusMethodNotAllowed,
+				header: http.Header{
+					"Allow": []string{http.MethodGet + "," + http.MethodPost},
+				},
 			}
 		}
-		o.methodConf = methodConf
-		return nil
 	}
+	o.methodConf = methodConf
+	return nil
 }
 
 // reportError handles an error that occurs while setting up the operation. It should not be used

transcoder_test.go

@@ -1042,11 +1042,11 @@ func TestTranscoder_PassThrough(t *testing.T) {
 			}
 
 			_, isWrapped := respWriter.(*responseWriter)
-			require.False(t, isWrapped)
+			assert.False(t, isWrapped)
 			_, isWrapped = request.Body.(*envelopingReader)
-			require.False(t, isWrapped)
+			assert.False(t, isWrapped)
 			_, isWrapped = request.Body.(*transformingReader)
-			require.False(t, isWrapped)
+			assert.False(t, isWrapped)
 
 			// carry on...
 			handler.ServeHTTP(respWriter, request)

vanguard_test.go

@@ -376,7 +376,7 @@ func (i *testInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc {
 		if err := assertTestTimeoutEncoded(ctx); err != nil {
 			return nil, err
 		}
-		val := req.Header().Get("test")
+		val := req.Header().Get("Test")
 		if val == "" {
 			return next(ctx, req)
 		}
@@ -462,7 +462,7 @@ func (i *testInterceptor) WrapStreamingHandler(next connect.StreamingHandlerFunc
 		if err := assertTestTimeoutEncoded(ctx); err != nil {
 			return err
 		}
-		val := conn.RequestHeader().Get("test")
+		val := conn.RequestHeader().Get("Test")
 		if val == "" {
 			return next(ctx, conn)
 		}
@@ -635,7 +635,7 @@ func (i *testInterceptor) restUnaryHandler(
 		return nil
 	}
 	return func(rsp http.ResponseWriter, req *http.Request) {
-		val := req.Header.Get("test")
+		val := req.Header.Get("Test")
 		if val == "" {
 			http.Error(rsp, "missing test header", http.StatusInternalServerError)
 			return
@@ -706,7 +706,7 @@ type testServer struct {
 
 func appendClientProtocolOptions(t *testing.T, opts []connect.ClientOption, protocol Protocol) []connect.ClientOption {
 	t.Helper()
-	switch protocol {
+	switch protocol { //nolint:exhaustive
 	case ProtocolGRPC:
 		return append(opts, connect.WithGRPC())
 	case ProtocolGRPCWeb:
@@ -914,7 +914,7 @@ func protocolAssertMiddleware(
 	}
 	return func(rsp http.ResponseWriter, req *http.Request) {
 		var wantHdr map[string][]string
-		switch protocol {
+		switch protocol { //nolint:exhaustive
 		case ProtocolGRPC:
 			wantContentType := []string{"application/grpc+" + codec}
 			if codec == CodecProto {