From 8e3c3acddca2976a0e029050d0d69e6aeac4ed4e Mon Sep 17 00:00:00 2001
From: Vilen Topchii <32271530+vtopc@users.noreply.github.com>
Date: Sat, 7 Dec 2024 14:20:44 +0200
Subject: [PATCH] DE-1369 Add gocritic linter (#360)

---
 .golangci.yml           | 19 ++++++++-----------
 analytics.go            |  2 +-
 attachments_test.go     |  2 +-
 bounces.go              |  6 +++---
 credentials.go          |  4 ++--
 domains.go              |  6 +++---
 domains_test.go         |  4 ++--
 email_validation.go     |  5 +++--
 events/events.go        |  2 --
 mailgun.go              |  1 -
 parse_test.go           |  2 +-
 routes.go               |  9 +++++----
 spam_complaints_test.go |  1 -
 subaccounts.go          |  4 ++--
 tags.go                 |  3 ++-
 15 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/.golangci.yml b/.golangci.yml
index 39864137..395abf23 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -13,6 +13,7 @@ linters:
     - govet
     - gosimple
     - goimports
+    - gocritic
     - revive
     - gocyclo
     - noctx
@@ -20,8 +21,7 @@ linters:
     - nolintlint
 
 # TODO:
-#    - gocritic
-#    - stylecheck
+#    - stylecheck # TODO(v5): enable
 #    - unused
 
 linters-settings:
@@ -32,6 +32,12 @@ linters-settings:
       - opinionated
       - performance
       - style
+    disabled-checks:
+      - singleCaseSwitch
+      - deferInLoop # TODO(DE-1373): enable
+      - hugeParam # TODO(v5): enable?
+      - sprintfQuotedString # noisy # TODO: enable
+      - exitAfterDefer # TODO: enable?
 
   errcheck:
     # List of functions to exclude from checking, where each entry is a single function to exclude.
@@ -144,15 +150,6 @@ linters-settings:
         disabled: false
         exclude: [""]
 
-      # TODO(DE-1373): enable:
-      #      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#defer
-      #      - name: defer
-      #        severity: warning
-      #        disabled: false
-      #        exclude: [""]
-      #        arguments:
-      #          - [ "call-chain", "loop" ]
-
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#dot-imports
       - name: dot-imports
         severity: warning
diff --git a/analytics.go b/analytics.go
index 001703f2..3163da93 100644
--- a/analytics.go
+++ b/analytics.go
@@ -79,7 +79,7 @@ func (iter *MetricsIterator) Next(ctx context.Context, resp *MetricsResponse) (m
 		return false
 	}
 
-	iter.opts.Pagination.Skip = iter.opts.Pagination.Skip + iter.opts.Pagination.Limit
+	iter.opts.Pagination.Skip += iter.opts.Pagination.Limit
 
 	return len(resp.Items) == iter.opts.Pagination.Limit
 }
