fix: style and performance issues found by golangci-lint (#97)

* fix: use to format error * fix: use errors.New instead of fmt.Errorf * refactor: simplify assign operation * refactor: return early on error * refactor: omit unnecessary type declaration * refactor: replace single case switch with if * style: remove extra empty lines * test: fix linter issues * docs: update CHANGELOG Co-authored-by: Jakub Bednář <[email protected]> --------- Co-authored-by: Jakub Bednář <[email protected]>
InfluxCommunity · Sep 2, 2024 · ece5518 · ece5518
1 parent 84efb2c
commit ece5518
Show file tree

Hide file tree

Showing 20 changed files with 210 additions and 89 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,12 +1,131 @@
+run:
+  # Timeout for analysis, e.g. 30s, 5m.
+  # Default: 1m
+  timeout: 90s
+
 linters:
-  disable-all: true
-  enable:
-    - errcheck
-    - gosimple
-    - govet
-    - ineffassign
-    - staticcheck
-    - unused
+  enable-all: true
+  disable:
+    - cyclop
+    - depguard
+    - err113
+    - exhaustruct
+    - exhaustive
+    - forcetypeassert
+    - funlen
+    - godot
+    - gochecknoglobals
+    - gofumpt
+    - gomnd
+    - mnd
+    - nilnil
+    - nlreturn
+    - tagliatelle
+    - varnamelen
+    - whitespace
+    - wrapcheck
+    - wsl
+
+linters-settings:
+  gosec:
+    excludes:
+      - G104
+  lll:
+    # Max line length, lines longer will be reported.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option.
+    # Default: 120.
+    line-length: 160
+  nestif:
+    min-complexity: 7
+  revive:
+    # Enable all available rules.
+    # Default: false
+    enable-all-rules: true
+    rules:
+      - name: add-constant
+        disabled: true
+      - name: cognitive-complexity
+        disabled: true
+      - name: confusing-naming
+        disabled: true
+      - name: cyclomatic
+        disabled: true
+      - name: exported
+        disabled: true
+      - name: flag-parameter
+        disabled: true
+      - name: function-length
+        disabled: true
+      # same as lll
+      - name: line-length-limit
+        disabled: true
+      - name: max-public-structs
+        disabled: true
+      - name: nested-structs
+        disabled: true
+      # same as errcheck
+      - name: unhandled-error
+        disabled: true
+      - name: unused-receiver
+        disabled: true
+      - name: use-any
+        disabled: true
+  tenv:
+    # The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures.
+    # Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked.
+    # Default: false
+    all: true
+
+issues:
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    # Some code is just exceptional. Review whenever the code is changed.
+    - path: influxdb3/query_iterator\.go
+      linters:
+        - gocyclo
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters:
+        - goconst
+        - gofmt
+        - paralleltest
+        - tagalign
+        - testableexamples
+        - testpackage
+        - thelper
+        - tparallel
+        - unparam
+    - path: _test\.go
+      text: "Error return value of .((os.)?std(out|err)..*|.*Close.*|.*Flush|.*Disconnect|.*Clear|os.Remove(All)?|.*print(f|ln)?|os.(Un)?Setenv). is not checked"
+    - path: _test\.go
+      text: "G404: Use of weak random number generator" #gosec:G404
+    - path: _test\.go
+      text: "unused-parameter:" #revive:unused-parameter
+    - path: _test\.go
+      text: "require-error:" #testifylint:require-error
+    - path: _test\.go
+      text: "go-require:" #testifylint:go-require
+    # Relax some specific check for examples (for now, we should have really nice examples) - they are not executed in CI.
+    - path: .*example(_.*)?_test\.go$
+      text: "exitAfterDefer:" #gocritic:exitAfterDefer
+    - path: .*example(_.*)?_test\.go$
+      text: "deep-exit:" #revive:deep-exit
+
+  # Independently of option `exclude` we use default exclude patterns,
+  # it can be disabled by this option.
+  # To list all excluded by default patterns execute `golangci-lint run --help`.
+  # Default: true
+  exclude-use-default: false
+
+  # Maximum issues count per one linter.
+  # Set to 0 to disable.
+  # Default: 50
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text.
+  # Set to 0 to disable.
+  # Default: 3
+  max-same-issues: 0
 
 output:
   # The formats used to render issues.

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ### Bug Fixes
 
 1. [#94](https://github.com/InfluxCommunity/influxdb3-go/pull/94): Resource leak from unclosed `Response`
+1. [#97](https://github.com/InfluxCommunity/influxdb3-go/pull/97): Style and performance improvements discovered by `golangci-lint`
 1. [#98](https://github.com/InfluxCommunity/influxdb3-go/pull/98): Cloud Dedicated database creation ignores the name given by an argument
 
 ### CI

diff --git a/influxdb3/client.go b/influxdb3/client.go
@@ -164,7 +164,7 @@ func (c *Client) makeAPICall(ctx context.Context, params httpParams) (*http.Resp
 
 	req, err := http.NewRequestWithContext(ctx, params.httpMethod, fullURL, params.body)
 	if err != nil {
-		return nil, fmt.Errorf("error calling %s: %v", fullURL, err)
+		return nil, fmt.Errorf("error calling %s: %w", fullURL, err)
 	}
 	for k, v := range c.config.Headers {
 		for _, i := range v {
@@ -183,7 +183,7 @@ func (c *Client) makeAPICall(ctx context.Context, params httpParams) (*http.Resp
 
 	resp, err := c.config.HTTPClient.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("error calling %s: %v", fullURL, err)
+		return nil, fmt.Errorf("error calling %s: %w", fullURL, err)
 	}
 	err = c.resolveHTTPError(resp)
 	if err != nil {

diff --git a/influxdb3/client_test.go b/influxdb3/client_test.go
@@ -24,7 +24,6 @@ package influxdb3
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -33,7 +32,6 @@ import (
 	"testing"
 
 	"github.com/influxdata/line-protocol/v2/lineprotocol"
-
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -52,7 +50,7 @@ func TestNew(t *testing.T) {
 	require.Nil(t, c)
 	if assert.Error(t, err) {
 		expectedMessage := "parsing host URL:"
-		assert.True(t, strings.HasPrefix(err.Error(), expectedMessage), fmt.Sprintf("\nexpected prefix : %s\nactual message  : %s", expectedMessage, err.Error()))
+		assert.True(t, strings.HasPrefix(err.Error(), expectedMessage), "\nexpected prefix : %s\nactual message  : %s", expectedMessage, err.Error())
 	}
 
 	c, err = New(ClientConfig{Host: "http@localhost:8086", Token: "my-token"})
@@ -351,7 +349,7 @@ func TestMakeAPICall(t *testing.T) {
 	html := `<html><body><h1>Response</h1></body></html>`
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "text/html")
-		w.WriteHeader(200)
+		w.WriteHeader(http.StatusOK)
 		_, _ = w.Write([]byte(html))
 	}))
 	defer ts.Close()
@@ -367,9 +365,10 @@ func TestMakeAPICall(t *testing.T) {
 		body:        nil,
 	})
 	assert.NotNil(t, res)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, "Token my-token", res.Request.Header.Get("Authorization"))
-	assert.Nil(t, err)
+	assert.NoError(t, err)
+	_ = res.Body.Close()
 
 	res, err = client.makeAPICall(context.Background(), httpParams{
 		endpointURL: turl,
@@ -379,7 +378,8 @@ func TestMakeAPICall(t *testing.T) {
 		body:        nil,
 	})
 	assert.Equal(t, "Bearer management-api-token", res.Request.Header.Get("Authorization"))
-	assert.Nil(t, err)
+	assert.NoError(t, err)
+	_ = res.Body.Close()
 
 	client, err = New(ClientConfig{Host: ts.URL, Token: "my-token", AuthScheme: "Bearer"})
 	require.NoError(t, err)
@@ -388,22 +388,23 @@ func TestMakeAPICall(t *testing.T) {
 		httpMethod:  "GET",
 	})
 	assert.Equal(t, "Bearer my-token", res.Request.Header.Get("Authorization"))
-	assert.Nil(t, err)
+	assert.NoError(t, err)
+	_ = res.Body.Close()
 }
 
 func TestResolveErrorMessage(t *testing.T) {
 	errMsg := "compilation failed: error at @1:170-1:171: invalid expression @1:167-1:168: |"
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "application/json")
-		w.WriteHeader(400)
+		w.WriteHeader(http.StatusBadRequest)
 		_, _ = w.Write([]byte(`{"code":"invalid","message":"` + errMsg + `"}`))
 	}))
 	defer ts.Close()
 	client, err := New(ClientConfig{Host: ts.URL, Token: "my-token"})
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -419,15 +420,15 @@ func TestResolveErrorHTML(t *testing.T) {
 	html := `<html><body><h1>Not found</h1></body></html>`
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "text/html")
-		w.WriteHeader(404)
+		w.WriteHeader(http.StatusNotFound)
 		_, _ = w.Write([]byte(html))
 	}))
 	defer ts.Close()
 	client, err := New(ClientConfig{Host: ts.URL, Token: "my-token"})
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -452,7 +453,7 @@ func TestResolveErrorRetryAfter(t *testing.T) {
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -468,7 +469,7 @@ func TestResolveErrorWrongJsonResponse(t *testing.T) {
 	errMsg := "compilation failed: error at @1:170-1:171: invalid expression @1:167-1:168: |"
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "application/json")
-		w.WriteHeader(400)
+		w.WriteHeader(http.StatusBadRequest)
 		// Missing closing }
 		_, _ = w.Write([]byte(`{"error": "` + errMsg + `"`))
 	}))
