From 58ba5644d1865690284dd6a06bc0d7bb7f5f7119 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Fri, 11 Oct 2024 14:41:05 +0300 Subject: [PATCH 1/2] refactor: Enable perfsprint; fix appeared lint issues --- .golangci.yml | 4 ++++ .../codespaces/newreposecretwithxcrypto/main.go | 3 ++- .../codespaces/newusersecretwithxcrypto/main.go | 3 ++- example/newreposecretwithxcrypto/main.go | 3 ++- github/copilot.go | 3 ++- github/dependabot_secrets.go | 3 ++- github/git_commits.go | 2 +- github/git_commits_test.go | 3 ++- github/github_test.go | 16 ++++++++-------- github/orgs_properties.go | 3 ++- github/pulls.go | 3 ++- tools/metadata/main.go | 9 +++++---- 12 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2bfeaaa24dc..30628b213fa 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -11,6 +11,7 @@ linters: - gosec - misspell - nakedret + - perfsprint - paralleltest - stylecheck - tparallel @@ -24,6 +25,9 @@ linters-settings: - G104 # int(os.Stdin.Fd()) - G115 + perfsprint: + errorf: true + strconcat: false issues: exclude-use-default: false exclude-rules: diff --git a/example/codespaces/newreposecretwithxcrypto/main.go b/example/codespaces/newreposecretwithxcrypto/main.go index bd6033ec5af..df7cd97fb36 100644 --- a/example/codespaces/newreposecretwithxcrypto/main.go +++ b/example/codespaces/newreposecretwithxcrypto/main.go @@ -31,6 +31,7 @@ import ( "context" crypto_rand "crypto/rand" "encoding/base64" + "errors" "flag" "fmt" "log" @@ -84,7 +85,7 @@ func main() { func getSecretName() (string, error) { secretName := flag.Arg(0) if secretName == "" { - return "", fmt.Errorf("missing argument secret name") + return "", errors.New("missing argument secret name") } return secretName, nil } diff --git a/example/codespaces/newusersecretwithxcrypto/main.go b/example/codespaces/newusersecretwithxcrypto/main.go index e1fec87de33..e306ae3de79 100644 --- a/example/codespaces/newusersecretwithxcrypto/main.go +++ b/example/codespaces/newusersecretwithxcrypto/main.go @@ -32,6 +32,7 @@ import ( "context" crypto_rand "crypto/rand" "encoding/base64" + "errors" "flag" "fmt" "log" @@ -77,7 +78,7 @@ func main() { func getSecretName() (string, error) { secretName := flag.Arg(0) if secretName == "" { - return "", fmt.Errorf("missing argument secret name") + return "", errors.New("missing argument secret name") } return secretName, nil } diff --git a/example/newreposecretwithxcrypto/main.go b/example/newreposecretwithxcrypto/main.go index 6737263aa08..9c1a42b04eb 100644 --- a/example/newreposecretwithxcrypto/main.go +++ b/example/newreposecretwithxcrypto/main.go @@ -31,6 +31,7 @@ import ( "context" crypto_rand "crypto/rand" "encoding/base64" + "errors" "flag" "fmt" "log" @@ -84,7 +85,7 @@ func main() { func getSecretName() (string, error) { secretName := flag.Arg(0) if secretName == "" { - return "", fmt.Errorf("missing argument secret name") + return "", errors.New("missing argument secret name") } return secretName, nil } diff --git a/github/copilot.go b/github/copilot.go index 2697b71850c..349b16ebde6 100644 --- a/github/copilot.go +++ b/github/copilot.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" ) @@ -87,7 +88,7 @@ func (cp *CopilotSeatDetails) UnmarshalJSON(data []byte) error { } if v["type"] == nil { - return fmt.Errorf("assignee type field is not set") + return errors.New("assignee type field is not set") } if t, ok := v["type"].(string); ok && t == "User" { diff --git a/github/dependabot_secrets.go b/github/dependabot_secrets.go index e85c805a63f..5adca6b6073 100644 --- a/github/dependabot_secrets.go +++ b/github/dependabot_secrets.go @@ -8,6 +8,7 @@ package github import ( "context" "fmt" + "strconv" ) func (s *DependabotService) getPublicKey(ctx context.Context, url string) (*PublicKey, *Response, error) { @@ -162,7 +163,7 @@ func (s *DependabotService) CreateOrUpdateRepoSecret(ctx context.Context, owner, func (s *DependabotService) CreateOrUpdateOrgSecret(ctx context.Context, org string, eSecret *DependabotEncryptedSecret) (*Response, error) { repoIDs := make([]string, len(eSecret.SelectedRepositoryIDs)) for i, secret := range eSecret.SelectedRepositoryIDs { - repoIDs[i] = fmt.Sprintf("%v", secret) + repoIDs[i] = strconv.FormatInt(secret, 10) } params := struct { *DependabotEncryptedSecret diff --git a/github/git_commits.go b/github/git_commits.go index 573d38be528..fdb3d26ceab 100644 --- a/github/git_commits.go +++ b/github/git_commits.go @@ -129,7 +129,7 @@ type CreateCommitOptions struct { //meta:operation POST /repos/{owner}/{repo}/git/commits func (s *GitService) CreateCommit(ctx context.Context, owner string, repo string, commit *Commit, opts *CreateCommitOptions) (*Commit, *Response, error) { if commit == nil { - return nil, nil, fmt.Errorf("commit must be provided") + return nil, nil, errors.New("commit must be provided") } if opts == nil { opts = &CreateCommitOptions{} diff --git a/github/git_commits_test.go b/github/git_commits_test.go index d99787fab1c..9ea876050e6 100644 --- a/github/git_commits_test.go +++ b/github/git_commits_test.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -402,7 +403,7 @@ func TestGitService_createSignature_signerError(t *testing.T) { Author: &CommitAuthor{Name: String("go-github")}, } - signer := mockSigner(t, "", fmt.Errorf("signer error"), "") + signer := mockSigner(t, "", errors.New("signer error"), "") _, err := createSignature(signer, a) if err == nil { diff --git a/github/github_test.go b/github/github_test.go index a3eb29720c8..83ba1a81a64 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -1318,7 +1318,7 @@ func TestDo_rateLimit_noNetworkCall(t *testing.T) { mux.HandleFunc("/first", func(w http.ResponseWriter, r *http.Request) { w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1378,7 +1378,7 @@ func TestDo_rateLimit_ignoredFromCache(t *testing.T) { w.Header().Set("X-From-Cache", "1") w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1425,7 +1425,7 @@ func TestDo_rateLimit_sleepUntilResponseResetLimit(t *testing.T) { firstRequest = false w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1436,7 +1436,7 @@ func TestDo_rateLimit_sleepUntilResponseResetLimit(t *testing.T) { } w.Header().Set(headerRateLimit, "5000") w.Header().Set(headerRateRemaining, "5000") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Add(time.Hour).Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusOK) fmt.Fprintln(w, `{}`) @@ -1465,7 +1465,7 @@ func TestDo_rateLimit_sleepUntilResponseResetLimitRetryOnce(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1497,7 +1497,7 @@ func TestDo_rateLimit_sleepUntilClientResetLimit(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "5000") w.Header().Set(headerRateRemaining, "5000") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Add(time.Hour).Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusOK) fmt.Fprintln(w, `{}`) @@ -1528,7 +1528,7 @@ func TestDo_rateLimit_abortSleepContextCancelled(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1561,7 +1561,7 @@ func TestDo_rateLimit_abortSleepContextCancelledClientLimit(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "5000") w.Header().Set(headerRateRemaining, "5000") - w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) + w.Header().Set(headerRateReset, strconv.FormatInt(reset.Add(time.Hour).Unix(), 10)) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusOK) fmt.Fprintln(w, `{}`) diff --git a/github/orgs_properties.go b/github/orgs_properties.go index 3387d98d76f..d8db48fc94e 100644 --- a/github/orgs_properties.go +++ b/github/orgs_properties.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" ) @@ -69,7 +70,7 @@ func (cpv *CustomPropertyValue) UnmarshalJSON(data []byte) error { if str, ok := item.(string); ok { strSlice[i] = str } else { - return fmt.Errorf("non-string value in string array") + return errors.New("non-string value in string array") } } cpv.Value = strSlice diff --git a/github/pulls.go b/github/pulls.go index b668502691e..35ceda44679 100644 --- a/github/pulls.go +++ b/github/pulls.go @@ -8,6 +8,7 @@ package github import ( "bytes" "context" + "errors" "fmt" ) @@ -352,7 +353,7 @@ type pullRequestUpdate struct { //meta:operation PATCH /repos/{owner}/{repo}/pulls/{pull_number} func (s *PullRequestsService) Edit(ctx context.Context, owner string, repo string, number int, pull *PullRequest) (*PullRequest, *Response, error) { if pull == nil { - return nil, nil, fmt.Errorf("pull must be provided") + return nil, nil, errors.New("pull must be provided") } u := fmt.Sprintf("repos/%v/%v/pulls/%d", owner, repo, number) diff --git a/tools/metadata/main.go b/tools/metadata/main.go index 238fc468856..7be3cbed656 100644 --- a/tools/metadata/main.go +++ b/tools/metadata/main.go @@ -10,6 +10,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -70,7 +71,7 @@ func (c *rootCmd) opsFile() (string, *operationsFile, error) { func githubClient(apiURL string) (*github.Client, error) { token := os.Getenv("GITHUB_TOKEN") if token == "" { - return nil, fmt.Errorf("GITHUB_TOKEN environment variable must be set to a GitHub personal access token with the public_repo scope") + return nil, errors.New("GITHUB_TOKEN environment variable must be set to a GitHub personal access token with the public_repo scope") } return github.NewClient(nil).WithAuthToken(token).WithEnterpriseURLs(apiURL, "") } @@ -83,7 +84,7 @@ type updateOpenAPICmd struct { func (c *updateOpenAPICmd) Run(root *rootCmd) error { ctx := context.Background() if c.ValidateGithub && c.Ref != "main" { - return fmt.Errorf("--validate and --ref are mutually exclusive") + return errors.New("--validate and --ref are mutually exclusive") } filename, opsFile, err := root.opsFile() if err != nil { @@ -102,7 +103,7 @@ func (c *updateOpenAPICmd) Run(root *rootCmd) error { if c.ValidateGithub { ref = opsFile.GitCommit if ref == "" { - return fmt.Errorf("openapi_operations.yaml does not have an openapi_commit field") + return errors.New("openapi_operations.yaml does not have an openapi_commit field") } } err = opsFile.updateFromGithub(ctx, client, ref) @@ -113,7 +114,7 @@ func (c *updateOpenAPICmd) Run(root *rootCmd) error { return opsFile.saveFile(filename) } if !operationsEqual(origOps, opsFile.OpenapiOps) { - return fmt.Errorf("openapi_operations.yaml does not match the OpenAPI descriptions in github.com/github/rest-api-description") + return errors.New("openapi_operations.yaml does not match the OpenAPI descriptions in github.com/github/rest-api-description") } return nil } From 3d9f8e25f1eeff6acd07e5aa285bf8059bae4a2e Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Fri, 11 Oct 2024 15:12:23 +0300 Subject: [PATCH 2/2] Revert strconv.FormatInt changes --- .golangci.yml | 4 ++++ github/dependabot_secrets.go | 3 +-- github/github_test.go | 16 ++++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 30628b213fa..d97416041cd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,3 +60,7 @@ issues: # We don't run parallel integration tests - linters: [ paralleltest, tparallel ] path: '^test/integration' + + # Because fmt.Sprint(reset.Unix())) is more readable than strconv.FormatInt(reset.Unix(), 10). + - linters: [ perfsprint] + text: 'fmt.Sprint.* can be replaced with faster strconv.FormatInt' diff --git a/github/dependabot_secrets.go b/github/dependabot_secrets.go index 5adca6b6073..e85c805a63f 100644 --- a/github/dependabot_secrets.go +++ b/github/dependabot_secrets.go @@ -8,7 +8,6 @@ package github import ( "context" "fmt" - "strconv" ) func (s *DependabotService) getPublicKey(ctx context.Context, url string) (*PublicKey, *Response, error) { @@ -163,7 +162,7 @@ func (s *DependabotService) CreateOrUpdateRepoSecret(ctx context.Context, owner, func (s *DependabotService) CreateOrUpdateOrgSecret(ctx context.Context, org string, eSecret *DependabotEncryptedSecret) (*Response, error) { repoIDs := make([]string, len(eSecret.SelectedRepositoryIDs)) for i, secret := range eSecret.SelectedRepositoryIDs { - repoIDs[i] = strconv.FormatInt(secret, 10) + repoIDs[i] = fmt.Sprintf("%v", secret) } params := struct { *DependabotEncryptedSecret diff --git a/github/github_test.go b/github/github_test.go index 83ba1a81a64..a3eb29720c8 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -1318,7 +1318,7 @@ func TestDo_rateLimit_noNetworkCall(t *testing.T) { mux.HandleFunc("/first", func(w http.ResponseWriter, r *http.Request) { w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1378,7 +1378,7 @@ func TestDo_rateLimit_ignoredFromCache(t *testing.T) { w.Header().Set("X-From-Cache", "1") w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1425,7 +1425,7 @@ func TestDo_rateLimit_sleepUntilResponseResetLimit(t *testing.T) { firstRequest = false w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1436,7 +1436,7 @@ func TestDo_rateLimit_sleepUntilResponseResetLimit(t *testing.T) { } w.Header().Set(headerRateLimit, "5000") w.Header().Set(headerRateRemaining, "5000") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Add(time.Hour).Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusOK) fmt.Fprintln(w, `{}`) @@ -1465,7 +1465,7 @@ func TestDo_rateLimit_sleepUntilResponseResetLimitRetryOnce(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1497,7 +1497,7 @@ func TestDo_rateLimit_sleepUntilClientResetLimit(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "5000") w.Header().Set(headerRateRemaining, "5000") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Add(time.Hour).Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusOK) fmt.Fprintln(w, `{}`) @@ -1528,7 +1528,7 @@ func TestDo_rateLimit_abortSleepContextCancelled(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "60") w.Header().Set(headerRateRemaining, "0") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusForbidden) fmt.Fprintln(w, `{ @@ -1561,7 +1561,7 @@ func TestDo_rateLimit_abortSleepContextCancelledClientLimit(t *testing.T) { requestCount++ w.Header().Set(headerRateLimit, "5000") w.Header().Set(headerRateRemaining, "5000") - w.Header().Set(headerRateReset, strconv.FormatInt(reset.Add(time.Hour).Unix(), 10)) + w.Header().Set(headerRateReset, fmt.Sprint(reset.Add(time.Hour).Unix())) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusOK) fmt.Fprintln(w, `{}`)