From 2d9319601e32e1e9d3abcfe463dfdc2297296a12 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 23 Oct 2023 13:12:50 -0700 Subject: [PATCH] :seedling: 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 * remove or grant exceptions for existing print statements Signed-off-by: Spencer Schrock * swap stdout to stderr Signed-off-by: Spencer Schrock * separate msg from regex for better readability. Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --- .golangci.yml | 5 +++++ attestor/command/cli.go | 2 +- attestor/policy/attestation_policy_test.go | 5 ++--- clients/gitlabrepo/repo_test.go | 7 +++---- cmd/root.go | 2 +- cmd/serve.go | 2 +- cron/internal/cii/main.go | 7 ++++--- cron/internal/webhook/main.go | 2 +- options/options.go | 7 ++++--- 9 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8f02cee9c03..f046e3f76b6 100644 --- a/.golangci.yml +++ b/.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 diff --git a/attestor/command/cli.go b/attestor/command/cli.go index 0f8e89239c8..27e8b829689 100644 --- a/attestor/command/cli.go +++ b/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) } } diff --git a/attestor/policy/attestation_policy_test.go b/attestor/policy/attestation_policy_test.go index 61e222b906d..0c1f991e875 100644 --- a/attestor/policy/attestation_policy_test.go +++ b/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) } }) diff --git a/clients/gitlabrepo/repo_test.go b/clients/gitlabrepo/repo_test.go index 87b36894355..a46250de82a 100644 --- a/clients/gitlabrepo/repo_test.go +++ b/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) { diff --git a/cmd/root.go b/cmd/root.go index 4dccf166d64..506cdb819ed 100644 --- a/cmd/root.go +++ b/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( diff --git a/cmd/serve.go b/cmd/serve.go index 65f4ea50bca..31825104527 100644 --- a/cmd/serve.go +++ b/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 { diff --git a/cron/internal/cii/main.go b/cron/internal/cii/main.go index fa8616606da..dce89c8a571 100644 --- a/cron/internal/cii/main.go +++ b/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") } diff --git a/cron/internal/webhook/main.go b/cron/internal/webhook/main.go index 2324cd908d4..9724f79a0fa 100644 --- a/cron/internal/webhook/main.go +++ b/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) diff --git a/options/options.go b/options/options.go index f4d528b44f8..30acfe0efe3 100644 --- a/options/options.go +++ b/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")