🌱 use forbidigo linter to prevent print statements (#3585)

* enable forbidigo for print statements.

include reasoning as message exposed to developer.

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

* remove or grant exceptions for existing print statements

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

* swap stdout to stderr

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

* separate msg from regex for better readability.

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

---------

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

.golangci.yml

@@ -29,6 +29,7 @@ linters:
     - errorlint
     - exhaustive
     - exportloopref
+    - forbidigo
     - gci
     - gochecknoinits
     - gocognit
@@ -75,6 +76,10 @@ linters-settings:
   exhaustive:
     # https://golangci-lint.run/usage/linters/#exhaustive
     default-signifies-exhaustive: true
+  forbidigo:
+    forbid:
+      - p: "^fmt\\.Print.*$"
+        msg: "Do not commit print statements. Output to stdout interferes with users who redirect JSON results to files."
   govet:
     enable:
       - fieldalignment

attestor/command/cli.go

@@ -114,7 +114,7 @@ func init() {
 
 func Execute() {
 	if err := RootCmd.Execute(); err != nil {
-		fmt.Println(err)
+		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
 	}
 }

attestor/policy/attestation_policy_test.go

@@ -17,7 +17,6 @@ package policy
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -533,8 +532,8 @@ func TestAttestationPolicyRead(t *testing.T) {
 			// Compare outputs only if the error is nil.
 			// TODO: compare objects.
 			if p.ToJSON() != tt.result.ToJSON() {
-				fmt.Printf("p.ToJSON(): %v\n", p.ToJSON())
-				fmt.Printf("tt.result.ToJSON(): %v\n", tt.result.ToJSON())
+				t.Logf("p.ToJSON(): %v\n", p.ToJSON())
+				t.Logf("tt.result.ToJSON(): %v\n", tt.result.ToJSON())
 				t.Fatalf("%s: invalid result", tt.name)
 			}
 		})

clients/gitlabrepo/repo_test.go

@@ -15,7 +15,6 @@
 package gitlabrepo
 
 import (
-	"fmt"
 	"os"
 	"testing"
 
@@ -107,9 +106,9 @@ func TestRepoURL_IsValid(t *testing.T) {
 				t.Errorf("repoURL.IsValid() error = %v, wantErr %v", err, tt.wantErr)
 			}
 			if !tt.wantErr && !cmp.Equal(tt.expected, r, cmpopts.IgnoreUnexported(repoURL{})) {
-				fmt.Println("expected: " + tt.expected.host + " GOT: " + r.host)
-				fmt.Println("expected: " + tt.expected.owner + " GOT: " + r.owner)
-				fmt.Println("expected: " + tt.expected.project + " GOT: " + r.project)
+				t.Logf("expected: %s GOT: %s", tt.expected.host, r.host)
+				t.Logf("expected: %s GOT: %s", tt.expected.owner, r.owner)
+				t.Logf("expected: %s GOT: %s", tt.expected.project, r.project)
 				t.Errorf("Got diff: %s", cmp.Diff(tt.expected, r))
 			}
 			if !cmp.Equal(r.Host(), tt.expected.host) {

cmd/root.go

@@ -151,7 +151,7 @@ func rootCmd(o *options.Options) error {
 		for checkName := range enabledChecks {
 			fmt.Fprintf(os.Stderr, "Finished [%s]\n", checkName)
 		}
-		fmt.Println("\nRESULTS\n-------")
+		fmt.Fprintln(os.Stderr, "\nRESULTS\n-------")
 	}
 
 	resultsErr := pkg.FormatResults(

cmd/serve.go

@@ -95,7 +95,7 @@ func serveCmd(o *options.Options) *cobra.Command {
 			if port == "" {
 				port = "8080"
 			}
-			fmt.Printf("Listening on localhost:%s\n", port)
+			logger.Info("Listening on localhost:" + port + "\n")
 			//nolint: gosec // unsused.
 			err = http.ListenAndServe(fmt.Sprintf("0.0.0.0:%s", port), nil)
 			if err != nil {

cron/internal/cii/main.go

@@ -21,6 +21,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"log"
 	"net/http"
 	"strings"
 
@@ -46,7 +47,7 @@ func writeToCIIDataBucket(ctx context.Context, pageResp []ciiPageResp, bucketURL
 		if err != nil {
 			return fmt.Errorf("error during AsJSON: %w", err)
 		}
-		fmt.Printf("Writing result for: %s\n", projectURL)
+		log.Printf("Writing result for: %s\n", projectURL)
 		if err := data.WriteToBlobStore(ctx, bucketURL,
 			fmt.Sprintf("%s/result.json", projectURL), jsonData); err != nil {
 			return fmt.Errorf("error during data.WriteToBlobStore: %w", err)
@@ -82,7 +83,7 @@ func getPage(ctx context.Context, pageNum int) ([]ciiPageResp, error) {
 
 func main() {
 	ctx := context.Background()
-	fmt.Println("Starting...")
+	log.Println("Starting...")
 
 	flag.Parse()
 	if err := config.ReadConfig(); err != nil {
@@ -107,5 +108,5 @@ func main() {
 		panic(err)
 	}
 
-	fmt.Println("Job completed")
+	log.Println("Job completed")
 }

cron/internal/webhook/main.go

@@ -80,7 +80,7 @@ func scriptHandler(w http.ResponseWriter, r *http.Request) {
 
 func main() {
 	http.HandleFunc("/", scriptHandler)
-	fmt.Printf("Starting HTTP server on port 8080 ...\n")
+	log.Printf("Starting HTTP server on port 8080 ...\n")
 	// nolint:gosec // internal server.
 	if err := http.ListenAndServe(":8080", nil); err != nil {
 		log.Fatal(err)

options/options.go

@@ -18,13 +18,14 @@ package options
 import (
 	"errors"
 	"fmt"
+	"log"
 	"os"
 	"strings"
 
 	"github.com/caarlos0/env/v6"
 
 	"github.com/ossf/scorecard/v4/clients"
-	"github.com/ossf/scorecard/v4/log"
+	sclog "github.com/ossf/scorecard/v4/log"
 )
 
 // Options define common options for configuring scorecard.
@@ -54,7 +55,7 @@ type Options struct {
 func New() *Options {
 	opts := &Options{}
 	if err := env.Parse(opts); err != nil {
-		fmt.Printf("could not parse env vars, using default options: %v", err)
+		log.Printf("could not parse env vars, using default options: %v", err)
 	}
 	// Defaulting.
 	// TODO(options): Consider moving this to a separate function/method.
@@ -105,7 +106,7 @@ const (
 
 var (
 	// DefaultLogLevel retrieves the default log level.
-	DefaultLogLevel = log.DefaultLevel.String()
+	DefaultLogLevel = sclog.DefaultLevel.String()
 
 	errCommitIsEmpty                   = errors.New("commit should be non-empty")
 	errFormatNotSupported              = errors.New("unsupported format")