Skip to content

Commit

Permalink
chore: Enable dynamicFmtString and sprintfQuotedString checkers for g…
Browse files Browse the repository at this point in the history
…ocritic (influxdata#13279)

Co-authored-by: Pawel Zak <Pawel Zak>
  • Loading branch information
zak-pawel committed Jun 9, 2023
1 parent 16786d2 commit 02f0b15
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 39 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ linters-settings:
- dupBranchBody
- dupCase
- dupSubExpr
- dynamicFmtString
- emptyDecl
- evalOrder
- exitAfterDefer
Expand All @@ -110,6 +111,7 @@ linters-settings:
- regexpPattern
- sloppyTypeAssert
- sortSlice
- sprintfQuotedString
- sqlQuery
- uncheckedInlineErr
- unnecessaryDefer
Expand Down
6 changes: 3 additions & 3 deletions plugins/inputs/dpdk/dpdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func Test_getCommandsAndParamsCombinations(t *testing.T) {
t.Run("when 2 ethdev commands are enabled, then 2*numberOfIds new commands should be appended", func(t *testing.T) {
mockConn, dpdk, mockAcc := prepareEnvironment()
defer mockConn.AssertExpectations(t)
response := fmt.Sprintf(`{"%s": [1, 123]}`, ethdevListCommand)
response := fmt.Sprintf(`{%q: [1, 123]}`, ethdevListCommand)
simulateResponse(mockConn, response, nil)
expectedCommands := []string{"/ethdev/stats,1", "/ethdev/stats,123", "/ethdev/xstats,1", "/ethdev/xstats,123"}

Expand All @@ -255,7 +255,7 @@ func Test_getCommandsAndParamsCombinations(t *testing.T) {
t.Run("when 1 rawdev command is enabled, then 2*numberOfIds new commands should be appended", func(t *testing.T) {
mockConn, dpdk, mockAcc := prepareEnvironment()
defer mockConn.AssertExpectations(t)
response := fmt.Sprintf(`{"%s": [1, 123]}`, rawdevListCommand)
response := fmt.Sprintf(`{%q: [1, 123]}`, rawdevListCommand)
simulateResponse(mockConn, response, nil)
expectedCommands := []string{"/rawdev/xstats,1", "/rawdev/xstats,123"}

Expand All @@ -271,7 +271,7 @@ func Test_getCommandsAndParamsCombinations(t *testing.T) {
t.Run("when 2 ethdev commands are enabled but one command is disabled, then numberOfIds new commands should be appended", func(t *testing.T) {
mockConn, dpdk, mockAcc := prepareEnvironment()
defer mockConn.AssertExpectations(t)
response := fmt.Sprintf(`{"%s": [1, 123]}`, ethdevListCommand)
response := fmt.Sprintf(`{%q: [1, 123]}`, ethdevListCommand)
simulateResponse(mockConn, response, nil)
expectedCommands := []string{"/ethdev/stats,1", "/ethdev/stats,123"}

Expand Down
4 changes: 2 additions & 2 deletions plugins/inputs/dpdk/dpdk_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func Test_jsonToArray(t *testing.T) {
t.Run("when got numeric array then string array should be returned", func(t *testing.T) {
firstValue := int64(0)
secondValue := int64(1)
jsonString := fmt.Sprintf(`{"%s": [%d, %d]}`, key, firstValue, secondValue)
jsonString := fmt.Sprintf(`{%q: [%d, %d]}`, key, firstValue, secondValue)

arr, err := jsonToArray([]byte(jsonString), key)

Expand All @@ -120,7 +120,7 @@ func Test_jsonToArray(t *testing.T) {
})

t.Run("when valid json with json-object is supplied as input then error should be returned", func(t *testing.T) {
jsonString := fmt.Sprintf(`{"%s": {"testKey": "testValue"}}`, key)
jsonString := fmt.Sprintf(`{%q: {"testKey": "testValue"}}`, key)

_, err := jsonToArray([]byte(jsonString), key)

Expand Down
15 changes: 8 additions & 7 deletions plugins/inputs/elasticsearch/elasticsearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package elasticsearch
import (
_ "embed"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -204,14 +205,14 @@ func (e *Elasticsearch) Gather(acc telegraf.Accumulator) error {

// Gather node ID
if info.nodeID, err = e.gatherNodeID(s + "/_nodes/_local/name"); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}

// get cat/master information here so NodeStats can determine
// whether this node is the Master
if info.masterID, err = e.getCatMaster(s + "/_cat/master"); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}

Expand All @@ -233,7 +234,7 @@ func (e *Elasticsearch) Gather(acc telegraf.Accumulator) error {

// Always gather node stats
if err := e.gatherNodeStats(url, acc); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}

Expand All @@ -243,27 +244,27 @@ func (e *Elasticsearch) Gather(acc telegraf.Accumulator) error {
url = url + "?level=" + e.ClusterHealthLevel
}
if err := e.gatherClusterHealth(url, acc); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}
}

if e.ClusterStats && (e.serverInfo[s].isMaster() || !e.ClusterStatsOnlyFromMaster || !e.Local) {
if err := e.gatherClusterStats(s+"/_cluster/stats", acc); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}
}

if len(e.IndicesInclude) > 0 && (e.serverInfo[s].isMaster() || !e.ClusterStatsOnlyFromMaster || !e.Local) {
if e.IndicesLevel != "shards" {
if err := e.gatherIndicesStats(s+"/"+strings.Join(e.IndicesInclude, ",")+"/_stats", acc); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}
} else {
if err := e.gatherIndicesStats(s+"/"+strings.Join(e.IndicesInclude, ",")+"/_stats?level=shards", acc); err != nil {
acc.AddError(fmt.Errorf(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
acc.AddError(errors.New(mask.ReplaceAllString(err.Error(), "http(s)://XXX:XXX@")))
return
}
}
Expand Down
23 changes: 12 additions & 11 deletions plugins/inputs/intel_dlb/intel_dlb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (
"testing"
"time"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/influxdata/telegraf"
"github.com/influxdata/telegraf/plugins/inputs/dpdk/mocks"
"github.com/influxdata/telegraf/testutil"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func TestDLB_Init(t *testing.T) {
Expand Down Expand Up @@ -310,7 +311,7 @@ func TestDLB_gatherCommandsWithDeviceIndex(t *testing.T) {
maxInitMessageLength: 1024,
EventdevCommands: []string{"/eventdev/dev_xstats"},
}
response := fmt.Sprintf(`{"%s": [0, 1]}`, eventdevListCommand)
response := fmt.Sprintf(`{%q: [0, 1]}`, eventdevListCommand)
simulateResponse(mockConn, response, nil)

expectedCommands := []string{"/eventdev/dev_xstats,0", "/eventdev/dev_xstats,1"}
Expand All @@ -330,7 +331,7 @@ func TestDLB_gatherCommandsWithDeviceIndex(t *testing.T) {
maxInitMessageLength: 1024,
EventdevCommands: []string{"/eventdev/queue_links"},
}
responseDevList := fmt.Sprintf(`{"%s": [0]}`, eventdevListCommand)
responseDevList := fmt.Sprintf(`{%q: [0]}`, eventdevListCommand)
simulateResponse(mockConn, responseDevList, nil)
responseQueueLinks := `{"0": [0]}`
simulateResponse(mockConn, responseQueueLinks, nil)
Expand All @@ -352,7 +353,7 @@ func TestDLB_gatherCommandsWithDeviceIndex(t *testing.T) {
maxInitMessageLength: 1024,
EventdevCommands: []string{"/eventdev/dev_xstats", "/eventdev/wrong"},
}
response := fmt.Sprintf(`{"%s": [0, 1]}`, eventdevListCommand)
response := fmt.Sprintf(`{%q: [0, 1]}`, eventdevListCommand)
mockConn.On("Write", mock.Anything).Return(0, nil).Once()
mockConn.On("Read", mock.Anything).Run(func(arg mock.Arguments) {
elem := arg.Get(0).([]byte)
Expand Down Expand Up @@ -443,7 +444,7 @@ func TestDLB_gatherSecondDeviceIndex(t *testing.T) {
EventdevCommands: []string{"/eventdev/port_xstats"},
}
eventdevListWithSecondIndex := []string{"/eventdev/port_list", "/eventdev/queue_list"}
response := fmt.Sprintf(`{"%s": [0, 1]}`, eventdevListWithSecondIndex[0])
response := fmt.Sprintf(`{%q: [0, 1]}`, eventdevListWithSecondIndex[0])
simulateResponse(mockConn, response, nil)

expectedCommands := []string{"/eventdev/port_xstats,0,0", "/eventdev/port_xstats,0,1"}
Expand All @@ -468,7 +469,7 @@ func TestDLB_processCommandResult(t *testing.T) {
maxInitMessageLength: 1024,
EventdevCommands: []string{"/eventdev/dev_xstats"},
}
response := fmt.Sprintf(`{"%s": [0]}`, eventdevListCommand)
response := fmt.Sprintf(`{%q: [0]}`, eventdevListCommand)
simulateResponse(mockConn, response, nil)

response = `{"/eventdev/dev_xstats": {"dev_rx_ok": 0}}`
Expand Down Expand Up @@ -506,7 +507,7 @@ func TestDLB_processCommandResult(t *testing.T) {
rasReader: fileMock,
maxInitMessageLength: 1024,
}
responseGather := fmt.Sprintf(`{"%s": [0]}`, eventdevListCommand)
responseGather := fmt.Sprintf(`{%q: [0]}`, eventdevListCommand)
mockConn.On("Write", mock.Anything).Return(0, nil).Twice()
mockConn.On("Read", mock.Anything).Run(func(arg mock.Arguments) {
elem := arg.Get(0).([]byte)
Expand Down Expand Up @@ -537,7 +538,7 @@ func TestDLB_processCommandResult(t *testing.T) {
Log: testutil.Logger{},
EventdevCommands: []string{"/eventdev/dev_xstats"},
}
response := fmt.Sprintf(`{"%s": [0]}`, eventdevListCommand)
response := fmt.Sprintf(`{%q: [0]}`, eventdevListCommand)
simulateResponse(mockConn, response, nil)

simulateResponse(mockConn, "/wrong/json", nil)
Expand Down Expand Up @@ -622,7 +623,7 @@ func TestDLB_processCommandResult(t *testing.T) {
}
mockConn.On("Close").Return(nil)

responseGather := fmt.Sprintf(`{"%s": [0]}`, eventdevListCommand)
responseGather := fmt.Sprintf(`{%q: [0]}`, eventdevListCommand)
mockConn.On("Write", mock.Anything).Return(0, nil).Once().
On("Read", mock.Anything).Run(func(arg mock.Arguments) {
elem := arg.Get(0).([]byte)
Expand Down Expand Up @@ -949,7 +950,7 @@ func simulateSocketResponseForGather(socket net.Listener, t *testing.T) {

require.NoError(t, err)
eventdevListWithSecondIndex := []string{"/eventdev/port_list", "/eventdev/queue_list"}
_, err = conn.Write([]byte(fmt.Sprintf(`{"%s": [0, 1]}`, eventdevListWithSecondIndex[0])))
_, err = conn.Write([]byte(fmt.Sprintf(`{%q: [0, 1]}`, eventdevListWithSecondIndex[0])))
require.NoError(t, err)
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/inputs/intel_rdt/intel_rdt.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ func validateInterval(interval int32) error {
func splitMeasurementLine(line string) ([]string, error) {
values := strings.Split(line, ",")
if len(values) < 8 {
return nil, fmt.Errorf(fmt.Sprintf("not valid line format from pqos: %s", values))
return nil, fmt.Errorf("not valid line format from pqos: %s", values)
}
return values, nil
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/inputs/stackdriver/stackdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (s *Stackdriver) newListTimeSeriesFilter(metricType string) string {
"has_substring",
"one_of",
}
filterString := fmt.Sprintf(`metric.type = "%s"`, metricType)
filterString := fmt.Sprintf(`metric.type = %q`, metricType)
if s.Filter == nil {
return filterString
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/outputs/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ func TestOAuthAuthorizationCodeGrant(t *testing.T) {
},
tokenHandler: func(t *testing.T, w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
authHeader := fmt.Sprintf(`{"id_token":"%s"}`, token)
authHeader := fmt.Sprintf(`{"id_token":%q}`, token)
_, err = w.Write([]byte(authHeader))
require.NoError(t, err)
},
Expand Down
1 change: 1 addition & 0 deletions plugins/outputs/influxdb/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ func (c *httpClient) Database() string {
// Note that some names are not allowed by the server, notably those with
// non-printable characters or slashes.
func (c *httpClient) CreateDatabase(ctx context.Context, database string) error {
//nolint:gocritic // sprintfQuotedString - "%s" used by purpose, string escaping is done by special function
query := fmt.Sprintf(`CREATE DATABASE "%s"`, escapeIdentifier.Replace(database))

req, err := c.makeQueryRequest(query)
Expand Down
7 changes: 4 additions & 3 deletions plugins/outputs/warp10/warp10.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package warp10
import (
"bytes"
_ "embed"
"errors"
"fmt"
"io"
"math"
Expand Down Expand Up @@ -142,14 +143,14 @@ func (w *Warp10) Write(metrics []telegraf.Metric) error {
if resp.StatusCode != http.StatusOK {
if w.PrintErrorBody {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf(w.WarpURL + ": " + w.HandleError(string(body), w.MaxStringErrorSize))
return errors.New(w.WarpURL + ": " + w.HandleError(string(body), w.MaxStringErrorSize))
}

if len(resp.Status) < w.MaxStringErrorSize {
return fmt.Errorf(w.WarpURL + ": " + resp.Status)
return errors.New(w.WarpURL + ": " + resp.Status)
}

return fmt.Errorf(w.WarpURL + ": " + resp.Status[0:w.MaxStringErrorSize])
return errors.New(w.WarpURL + ": " + resp.Status[0:w.MaxStringErrorSize])
}

return nil
Expand Down
6 changes: 3 additions & 3 deletions tools/package_lxd_test/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (c *Container) configureYum() error {
err := c.client.Exec(
c.Name,
"bash", "-c", "--",
fmt.Sprintf("echo \"%s\" > /etc/yum.repos.d/influxdata.repo", influxDataRPMRepo),
fmt.Sprintf("echo %q > /etc/yum.repos.d/influxdata.repo", influxDataRPMRepo),
)
if err != nil {
return err
Expand All @@ -205,7 +205,7 @@ func (c *Container) configureDnf() error {
err := c.client.Exec(
c.Name,
"bash", "-c", "--",
fmt.Sprintf("echo \"%s\" > /etc/yum.repos.d/influxdata.repo", influxDataRPMRepo),
fmt.Sprintf("echo %q > /etc/yum.repos.d/influxdata.repo", influxDataRPMRepo),
)
if err != nil {
return err
Expand All @@ -219,7 +219,7 @@ func (c *Container) configureDnf() error {
func (c *Container) configureZypper() error {
err := c.client.Exec(
c.Name,
"echo", fmt.Sprintf("\"%s\"", influxDataRPMRepo), ">", "/etc/zypp/repos.d/influxdata.repo",
"echo", fmt.Sprintf("%q", influxDataRPMRepo), ">", "/etc/zypp/repos.d/influxdata.repo",
)
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion tools/package_lxd_test/lxd.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"bytes"
"errors"
"fmt"
"os"
"strconv"
Expand Down Expand Up @@ -140,7 +141,7 @@ func (c *LXDClient) Exec(name string, command ...string) error {
rc := int(opAPI.Metadata["return"].(float64))

if rc != 0 {
return fmt.Errorf(output.String())
return errors.New(output.String())
}

fmt.Println(output.String())
Expand Down
12 changes: 6 additions & 6 deletions tools/update_goversion/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,32 +190,32 @@ func main() {
{
FileName: "scripts/installgo_linux.sh",
Regex: `(GO_VERSION)=("\d.\d*.\d")`,
Replace: fmt.Sprintf("$1=\"%s\"", zeroPatchVersion),
Replace: fmt.Sprintf("$1=%q", zeroPatchVersion),
},
{
FileName: "scripts/installgo_mac.sh",
Regex: `(GO_VERSION)=("\d.\d*.\d")`,
Replace: fmt.Sprintf("$1=\"%s\"", zeroPatchVersion),
Replace: fmt.Sprintf("$1=%q", zeroPatchVersion),
},
{
FileName: "scripts/installgo_windows.sh",
Regex: `(GO_VERSION)=("\d.\d*.\d")`,
Replace: fmt.Sprintf("$1=\"%s\"", zeroPatchVersion),
Replace: fmt.Sprintf("$1=%q", zeroPatchVersion),
},
{
FileName: "scripts/installgo_linux.sh",
Regex: `(GO_VERSION_SHA)=".*"`,
Replace: fmt.Sprintf("$1=\"%s\"", hashes[fmt.Sprintf("go%s.linux-amd64.tar.gz", zeroPatchVersion)]),
Replace: fmt.Sprintf("$1=%q", hashes[fmt.Sprintf("go%s.linux-amd64.tar.gz", zeroPatchVersion)]),
},
{
FileName: "scripts/installgo_mac.sh",
Regex: `(GO_VERSION_SHA_arm64)=".*"`,
Replace: fmt.Sprintf("$1=\"%s\"", hashes[fmt.Sprintf("go%s.darwin-arm64.tar.gz", zeroPatchVersion)]),
Replace: fmt.Sprintf("$1=%q", hashes[fmt.Sprintf("go%s.darwin-arm64.tar.gz", zeroPatchVersion)]),
},
{
FileName: "scripts/installgo_mac.sh",
Regex: `(GO_VERSION_SHA_amd64)=".*"`,
Replace: fmt.Sprintf("$1=\"%s\"", hashes[fmt.Sprintf("go%s.darwin-amd64.tar.gz", zeroPatchVersion)]),
Replace: fmt.Sprintf("$1=%q", hashes[fmt.Sprintf("go%s.darwin-amd64.tar.gz", zeroPatchVersion)]),
},
}

Expand Down

0 comments on commit 02f0b15

Please sign in to comment.