DE-1133 Add linter and fix warnings (#123)

Author: vtopc

.github/workflows/main.yml

@@ -36,9 +36,8 @@ jobs:
       - name: nilaway
         run: make nilaway
 
-# TODO(vtopc):
-#      - name: lint
-#        run: make lint
+      - name: lint
+        run: make lint
 
       - name: Test
         run: go test ./... -race -count=1

.golangci.yml

@@ -0,0 +1,76 @@
+linters:
+  # Please, do not use `enable-all`: it's deprecated and will be removed soon.
+  # Inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint.
+  # Full list of linters - https://golangci-lint.run/usage/linters
+  disable-all: true
+  enable:
+    - typecheck
+    - errcheck # Mandatory. Do not disable.
+    - ineffassign # Mandatory. Do not disable.
+    - staticcheck # Mandatory. Do not disable.
+    - govet
+    - gosimple
+    - gosec
+    - bodyclose # https://github.com/timakin/bodyclose
+    - goimports
+    - stylecheck
+    - gocritic
+    - gomodguard
+    - nolintlint
+
+# TODO:
+#    - unused
+
+# TODO(v5): enable
+#    - noctx
+
+linters-settings:
+  errcheck:
+    # List of functions to exclude from checking, where each entry is a single function to exclude.
+    # See https://github.com/kisielk/errcheck#excluding-functions for details.
+    exclude-functions:
+      - (io.Closer).Close
+      - (io.ReadCloser).Close
+
+  govet:
+    enable-all: true
+    disable:
+     - shadow
+     - fieldalignment
+
+  staticcheck:
+    # SAxxxx checks in https://staticcheck.dev/docs/configuration/options/#checks
+    # Example (to disable some checks): [ "all", "-SA1000", "-SA1001"]
+    # Default: ["*"]
+    # TODO(Go1.20+): enable SA1019
+    checks: ["all", "-SA1019"]
+
+  stylecheck:
+    # https://staticcheck.io/docs/options#checks
+    # TODO(v5): enable all:
+    checks: ["all", "-ST1003", "-ST1005"]
+
+  gomodguard:
+    blocked:
+      # List of blocked modules.
+      modules:
+        - github.com/pkg/errors:
+            recommendations:
+              - errors
+              - github.com/mailgun/errors
+            reason: "Deprecated"
+
+issues:
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 50
+
+run:
+  # include test files or not, default is true
+  tests: true
+
+  # Timeout for analysis, e.g. 30s, 5m.
+  # Default: 1m
+  timeout: 5m

Makefile

@@ -23,10 +23,6 @@ GOLINT = $(GOPATH)/bin/golangci-lint
 $(GOLINT):
 	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v1.61.0
 
-.PHONY: lint-new
-lint-new: $(GOLINT)
-	$(GOLINT) run -new-from-rev=master
-
 .PHONY: lint
 lint: $(GOLINT)
 	$(GOLINT) run

core.go

@@ -186,7 +186,7 @@ func buildDataURL(baseURL string, info *DataRequest) string {
 	if info.DataTypeID != 0 {
 		DataTypeID := strconv.FormatInt(info.DataTypeID, 10)
 		tokens = append(tokens, DataTypeID)
-	} else if info.LastID == true {
+	} else if info.LastID {
 		tokens = append(tokens, "LAST")
 	}
 	return strings.Join(tokens, "/")

data_api.go

@@ -5,36 +5,36 @@ import "strings"
 // ListData issues a GET to list the specified data resource
 // and stores the result in the value pointed to by res.
 // Filters can be add via functional options.
-func (mj *Client) ListData(resource string, resp interface{}, options ...RequestOptions) (count, total int, err error) {
-	url := buildDataURL(mj.apiBase, &DataRequest{SourceType: resource})
+func (c *Client) ListData(resource string, resp interface{}, options ...RequestOptions) (count, total int, err error) {
+	url := buildDataURL(c.apiBase, &DataRequest{SourceType: resource})
 	req, err := createRequest("GET", url, nil, nil, options...)
 	if err != nil {
 		return count, total, err
 	}
 
-	return mj.httpClient.Send(req).Read(resp).Call()
+	return c.httpClient.Send(req).Read(resp).Call()
 }
 
 // GetData issues a GET to view a resource specifying an id
 // and stores the result in the value pointed to by res.
 // Filters can be add via functional options.
 // Without an specified SourceTypeID in MailjetDataRequest, it is the same as ListData.
-func (mj *Client) GetData(mdr *DataRequest, res interface{}, options ...RequestOptions) (err error) {
-	url := buildDataURL(mj.apiBase, mdr)
+func (c *Client) GetData(mdr *DataRequest, res interface{}, options ...RequestOptions) (err error) {
+	url := buildDataURL(c.apiBase, mdr)
 	req, err := createRequest("GET", url, nil, nil, options...)
 	if err != nil {
 		return err
 	}
 
-	_, _, err = mj.httpClient.Send(req).Read(res).Call()
+	_, _, err = c.httpClient.Send(req).Read(res).Call()
 	return err
 }
 
 // PostData issues a POST to create a new data resource
 // and stores the result in the value pointed to by res.
 // Filters can be add via functional options.
-func (mj *Client) PostData(fmdr *FullDataRequest, res interface{}, options ...RequestOptions) (err error) {
-	url := buildDataURL(mj.apiBase, fmdr.Info)
+func (c *Client) PostData(fmdr *FullDataRequest, res interface{}, options ...RequestOptions) (err error) {
+	url := buildDataURL(c.apiBase, fmdr.Info)
 	req, err := createRequest("POST", url, fmdr.Payload, nil, options...)
 	if err != nil {
 		return err
@@ -46,36 +46,36 @@ func (mj *Client) PostData(fmdr *FullDataRequest, res interface{}, options ...Re
 		headers = map[string]string{"Content-Type": contentType}
 	}
 
-	_, _, err = mj.httpClient.Send(req).With(headers).Read(res).Call()
+	_, _, err = c.httpClient.Send(req).With(headers).Read(res).Call()
 	return err
 }
 
 // PutData is used to update a data resource.
 // Fields to be updated must be specified by the string array onlyFields.
 // If onlyFields is nil, all fields except these with the tag read_only, are updated.
 // Filters can be add via functional options.
-func (mj *Client) PutData(fmr *FullDataRequest, onlyFields []string, options ...RequestOptions) (err error) {
-	url := buildDataURL(mj.apiBase, fmr.Info)
+func (c *Client) PutData(fmr *FullDataRequest, onlyFields []string, options ...RequestOptions) (err error) {
+	url := buildDataURL(c.apiBase, fmr.Info)
 	req, err := createRequest("PUT", url, fmr.Payload, onlyFields, options...)
 	if err != nil {
 		return err
 	}
 
 	headers := map[string]string{"Content-Type": "application/json"}
-	_, _, err = mj.httpClient.Send(req).With(headers).Call()
+	_, _, err = c.httpClient.Send(req).With(headers).Call()
 
 	return err
 }
 
 // DeleteData is used to delete a data resource.
-func (mj *Client) DeleteData(mdr *DataRequest, options ...RequestOptions) (err error) {
-	url := buildDataURL(mj.apiBase, mdr)
+func (c *Client) DeleteData(mdr *DataRequest, options ...RequestOptions) (err error) {
+	url := buildDataURL(c.apiBase, mdr)
 	req, err := createRequest("DELETE", url, nil, nil, options...)
 	if err != nil {
 		return err
 	}
 
-	_, _, err = mj.httpClient.Send(req).Call()
+	_, _, err = c.httpClient.Send(req).Call()
 
 	return err
 }

example_test.go

@@ -5,7 +5,7 @@ import (
 	"net/textproto"
 	"os"
 
-	mailjet "github.com/mailjet/mailjet-apiv3-go/v4"
+	"github.com/mailjet/mailjet-apiv3-go/v4"
 	"github.com/mailjet/mailjet-apiv3-go/v4/resources"
 )
 

fixtures/fixtures.go

@@ -33,8 +33,7 @@ func New() *Fixtures {
 func (f *Fixtures) Read(v interface{}) error {
 	for t, val := range f.data {
 		if reflect.ValueOf(v).Type().String() == reflect.ValueOf(t).Type().String() {
-			json.Unmarshal(val, v)
-			return nil
+			return json.Unmarshal(val, v)
 		}
 	}
 	return errors.New("not found")

http_client_mock.go

@@ -2,6 +2,8 @@ package mailjet
 
 import (
 	"errors"
+	"fmt"
+	"log"
 	"net/http"
 
 	"github.com/mailjet/mailjet-apiv3-go/v4/fixtures"
@@ -30,7 +32,7 @@ func NewhttpClientMock(valid bool) *HTTPClientMock {
 		validCreds:    valid,
 		fx:            fixtures.New(),
 		CallFunc: func() (int, int, error) {
-			if valid == true {
+			if valid {
 				return 1, 1, nil
 			}
 			return 0, 0, errors.New("Unexpected error: Unexpected server response code: 401: EOF")
@@ -75,7 +77,11 @@ func (c *HTTPClientMock) With(headers map[string]string) HTTPClientInterface {
 
 // Read allow you to bind the response received through the underlying http client
 func (c *HTTPClientMock) Read(response interface{}) HTTPClientInterface {
-	c.fx.Read(response)
+	err := c.fx.Read(response)
+	if err != nil {
+		log.Println(fmt.Errorf("c.fx.Read: %w", err))
+	}
+
 	return c
 }
 

mailjet_client.go

@@ -7,10 +7,8 @@ package mailjet
 import (
 	"bytes"
 	"context"
-	"fmt"
-
 	"encoding/json"
-
+	"fmt"
 	"io"
 	"log"
 	"net/http"
@@ -123,7 +121,7 @@ func SetDebugOutput(w io.Writer) {
 // Sort applies the Sort filter to the request.
 func Sort(value string, order SortOrder) RequestOptions {
 	if order == SortDesc {
-		value = value + "+DESC"
+		value += "+DESC"
 	}
 	return Filter("Sort", value)
 }

mailjet_client_test.go

@@ -12,7 +12,7 @@ import (
 	"testing"
 	"time"
 
-	mailjet "github.com/mailjet/mailjet-apiv3-go/v4"
+	"github.com/mailjet/mailjet-apiv3-go/v4"
 	"github.com/mailjet/mailjet-apiv3-go/v4/resources"
 )
 
@@ -59,14 +59,15 @@ func handle(path, response string) {
 	mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
-		fmt.Fprintf(w, response)
+		fmt.Fprint(w, response)
 	})
 }
 
 func randSeq(n int) string {
 	rand.Seed(time.Now().UnixNano())
 	b := make([]rune, n)
 	for i := range b {
+		//nolint:gosec // G404 crypto random is not required here
 		b[i] = letters[rand.Intn(len(letters))]
 	}
 	return string(b)
@@ -588,6 +589,9 @@ func TestSendMailV31(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			// TODO(Go1.22+): remove:
+			messages := test.messages // https://go.dev/wiki/CommonMistakes
+
 			httpClientMocked := mailjet.NewhttpClientMock(true)
 			httpClientMocked.SendMailV31Func = func(req *http.Request) (*http.Response, error) {
 				if req.Header.Get("Content-Type") != "application/json" {
@@ -619,7 +623,7 @@ func TestSendMailV31(t *testing.T) {
 
 			m := mailjet.NewClient(httpClientMocked, mailjet.NewSMTPClientMock(true))
 
-			res, err := m.SendMailV31(&test.messages)
+			res, err := m.SendMailV31(&messages)
 			if !reflect.DeepEqual(err, test.wantErr) {
 				t.Fatalf("Wanted error: %+v, got: %+v", err, test.wantErr)
 			}

resources/resources.go

@@ -1033,7 +1033,7 @@ type MJMLContent struct {
 type Toplinkclicked struct {
 	ClickedCount int64  `mailjet:"read_only"`
 	ID           int64  `mailjet:"read_only"`
-	LinkID       int64  `json:"LinkId"mailjet:"read_only"`
+	LinkID       int64  `json:"LinkId" mailjet:"read_only"`
 	URL          string `json:"Url" mailjet:"read_only"`
 }
 

smtp_client_mock.go

@@ -18,7 +18,7 @@ func NewSMTPClientMock(valid bool) *SMTPClientMock {
 
 // SendMail wraps smtp.SendMail
 func (s SMTPClientMock) SendMail(from string, to []string, msg []byte) error {
-	if s.valid == true {
+	if s.valid {
 		return nil
 	}
 	return errors.New("smtp send error")

tests/main.go

@@ -19,6 +19,7 @@ func randSeq(n int) string {
 	rand.Seed(time.Now().UnixNano())
 	b := make([]rune, n)
 	for i := range b {
+		//nolint:gosec // G404 crypto random is not required here
 		b[i] = letters[rand.Intn(len(letters))]
 	}
 	return string(b)