From d0cefa519a6641f89ecf097ea03cdf52baf4ec7d Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 23 Oct 2023 09:35:40 -0700 Subject: [PATCH] :seedling: enable the golangci-lint `bugs` preset (#3583) * enable bugs preset Signed-off-by: Spencer Schrock * fix noctx linter Signed-off-by: Spencer Schrock * fix bodyclose linter Signed-off-by: Spencer Schrock * fix contextcheck linter Signed-off-by: Spencer Schrock * This ignores all existing cases of musttag linter complaints. This analyzer seems useful in the future, but some of this code is old and I don't want to change it for existing code now. Signed-off-by: Spencer Schrock * ignore existing nilerr lints. This behavior is from the initial commit, and primarily affects metrics. Leaving as is, and hope to benefit from the linter in the future. Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --- .golangci.yml | 2 ++ attestor/policy/attestation_policy.go | 2 +- clients/githubrepo/roundtripper/rate_limit.go | 2 ++ clients/githubrepo/roundtripper/rate_limit_test.go | 7 +++++-- clients/gitlabrepo/graphql.go | 2 +- clients/ossfuzz/client.go | 14 ++++++++++---- cmd/internal/packagemanager/client_test.go | 2 ++ cron/internal/format/json.go | 4 ++-- pkg/dependencydiff_result.go | 2 ++ pkg/json.go | 3 ++- 10 files changed, 29 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3f7d7108..8f02cee9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -62,6 +62,8 @@ linters: - unused - whitespace - wrapcheck + presets: + - bugs linters-settings: errcheck: check-type-assertions: true diff --git a/attestor/policy/attestation_policy.go b/attestor/policy/attestation_policy.go index 1052bbad..cfc991db 100644 --- a/attestor/policy/attestation_policy.go +++ b/attestor/policy/attestation_policy.go @@ -29,7 +29,7 @@ import ( sclog "github.com/ossf/scorecard/v4/log" ) -//nolint:govet +//nolint:govet,musttag // JSON usage is test only type AttestationPolicy struct { // PreventBinaryArtifacts : set to true to require that this project's SCM repo is // free of binary artifacts diff --git a/clients/githubrepo/roundtripper/rate_limit.go b/clients/githubrepo/roundtripper/rate_limit.go index d2ae8378..6d7d76b4 100644 --- a/clients/githubrepo/roundtripper/rate_limit.go +++ b/clients/githubrepo/roundtripper/rate_limit.go @@ -62,6 +62,7 @@ func (gh *rateLimitTransport) RoundTrip(r *http.Request) (*http.Response, error) rateLimit := resp.Header.Get("X-RateLimit-Remaining") remaining, err := strconv.Atoi(rateLimit) if err != nil { + //nolint:nilerr // just an error in metadata, response may still be useful? return resp, nil } ctx, err := tag.New(r.Context(), tag.Upsert(githubstats.ResourceType, resp.Header.Get("X-RateLimit-Resource"))) @@ -73,6 +74,7 @@ func (gh *rateLimitTransport) RoundTrip(r *http.Request) (*http.Response, error) if remaining <= 0 { reset, err := strconv.Atoi(resp.Header.Get("X-RateLimit-Reset")) if err != nil { + //nolint:nilerr // just an error in metadata, response may still be useful? return resp, nil } diff --git a/clients/githubrepo/roundtripper/rate_limit_test.go b/clients/githubrepo/roundtripper/rate_limit_test.go index b74b7d99..83b50291 100644 --- a/clients/githubrepo/roundtripper/rate_limit_test.go +++ b/clients/githubrepo/roundtripper/rate_limit_test.go @@ -14,6 +14,7 @@ package roundtripper import ( + "context" "net/http" "net/http/httptest" "testing" @@ -60,7 +61,7 @@ func TestRoundTrip(t *testing.T) { } t.Run("Successful response", func(t *testing.T) { - req, err := http.NewRequest(http.MethodGet, ts.URL+"/success", nil) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, ts.URL+"/success", nil) if err != nil { t.Fatalf("Failed to create request: %v", err) } @@ -69,13 +70,14 @@ func TestRoundTrip(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %v", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Errorf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode) } }) t.Run("Retry-After header set", func(t *testing.T) { - req, err := http.NewRequest(http.MethodGet, ts.URL+"/retry", nil) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, ts.URL+"/retry", nil) if err != nil { t.Fatalf("Failed to create request: %v", err) } @@ -84,6 +86,7 @@ func TestRoundTrip(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %v", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Errorf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode) } diff --git a/clients/gitlabrepo/graphql.go b/clients/gitlabrepo/graphql.go index 34a7694d..4318e289 100644 --- a/clients/gitlabrepo/graphql.go +++ b/clients/gitlabrepo/graphql.go @@ -45,7 +45,7 @@ func (handler *graphqlHandler) init(ctx context.Context, repourl *repoURL) { src := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: os.Getenv("GITLAB_AUTH_TOKEN")}, ) - handler.client = oauth2.NewClient(context.Background(), src) + handler.client = oauth2.NewClient(ctx, src) handler.graphClient = graphql.NewClient(fmt.Sprintf("%s/api/graphql", repourl.Host()), handler.client) } diff --git a/clients/ossfuzz/client.go b/clients/ossfuzz/client.go index 054ea50a..8c19d803 100644 --- a/clients/ossfuzz/client.go +++ b/clients/ossfuzz/client.go @@ -39,6 +39,7 @@ var ( ) type client struct { + ctx context.Context err error projects map[string]bool statusURL string @@ -54,6 +55,7 @@ type ossFuzzStatus struct { // CreateOSSFuzzClient returns a client which implements RepoClient interface. func CreateOSSFuzzClient(ossFuzzStatusURL string) clients.RepoClient { return &client{ + ctx: context.Background(), statusURL: ossFuzzStatusURL, projects: map[string]bool{}, } @@ -62,6 +64,7 @@ func CreateOSSFuzzClient(ossFuzzStatusURL string) clients.RepoClient { // CreateOSSFuzzClientEager returns a OSS Fuzz Client which has already fetched and parsed the status file. func CreateOSSFuzzClientEager(ossFuzzStatusURL string) (clients.RepoClient, error) { c := client{ + ctx: context.Background(), statusURL: ossFuzzStatusURL, projects: map[string]bool{}, } @@ -91,7 +94,7 @@ func (c *client) Search(request clients.SearchRequest) (clients.SearchResponse, } func (c *client) init() { - b, err := fetchStatusFile(c.statusURL) + b, err := fetchStatusFile(c.ctx, c.statusURL) if err != nil { c.err = err return @@ -118,9 +121,12 @@ func parseStatusFile(contents []byte, m map[string]bool) error { return nil } -func fetchStatusFile(uri string) ([]byte, error) { - //nolint:gosec // URI comes from a constant or a test HTTP server, not user input - resp, err := http.Get(uri) +func fetchStatusFile(ctx context.Context, uri string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, uri, nil) + if err != nil { + return nil, fmt.Errorf("making status file request: %w", err) + } + resp, err := http.DefaultClient.Do(req) if err != nil { return nil, fmt.Errorf("http.Get: %w", err) } diff --git a/cmd/internal/packagemanager/client_test.go b/cmd/internal/packagemanager/client_test.go index 97d9c915..04ffd40e 100644 --- a/cmd/internal/packagemanager/client_test.go +++ b/cmd/internal/packagemanager/client_test.go @@ -63,6 +63,7 @@ func Test_GetURI_calls_client_get_with_input(t *testing.T) { t.Errorf("Test_GetURI_calls_client_get_with_input() error in Get= %v", err) return } + defer got.Body.Close() body, err := io.ReadAll(got.Body) if err != nil { t.Errorf("Test_GetURI_calls_client_get_with_input() error in ReadAll= %v", err) @@ -118,6 +119,7 @@ func Test_Get_calls_client_get_with_input(t *testing.T) { t.Errorf("Test_Get_calls_client_get_with_input() error in Get = %v", err) return } + defer got.Body.Close() body, err := io.ReadAll(got.Body) if err != nil { t.Errorf("Test_Get_calls_client_get_with_input() error in ReadAll = %v", err) diff --git a/cron/internal/format/json.go b/cron/internal/format/json.go index b07f2439..88c78cf4 100644 --- a/cron/internal/format/json.go +++ b/cron/internal/format/json.go @@ -26,7 +26,6 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) -//nolint type jsonCheckResult struct { Name string Details []string @@ -34,6 +33,7 @@ type jsonCheckResult struct { Pass bool } +//nolint:musttag type jsonScorecardResult struct { Repo string Date string @@ -47,7 +47,7 @@ type jsonCheckDocumentationV2 struct { // Can be extended if needed. } -//nolint +//nolint:govet type jsonCheckResultV2 struct { Details []string `json:"details"` Score int `json:"score"` diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index d0a7bd2e..88038983 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -55,6 +55,8 @@ type ScorecardResultWithError struct { } // DependencyCheckResult is the dependency structure used in the returned results. +// +//nolint:musttag // functionality is deprecated anyway type DependencyCheckResult struct { // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType *ChangeType diff --git a/pkg/json.go b/pkg/json.go index a1fab73f..fd81223c 100644 --- a/pkg/json.go +++ b/pkg/json.go @@ -27,7 +27,7 @@ import ( "github.com/ossf/scorecard/v4/log" ) -// nolint: govet +//nolint:govet type jsonCheckResult struct { Name string Details []string @@ -35,6 +35,7 @@ type jsonCheckResult struct { Pass bool } +//nolint:musttag type jsonScorecardResult struct { Repo string Date string