From fcfad5c0b904b6213504116af3f2d78e9f0e0454 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Fri, 8 Nov 2024 09:54:31 +0100 Subject: [PATCH] [chore]: enable perfsprint linter (#1676) --- .golangci.yml | 12 ++++ api/prometheus/v1/api_test.go | 68 +++++++++---------- examples/exemplars/main.go | 4 +- examples/random/main.go | 4 +- prometheus/counter_test.go | 5 +- prometheus/internal/difflib.go | 3 +- prometheus/process_collector_cgo_darwin.go | 4 +- prometheus/promhttp/http_test.go | 2 +- prometheus/registry_test.go | 3 +- prometheus/testutil/promlint/promlint_test.go | 5 +- .../validations/duplicate_validations.go | 4 +- prometheus/value_test.go | 3 +- prometheus/vec_test.go | 5 +- 13 files changed, 67 insertions(+), 55 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d2580cc..fb5d208 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,6 +25,7 @@ linters: - ineffassign - misspell - nolintlint + - perfsprint - predeclared - revive - staticcheck @@ -66,6 +67,17 @@ linters-settings: local-prefixes: github.com/prometheus/client_golang gofumpt: extra-rules: true + perfsprint: + # Optimizes even if it requires an int or uint type cast. + int-conversion: true + # Optimizes into `err.Error()` even if it is only equivalent for non-nil errors. + err-error: true + # Optimizes `fmt.Errorf`. + errorf: true + # Optimizes `fmt.Sprintf` with only one argument. + sprintf1: true + # Optimizes into strings concatenation. + strconcat: true revive: rules: # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 01ddf45..bacf6a0 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -16,13 +16,13 @@ package v1 import ( "context" "errors" - "fmt" "io" "math" "net/http" "net/http/httptest" "net/url" "reflect" + "strconv" "strings" "testing" "time" @@ -260,7 +260,7 @@ func TestAPIs(t *testing.T) { }, { do: doQuery("2", testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/query", @@ -336,7 +336,7 @@ func TestAPIs(t *testing.T) { End: testTime, Step: 1 * time.Minute, }, WithTimeout(5*time.Second)), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/query_range", @@ -361,14 +361,14 @@ func TestAPIs(t *testing.T) { { do: doLabelNames(nil, testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/labels", err: errors.New("some error"), }, { do: doLabelNames(nil, testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), inWarnings: []string{"a"}, reqMethod: "POST", reqPath: "/api/v1/labels", @@ -400,14 +400,14 @@ func TestAPIs(t *testing.T) { { do: doLabelValues(nil, "mylabel", testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "GET", reqPath: "/api/v1/label/mylabel/values", err: errors.New("some error"), }, { do: doLabelValues(nil, "mylabel", testTime.Add(-100*time.Hour), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), inWarnings: []string{"a"}, reqMethod: "GET", reqPath: "/api/v1/label/mylabel/values", @@ -464,7 +464,7 @@ func TestAPIs(t *testing.T) { { do: doSeries("up", testTime.Add(-time.Minute), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/series", err: errors.New("some error"), @@ -472,7 +472,7 @@ func TestAPIs(t *testing.T) { // Series with error and warning. { do: doSeries("up", testTime.Add(-time.Minute), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), inWarnings: []string{"a"}, reqMethod: "POST", reqPath: "/api/v1/series", @@ -493,7 +493,7 @@ func TestAPIs(t *testing.T) { { do: doSnapshot(true), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/admin/tsdb/snapshot", err: errors.New("some error"), @@ -507,7 +507,7 @@ func TestAPIs(t *testing.T) { { do: doCleanTombstones(), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/admin/tsdb/clean_tombstones", err: errors.New("some error"), @@ -528,7 +528,7 @@ func TestAPIs(t *testing.T) { { do: doDeleteSeries("up", testTime.Add(-time.Minute), testTime), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "POST", reqPath: "/api/v1/admin/tsdb/delete_series", err: errors.New("some error"), @@ -550,8 +550,8 @@ func TestAPIs(t *testing.T) { do: doConfig(), reqMethod: "GET", reqPath: "/api/v1/status/config", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -578,16 +578,16 @@ func TestAPIs(t *testing.T) { do: doFlags(), reqMethod: "GET", reqPath: "/api/v1/status/flags", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { do: doBuildinfo(), reqMethod: "GET", reqPath: "/api/v1/status/buildinfo", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -616,8 +616,8 @@ func TestAPIs(t *testing.T) { do: doRuntimeinfo(), reqMethod: "GET", reqPath: "/api/v1/status/runtimeinfo", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -684,8 +684,8 @@ func TestAPIs(t *testing.T) { do: doAlertManagers(), reqMethod: "GET", reqPath: "/api/v1/alertmanagers", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -891,8 +891,8 @@ func TestAPIs(t *testing.T) { do: doRules(), reqMethod: "GET", reqPath: "/api/v1/rules", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -971,8 +971,8 @@ func TestAPIs(t *testing.T) { do: doTargets(), reqMethod: "GET", reqPath: "/api/v1/targets", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -1005,7 +1005,7 @@ func TestAPIs(t *testing.T) { { do: doTargetsMetadata("{job=\"prometheus\"}", "go_goroutines", "1"), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "GET", reqPath: "/api/v1/targets/metadata", err: errors.New("some error"), @@ -1037,7 +1037,7 @@ func TestAPIs(t *testing.T) { { do: doMetadata("", "1"), - inErr: fmt.Errorf("some error"), + inErr: errors.New("some error"), reqMethod: "GET", reqPath: "/api/v1/metadata", err: errors.New("some error"), @@ -1047,8 +1047,8 @@ func TestAPIs(t *testing.T) { do: doTSDB(), reqMethod: "GET", reqPath: "/api/v1/status/tsdb", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -1127,8 +1127,8 @@ func TestAPIs(t *testing.T) { do: doWalReply(), reqMethod: "GET", reqPath: "/api/v1/status/walreplay", - inErr: fmt.Errorf("some error"), - err: fmt.Errorf("some error"), + inErr: errors.New("some error"), + err: errors.New("some error"), }, { @@ -1212,7 +1212,7 @@ func TestAPIs(t *testing.T) { tests = append(tests, queryTests...) for i, test := range tests { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(strconv.Itoa(i), func(t *testing.T) { tc.curTest = test res, warnings, err := test.do() @@ -1430,7 +1430,7 @@ func TestAPIClientDo(t *testing.T) { } for i, test := range tests { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(strconv.Itoa(i), func(t *testing.T) { tc.ch <- test _, body, warnings, err := client.Do(context.Background(), tc.req) diff --git a/examples/exemplars/main.go b/examples/exemplars/main.go index 798c2df..618224a 100644 --- a/examples/exemplars/main.go +++ b/examples/exemplars/main.go @@ -17,10 +17,10 @@ package main import ( - "fmt" "log" "math/rand" "net/http" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -50,7 +50,7 @@ func main() { // Record fictional latency. now := time.Now() requestDurations.(prometheus.ExemplarObserver).ObserveWithExemplar( - time.Since(now).Seconds(), prometheus.Labels{"dummyID": fmt.Sprint(rand.Intn(100000))}, + time.Since(now).Seconds(), prometheus.Labels{"dummyID": strconv.Itoa(rand.Intn(100000))}, ) time.Sleep(600 * time.Millisecond) } diff --git a/examples/random/main.go b/examples/random/main.go index b4f6280..9192e94 100644 --- a/examples/random/main.go +++ b/examples/random/main.go @@ -18,11 +18,11 @@ package main import ( "flag" - "fmt" "log" "math" "math/rand" "net/http" + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -116,7 +116,7 @@ func main() { // the ExemplarObserver interface and thus don't need to // check the outcome of the type assertion. m.rpcDurationsHistogram.(prometheus.ExemplarObserver).ObserveWithExemplar( - v, prometheus.Labels{"dummyID": fmt.Sprint(rand.Intn(100000))}, + v, prometheus.Labels{"dummyID": strconv.Itoa(rand.Intn(100000))}, ) time.Sleep(time.Duration(75*oscillationFactor()) * time.Millisecond) } diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index 3686199..29d94bf 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -14,7 +14,6 @@ package prometheus import ( - "fmt" "math" "strings" "testing" @@ -120,10 +119,10 @@ func TestCounterVecGetMetricWithInvalidLabelValues(t *testing.T) { expectPanic(t, func() { counterVec.WithLabelValues(labelValues...) - }, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc)) + }, "WithLabelValues: expected panic because: "+test.desc) expectPanic(t, func() { counterVec.With(test.labels) - }, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc)) + }, "WithLabelValues: expected panic because: "+test.desc) if _, err := counterVec.GetMetricWithLabelValues(labelValues...); err == nil { t.Errorf("GetMetricWithLabelValues: expected error because: %s", test.desc) diff --git a/prometheus/internal/difflib.go b/prometheus/internal/difflib.go index ba09bdd..8b01635 100644 --- a/prometheus/internal/difflib.go +++ b/prometheus/internal/difflib.go @@ -22,6 +22,7 @@ import ( "bytes" "fmt" "io" + "strconv" "strings" ) @@ -524,7 +525,7 @@ func formatRangeUnified(start, stop int) string { beginning := start + 1 // lines start numbering with one length := stop - start if length == 1 { - return fmt.Sprintf("%d", beginning) + return strconv.Itoa(beginning) } if length == 0 { beginning-- // empty ranges begin at line just before the range diff --git a/prometheus/process_collector_cgo_darwin.go b/prometheus/process_collector_cgo_darwin.go index 6f48e58..fc4c247 100644 --- a/prometheus/process_collector_cgo_darwin.go +++ b/prometheus/process_collector_cgo_darwin.go @@ -22,9 +22,7 @@ import "C" import "fmt" func getMemory() (*memoryInfo, error) { - var ( - rss, vsize C.ulonglong - ) + var rss, vsize C.ulonglong if err := C.get_memory_info(&rss, &vsize); err != 0 { return nil, fmt.Errorf("task_info() failed with 0x%x", int(err)) diff --git a/prometheus/promhttp/http_test.go b/prometheus/promhttp/http_test.go index 3ad2d1d..c3a433c 100644 --- a/prometheus/promhttp/http_test.go +++ b/prometheus/promhttp/http_test.go @@ -98,7 +98,7 @@ func readCompressedBody(r io.Reader, comp Compression) (string, error) { got, err := io.ReadAll(reader) return string(got), err } - return "", fmt.Errorf("Unsupported compression") + return "", errors.New("Unsupported compression") } func TestHandlerErrorHandling(t *testing.T) { diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index 35a74d2..f6bca22 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -27,6 +27,7 @@ import ( "net/http" "net/http/httptest" "os" + "strconv" "sync" "testing" "time" @@ -1282,7 +1283,7 @@ func ExampleRegistry_grouping() { ConstLabels: prometheus.Labels{ // Generate a label unique to this worker so its metric doesn't // collide with the metrics from other workers. - "worker_id": fmt.Sprintf("%d", workerID), + "worker_id": strconv.Itoa(workerID), }, }) workerReg.MustRegister(workTime) diff --git a/prometheus/testutil/promlint/promlint_test.go b/prometheus/testutil/promlint/promlint_test.go index c60507c..c6a4e27 100644 --- a/prometheus/testutil/promlint/promlint_test.go +++ b/prometheus/testutil/promlint/promlint_test.go @@ -14,6 +14,7 @@ package promlint_test import ( + "errors" "fmt" "reflect" "strings" @@ -733,7 +734,7 @@ request_duration_seconds{httpService="foo"} 10 func TestLintUnitAbbreviations(t *testing.T) { genTest := func(n string) test { return test{ - name: fmt.Sprintf("%s with abbreviated unit", n), + name: n + " with abbreviated unit", in: fmt.Sprintf(` # HELP %s Test metric. # TYPE %s gauge @@ -820,7 +821,7 @@ mc_something_total 10 prefixValidation := func(mf *dto.MetricFamily) []error { if !strings.HasPrefix(mf.GetName(), "memcached_") { - return []error{fmt.Errorf("expected metric name to start with 'memcached_'")} + return []error{errors.New("expected metric name to start with 'memcached_'")} } return nil } diff --git a/prometheus/testutil/promlint/validations/duplicate_validations.go b/prometheus/testutil/promlint/validations/duplicate_validations.go index fdc1e62..68645ed 100644 --- a/prometheus/testutil/promlint/validations/duplicate_validations.go +++ b/prometheus/testutil/promlint/validations/duplicate_validations.go @@ -14,7 +14,7 @@ package validations import ( - "fmt" + "errors" "reflect" dto "github.com/prometheus/client_model/go" @@ -27,7 +27,7 @@ func LintDuplicateMetric(mf *dto.MetricFamily) []error { for i, m := range mf.Metric { for _, k := range mf.Metric[i+1:] { if reflect.DeepEqual(m.Label, k.Label) { - problems = append(problems, fmt.Errorf("metric not unique")) + problems = append(problems, errors.New("metric not unique")) break } } diff --git a/prometheus/value_test.go b/prometheus/value_test.go index 004c3bb..23da6b2 100644 --- a/prometheus/value_test.go +++ b/prometheus/value_test.go @@ -14,7 +14,6 @@ package prometheus import ( - "fmt" "testing" "time" @@ -51,7 +50,7 @@ func TestNewConstMetricInvalidLabelValues(t *testing.T) { expectPanic(t, func() { MustNewConstMetric(metricDesc, CounterValue, 0.3, "\xFF") - }, fmt.Sprintf("WithLabelValues: expected panic because: %s", test.desc)) + }, "WithLabelValues: expected panic because: "+test.desc) if _, err := NewConstMetric(metricDesc, CounterValue, 0.3, "\xFF"); err == nil { t.Errorf("NewConstMetric: expected error because: %s", test.desc) diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 89bd854..fdd3abc 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -16,6 +16,7 @@ package prometheus import ( "fmt" "reflect" + "strconv" "testing" dto "github.com/prometheus/client_model/go" @@ -291,7 +292,7 @@ func testMetricVec(t *testing.T, vec *GaugeVec) { expected := map[[2]string]int{} for i := 0; i < 1000; i++ { - pair[0], pair[1] = fmt.Sprint(i%4), fmt.Sprint(i%5) // Varying combinations multiples. + pair[0], pair[1] = strconv.Itoa(i%4), strconv.Itoa(i%5) // Varying combinations multiples. expected[pair]++ vec.WithLabelValues(pair[0], pair[1]).Inc() @@ -363,7 +364,7 @@ func testConstrainedMetricVec(t *testing.T, vec *GaugeVec, constrain func(string expected := map[[2]string]int{} for i := 0; i < 1000; i++ { - pair[0], pair[1] = fmt.Sprint(i%4), fmt.Sprint(i%5) // Varying combinations multiples. + pair[0], pair[1] = strconv.Itoa(i%4), strconv.Itoa(i%5) // Varying combinations multiples. expected[[2]string{pair[0], constrain(pair[1])}]++ vec.WithLabelValues(pair[0], pair[1]).Inc()