@@ -477,7 +478,7 @@ func TestResolveErrorWrongJsonResponse(t *testing.T) {
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -493,15 +494,15 @@ func TestResolveErrorEdge(t *testing.T) {
 	errMsg := "compilation failed: error at @1:170-1:171: invalid expression @1:167-1:168: |"
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "application/json")
-		w.WriteHeader(400)
+		w.WriteHeader(http.StatusBadRequest)
 		_, _ = w.Write([]byte(`{"error": "` + errMsg + `"}`))
 	}))
 	defer ts.Close()
 	client, err := New(ClientConfig{Host: ts.URL, Token: "my-token"})
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -518,15 +519,15 @@ func TestResolveErrorEdgeWithData(t *testing.T) {
 	dataErrMsg := "compilation failed: error at @1:170-1:171: invalid expression @1:167-1:168: |"
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "application/json")
-		w.WriteHeader(400)
+		w.WriteHeader(http.StatusBadRequest)
 		_, _ = w.Write([]byte(`{"error": "` + errMsg + `", "data": {"error_message": "` + dataErrMsg + `"}}`))
 	}))
 	defer ts.Close()
 	client, err := New(ClientConfig{Host: ts.URL, Token: "my-token"})
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -540,14 +541,14 @@ func TestResolveErrorEdgeWithData(t *testing.T) {
 
 func TestResolveErrorNoError(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.WriteHeader(500)
+		w.WriteHeader(http.StatusInternalServerError)
 	}))
 	defer ts.Close()
 	client, err := New(ClientConfig{Host: ts.URL, Token: "my-token"})
 	require.NoError(t, err)
 	turl, err := url.Parse(ts.URL)
 	require.NoError(t, err)
