Merge pull request #12084 from cli/babakks/add-basic-linters

chore: add basic linters
This commit is contained in:
Kynan Ware 2025-11-04 10:46:40 -07:00 committed by GitHub
commit 152d328db8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 119 additions and 81 deletions

View file

@ -1,16 +1,50 @@
version: "2"
linters:
default: none
enable:
- bodyclose
- copyloopvar
- durationcheck
- gocritic
- govet
- ineffassign
- nilerr
- nolintlint
disable:
# The following linters are disabled purely because this config was migrated to v2 where they are in the default
# set, and we should have separate work to enable them if we truly want them.
- staticcheck
- errcheck
# To enable later due to too many issues, and confirm we need them:
# - gosec
# - staticcheck
# - errcheck
exclusions:
paths:
- third-party
rules:
- path: _test\.go$
linters:
- bodyclose
- gosec
settings:
gocritic:
disabled-checks:
- appendAssign
disabled-tags:
- style
gosec:
excludes:
- G110
- G204
- G301
- G302
- G304
- G307
- G404
config:
G104:
os:
- Setenv
govet:
enable:
- httpresponse
formatters:
enable:

View file

@ -152,6 +152,8 @@ func (c Client) RESTWithNext(hostname string, method string, p string, body io.R
}
// HandleHTTPError parses a http.Response into a HTTPError.
//
// The caller is responsible to close the response body stream.
func HandleHTTPError(resp *http.Response) error {
return handleResponse(ghAPI.HandleHTTPError(resp))
}
@ -196,12 +198,11 @@ func ScopesSuggestion(resp *http.Response) string {
// EndpointNeedsScopes adds additional OAuth scopes to an HTTP response as if they were returned from the
// server endpoint. This improves HTTP 4xx error messaging for endpoints that don't explicitly list the
// OAuth scopes they need.
func EndpointNeedsScopes(resp *http.Response, s string) *http.Response {
func EndpointNeedsScopes(resp *http.Response, s string) {
if resp.StatusCode >= 400 && resp.StatusCode < 500 {
oldScopes := resp.Header.Get("X-Accepted-Oauth-Scopes")
resp.Header.Set("X-Accepted-Oauth-Scopes", fmt.Sprintf("%s, %s", oldScopes, s))
}
return resp
}
func generateScopesSuggestion(statusCode int, endpointNeedsScopes, tokenHasScopes, hostname string) string {

View file

@ -127,7 +127,6 @@ func TestChecksStatus_SummarisingCheckRuns(t *testing.T) {
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
@ -180,7 +179,6 @@ func TestChecksStatus_SummarisingStatusContexts(t *testing.T) {
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

View file

@ -1549,6 +1549,8 @@ func RepoExists(client *Client, repo ghrepo.Interface) (bool, error) {
return false, err
}
defer resp.Body.Close()
switch resp.StatusCode {
case 200:
return true, nil

View file

@ -83,7 +83,6 @@ func TestJoin(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
joinedPath, err := tt.base.Join(tt.elems...)

View file

@ -696,7 +696,7 @@ func Test_parsePathFromFileArg(t *testing.T) {
{
name: "go to root of repository",
currentDir: "pkg/cmd/browse/",
fileArg: filepath.Join("../../../"),
fileArg: filepath.FromSlash("../../../"),
expectedPath: "",
},
{

View file

@ -329,7 +329,6 @@ func (a *App) ForwardPorts(ctx context.Context, selector *CodespaceSelector, por
// them at the first failure, including cancellation of the context.
group, ctx := errgroup.WithContext(ctx)
for _, pair := range portPairs {
pair := pair
group.Go(func() error {
listen, _, err := codespaces.ListenTCP(pair.local, true)
if err != nil {

View file

@ -584,9 +584,8 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
continue
}
cs := cs
wg.Add(1)
go func() {
go func(cs *api.Codespace) {
result := sshResult{}
defer wg.Done()
@ -622,7 +621,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
result.codespace = cs
sshUsers <- result
}()
}(cs)
}
go func() {

View file

@ -286,7 +286,8 @@ func createGist(client *http.Client, hostname, description string, public bool,
defer resp.Body.Close()
if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(api.EndpointNeedsScopes(resp, "gist"))
api.EndpointNeedsScopes(resp, "gist")
return nil, api.HandleHTTPError(resp)
}
result := &shared.Gist{}

View file

@ -293,7 +293,7 @@ func highlightMatch(s string, filter *regexp.Regexp, matched *bool, color, highl
text = s[match[1]:matches[i+1][0]]
}
if _, err := out.WriteString(color(text)); err != nil {
return "", nil
return "", err
}
}

View file

@ -265,7 +265,8 @@ func (m *templateManager) fetch() error {
gitClient := &git.Client{}
dir, err = gitClient.ToplevelDir(context.Background())
if err != nil {
return nil // abort silently
//nolint:nilerr // intentional, abort silently
return nil
}
}

View file

@ -32,7 +32,8 @@ func deleteRepo(client *http.Client, repo ghrepo.Interface) error {
defer resp.Body.Close()
if resp.StatusCode > 299 {
return api.HandleHTTPError(api.EndpointNeedsScopes(resp, "delete_repo"))
api.EndpointNeedsScopes(resp, "delete_repo")
return api.HandleHTTPError(resp)
}
return nil

View file

@ -526,6 +526,8 @@ func getTopics(ctx context.Context, httpClient *http.Client, repo ghrepo.Interfa
if err != nil {
return nil, err
}
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
return nil, api.HandleHTTPError(res)
}
@ -563,6 +565,7 @@ func setTopics(ctx context.Context, httpClient *http.Client, repo ghrepo.Interfa
if err != nil {
return err
}
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
return api.HandleHTTPError(res)

View file

@ -347,7 +347,7 @@ func forkRun(opts *ForkOptions) error {
forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol)
dir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs)
if err == nil {
return dir, err
return dir, nil
}
var execError errWithExitCode
if errors.As(err, &execError) && execError.ExitCode() == 128 {

View file

@ -36,7 +36,7 @@ func getCommits(client *http.Client, repo ghrepo.Interface, maxCommits int) ([]*
break
}
result := Result{}
resp, err := getResponse(client, repo.RepoHost(), pathF(page), &result)
links, err := getResponse(client, repo.RepoHost(), pathF(page), &result)
if err != nil {
return nil, err
}
@ -52,8 +52,7 @@ func getCommits(client *http.Client, repo ghrepo.Interface, maxCommits int) ([]*
Char: colorFunc(string(handle[0])),
})
}
link := resp.Header["Link"]
if len(link) == 0 || !strings.Contains(link[0], "last") {
if len(links) == 0 || !strings.Contains(links[0], "last") {
paginating = false
}
page++
@ -68,7 +67,9 @@ func getCommits(client *http.Client, repo ghrepo.Interface, maxCommits int) ([]*
return commits, nil
}
func getResponse(client *http.Client, host, path string, data interface{}) (*http.Response, error) {
// getResponse performs the API call and returns the response's link header values.
// If the "Link" header is missing, the returned slice will be nil.
func getResponse(client *http.Client, host, path string, data interface{}) ([]string, error) {
url := ghinstance.RESTPrefix(host) + path
req, err := http.NewRequest("GET", url, nil)
if err != nil {
@ -87,8 +88,10 @@ func getResponse(client *http.Client, host, path string, data interface{}) (*htt
return nil, errors.New("api call failed")
}
links := resp.Header["Link"]
if resp.StatusCode == http.StatusNoContent {
return resp, nil
return links, nil
}
b, err := io.ReadAll(resp.Body)
@ -101,5 +104,5 @@ func getResponse(client *http.Client, host, path string, data interface{}) (*htt
return nil, err
}
return resp, nil
return links, nil
}

View file

@ -60,7 +60,6 @@ func TestCmdHelpTopic(t *testing.T) {
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

View file

@ -527,7 +527,7 @@ func Test_runDownload(t *testing.T) {
},
},
expectedFiles: []string{
filepath.Join("non-artifact-2-file"),
"non-artifact-2-file",
},
},
{
@ -681,7 +681,7 @@ func Test_runDownload(t *testing.T) {
})
},
expectedFiles: []string{
filepath.Join("artifact-2-file"),
"artifact-2-file",
},
},
{

View file

@ -249,6 +249,7 @@ func GraphQLQuery(body string, cb func(string, map[string]interface{})) Responde
// ScopesResponder returns a response with a 200 status code and the given OAuth scopes.
func ScopesResponder(scopes string) func(*http.Request) (*http.Response, error) {
//nolint:bodyclose
return StatusScopesResponder(http.StatusOK, scopes)
}

View file

@ -65,9 +65,6 @@ func NewSearcher(client *http.Client, host string, detector fd.Detector) Searche
func (s searcher) Code(query Query) (CodeResult, error) {
result := CodeResult{}
var resp *http.Response
var err error
// We will request either the query limit if it's less than 1 page, or our max page size.
// This number doesn't change to keep a valid offset.
//
@ -77,15 +74,11 @@ func (s searcher) Code(query Query) (CodeResult, error) {
// If we were to request page #2 for 50 items, we would instead get items 50 to 99.
numItemsToRetrieve := query.Limit
query.Limit = min(numItemsToRetrieve, maxPerPage)
query.Page = 1
for numItemsToRetrieve > 0 {
query.Page = nextPage(resp)
if query.Page == 0 {
break
}
page := CodeResult{}
resp, err = s.search(query, &page)
link, err := s.search(query, &page)
if err != nil {
return result, err
}
@ -99,6 +92,11 @@ func (s searcher) Code(query Query) (CodeResult, error) {
result.Total = page.Total
result.Items = append(result.Items, page.Items[:numItemsToAdd]...)
numItemsToRetrieve = numItemsToRetrieve - numItemsToAdd
query.Page = nextPage(link)
if query.Page == 0 {
break
}
}
return result, nil
@ -107,20 +105,13 @@ func (s searcher) Code(query Query) (CodeResult, error) {
func (s searcher) Commits(query Query) (CommitsResult, error) {
result := CommitsResult{}
var resp *http.Response
var err error
numItemsToRetrieve := query.Limit
query.Limit = min(numItemsToRetrieve, maxPerPage)
query.Page = 1
for numItemsToRetrieve > 0 {
query.Page = nextPage(resp)
if query.Page == 0 {
break
}
page := CommitsResult{}
resp, err = s.search(query, &page)
link, err := s.search(query, &page)
if err != nil {
return result, err
}
@ -130,6 +121,11 @@ func (s searcher) Commits(query Query) (CommitsResult, error) {
result.Total = page.Total
result.Items = append(result.Items, page.Items[:numItemsToAdd]...)
numItemsToRetrieve = numItemsToRetrieve - numItemsToAdd
query.Page = nextPage(link)
if query.Page == 0 {
break
}
}
return result, nil
}
@ -137,20 +133,13 @@ func (s searcher) Commits(query Query) (CommitsResult, error) {
func (s searcher) Repositories(query Query) (RepositoriesResult, error) {
result := RepositoriesResult{}
var resp *http.Response
var err error
numItemsToRetrieve := query.Limit
query.Limit = min(numItemsToRetrieve, maxPerPage)
query.Page = 1
for numItemsToRetrieve > 0 {
query.Page = nextPage(resp)
if query.Page == 0 {
break
}
page := RepositoriesResult{}
resp, err = s.search(query, &page)
link, err := s.search(query, &page)
if err != nil {
return result, err
}
@ -160,6 +149,11 @@ func (s searcher) Repositories(query Query) (RepositoriesResult, error) {
result.Total = page.Total
result.Items = append(result.Items, page.Items[:numItemsToAdd]...)
numItemsToRetrieve = numItemsToRetrieve - numItemsToAdd
query.Page = nextPage(link)
if query.Page == 0 {
break
}
}
return result, nil
}
@ -167,20 +161,12 @@ func (s searcher) Repositories(query Query) (RepositoriesResult, error) {
func (s searcher) Issues(query Query) (IssuesResult, error) {
result := IssuesResult{}
var resp *http.Response
var err error
numItemsToRetrieve := query.Limit
query.Limit = min(numItemsToRetrieve, maxPerPage)
query.Page = 1
for numItemsToRetrieve > 0 {
query.Page = nextPage(resp)
if query.Page == 0 {
break
}
page := IssuesResult{}
resp, err = s.search(query, &page)
link, err := s.search(query, &page)
if err != nil {
return result, err
}
@ -190,11 +176,18 @@ func (s searcher) Issues(query Query) (IssuesResult, error) {
result.Total = page.Total
result.Items = append(result.Items, page.Items[:numItemsToAdd]...)
numItemsToRetrieve = numItemsToRetrieve - numItemsToAdd
query.Page = nextPage(link)
if query.Page == 0 {
break
}
}
return result, nil
}
// search makes a single-page REST search request for code, commits, issues, prs, or repos.
// search makes a single-page REST search request for code, commits, issues, prs, or repos,
// and returns the link header from response for further pagination calls. If the link header
// is not set on the response, empty string is returned.
//
// The result argument is populated with the following information:
//
@ -203,7 +196,7 @@ func (s searcher) Issues(query Query) (IssuesResult, error) {
// - Items: the actual matching search results, up to 100 max items per page
//
// For more information, see https://docs.github.com/en/rest/search/search?apiVersion=2022-11-28.
func (s searcher) search(query Query, result interface{}) (*http.Response, error) {
func (s searcher) search(query Query, result interface{}) (string, error) {
path := fmt.Sprintf("%ssearch/%s", ghinstance.RESTPrefix(s.host), query.Kind)
qs := url.Values{}
qs.Set("page", strconv.Itoa(query.Page))
@ -216,7 +209,7 @@ func (s searcher) search(query Query, result interface{}) (*http.Response, error
// issues.
features, err := s.detector.SearchFeatures()
if err != nil {
return nil, err
return "", err
}
if !features.AdvancedIssueSearchAPI {
@ -242,7 +235,7 @@ func (s searcher) search(query Query, result interface{}) (*http.Response, error
url := fmt.Sprintf("%s?%s", path, qs.Encode())
req, err := http.NewRequest("GET", url, nil)
if err != nil {
return nil, err
return "", err
}
req.Header.Set("Content-Type", "application/json; charset=utf-8")
req.Header.Set("Accept", "application/vnd.github.v3+json")
@ -252,19 +245,22 @@ func (s searcher) search(query Query, result interface{}) (*http.Response, error
resp, err := s.client.Do(req)
if err != nil {
return nil, err
return "", err
}
defer resp.Body.Close()
link := resp.Header.Get("Link")
success := resp.StatusCode >= 200 && resp.StatusCode < 300
if !success {
return resp, handleHTTPError(resp)
return link, handleHTTPError(resp)
}
decoder := json.NewDecoder(resp.Body)
err = decoder.Decode(result)
if err != nil {
return resp, err
return link, err
}
return resp, nil
return link, nil
}
// URL returns URL to the global search in web GUI (i.e. github.com/search).
@ -317,16 +313,17 @@ func handleHTTPError(resp *http.Response) error {
return httpError
}
// https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api
func nextPage(resp *http.Response) (page int) {
if resp == nil {
return 1
}
// nextPage extracts the next page number from an API response's link header. if
// the provided link header is empty or there is no next page, zero is returned.
//
// See API [docs] on pagination for more information.
//
// [docs]: https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api
func nextPage(link string) (page int) {
// When using pagination, responses get a "Link" field in their header.
// When a next page is available, "Link" contains a link to the next page
// tagged with rel="next".
for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) {
for _, m := range linkRE.FindAllStringSubmatch(link, -1) {
if !(len(m) > 2 && m[2] == "next") {
continue
}