From 61d81a79030635ffca9ceb07739e56b80eaf3582 Mon Sep 17 00:00:00 2001 From: jorge-ferrero-ag Date: Sat, 23 Aug 2025 15:45:17 +0200 Subject: [PATCH 1/2] chore: Replace http.Method* constants with string literals Updated all usages of http.Method* constants to use string literals for HTTP methods (e.g., "GET", "POST", etc.) throughout the codebase. Added forbidigo linter to enforce this rule and updated .golangci.yml accordingly. This change aligns with new linting requirements and ensures consistency in HTTP method usage. --- .golangci.yml | 5 ++++ github/classroom.go | 3 +-- github/github.go | 10 +++---- github/github_test.go | 26 +++++++++---------- github/orgs_credential_authorizations.go | 5 ++-- github/orgs_credential_authorizations_test.go | 4 +-- github/orgs_personal_access_tokens.go | 5 ++-- github/orgs_personal_access_tokens_test.go | 2 +- github/reactions.go | 3 +-- github/repos_contents.go | 4 +-- github/repos_hooks_test.go | 4 +-- 11 files changed, 36 insertions(+), 35 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 80c21f9d99b..c96bed884e1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,6 +8,7 @@ linters: - copyloopvar - dogsled - dupl + - forbidigo - gocritic - godot - goheader @@ -114,6 +115,10 @@ linters: checks: - "all" - "-QF1008" # allow embedded field in selector + forbidigo: + forbid: + - pattern: "^http\\.Method[A-Z][a-z]*$" + msg: "Use string literals instead of http.Method constants (https://pkg.go.dev/net/http#pkg-constants)" custom: sliceofpointers: type: module diff --git a/github/classroom.go b/github/classroom.go index 8f6ce14511e..66958d583d1 100644 --- a/github/classroom.go +++ b/github/classroom.go @@ -8,7 +8,6 @@ package github import ( "context" "fmt" - "net/http" ) // ClassroomService handles communication with the GitHub Classroom related @@ -67,7 +66,7 @@ func (a ClassroomAssignment) String() string { func (s *ClassroomService) GetAssignment(ctx context.Context, assignmentID int64) (*ClassroomAssignment, *Response, error) { u := fmt.Sprintf("assignments/%v", assignmentID) - req, err := s.client.NewRequest(http.MethodGet, u, nil) + req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err } diff --git a/github/github.go b/github/github.go index b1e7b3ddaf1..4ebb1750ea4 100644 --- a/github/github.go +++ b/github/github.go @@ -587,7 +587,7 @@ func (c *Client) NewFormRequest(urlStr string, body io.Reader, opts ...RequestOp return nil, err } - req, err := http.NewRequest(http.MethodPost, u.String(), body) + req, err := http.NewRequest("POST", u.String(), body) if err != nil { return nil, err } @@ -1525,7 +1525,7 @@ func GetRateLimitCategory(method, path string) RateLimitCategory { // https://docs.github.com/en/rest/search/search#search-code case strings.HasPrefix(path, "/search/code") && - method == http.MethodGet: + method == "GET": return CodeSearchCategory case strings.HasPrefix(path, "/search/"): @@ -1534,13 +1534,13 @@ func GetRateLimitCategory(method, path string) RateLimitCategory { return GraphqlCategory case strings.HasPrefix(path, "/app-manifests/") && strings.HasSuffix(path, "/conversions") && - method == http.MethodPost: + method == "POST": return IntegrationManifestCategory // https://docs.github.com/rest/migrations/source-imports#start-an-import case strings.HasPrefix(path, "/repos/") && strings.HasSuffix(path, "/import") && - method == http.MethodPut: + method == "PUT": return SourceImportCategory // https://docs.github.com/rest/code-scanning#upload-an-analysis-as-sarif-data @@ -1554,7 +1554,7 @@ func GetRateLimitCategory(method, path string) RateLimitCategory { // https://docs.github.com/en/rest/dependency-graph/dependency-submission#create-a-snapshot-of-dependencies-for-a-repository case strings.HasPrefix(path, "/repos/") && strings.HasSuffix(path, "/dependency-graph/snapshots") && - method == http.MethodPost: + method == "POST": return DependencySnapshotsCategory // https://docs.github.com/en/enterprise-cloud@latest/rest/orgs/orgs?apiVersion=2022-11-28#get-the-audit-log-for-an-organization diff --git a/github/github_test.go b/github/github_test.go index ff0fa16a4a3..42a5a6810d5 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -646,7 +646,7 @@ func TestNewRequest_errorForNoTrailingSlash(t *testing.T) { t.Fatalf("url.Parse returned unexpected error: %v.", err) } c.BaseURL = u - if _, err := c.NewRequest(http.MethodGet, "test", nil); test.wantError && err == nil { + if _, err := c.NewRequest("GET", "test", nil); test.wantError && err == nil { t.Fatal("Expected error to be returned.") } else if !test.wantError && err != nil { t.Fatalf("NewRequest returned unexpected error: %v.", err) @@ -1241,62 +1241,62 @@ func TestDo_rateLimitCategory(t *testing.T) { category RateLimitCategory }{ { - method: http.MethodGet, + method: "GET", url: "/", category: CoreCategory, }, { - method: http.MethodGet, + method: "GET", url: "/search/issues?q=rate", category: SearchCategory, }, { - method: http.MethodGet, + method: "GET", url: "/graphql", category: GraphqlCategory, }, { - method: http.MethodPost, + method: "POST", url: "/app-manifests/code/conversions", category: IntegrationManifestCategory, }, { - method: http.MethodGet, + method: "GET", url: "/app-manifests/code/conversions", category: CoreCategory, // only POST requests are in the integration manifest category }, { - method: http.MethodPut, + method: "PUT", url: "/repos/google/go-github/import", category: SourceImportCategory, }, { - method: http.MethodGet, + method: "GET", url: "/repos/google/go-github/import", category: CoreCategory, // only PUT requests are in the source import category }, { - method: http.MethodPost, + method: "POST", url: "/repos/google/go-github/code-scanning/sarifs", category: CodeScanningUploadCategory, }, { - method: http.MethodGet, + method: "GET", url: "/scim/v2/organizations/ORG/Users", category: ScimCategory, }, { - method: http.MethodPost, + method: "POST", url: "/repos/google/go-github/dependency-graph/snapshots", category: DependencySnapshotsCategory, }, { - method: http.MethodGet, + method: "GET", url: "/search/code?q=rate", category: CodeSearchCategory, }, { - method: http.MethodGet, + method: "GET", url: "/orgs/google/audit-log", category: AuditLogCategory, }, diff --git a/github/orgs_credential_authorizations.go b/github/orgs_credential_authorizations.go index dca42433c30..326922c3bcb 100644 --- a/github/orgs_credential_authorizations.go +++ b/github/orgs_credential_authorizations.go @@ -8,7 +8,6 @@ package github import ( "context" "fmt" - "net/http" ) // CredentialAuthorization represents a credential authorized through SAML SSO. @@ -78,7 +77,7 @@ func (s *OrganizationsService) ListCredentialAuthorizations(ctx context.Context, return nil, nil, err } - req, err := s.client.NewRequest(http.MethodGet, u, nil) + req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err } @@ -100,7 +99,7 @@ func (s *OrganizationsService) ListCredentialAuthorizations(ctx context.Context, //meta:operation DELETE /orgs/{org}/credential-authorizations/{credential_id} func (s *OrganizationsService) RemoveCredentialAuthorization(ctx context.Context, org string, credentialID int64) (*Response, error) { u := fmt.Sprintf("orgs/%v/credential-authorizations/%v", org, credentialID) - req, err := s.client.NewRequest(http.MethodDelete, u, nil) + req, err := s.client.NewRequest("DELETE", u, nil) if err != nil { return nil, err } diff --git a/github/orgs_credential_authorizations_test.go b/github/orgs_credential_authorizations_test.go index dfd7e559507..cd760a629b8 100644 --- a/github/orgs_credential_authorizations_test.go +++ b/github/orgs_credential_authorizations_test.go @@ -20,7 +20,7 @@ func TestOrganizationsService_ListCredentialAuthorizations(t *testing.T) { client, mux, _ := setup(t) mux.HandleFunc("/orgs/o/credential-authorizations", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, http.MethodGet) + testMethod(t, r, "GET") testFormValues(t, r, values{"per_page": "2", "page": "2", "login": "l"}) fmt.Fprint(w, `[ { @@ -77,7 +77,7 @@ func TestOrganizationsService_RemoveCredentialAuthorization(t *testing.T) { client, mux, _ := setup(t) mux.HandleFunc("/orgs/o/credential-authorizations/1", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, http.MethodDelete) + testMethod(t, r, "DELETE") w.WriteHeader(http.StatusNoContent) }) diff --git a/github/orgs_personal_access_tokens.go b/github/orgs_personal_access_tokens.go index 4abd6665ad5..651dcc26d77 100644 --- a/github/orgs_personal_access_tokens.go +++ b/github/orgs_personal_access_tokens.go @@ -9,7 +9,6 @@ import ( "context" "errors" "fmt" - "net/http" "net/url" "strings" ) @@ -101,7 +100,7 @@ func (s *OrganizationsService) ListFineGrainedPersonalAccessTokens(ctx context.C return nil, nil, err } - req, err := s.client.NewRequest(http.MethodGet, u, opts) + req, err := s.client.NewRequest("GET", u, opts) if err != nil { return nil, nil, err } @@ -132,7 +131,7 @@ type ReviewPersonalAccessTokenRequestOptions struct { func (s *OrganizationsService) ReviewPersonalAccessTokenRequest(ctx context.Context, org string, requestID int64, opts ReviewPersonalAccessTokenRequestOptions) (*Response, error) { u := fmt.Sprintf("orgs/%v/personal-access-token-requests/%v", org, requestID) - req, err := s.client.NewRequest(http.MethodPost, u, &opts) + req, err := s.client.NewRequest("POST", u, &opts) if err != nil { return nil, err } diff --git a/github/orgs_personal_access_tokens_test.go b/github/orgs_personal_access_tokens_test.go index 845dcf63e6b..9040fa9a10f 100644 --- a/github/orgs_personal_access_tokens_test.go +++ b/github/orgs_personal_access_tokens_test.go @@ -172,7 +172,7 @@ func TestOrganizationsService_ReviewPersonalAccessTokenRequest(t *testing.T) { v := new(ReviewPersonalAccessTokenRequestOptions) assertNilError(t, json.NewDecoder(r.Body).Decode(v)) - testMethod(t, r, http.MethodPost) + testMethod(t, r, "POST") if !cmp.Equal(v, &input) { t.Errorf("Request body = %+v, want %+v", v, input) } diff --git a/github/reactions.go b/github/reactions.go index 5233f78d3d4..6f84c673c43 100644 --- a/github/reactions.go +++ b/github/reactions.go @@ -8,7 +8,6 @@ package github import ( "context" "fmt" - "net/http" ) // ReactionsService provides access to the reactions-related functions in the @@ -530,7 +529,7 @@ func (s *ReactionsService) DeleteTeamDiscussionCommentReactionByOrgIDAndTeamID(c } func (s *ReactionsService) deleteReaction(ctx context.Context, url string) (*Response, error) { - req, err := s.client.NewRequest(http.MethodDelete, url, nil) + req, err := s.client.NewRequest("DELETE", url, nil) if err != nil { return nil, err } diff --git a/github/repos_contents.go b/github/repos_contents.go index 3b24b6d4e6d..d5b0582b826 100644 --- a/github/repos_contents.go +++ b/github/repos_contents.go @@ -157,7 +157,7 @@ func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, if contents.GetDownloadURL() == "" { return nil, resp, fmt.Errorf("no download link found for %s", filepath) } - dlReq, err := http.NewRequestWithContext(ctx, http.MethodGet, *contents.DownloadURL, nil) + dlReq, err := http.NewRequestWithContext(ctx, "GET", *contents.DownloadURL, nil) if err != nil { return nil, resp, err } @@ -206,7 +206,7 @@ func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owne if contents.GetDownloadURL() == "" { return nil, contents, resp, fmt.Errorf("no download link found for %s", filepath) } - dlReq, err := http.NewRequestWithContext(ctx, http.MethodGet, *contents.DownloadURL, nil) + dlReq, err := http.NewRequestWithContext(ctx, "GET", *contents.DownloadURL, nil) if err != nil { return nil, contents, resp, err } diff --git a/github/repos_hooks_test.go b/github/repos_hooks_test.go index ff3bbc95ea5..3a778b595cb 100644 --- a/github/repos_hooks_test.go +++ b/github/repos_hooks_test.go @@ -575,7 +575,7 @@ func TestRepositoriesService_Subscribe(t *testing.T) { client, mux, _ := setup(t) mux.HandleFunc("/hub", func(_ http.ResponseWriter, r *http.Request) { - testMethod(t, r, http.MethodPost) + testMethod(t, r, "POST") testHeader(t, r, "Content-Type", "application/x-www-form-urlencoded") testFormValues(t, r, values{ "hub.mode": "subscribe", @@ -608,7 +608,7 @@ func TestRepositoriesService_Unsubscribe(t *testing.T) { client, mux, _ := setup(t) mux.HandleFunc("/hub", func(_ http.ResponseWriter, r *http.Request) { - testMethod(t, r, http.MethodPost) + testMethod(t, r, "POST") testHeader(t, r, "Content-Type", "application/x-www-form-urlencoded") testFormValues(t, r, values{ "hub.mode": "unsubscribe", From 661a4eaf35fa2f67b63908c27e48ac780aba14a6 Mon Sep 17 00:00:00 2001 From: jferrl Date: Tue, 23 Sep 2025 09:00:26 +0200 Subject: [PATCH 2/2] chore: Refactor forbidigo linter config in .golangci.yml Moved forbidigo linter rules to the main settings section for better organization and clarity. No changes to the actual forbidden patterns or messages. --- .golangci.yml | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4dffebb7f99..b25a83383c6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,6 +25,12 @@ linters: - unparam - whitespace settings: + forbidigo: + forbid: + - pattern: ^reflect\.DeepEqual$ + msg: "Use cmp.Equal instead of reflect.DeepEqual" + - pattern: "^http\\.Method[A-Z][a-z]*$" + msg: "Use string literals instead of http.Method constants (https://pkg.go.dev/net/http#pkg-constants)" gocritic: disable-all: true enabled-checks: @@ -41,10 +47,6 @@ linters: Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. - forbidigo: - forbid: - - pattern: ^reflect\.DeepEqual$ - msg: "Use cmp.Equal instead of reflect.DeepEqual" gosec: excludes: # duplicates errcheck @@ -119,10 +121,6 @@ linters: checks: - "all" - "-QF1008" # allow embedded field in selector - forbidigo: - forbid: - - pattern: "^http\\.Method[A-Z][a-z]*$" - msg: "Use string literals instead of http.Method constants (https://pkg.go.dev/net/http#pkg-constants)" custom: sliceofpointers: type: module