-	res, err := client.makeAPICall(context.Background(), httpParams{
+	res, err := client.makeAPICall(context.Background(), httpParams{ //nolint:bodyclose
 		endpointURL: turl,
 		queryParams: nil,
 		httpMethod:  "GET",
@@ -610,7 +611,7 @@ func TestFixUrl(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		t.Run(fmt.Sprintf("fix url: %s", tc.input),
+		t.Run(tc.input,
 			func(t *testing.T) {
 				u, safe := ReplaceURLProtocolWithPort(tc.input)
 				assert.Equal(t, tc.expected, u)

diff --git a/influxdb3/example_test.go b/influxdb3/example_test.go
@@ -22,7 +22,9 @@
 
 package influxdb3
 
-import "log"
+import (
+	"log"
+)
 
 func ExampleNew() {
 	client, err := New(ClientConfig{
@@ -51,4 +53,3 @@ func ExampleNewFromEnv() {
 	}
 	defer client.Close()
 }
-
diff --git a/influxdb3/example_write_test.go b/influxdb3/example_write_test.go
@@ -117,10 +117,9 @@ func ExampleClient_WriteData() {
 
 	// write points with options
 	err = client.WriteData(context.Background(), points, WithDefaultTags(map[string]string{
-		"version":"0.1",
+		"version": "0.1",
 	}))
 	if err != nil {
 		log.Fatal()
 	}
 }
-