linters fixes

Configured golangci-lint

Configured golangci-lint

.github/workflows/test.yml

@@ -51,4 +51,4 @@ jobs:
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.37
+          version: latest

.golangci.yml

@@ -1,10 +1,127 @@
+# Configuration reference: https://golangci-lint.run/usage/configuration/
+# Linters reference: https://golangci-lint.run/usage/linters/
 run:
   build-tags:
     - examples
   skip-dirs:
     - apiv6/localhost_apiserver/cloudexport
     - apiv6/localhost_apiserver/synthetics
+
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
-# TODO(dfurman): enable more linters
+  exclude:
+    # EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments
+    - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)
+    - (struct (of|with)|could be)
+  exclude-use-default: false
+  exclude-rules: # exclude linters impossible to exclude via //nolint
+    - path: ^kentikapi/models/enum_ # these files are generated and shouldn't be edited with //nolint
+      linters:
+        - gochecknoglobals
+        - lll
+        - gomnd
+
+# Disabled linters:
+# - cyclop - duplicates functionality of gocyclo
+# - exhaustivestruct - breaks "Make the zero value useful" proverb, meant to be used only for special cases
+# - funlen - only test functions exceeds the line limit of 60 - too strict, these functions are easily readable
+# - godox - requires all TODOs to be removed - too strict
+# - gomoddirectives - does not allow "replace" directives - too strict
+# - goerr113 - following check is too strict: "do not define dynamic errors, use wrapped static errors instead",
+#              the check cannot be disabled
+# - interfacer - deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner
+# - maligned - deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner
+# - nlreturn - leads to using too many line breaks
+# - prealloc - from docs:
+    # XXX: we don't recommend using this linter before doing performance profiling.
+    # For most programs usage of prealloc will be a premature optimization.
+# - scopelint - deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner
+# - thelper - enforcing t.Helper() everywhere is too strict
+# - wrapcheck - valuable linter which I think needs higher level knowledge on the project to be fixed
+# - wsl - leads to using too many line breaks
+linters:
+  enable:
+    - asciicheck
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
+    - dupl
+    - durationcheck
+    - errcheck
+    - errorlint
+    - exhaustive
+    - exportloopref
+    - forbidigo
+    - forcetypeassert
+    - gci
+    - gochecknoglobals
+    - gochecknoinits
+    - gocognit
+    - goconst
+    - gocritic
+    - gocyclo
+    - godot
+    - gofmt
+    - gofumpt
+    - goheader
+    - goimports
+    - revive
+    - gomnd
+    - gomodguard
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - ifshort
+    - importas
+    - ineffassign
+    - lll
+    - makezero
+    - misspell
+    - nakedret
+    - nestif
+    - nilerr
+    - noctx
+    - nolintlint
+    - paralleltest
+    - predeclared
+    - revive
+    - rowserrcheck
+    - sqlclosecheck
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - testpackage
+    - tparallel
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+    - wastedassign
+    - whitespace
+
+linters-settings:
+  dupl:
+    tests: false
+  errcheck:
+    check-type-assertions: true
+    check-blank: true
+  errorlint:
+    # Check whether fmt.Errorf uses the %w verb for formatting errors - too strict
+    errorf: false
+  gocyclo:
+    min-complexity: 11
+  golint:
+    min-confidence: 0
+  gosec:
+    tests: false
+  govet:
+    enable-all: true
+  lll:
+    # lines longer than 127 will be reported, 120 is default
+    line-length: 127
+  nakedret:
+    max-func-lines: 5

apiv6/kentikapi/client.go

