DE-1369 Add gocritic linter (#360)

mailgun · Dec 7, 2024 · 8e3c3ac · 8e3c3ac
1 parent b3b8931
commit 8e3c3ac
Show file tree

Hide file tree

Showing 15 changed files with 33 additions and 37 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -13,15 +13,15 @@ linters:
     - govet
     - gosimple
     - goimports
+    - gocritic
     - revive
     - gocyclo
     - noctx
     - gomodguard
     - 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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