diff --git a/attachments_test.go b/attachments_test.go
index d843371b..9812eb85 100644
--- a/attachments_test.go
+++ b/attachments_test.go
@@ -19,7 +19,7 @@ func createAttachment(t *testing.T) string {
 	f, err := os.Create(name)
 	require.NoError(t, err)
 
-	_, err = f.Write([]byte(randomString(100, "")))
+	_, err = f.WriteString(randomString(100, ""))
 	require.NoError(t, err)
 	require.Nil(t, f.Close())
 	return name
diff --git a/bounces.go b/bounces.go
index 0e576c75..d27948ae 100644
--- a/bounces.go
+++ b/bounces.go
@@ -172,7 +172,7 @@ func (mg *MailgunImpl) GetBounce(ctx context.Context, address string) (Bounce, e
 //
 // Note that both code and error exist as strings, even though
 // code will report as a number.
-func (mg *MailgunImpl) AddBounce(ctx context.Context, address, code, error string) error {
+func (mg *MailgunImpl) AddBounce(ctx context.Context, address, code, bounceError string) error {
 	r := newHTTPRequest(generateApiUrl(mg, bouncesEndpoint))
 	r.setClient(mg.Client())
 	r.setBasicAuth(basicAuthUser, mg.APIKey())
@@ -182,8 +182,8 @@ func (mg *MailgunImpl) AddBounce(ctx context.Context, address, code, error strin
 	if code != "" {
 		payload.addValue("code", code)
 	}
-	if error != "" {
-		payload.addValue("error", error)
+	if bounceError != "" {
+		payload.addValue("error", bounceError)
 	}
 	_, err := makePostRequest(ctx, r, payload)
 	return err
diff --git a/credentials.go b/credentials.go
index ae7c6d77..05873a57 100644
--- a/credentials.go
+++ b/credentials.go
@@ -79,7 +79,7 @@ func (ri *CredentialsIterator) Next(ctx context.Context, items *[]Credential) bo
 	if len(ri.Items) == 0 {
 		return false
 	}
-	ri.offset = ri.offset + len(ri.Items)
+	ri.offset += len(ri.Items)
 	return true
 }
 
@@ -141,7 +141,7 @@ func (ri *CredentialsIterator) Previous(ctx context.Context, items *[]Credential
 		return false
 	}
 
-	ri.offset = ri.offset - (ri.limit * 2)
+	ri.offset -= ri.limit * 2
 	if ri.offset < 0 {
 		ri.offset = 0
 	}
diff --git a/domains.go b/domains.go
index 82d5e048..8fcb81f8 100644
--- a/domains.go
+++ b/domains.go
@@ -137,7 +137,7 @@ func (ri *DomainsIterator) Next(ctx context.Context, items *[]Domain) bool {
 	if len(ri.Items) == 0 {
 		return false
 	}
-	ri.offset = ri.offset + len(ri.Items)
+	ri.offset += len(ri.Items)
 	return true
 }
 
@@ -199,7 +199,7 @@ func (ri *DomainsIterator) Previous(ctx context.Context, items *[]Domain) bool {
 		return false
 	}
 
-	ri.offset = ri.offset - (ri.limit * 2)
+	ri.offset -= ri.limit * 2
 	if ri.offset < 0 {
 		ri.offset = 0
 	}
@@ -305,7 +305,7 @@ func (mg *MailgunImpl) CreateDomain(ctx context.Context, name string, opts *Crea
 		if len(opts.IPS) != 0 {
 			payload.addValue("ips", strings.Join(opts.IPS, ","))
 		}
-		if len(opts.Password) != 0 {
+		if opts.Password != "" {
 			payload.addValue("smtp_password", opts.Password)
 		}
 		if opts.WebScheme != "" {
diff --git a/domains_test.go b/domains_test.go
index 9cfb0993..fc459eda 100644
--- a/domains_test.go
+++ b/domains_test.go
@@ -120,8 +120,8 @@ func TestDomainTracking(t *testing.T) {
 	require.NoError(t, err)
 
 	require.False(t, info.Unsubscribe.Active)
-	require.True(t, len(info.Unsubscribe.HTMLFooter) != 0)
-	require.True(t, len(info.Unsubscribe.TextFooter) != 0)
+	require.True(t, info.Unsubscribe.HTMLFooter != "")
+	require.True(t, info.Unsubscribe.TextFooter != "")
 	require.True(t, info.Click.Active)
 	require.True(t, info.Open.Active)
 
diff --git a/email_validation.go b/email_validation.go
index 25cf2522..b1ff5c46 100644
--- a/email_validation.go
+++ b/email_validation.go
@@ -216,14 +216,15 @@ func (m *EmailValidatorImpl) validateV4(ctx context.Context, email string, mailB
 // as `https://api.mailgun.net/v3` or set `v.SetAPIBase("https://api.mailgun.net/v3")`
 //
 // Deprecated: /v3/address/parse is deprecated use ValidateEmail instead.
-func (m *EmailValidatorImpl) ParseAddresses(ctx context.Context, addresses ...string) ([]string, []string, error) {
+// TODO(v5): remove
+func (m *EmailValidatorImpl) ParseAddresses(ctx context.Context, addresses ...string) (parsed, unparseable []string, err error) {
 	r := newHTTPRequest(m.getAddressURL("parse"))
 	r.setClient(m.Client())
 	r.addParameter("addresses", strings.Join(addresses, ","))
 	r.setBasicAuth(basicAuthUser, m.APIKey())
 
 	var response addressParseResult
-	err := getResponseFromJSON(ctx, r, &response)
+	err = getResponseFromJSON(ctx, r, &response)
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/events/events.go b/events/events.go
index 72405542..da265010 100644
--- a/events/events.go
+++ b/events/events.go
@@ -30,8 +30,6 @@ func (g *Generic) GetTimestamp() time.Time {
 }
 
 func (g *Generic) SetTimestamp(t time.Time) {
-	// convert := fmt.Sprintf("%d.%06d", t.Unix(), t.Nanosecond()/int(time.Microsecond))
-	// ts, err := strconv.ParseFloat(convert, 64)
 	g.Timestamp = float64(t.Unix()) + (float64(t.Nanosecond()/int(time.Microsecond)) / float64(1000000))
 }
 
diff --git a/mailgun.go b/mailgun.go
index cbff246d..24271ca1 100644
--- a/mailgun.go
+++ b/mailgun.go
@@ -445,7 +445,6 @@ func generateCredentialsUrl(m Mailgun, login string) string {
 		tail = fmt.Sprintf("/%s", login)
 	}
 	return generateDomainApiUrl(m, fmt.Sprintf("credentials%s", tail))
-	// return fmt.Sprintf("%s/domains/%s/credentials%s", apiBase, m.Domain(), tail)
 }
 
 // generatePublicApiUrl works as generateApiUrl, except that generatePublicApiUrl has no need for the domain.
diff --git a/parse_test.go b/parse_test.go
index 82d2403d..b65bdb04 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -158,7 +158,7 @@ func TestTimeStamp(t *testing.T) {
 	assert.Equal(t, ts, event.GetTimestamp())
 
 	event.Timestamp = 1546899001.019501
-	assert.Equal(t, time.Date(2019, 1, 7, 22, 10, 01, 19501056, time.UTC), event.GetTimestamp())
+	assert.Equal(t, time.Date(2019, 1, 7, 22, 10, 1, 19501056, time.UTC), event.GetTimestamp())
 }
 
 func TestEventNames(t *testing.T) {
diff --git a/routes.go b/routes.go
index bfce338f..e86895b0 100644
--- a/routes.go
+++ b/routes.go
@@ -163,7 +163,7 @@ func (ri *RoutesIterator) Next(ctx context.Context, items *[]Route) bool {
 	if len(ri.Items) == 0 {
 		return false
 	}
-	ri.offset = ri.offset + len(ri.Items)
+	ri.offset += len(ri.Items)
 	return true
 }
 
@@ -225,7 +225,7 @@ func (ri *RoutesIterator) Previous(ctx context.Context, items *[]Route) bool {
 		return false
 	}
 
-	ri.offset = ri.offset - (ri.limit * 2)
+	ri.offset -= ri.limit * 2
 	if ri.offset < 0 {
 		ri.offset = 0
 	}
@@ -273,10 +273,11 @@ func (mg *MailgunImpl) CreateRoute(ctx context.Context, prototype Route) (_ignor
 		p.addValue("action", action)
 	}
 	var resp createRouteResp
-	if err = postResponseFromJSON(ctx, r, p, &resp); err != nil {
+	if err := postResponseFromJSON(ctx, r, p, &resp); err != nil {
 		return _ignored, err
 	}
-	return resp.Route, err
+
+	return resp.Route, nil
 }
 
 // DeleteRoute removes the specified route from your domain's configuration.
diff --git a/spam_complaints_test.go b/spam_complaints_test.go
index 9e271c9f..3d4ea20d 100644
--- a/spam_complaints_test.go
+++ b/spam_complaints_test.go
@@ -19,7 +19,6 @@ func TestGetComplaints(t *testing.T) {
 	it := mg.ListComplaints(nil)
 	var page []mailgun.Complaint
 	for it.Next(ctx, &page) {
-		// spew.Dump(page)
 	}
 	require.NoError(t, it.Err())
 }
diff --git a/subaccounts.go b/subaccounts.go
index fb9bb305..8b6bc1ed 100644
--- a/subaccounts.go
+++ b/subaccounts.go
@@ -101,7 +101,7 @@ func (ri *SubaccountsIterator) Next(ctx context.Context, items *[]Subaccount) bo
 	if len(ri.Items) == 0 {
 		return false
 	}
-	ri.offset = ri.offset + len(ri.Items)
+	ri.offset += len(ri.Items)
 	return true
 }
 
@@ -163,7 +163,7 @@ func (ri *SubaccountsIterator) Previous(ctx context.Context, items *[]Subaccount
 		return false
 	}
 
-	ri.offset = ri.offset - (ri.limit * 2)
+	ri.offset -= ri.limit * 2
 	if ri.offset < 0 {
 		ri.offset = 0
 	}
diff --git a/tags.go b/tags.go
index 85214004..1fc7dd1e 100644
--- a/tags.go
+++ b/tags.go
@@ -41,7 +41,8 @@ func (mg *MailgunImpl) GetTag(ctx context.Context, tag string) (Tag, error) {
 	r.setClient(mg.Client())
 	r.setBasicAuth(basicAuthUser, mg.APIKey())
 	var tagItem Tag
-	return tagItem, getResponseFromJSON(ctx, r, &tagItem)
+	err := getResponseFromJSON(ctx, r, &tagItem)
+	return tagItem, err
 }
 
 // ListTags returns a cursor used to iterate through a list of tags