@@ -1,3 +1,4 @@
+//nolint:revive,stylecheck // Changing Api to API forces changes in generated files
 package kentikapi
 
 import (
@@ -6,6 +7,7 @@ import (
 	"github.com/kentik/community_sdk_golang/apiv6/kentikapi/synthetics"
 )
 
+//nolint:gosec
 const (
 	authAPITokenKey   = "X-CH-Auth-API-Token"
 	authEmailKey      = "X-CH-Auth-Email"

apiv6/kentikapi/client_integration_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+//nolint:gosec
 const (
 	authEmailKey    = "X-CH-Auth-Email"
 	authAPITokenKey = "X-CH-Auth-API-Token"
@@ -24,7 +25,10 @@ const (
 	testAgentID     = "968"
 )
 
+//nolint:errcheck // Conflict with defer Close() (additional info: https://github.com/kisielk/errcheck/issues/55)
 func TestClient_PatchAgent(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		name             string
 		retryMax         *int
@@ -160,7 +164,10 @@ func TestClient_PatchAgent(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
 			// arrange
 			h := newSpyHTTPHandler(t, tt.responses)
 			s := httptest.NewServer(h)
@@ -185,6 +192,7 @@ func TestClient_PatchAgent(t *testing.T) {
 				AgentPatch(context.Background(), testAgentID).
 				Body(tt.request).
 				Execute()
+			defer httpResp.Body.Close()
 
 			// assert
 			t.Logf("Got result: %v, httpResp: %v, err: %v", result, httpResp, err)
@@ -197,7 +205,7 @@ func TestClient_PatchAgent(t *testing.T) {
 			assert.Equal(t, len(h.responses), len(h.requests), "invalid number of requests")
 			for _, r := range h.requests {
 				assert.Equal(t, http.MethodPatch, r.method)
-				assert.Equal(t, fmt.Sprintf("/synthetics/v202101beta1/agents/%v", testAgentID), r.url_.Path)
+				assert.Equal(t, fmt.Sprintf("/synthetics/v202101beta1/agents/%v", testAgentID), r.url.Path)
 				assert.Equal(t, dummyAuthEmail, r.header.Get(authEmailKey))
 				assert.Equal(t, dummyAuthToken, r.header.Get(authAPITokenKey))
 				assert.Equal(t, tt.expectedRequestBody, unmarshalJSONToIf(t, r.body))
@@ -225,26 +233,34 @@ func newDummyAgent() *synthetics.V202101beta1Agent {
 	status := synthetics.V202101BETA1AGENTSTATUS_WAIT
 	family := synthetics.V202101BETA1IPFAMILY_DUAL
 	agent := &synthetics.V202101beta1Agent{
-		Id:         stringPtr(testAgentID),
-		Name:       stringPtr("dummy-agent"),
-		Status:     &status,
-		Alias:      stringPtr("probe-4-ams-1"),
-		Type:       stringPtr("global"),
-		Os:         stringPtr("I use Manjaro BTW"),
-		Ip:         stringPtr("95.179.136.58"),
-		Lat:        float64Ptr(52.374031),
-		Long:       float64Ptr(4.88969),
-		LastAuthed: timePtr(time.Date(2020, time.July, 9, 21, 37, 00, 826*1000000, time.UTC)),
-		Family:     &family,
-		Asn:        int64Ptr(20473),
-		SiteId:     stringPtr("2137"),
-		Version:    stringPtr("0.0.2"),
-		Challenge:  stringPtr("dummy-challenge"),
-		City:       stringPtr("Amsterdam"),
-		Region:     stringPtr("Noord-Holland"),
-		Country:    stringPtr("Netherlands"),
-		TestIds:    &[]string{"13", "133", "1337"},
-		LocalIp:    stringPtr("10.10.10.10"),
+		Id:     stringPtr(testAgentID),
+		Name:   stringPtr("dummy-agent"),
+		Status: &status,
+		Alias:  stringPtr("probe-4-ams-1"),
+		Type:   stringPtr("global"),
+		Os:     stringPtr("I use Manjaro BTW"),
+		Ip:     stringPtr("95.179.136.58"),
+		Lat:    float64Ptr(52.374031),
+		Long:   float64Ptr(4.88969),
+		LastAuthed: timePtr(time.Date(2020,
+			time.July,
+			9,
+			21,
+			37,
+			0,
+			826*1000000,
+			time.UTC,
+		)),
+		Family:    &family,
+		Asn:       int64Ptr(20473),
+		SiteId:    stringPtr("2137"),
+		Version:   stringPtr("0.0.2"),
+		Challenge: stringPtr("dummy-challenge"),
+		City:      stringPtr("Amsterdam"),
+		Region:    stringPtr("Noord-Holland"),
+		Country:   stringPtr("Netherlands"),
+		TestIds:   &[]string{"13", "133", "1337"},
+		LocalIp:   stringPtr("10.10.10.10"),
 	}
 
 	return agent
@@ -336,7 +352,7 @@ func (h *spyHTTPHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 
 	h.requests = append(h.requests, httpRequest{
 		method: r.Method,
-		url_:   r.URL,
+		url:    r.URL,
 		header: r.Header,
 		body:   string(body),
 	})
@@ -364,7 +380,7 @@ func (h *spyHTTPHandler) response() httpResponse {
 
 type httpRequest struct {
 	method string
-	url_   *url.URL
+	url    *url.URL
 	header http.Header
 	body   string
 }

apiv6/kentikapi/httputil/httputil.go

@@ -85,6 +85,7 @@ func (cfg *ClientConfig) FillDefaults() {
 	}
 }
 
+//nolint:gomnd // This is the only place for these numbers to turn up.
 func defaultHTTPClient() *http.Client {
 	return &http.Client{
 		Transport: &http.Transport{

apiv6/kentikapi/httputil/httputil_test.go

@@ -1,4 +1,4 @@
-package httputil
+package httputil_test
 
 import (
 	"context"
@@ -10,7 +10,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-retryablehttp"
-
+	"github.com/kentik/community_sdk_golang/apiv6/kentikapi/httputil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -22,15 +22,23 @@ import (
 // 4. retryingClient.retryableRoundTripper.retryableClient.httpClient.Do()
 // 5. retryingClient.retryableRoundTripper.retryableClient.httpClient.httpTransport.RoundTrip()
 
+//nolint:errcheck // https://github.com/kisielk/errcheck/issues/55
+// Closing a response you only read from cannot yield a meaningful error.
 func TestRetryingClient_Do_ReturnsHTTPTransportError(t *testing.T) {
+	t.Parallel()
+
 	// arrange
-	c := NewRetryingClient(ClientConfig{})
+	c := httputil.NewRetryingClient(httputil.ClientConfig{})
 
 	req, err := retryablehttp.NewRequest(http.MethodGet, "https://invalid.url", nil)
 	require.NoError(t, err)
 
 	// act
 	resp, err := c.Do(req.WithContext(context.Background()))
+	if err != nil {
+		return
+	}
+	defer resp.Body.Close()
 
 	// assert
 	t.Logf("Got response: %v, err: %v", resp, err)
@@ -40,7 +48,11 @@ func TestRetryingClient_Do_ReturnsHTTPTransportError(t *testing.T) {
 	assert.Equal(t, "no such host", dnsErr.Err)
 }
 
+//nolint:errcheck // https://github.com/kisielk/errcheck/issues/55
+// Closing a response you only read from cannot yield a meaningful error.
 func TestRetryingClientWithSpyHTTPTransport_Do(t *testing.T) {
+	t.Parallel()
+
 	const retryMax = 5
 
 	tests := []struct {
@@ -73,15 +85,17 @@ func TestRetryingClientWithSpyHTTPTransport_Do(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			// arrange
+			t.Parallel()
 
+			// arrange
 			st := spyTransport{transportError: tt.transportError}
-			c := NewRetryingClient(ClientConfig{
+			c := httputil.NewRetryingClient(httputil.ClientConfig{
 				HTTPClient: &http.Client{
 					Transport: &st,
 				},
-				RetryCfg: RetryConfig{
+				RetryCfg: httputil.RetryConfig{
 					MaxAttempts: intPtr(retryMax),
 					MinDelay:    durationPtr(1 * time.Microsecond),
 					MaxDelay:    durationPtr(10 * time.Microsecond),
@@ -93,6 +107,10 @@ func TestRetryingClientWithSpyHTTPTransport_Do(t *testing.T) {
 
 			// act
 			resp, err := c.Do(req.WithContext(context.Background()))
+			if err != nil {
+				return
+			}
+			defer resp.Body.Close()
 
 			// assert
 			t.Logf("Got response: %v, err: %v", resp, err)