🌱 enable the golangci-lint bugs preset (#3583)

* enable bugs preset

Signed-off-by: Spencer Schrock <sschrock@google.com>

* fix noctx linter

Signed-off-by: Spencer Schrock <sschrock@google.com>

* fix bodyclose linter

Signed-off-by: Spencer Schrock <sschrock@google.com>

* fix contextcheck linter

Signed-off-by: Spencer Schrock <sschrock@google.com>

* 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 <sschrock@google.com>

* 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 <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>

.golangci.yml

@@ -62,6 +62,8 @@ linters:
     - unused
     - whitespace
     - wrapcheck
+  presets:
+    - bugs
 linters-settings:
   errcheck:
     check-type-assertions: true

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

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
 		}
 

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)
 		}

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)
 }
 

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)
 	}

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)

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"`

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

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