From c9e25f7528737d7b0217fe48b8cbd86c84b31d5d Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 18 Oct 2023 16:29:00 -0700 Subject: [PATCH 1/4] enable forbidigo for print statements. include reasoning as message exposed to developer. Signed-off-by: Spencer Schrock --- .golangci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 3f7d7108bd1..ade8ef2d914 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -29,6 +29,7 @@ linters: - errorlint - exhaustive - exportloopref + - forbidigo - gci - gochecknoinits - gocognit @@ -73,6 +74,9 @@ linters-settings: exhaustive: # https://golangci-lint.run/usage/linters/#exhaustive default-signifies-exhaustive: true + forbidigo: + forbid: + - 'fmt\.Print.*(# Do not commit print statements\. Output to stdout interferes with users who redirect JSON results to files\.)?' govet: enable: - fieldalignment From 463d617eed6738d5e587ba2977e9bc33e4ab5090 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Wed, 18 Oct 2023 17:48:17 -0700 Subject: [PATCH 2/4] remove or grant exceptions for existing print statements Signed-off-by: Spencer Schrock --- 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 ++++--- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/attestor/command/cli.go b/attestor/command/cli.go index 0f8e89239c8..55632e0ebf0 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.Stdout, err) // can this just be stderr? 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") From fe8266cc5aecf0e8648dc3edef032603d5bf4103 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 23 Oct 2023 10:22:12 -0700 Subject: [PATCH 3/4] swap stdout to stderr Signed-off-by: Spencer Schrock --- attestor/command/cli.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/attestor/command/cli.go b/attestor/command/cli.go index 55632e0ebf0..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.Fprintln(os.Stdout, err) // can this just be stderr? + fmt.Fprintln(os.Stderr, err) os.Exit(1) } } From 88a9817ca093ae9a6a302485373f38be7773d743 Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Mon, 23 Oct 2023 11:07:35 -0700 Subject: [PATCH 4/4] separate msg from regex for better readability. Signed-off-by: Spencer Schrock --- .golangci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 5356758bd6d..f046e3f76b6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -78,7 +78,8 @@ linters-settings: default-signifies-exhaustive: true forbidigo: forbid: - - 'fmt\.Print.*(# Do not commit print statements\. Output to stdout interferes with users who redirect JSON results to files\.)?' + - p: "^fmt\\.Print.*$" + msg: "Do not commit print statements. Output to stdout interferes with users who redirect JSON results to files." govet: enable: - fieldalignment