diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c077223a..8ba1a1e18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,9 @@ jobs: EUREKA_ADDR: http://localhost:${{ job.services.eureka.ports[8761] }}/eureka run: go test -v -race -coverprofile=coverage.coverprofile -covermode=atomic -tags integration ./... + - name: Run linter + uses: actions-contrib/golangci-lint@v1 + - name: Upload coverage uses: codecov/codecov-action@v1 with: diff --git a/.gitignore b/.gitignore index 6062401c1..f49f19b87 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ examples/stringsvc1/stringsvc1 examples/stringsvc2/stringsvc2 examples/stringsvc3/stringsvc3 *.coverprofile +golangci-lint # Compiled Object files, Static and Dynamic libs (Shared Objects) *.o @@ -42,4 +43,3 @@ Session.vim *~ # auto-generated tag files tags - diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..1660f321a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,30 @@ +run: + skip-dirs: + - (^|/)pb($|/) + - (^|/)thrift($|/) + + timeout: 2m + +linters-settings: + golint: + min-confidence: 0.9 + goimports: + local-prefixes: github.com/go-kit/kit + +linters: + disable-all: true + enable: + - govet + - golint + - gofmt + - staticcheck + +issues: + exclude-rules: + - path: _test\.go + text: "should not use basic type string as key in context.WithValue" + linters: + - golint + +service: + golangci-lint-version: 1.23.x diff --git a/auth/jwt/middleware_test.go b/auth/jwt/middleware_test.go index 3278e13a7..c6f9f0b3e 100644 --- a/auth/jwt/middleware_test.go +++ b/auth/jwt/middleware_test.go @@ -136,7 +136,7 @@ func TestJWTParser(t *testing.T) { // Test for malformed token error response parser = NewParser(keys, method, StandardClaimsFactory)(e) ctx = context.WithValue(context.Background(), JWTTokenContextKey, malformedKey) - ctx1, err = parser(ctx, struct{}{}) + _, err = parser(ctx, struct{}{}) if want, have := ErrTokenMalformed, err; want != have { t.Fatalf("Expected %+v, got %+v", want, have) } @@ -149,7 +149,7 @@ func TestJWTParser(t *testing.T) { t.Fatalf("Unable to Sign Token: %+v", err) } ctx = context.WithValue(context.Background(), JWTTokenContextKey, token) - ctx1, err = parser(ctx, struct{}{}) + _, err = parser(ctx, struct{}{}) if want, have := ErrTokenExpired, err; want != have { t.Fatalf("Expected %+v, got %+v", want, have) } @@ -162,7 +162,7 @@ func TestJWTParser(t *testing.T) { t.Fatalf("Unable to Sign Token: %+v", err) } ctx = context.WithValue(context.Background(), JWTTokenContextKey, token) - ctx1, err = parser(ctx, struct{}{}) + _, err = parser(ctx, struct{}{}) if want, have := ErrTokenNotActive, err; want != have { t.Fatalf("Expected %+v, got %+v", want, have) } diff --git a/lint b/lint index 12e307273..09a8a51fb 100755 --- a/lint +++ b/lint @@ -4,23 +4,15 @@ set -o errexit set -o nounset set -o pipefail -if [ ! $(command -v gometalinter) ] +if [ $(command -v golangci-lint) ] then - go get github.com/alecthomas/gometalinter - gometalinter --update --install + golangci-lint run + exit $? fi -time gometalinter \ - --exclude='error return value not checked.*(Close|Log|Print).*\(errcheck\)$' \ - --exclude='.*_test\.go:.*error return value not checked.*\(errcheck\)$' \ - --exclude='/thrift/' \ - --exclude='/pb/' \ - --exclude='no args in Log call \(vet\)' \ - --disable=dupl \ - --disable=aligncheck \ - --disable=gotype \ - --cyclo-over=20 \ - --tests \ - --concurrency=2 \ - --deadline=300s \ - ./... +if [ ! -x ./golangci-lint ] +then + curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | bash -s -- -b $PWD +fi + +./golangci-lint run diff --git a/log/stdlib_test.go b/log/stdlib_test.go index 90ebc5fa4..ad9a6e354 100644 --- a/log/stdlib_test.go +++ b/log/stdlib_test.go @@ -53,11 +53,11 @@ func TestStdLibAdapterExtraction(t *testing.T) { logger := NewLogfmtLogger(buf) writer := NewStdlibAdapter(logger) for input, want := range map[string]string{ - "hello": "msg=hello\n", - "2009/01/23: hello": "ts=2009/01/23 msg=hello\n", - "2009/01/23 01:23:23: hello": "ts=\"2009/01/23 01:23:23\" msg=hello\n", - "01:23:23: hello": "ts=01:23:23 msg=hello\n", - "2009/01/23 01:23:23.123123: hello": "ts=\"2009/01/23 01:23:23.123123\" msg=hello\n", + "hello": "msg=hello\n", + "2009/01/23: hello": "ts=2009/01/23 msg=hello\n", + "2009/01/23 01:23:23: hello": "ts=\"2009/01/23 01:23:23\" msg=hello\n", + "01:23:23: hello": "ts=01:23:23 msg=hello\n", + "2009/01/23 01:23:23.123123: hello": "ts=\"2009/01/23 01:23:23.123123\" msg=hello\n", "2009/01/23 01:23:23.123123 /a/b/c/d.go:23: hello": "ts=\"2009/01/23 01:23:23.123123\" caller=/a/b/c/d.go:23 msg=hello\n", "01:23:23.123123 /a/b/c/d.go:23: hello": "ts=01:23:23.123123 caller=/a/b/c/d.go:23 msg=hello\n", "2009/01/23 01:23:23 /a/b/c/d.go:23: hello": "ts=\"2009/01/23 01:23:23\" caller=/a/b/c/d.go:23 msg=hello\n", diff --git a/metrics/cloudwatch/cloudwatch.go b/metrics/cloudwatch/cloudwatch.go index b14cdcd18..e6d9113f1 100644 --- a/metrics/cloudwatch/cloudwatch.go +++ b/metrics/cloudwatch/cloudwatch.go @@ -48,9 +48,9 @@ type CloudWatch struct { type option func(*CloudWatch) -func (s *CloudWatch) apply(opt option) { +func (cw *CloudWatch) apply(opt option) { if opt != nil { - opt(s) + opt(cw) } } diff --git a/metrics/cloudwatch/cloudwatch_test.go b/metrics/cloudwatch/cloudwatch_test.go index d534b7ec5..83b6bf2d8 100644 --- a/metrics/cloudwatch/cloudwatch_test.go +++ b/metrics/cloudwatch/cloudwatch_test.go @@ -77,7 +77,7 @@ LabelValues: } } } - return fmt.Errorf("could not find dimension with name %s and value %s", name, value) + return fmt.Errorf("could not find dimension with name %s and value %s", name, value) // nolint: staticcheck } return nil diff --git a/metrics/generic/generic.go b/metrics/generic/generic.go index ebde9c873..bc9f89c40 100644 --- a/metrics/generic/generic.go +++ b/metrics/generic/generic.go @@ -182,7 +182,7 @@ func (h *Histogram) LabelValues() []string { func (h *Histogram) Print(w io.Writer) { h.h.RLock() defer h.h.RUnlock() - fmt.Fprintf(w, h.h.String()) + fmt.Fprint(w, h.h.String()) } // safeHistogram exists as gohistogram.Histogram is not goroutine-safe. diff --git a/sd/etcd/client_test.go b/sd/etcd/client_test.go index 1bdb7aee9..10320eb02 100644 --- a/sd/etcd/client_test.go +++ b/sd/etcd/client_test.go @@ -1,11 +1,11 @@ package etcd import ( + "context" "errors" "reflect" "testing" "time" - "context" etcd "go.etcd.io/etcd/client" ) @@ -135,7 +135,6 @@ func (fw *fakeWatcher) Next(context.Context) (*etcd.Response, error) { return nil, nil case <-fw.err: return nil, errors.New("error from underlying etcd watcher") - default: } } } diff --git a/sd/zk/client.go b/sd/zk/client.go index 60ac1387e..8d511f243 100644 --- a/sd/zk/client.go +++ b/sd/zk/client.go @@ -189,7 +189,7 @@ func (c *client) CreateParentNodes(path string) error { if path[0] != '/' { return zk.ErrInvalidPath } - payload := []byte("") + var payload []byte pathString := "" pathNodes := strings.Split(path, "/") for i := 1; i < len(pathNodes); i++ { diff --git a/sd/zk/client_test.go b/sd/zk/client_test.go index e201536d8..06ef4ca57 100644 --- a/sd/zk/client_test.go +++ b/sd/zk/client_test.go @@ -18,7 +18,7 @@ func TestNewClient(t *testing.T) { payload = [][]byte{[]byte("Payload"), []byte("Test")} ) - c, err := NewClient( + _, err := NewClient( []string{"FailThisInvalidHost!!!"}, log.NewNopLogger(), ) @@ -36,7 +36,7 @@ func TestNewClient(t *testing.T) { } } - c, err = NewClient( + c, err := NewClient( []string{"localhost"}, log.NewNopLogger(), ACL(acl), @@ -115,7 +115,7 @@ func TestCreateParentNodes(t *testing.T) { t.Error("expected failed new Instancer") } - s, err = NewInstancer(c, "invalidpath", log.NewNopLogger()) + _, err = NewInstancer(c, "invalidpath", log.NewNopLogger()) if err != stdzk.ErrInvalidPath { t.Errorf("unexpected error: %v", err) } diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index 8eedd59b2..275b4600e 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -22,7 +22,7 @@ type EndpointOptions struct { // If the function is nil, or the returned name is empty, the existing name for the endpoint is used. GetName func(ctx context.Context, name string) string - // GetAttributes is an optional function that can extract trace attributes + // GetAttributes is an optional function that can extract trace attributes // from the context and add them to the span. GetAttributes func(ctx context.Context) []trace.Attribute } diff --git a/transport/amqp/publisher_test.go b/transport/amqp/publisher_test.go index 2c62be10b..0617183a1 100644 --- a/transport/amqp/publisher_test.go +++ b/transport/amqp/publisher_test.go @@ -7,8 +7,9 @@ import ( "testing" "time" - amqptransport "github.com/go-kit/kit/transport/amqp" "github.com/streadway/amqp" + + amqptransport "github.com/go-kit/kit/transport/amqp" ) var ( @@ -55,7 +56,7 @@ func TestBadDecode(t *testing.T) { f: nullFunc, c: make(chan amqp.Publishing, 1), deliveries: []amqp.Delivery{ - amqp.Delivery{ + { CorrelationId: cid, }, }, @@ -157,7 +158,7 @@ func TestSuccessfulPublisher(t *testing.T) { f: nullFunc, c: reqChan, deliveries: []amqp.Delivery{ - amqp.Delivery{ + { CorrelationId: cid, Body: b, }, diff --git a/transport/awslambda/handler_test.go b/transport/awslambda/handler_test.go index cf7aaf7f8..20834455c 100644 --- a/transport/awslambda/handler_test.go +++ b/transport/awslambda/handler_test.go @@ -134,7 +134,7 @@ func TestInvokeFailDecode(t *testing.T) { encodeResponse, HandlerErrorEncoder(func( ctx context.Context, - err error, + _ error, ) ([]byte, error) { apigwResp := events.APIGatewayProxyResponse{} apigwResp.Body = `{"error":"yes"}` @@ -184,7 +184,7 @@ func TestInvokeFailEndpoint(t *testing.T) { }), HandlerErrorEncoder(func( ctx context.Context, - err error, + _ error, ) ([]byte, error) { apigwResp := events.APIGatewayProxyResponse{} apigwResp.Body = `{"error":"yes"}` @@ -241,7 +241,7 @@ func TestInvokeFailEncode(t *testing.T) { }), HandlerErrorEncoder(func( ctx context.Context, - err error, + _ error, ) ([]byte, error) { // convert error into proper APIGateway response. apigwResp := events.APIGatewayProxyResponse{} diff --git a/transport/http/proto/proto_test.go b/transport/http/proto/proto_test.go index 7b59d71a2..070bbec02 100644 --- a/transport/http/proto/proto_test.go +++ b/transport/http/proto/proto_test.go @@ -1,6 +1,7 @@ package proto import ( + "context" "io/ioutil" "net/http" "net/http/httptest" @@ -14,7 +15,7 @@ func TestEncodeProtoRequest(t *testing.T) { r := httptest.NewRequest(http.MethodGet, "/cat", nil) - err := EncodeProtoRequest(nil, r, cat) + err := EncodeProtoRequest(context.TODO(), r, cat) if err != nil { t.Errorf("expected no encoding errors but got: %s", err) return @@ -51,7 +52,7 @@ func TestEncodeProtoResponse(t *testing.T) { wr := httptest.NewRecorder() - err := EncodeProtoResponse(nil, wr, cat) + err := EncodeProtoResponse(context.TODO(), wr, cat) if err != nil { t.Errorf("expected no encoding errors but got: %s", err) return diff --git a/transport/http/server_test.go b/transport/http/server_test.go index c4359272e..a066a80eb 100644 --- a/transport/http/server_test.go +++ b/transport/http/server_test.go @@ -263,7 +263,7 @@ func TestEncodeJSONResponse(t *testing.T) { type multiHeaderResponse struct{} -func (_ multiHeaderResponse) Headers() http.Header { +func (multiHeaderResponse) Headers() http.Header { return http.Header{"Vary": []string{"Origin", "User-Agent"}} } @@ -281,7 +281,7 @@ func TestAddMultipleHeaders(t *testing.T) { if err != nil { t.Fatal(err) } - expect := map[string]map[string]struct{}{"Vary": map[string]struct{}{"Origin": struct{}{}, "User-Agent": struct{}{}}} + expect := map[string]map[string]struct{}{"Vary": {"Origin": {}, "User-Agent": {}}} for k, vls := range resp.Header { for _, v := range vls { delete((expect[k]), v) @@ -318,7 +318,7 @@ func TestAddMultipleHeadersErrorEncoder(t *testing.T) { if err != nil { t.Fatal(err) } - expect := map[string]map[string]struct{}{"Vary": map[string]struct{}{"Origin": struct{}{}, "User-Agent": struct{}{}}} + expect := map[string]map[string]struct{}{"Vary": {"Origin": {}, "User-Agent": {}}} for k, vls := range resp.Header { for _, v := range vls { delete((expect[k]), v) diff --git a/transport/nats/encode_decode.go b/transport/nats/encode_decode.go index e819a8961..7a2902db8 100644 --- a/transport/nats/encode_decode.go +++ b/transport/nats/encode_decode.go @@ -29,4 +29,3 @@ type EncodeResponseFunc func(context.Context, string, *nats.Conn, interface{}) e // endpoints. One straightforward DecodeResponseFunc could be something that // JSON decodes from the response payload to the concrete response type. type DecodeResponseFunc func(context.Context, *nats.Msg) (response interface{}, err error) - diff --git a/transport/nats/subscriber_test.go b/transport/nats/subscriber_test.go index 69a097f1e..6fd999568 100644 --- a/transport/nats/subscriber_test.go +++ b/transport/nats/subscriber_test.go @@ -152,12 +152,17 @@ func TestSubscriberErrorEncoder(t *testing.T) { } func TestSubscriberHappySubject(t *testing.T) { - step, response := testSubscriber(t) + step, response, errs := testSubscriber(t) + err := <-errs + if err != nil { + t.Fatal(err) + } + step() r := <-response var resp TestResponse - err := json.Unmarshal(r.Data, &resp) + err = json.Unmarshal(r.Data, &resp) if err != nil { t.Fatal(err) } @@ -210,17 +215,23 @@ func TestMultipleSubscriberBefore(t *testing.T) { } defer sub.Unsubscribe() + errs := make(chan error, 1) + wg.Add(1) go func() { defer wg.Done() _, err := nc.Request("natstransport.test", []byte("test data"), 2*time.Second) - if err != nil { - t.Fatal(err) - } + errs <- err }() select { case <-done: + + case err := <-errs: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): t.Fatal("timeout waiting for finalizer") } @@ -271,17 +282,23 @@ func TestMultipleSubscriberAfter(t *testing.T) { } defer sub.Unsubscribe() + errs := make(chan error, 1) + wg.Add(1) go func() { defer wg.Done() _, err := nc.Request("natstransport.test", []byte("test data"), 2*time.Second) - if err != nil { - t.Fatal(err) - } + errs <- err }() select { case <-done: + + case err := <-errs: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): t.Fatal("timeout waiting for finalizer") } @@ -322,17 +339,23 @@ func TestSubscriberFinalizerFunc(t *testing.T) { } defer sub.Unsubscribe() + errs := make(chan error, 1) + wg.Add(1) go func() { defer wg.Done() _, err := nc.Request("natstransport.test", []byte("test data"), 2*time.Second) - if err != nil { - t.Fatal(err) - } + errs <- err }() select { case <-done: + + case err := <-errs: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): t.Fatal("timeout waiting for finalizer") } @@ -472,7 +495,7 @@ func TestNoOpRequestDecoder(t *testing.T) { } } -func testSubscriber(t *testing.T) (step func(), resp <-chan *nats.Msg) { +func testSubscriber(t *testing.T) (step func(), resp <-chan *nats.Msg, err <-chan error) { var ( stepch = make(chan bool) endpoint = func(context.Context, interface{}) (interface{}, error) { @@ -480,6 +503,7 @@ func testSubscriber(t *testing.T) (step func(), resp <-chan *nats.Msg) { return struct{}{}, nil } response = make(chan *nats.Msg) + errs = make(chan error) handler = natstransport.NewSubscriber( endpoint, func(context.Context, *nats.Msg) (interface{}, error) { return struct{}{}, nil }, @@ -495,19 +519,23 @@ func testSubscriber(t *testing.T) (step func(), resp <-chan *nats.Msg) { sub, err := nc.QueueSubscribe("natstransport.test", "natstransport", handler.ServeMsg(nc)) if err != nil { - t.Fatal(err) + errs <- err + return } defer sub.Unsubscribe() r, err := nc.Request("natstransport.test", []byte("test data"), 2*time.Second) if err != nil { - t.Fatal(err) + errs <- err + return } response <- r + + close(errs) }() - return func() { stepch <- true }, response + return func() { stepch <- true }, response, errs } func testRequest(t *testing.T, nc *nats.Conn, handler *natstransport.Subscriber) TestResponse {