From e87d9665b5734ec83cee8f0ecc1037ace157d691 Mon Sep 17 00:00:00 2001 From: Edward McFarlane <3036610+emcfarlane@users.noreply.github.com> Date: Wed, 12 Jun 2024 19:11:42 -0400 Subject: [PATCH] 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 --- .golangci.yml | 18 ++++++++---------- Makefile | 7 ++++--- client_stream_test.go | 4 ++-- connect_ext_test.go | 2 +- duplex_http_call.go | 5 ++--- error_test.go | 8 ++++---- handler.go | 5 ++--- header.go | 1 + protocol_connect.go | 11 +++++++---- protocol_grpc.go | 5 ++++- 10 files changed, 35 insertions(+), 31 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d94bf062..8b3bbde8 100644 --- a/.golangci.yml +++ b/.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' diff --git a/Makefile b/Makefile index 46a21425..d0315e71 100644 --- a/Makefile +++ b/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) diff --git a/client_stream_test.go b/client_stream_test.go index a812caa1..cf072392 100644 --- a/client_stream_test.go +++ b/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 { diff --git a/connect_ext_test.go b/connect_ext_test.go index 81ea030a..b93c5708 100644 --- a/connect_ext_test.go +++ b/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) diff --git a/duplex_http_call.go b/duplex_http_call.go index a1f8f36a..78bf13c2 100644 --- a/duplex_http_call.go +++ b/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 } diff --git a/error_test.go b/error_test.go index 93306a9c..3535b0b0 100644 --- a/error_test.go +++ b/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) { diff --git a/handler.go b/handler.go index 5eab6c71..9f95627b 100644 --- a/handler.go +++ b/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) diff --git a/header.go b/header.go index b3f05432..9958be28 100644 --- a/header.go +++ b/header.go @@ -20,6 +20,7 @@ import ( ) var ( + //nolint:gochecknoglobals protocolHeaders = map[string]struct{}{ // HTTP headers. headerContentType: {}, diff --git a/protocol_connect.go b/protocol_connect.go index bf26f1aa..45577d82 100644 --- a/protocol_connect.go +++ b/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 { diff --git a/protocol_grpc.go b/protocol_grpc.go index 5addf31e..e10ecad7 100644 --- a/protocol_grpc.go +++ b/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: {}, } )