From 673decc1f58306cd472ce24bcab4f53314f6b924 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Thu, 7 Nov 2024 00:26:49 +0100 Subject: [PATCH] chore: enable perfsprint linter Signed-off-by: Matthieu MOREL --- .golangci.yaml | 12 ++++++++++++ .../controllers/applicationset_controller.go | 15 ++++++++------- applicationset/utils/map.go | 5 +---- common/common_test.go | 3 ++- controller/cache/info.go | 2 +- controller/sharding/sharding_test.go | 2 +- controller/sharding/shuffle_test.go | 2 +- reposerver/repository/chart.go | 2 +- server/application/application_test.go | 4 ++-- server/logout/logout_test.go | 4 ++-- test/e2e/fixture/app/actions.go | 5 +++-- test/e2e/project_management_test.go | 8 ++++---- util/env/env_test.go | 17 +++++++++-------- util/exec/exec.go | 2 +- util/git/creds.go | 5 +++-- util/jwt/jwt_test.go | 3 +-- util/lua/oslib_safe.go | 4 ++-- util/session/sessionmanager_test.go | 4 ++-- 18 files changed, 56 insertions(+), 43 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index b2159df9de48e..5333f86b47f94 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -18,6 +18,7 @@ linters: - govet - ineffassign - misspell + - perfsprint - staticcheck - testifylint - thelper @@ -39,6 +40,17 @@ linters-settings: - typeSwitchVar goimports: local-prefixes: github.com/argoproj/argo-cd/v2 + 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: false + # Optimizes `fmt.Errorf`. + errorf: false + # Optimizes `fmt.Sprintf` with only one argument. + sprintf1: true + # Optimizes into strings concatenation. + strconcat: false testifylint: enable-all: true disable: diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index 7df5c46bfc16d..149e2766dfbdf 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -19,6 +19,7 @@ import ( "fmt" "reflect" "sort" + "strconv" "strings" "time" @@ -1047,7 +1048,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con LastTransitionTime: &now, Message: "No Application status found, defaulting status to Waiting.", Status: "Waiting", - Step: fmt.Sprint(getAppStep(app.Name, appStepMap)), + Step: strconv.Itoa(getAppStep(app.Name, appStepMap)), TargetRevisions: app.Status.GetRevisions(), } } else { @@ -1072,7 +1073,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con currentAppStatus.LastTransitionTime = &now currentAppStatus.Status = "Waiting" currentAppStatus.Message = "Application has pending changes, setting status to Waiting." - currentAppStatus.Step = fmt.Sprint(getAppStep(currentAppStatus.Application, appStepMap)) + currentAppStatus.Step = strconv.Itoa(getAppStep(currentAppStatus.Application, appStepMap)) currentAppStatus.TargetRevisions = app.Status.GetRevisions() } @@ -1090,14 +1091,14 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con currentAppStatus.LastTransitionTime = &now currentAppStatus.Status = "Progressing" currentAppStatus.Message = "Application resource completed a sync successfully, updating status from Pending to Progressing." - currentAppStatus.Step = fmt.Sprint(getAppStep(currentAppStatus.Application, appStepMap)) + currentAppStatus.Step = strconv.Itoa(getAppStep(currentAppStatus.Application, appStepMap)) } } else if operationPhaseString == "Running" || healthStatusString == "Progressing" { logCtx.Infof("Application %v has entered Progressing status, updating its ApplicationSet status to Progressing", app.Name) currentAppStatus.LastTransitionTime = &now currentAppStatus.Status = "Progressing" currentAppStatus.Message = "Application resource became Progressing, updating status from Pending to Progressing." - currentAppStatus.Step = fmt.Sprint(getAppStep(currentAppStatus.Application, appStepMap)) + currentAppStatus.Step = strconv.Itoa(getAppStep(currentAppStatus.Application, appStepMap)) } } @@ -1106,7 +1107,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con currentAppStatus.LastTransitionTime = &now currentAppStatus.Status = healthStatusString currentAppStatus.Message = "Application resource is already Healthy, updating status from Waiting to Healthy." - currentAppStatus.Step = fmt.Sprint(getAppStep(currentAppStatus.Application, appStepMap)) + currentAppStatus.Step = strconv.Itoa(getAppStep(currentAppStatus.Application, appStepMap)) } if currentAppStatus.Status == "Progressing" && isApplicationHealthy(app) { @@ -1114,7 +1115,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con currentAppStatus.LastTransitionTime = &now currentAppStatus.Status = healthStatusString currentAppStatus.Message = "Application resource became Healthy, updating status from Progressing to Healthy." - currentAppStatus.Step = fmt.Sprint(getAppStep(currentAppStatus.Application, appStepMap)) + currentAppStatus.Step = strconv.Itoa(getAppStep(currentAppStatus.Application, appStepMap)) } appStatuses = append(appStatuses, currentAppStatus) @@ -1185,7 +1186,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress appStatus.LastTransitionTime = &now appStatus.Status = "Pending" appStatus.Message = "Application moved to Pending status, watching for the Application resource to start Progressing." - appStatus.Step = fmt.Sprint(getAppStep(appStatus.Application, appStepMap)) + appStatus.Step = strconv.Itoa(getAppStep(appStatus.Application, appStepMap)) updateCountMap[appStepMap[appStatus.Application]] += 1 } diff --git a/applicationset/utils/map.go b/applicationset/utils/map.go index e15baec29bd9c..0d71f20b50a83 100644 --- a/applicationset/utils/map.go +++ b/applicationset/utils/map.go @@ -8,10 +8,7 @@ func ConvertToMapStringString(mapStringInterface map[string]interface{}) map[str mapStringString := make(map[string]string, len(mapStringInterface)) for key, value := range mapStringInterface { - strKey := fmt.Sprintf("%v", key) - strValue := fmt.Sprintf("%v", value) - - mapStringString[strKey] = strValue + mapStringString[key] = fmt.Sprintf("%v", value) } return mapStringString } diff --git a/common/common_test.go b/common/common_test.go index 4db2ad76ce037..88df43cd1b3e9 100644 --- a/common/common_test.go +++ b/common/common_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "strconv" "testing" "time" @@ -41,7 +42,7 @@ func Test_GRPCKeepAliveMinIsSet(t *testing.T) { // Test invalid env var set for EnvGRPCKeepAliveMin func Test_GRPCKeepAliveMinIncorrectlySet(t *testing.T) { numSeconds := 15 - os.Setenv(EnvGRPCKeepAliveMin, fmt.Sprintf("%d", numSeconds)) + os.Setenv(EnvGRPCKeepAliveMin, strconv.Itoa(numSeconds)) grpcKeepAliveMin := GetGRPCKeepAliveEnforcementMinimum() grpcKeepAliveExpectedMin := defaultGRPCKeepAliveEnforcementMinimum diff --git a/controller/cache/info.go b/controller/cache/info.go index 7260487af859f..635000e688ce3 100644 --- a/controller/cache/info.go +++ b/controller/cache/info.go @@ -451,7 +451,7 @@ func populatePodInfo(un *unstructured.Unstructured, res *ResourceInfo) { res.Info = append(res.Info, v1alpha1.InfoItem{Name: "Node", Value: pod.Spec.NodeName}) res.Info = append(res.Info, v1alpha1.InfoItem{Name: "Containers", Value: fmt.Sprintf("%d/%d", readyContainers, totalContainers)}) if restarts > 0 { - res.Info = append(res.Info, v1alpha1.InfoItem{Name: "Restart Count", Value: fmt.Sprintf("%d", restarts)}) + res.Info = append(res.Info, v1alpha1.InfoItem{Name: "Restart Count", Value: strconv.Itoa(restarts)}) } var urls []string diff --git a/controller/sharding/sharding_test.go b/controller/sharding/sharding_test.go index 5c36a0851919d..b76741be92e16 100644 --- a/controller/sharding/sharding_test.go +++ b/controller/sharding/sharding_test.go @@ -226,7 +226,7 @@ func TestGetShardByIndexModuloReplicasCountDistributionFunctionWhenClusterNumber // The other implementation was giving almost linear time of 400ms up to 10'000 clusters clusterPointers := []*v1alpha1.Cluster{} for i := 0; i < 2048; i++ { - cluster := createCluster(fmt.Sprintf("cluster-%d", i), fmt.Sprintf("%d", i)) + cluster := createCluster(fmt.Sprintf("cluster-%d", i), strconv.Itoa(i)) clusterPointers = append(clusterPointers, &cluster) } replicasCount := 2 diff --git a/controller/sharding/shuffle_test.go b/controller/sharding/shuffle_test.go index 4c05a8c7aeebd..390b82254555e 100644 --- a/controller/sharding/shuffle_test.go +++ b/controller/sharding/shuffle_test.go @@ -20,7 +20,7 @@ func TestLargeShuffle(t *testing.T) { clusterList := &v1alpha1.ClusterList{Items: []v1alpha1.Cluster{}} for i := 0; i < math.MaxInt/4096; i += 256 { // fmt.Fprintf(os.Stdout, "%d", i) - cluster := createCluster(fmt.Sprintf("cluster-%d", i), fmt.Sprintf("%d", i)) + cluster := createCluster(fmt.Sprintf("cluster-%d", i), strconv.Itoa(i)) clusterList.Items = append(clusterList.Items, cluster) } db.On("ListClusters", mock.Anything).Return(clusterList, nil) diff --git a/reposerver/repository/chart.go b/reposerver/repository/chart.go index c1ad7855049d3..3424db4b51fcd 100644 --- a/reposerver/repository/chart.go +++ b/reposerver/repository/chart.go @@ -20,7 +20,7 @@ func getChartDetails(chartYAML string) (*v1alpha1.ChartDetails, error) { if maintainer.Email != "" { maintainers = append(maintainers, strings.Trim(fmt.Sprintf("%v <%v>", maintainer.Name, maintainer.Email), " ")) } else { - maintainers = append(maintainers, fmt.Sprintf("%v", maintainer.Name)) + maintainers = append(maintainers, maintainer.Name) } } return &v1alpha1.ChartDetails{ diff --git a/server/application/application_test.go b/server/application/application_test.go index a030131f679a9..d4068b5c7675c 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -242,7 +242,7 @@ func newTestAppServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforcer), oldVersion = 0 } clonedApp := app.DeepCopy() - clonedApp.ResourceVersion = fmt.Sprintf("%d", oldVersion+1) + clonedApp.ResourceVersion = strconv.Itoa(oldVersion + 1) events <- &appsv1.ApplicationWatchEvent{Type: watch.Added, Application: *clonedApp} } } @@ -425,7 +425,7 @@ func newTestAppServerWithEnforcerConfigureWithBenchmark(b *testing.B, f func(*rb oldVersion = 0 } clonedApp := app.DeepCopy() - clonedApp.ResourceVersion = fmt.Sprintf("%d", oldVersion+1) + clonedApp.ResourceVersion = strconv.Itoa(oldVersion + 1) events <- &appsv1.ApplicationWatchEvent{Type: watch.Added, Application: *clonedApp} } } diff --git a/server/logout/logout_test.go b/server/logout/logout_test.go index 3d2bab2d3662d..28345407a8299 100644 --- a/server/logout/logout_test.go +++ b/server/logout/logout_test.go @@ -3,10 +3,10 @@ package logout import ( "context" "errors" - "fmt" "net/http" "net/http/httptest" "regexp" + "strconv" "testing" "github.com/argoproj/argo-cd/v2/common" @@ -385,7 +385,7 @@ func TestHandlerConstructLogoutURL(t *testing.T) { if status := tt.responseRecorder.Code; status != http.StatusSeeOther { if !tt.wantErr { t.Error(tt.responseRecorder.Body.String()) - t.Error("handler returned wrong status code: " + fmt.Sprintf("%d", tt.responseRecorder.Code)) + t.Error("handler returned wrong status code: " + strconv.Itoa(tt.responseRecorder.Code)) } } else { if tt.wantErr { diff --git a/test/e2e/fixture/app/actions.go b/test/e2e/fixture/app/actions.go index 2628b573ec5db..ca33ae151eb85 100644 --- a/test/e2e/fixture/app/actions.go +++ b/test/e2e/fixture/app/actions.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "strconv" log "github.com/sirupsen/logrus" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -345,7 +346,7 @@ func (a *Actions) Sync(args ...string) *Actions { if a.context.name != "" { args = append(args, a.context.AppQualifiedName()) } - args = append(args, "--timeout", fmt.Sprintf("%v", a.context.timeout)) + args = append(args, "--timeout", strconv.Itoa(a.context.timeout)) if a.context.async { args = append(args, "--async") @@ -441,7 +442,7 @@ func (a *Actions) Wait(args ...string) *Actions { if a.context.name != "" { args = append(args, a.context.AppQualifiedName()) } - args = append(args, "--timeout", fmt.Sprintf("%v", a.context.timeout)) + args = append(args, "--timeout", strconv.Itoa(a.context.timeout)) a.runCli(args...) return a } diff --git a/test/e2e/project_management_test.go b/test/e2e/project_management_test.go index 01b2b472d5acb..b931a5e102746 100644 --- a/test/e2e/project_management_test.go +++ b/test/e2e/project_management_test.go @@ -566,11 +566,11 @@ func TestGetVirtualProjectNoMatch(t *testing.T) { time.Sleep(time.Second * 2) // App trying to sync a resource which is not blacked listed anywhere - _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", "apps:Deployment:guestbook-ui", "--timeout", fmt.Sprintf("%v", 10)) + _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", "apps:Deployment:guestbook-ui", "--timeout", strconv.Itoa(10)) require.NoError(t, err) // app trying to sync a resource which is black listed by global project - _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", ":Service:guestbook-ui", "--timeout", fmt.Sprintf("%v", 10)) + _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", ":Service:guestbook-ui", "--timeout", strconv.Itoa(10)) require.NoError(t, err) } @@ -606,11 +606,11 @@ func TestGetVirtualProjectMatch(t *testing.T) { time.Sleep(time.Second * 2) // App trying to sync a resource which is not blacked listed anywhere - _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", "apps:Deployment:guestbook-ui", "--timeout", fmt.Sprintf("%v", 10)) + _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", "apps:Deployment:guestbook-ui", "--timeout", strconv.Itoa(10)) require.ErrorContains(t, err, "blocked by sync window") // app trying to sync a resource which is black listed by global project - _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", ":Service:guestbook-ui", "--timeout", fmt.Sprintf("%v", 10)) + _, err = fixture.RunCli("app", "sync", fixture.Name(), "--resource", ":Service:guestbook-ui", "--timeout", strconv.Itoa(10)) assert.ErrorContains(t, err, "blocked by sync window") } diff --git a/util/env/env_test.go b/util/env/env_test.go index 84e87052c9e33..5be065908084d 100644 --- a/util/env/env_test.go +++ b/util/env/env_test.go @@ -3,6 +3,7 @@ package env import ( "fmt" "math" + "strconv" "testing" "time" @@ -23,10 +24,10 @@ func TestParseNumFromEnv(t *testing.T) { {"Valid positive number", "200", 200}, {"Valid negative number", "-200", -200}, {"Invalid number", "abc", def}, - {"Equals minimum", fmt.Sprintf("%d", math.MinInt+1), min}, - {"Equals maximum", fmt.Sprintf("%d", math.MaxInt-1), max}, - {"Less than minimum", fmt.Sprintf("%d", math.MinInt), def}, - {"Greater than maximum", fmt.Sprintf("%d", math.MaxInt), def}, + {"Equals minimum", strconv.Itoa(math.MinInt + 1), min}, + {"Equals maximum", strconv.Itoa(math.MaxInt - 1), max}, + {"Less than minimum", strconv.Itoa(math.MinInt), def}, + {"Greater than maximum", strconv.Itoa(math.MaxInt), def}, {"Variable not set", "", def}, } @@ -81,10 +82,10 @@ func TestParseInt64FromEnv(t *testing.T) { }{ {"Valid int64", "200", 200}, {"Text as invalid int64", "abc", def}, - {"Equals maximum", fmt.Sprintf("%d", max), max}, - {"Equals minimum", fmt.Sprintf("%d", min), min}, - {"Greater than maximum", fmt.Sprintf("%d", max+1), def}, - {"Less than minimum", fmt.Sprintf("%d", min-1), def}, + {"Equals maximum", strconv.FormatInt(max, 10), max}, + {"Equals minimum", strconv.FormatInt(min, 10), min}, + {"Greater than maximum", strconv.FormatInt(max+1, 10), def}, + {"Less than minimum", strconv.FormatInt(min-1, 10), def}, {"Environment not set", "", def}, } diff --git a/util/exec/exec.go b/util/exec/exec.go index 493d8855c8c80..17eab41a20445 100644 --- a/util/exec/exec.go +++ b/util/exec/exec.go @@ -52,7 +52,7 @@ func RunWithRedactor(cmd *exec.Cmd, redactor func(text string) string) (string, func RunWithExecRunOpts(cmd *exec.Cmd, opts ExecRunOpts) (string, error) { cmdOpts := argoexec.CmdOpts{Timeout: timeout, Redactor: opts.Redactor, TimeoutBehavior: opts.TimeoutBehavior, SkipErrorLogging: opts.SkipErrorLogging} span := tracing.NewLoggingTracer(log.NewLogrusLogger(log.NewWithCurrentConfig())).StartSpan(fmt.Sprintf("exec %v", cmd.Args[0])) - span.SetBaggageItem("dir", fmt.Sprintf("%v", cmd.Dir)) + span.SetBaggageItem("dir", cmd.Dir) if cmdOpts.Redactor != nil { span.SetBaggageItem("args", opts.Redactor(fmt.Sprintf("%v", cmd.Args))) } else { diff --git a/util/git/creds.go b/util/git/creds.go index 5715925dcef9e..f48b2362dc22a 100644 --- a/util/git/creds.go +++ b/util/git/creds.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/base64" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -427,7 +428,7 @@ func (g GitHubAppCreds) getAccessToken() (string, error) { if err != nil { return "", err } - key := fmt.Sprintf("%x", h.Sum(nil)) + key := hex.EncodeToString(h.Sum(nil)) // Check cache for GitHub transport which helps fetch an API token t, found := githubAppTokenCache.Get(key) @@ -543,7 +544,7 @@ func (c GoogleCloudCreds) getAccessToken() (string, error) { if err != nil { return "", err } - key := fmt.Sprintf("%x", h.Sum(nil)) + key := hex.EncodeToString(h.Sum(nil)) t, found := googleCloudTokenSource.Get(key) if found { diff --git a/util/jwt/jwt_test.go b/util/jwt/jwt_test.go index 7daf855e3d9fb..1b0ac87fe9752 100644 --- a/util/jwt/jwt_test.go +++ b/util/jwt/jwt_test.go @@ -1,7 +1,6 @@ package jwt import ( - "fmt" "testing" "time" @@ -45,7 +44,7 @@ func TestIssuedAtTime_Int64(t *testing.T) { claims := jwt.MapClaims{"iat": int64(1606831200)} issuedAt, err := IssuedAtTime(claims) require.NoError(t, err) - str := fmt.Sprint(issuedAt.UTC().Format("Mon Jan _2 15:04:05 2006")) + str := issuedAt.UTC().Format("Mon Jan _2 15:04:05 2006") assert.Equal(t, "Tue Dec 1 14:00:00 2020", str) } diff --git a/util/lua/oslib_safe.go b/util/lua/oslib_safe.go index 1b84a18b61905..2d5a94e9463f2 100644 --- a/util/lua/oslib_safe.go +++ b/util/lua/oslib_safe.go @@ -6,7 +6,7 @@ package lua // github.com/yuin/gopher-lua. import ( - "fmt" + "strconv" "strings" "time" @@ -116,7 +116,7 @@ func strftime(t time.Time, cfmt string) string { } else { switch c { case 'w': - sc.AppendString(fmt.Sprint(int(t.Weekday()))) + sc.AppendString(strconv.Itoa(int(t.Weekday()))) default: sc.AppendChar('%') sc.AppendChar(c) diff --git a/util/session/sessionmanager_test.go b/util/session/sessionmanager_test.go index d41124d9a15e4..6faf51daf9db8 100644 --- a/util/session/sessionmanager_test.go +++ b/util/session/sessionmanager_test.go @@ -478,8 +478,8 @@ func TestCacheValueGetters(t *testing.T) { }) t.Run("Greater than allowed in environment overrides", func(t *testing.T) { - t.Setenv(envLoginMaxFailCount, fmt.Sprintf("%d", math.MaxInt32+1)) - t.Setenv(envLoginMaxCacheSize, fmt.Sprintf("%d", math.MaxInt32+1)) + t.Setenv(envLoginMaxFailCount, strconv.Itoa(math.MaxInt32+1)) + t.Setenv(envLoginMaxCacheSize, strconv.Itoa(math.MaxInt32+1)) mlf := getMaxLoginFailures() assert.Equal(t, defaultMaxLoginFailures, mlf)