lint: forbidigo (no print or printf in production, prefer fprint) (#4141)

Author: mmetc

.golangci.yml

@@ -58,7 +58,6 @@ linters:
     - paralleltest          # Detects missing usage of t.Parallel() method in your Go test
     - testpackage           # linter that makes you use a separate _test package
     - exhaustruct           # Checks if all structure fields are initialized
-    - forbidigo             # Forbids identifiers
     - gochecknoglobals      # Check that no global variables exist.
     - goconst               # Finds repeated strings that could be replaced by a constant
     - tagliatelle           # Checks the struct tags.
@@ -200,6 +199,14 @@ linters:
       # Default: false
       default-case-required: true
 
+    forbidigo:
+      forbid:
+      # prefer fprint to print calls, keep the latter for debugging
+      # so they're not comitted by mistake.
+      - pattern: ^print(ln)?$
+      - pattern: ^fmt\.Print.*$
+        msg: use the Fprint equivalent in production code
+
     gocritic:
       enable-all: true
       disabled-checks:
@@ -553,6 +560,12 @@ linters:
           - maintidx
         path: (.+)_test.go
 
+      # tolerate print in tests for now
+      - linters:
+          - forbidigo
+        path: (.+)_test.go
+        text: 'use the Fprint equivalent in production code'
+
       # tolerate long functions in tests
       - linters:
           - revive

cmd/crowdsec-cli/cliallowlists/allowlists.go

@@ -505,7 +505,7 @@ func (*cliAllowLists) add(ctx context.Context, db *database.Client, name string,
 		return fmt.Errorf("unable to apply allowlists to existing decisions: %w", err)
 	}
 	if deleted > 0 {
-		fmt.Printf("%d decisions deleted by allowlists\n", deleted)
+		fmt.Fprintf(os.Stdout, "%d decisions deleted by allowlists\n", deleted)
 	}
 
 	return nil

cmd/crowdsec-cli/clibouncer/add.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"os"
 
 	"github.com/spf13/cobra"
 
@@ -32,18 +33,18 @@ func (cli *cliBouncers) add(ctx context.Context, bouncerName string, key string)
 
 	switch cli.cfg().Cscli.Output {
 	case "human":
-		fmt.Printf("API key for '%s':\n\n", bouncerName)
-		fmt.Printf("   %s\n\n", key)
-		fmt.Print("Please keep this key since you will not be able to retrieve it!\n")
+		fmt.Fprintf(os.Stdout, "API key for '%s':\n\n", bouncerName)
+		fmt.Fprintf(os.Stdout, "   %s\n\n", key)
+		fmt.Fprintln(os.Stdout, "Please keep this key since you will not be able to retrieve it!")
 	case "raw":
-		fmt.Print(key)
+		fmt.Fprint(os.Stdout, key)
 	case "json":
 		j, err := json.Marshal(key)
 		if err != nil {
 			return errors.New("unable to serialize api key")
 		}
 
-		fmt.Print(string(j))
+		fmt.Fprint(os.Stdout, string(j))
 	}
 
 	return nil

cmd/crowdsec-cli/clibouncer/prune.go

@@ -24,7 +24,7 @@ func (cli *cliBouncers) prune(ctx context.Context, duration time.Duration, force
 				"This may remove active bouncers. Continue?", false); err != nil {
 			return err
 		} else if !yes {
-			fmt.Println("User aborted prune. No changes were made.")
+			fmt.Fprintln(os.Stdout, "User aborted prune. No changes were made.")
 			return nil
 		}
 	}
@@ -35,7 +35,7 @@ func (cli *cliBouncers) prune(ctx context.Context, duration time.Duration, force
 	}
 
 	if len(bouncers) == 0 {
-		fmt.Println("No bouncers to prune.")
+		fmt.Fprintln(os.Stdout, "No bouncers to prune.")
 		return nil
 	}
 
@@ -47,7 +47,7 @@ func (cli *cliBouncers) prune(ctx context.Context, duration time.Duration, force
 				"These will NOT be recoverable. Continue?", false); err != nil {
 			return err
 		} else if !yes {
-			fmt.Println("User aborted prune. No changes were made.")
+			fmt.Fprintln(os.Stdout, "User aborted prune. No changes were made.")
 			return nil
 		}
 	}

cmd/crowdsec-cli/cliconfig/feature_flags.go

@@ -2,6 +2,7 @@ package cliconfig
 
 import (
 	"fmt"
+	"os"
 	"path/filepath"
 
 	"github.com/fatih/color"
@@ -29,17 +30,17 @@ func (cli *cliConfig) featureFlags(showRetired bool) error {
 			status = green("✓")
 		}
 
-		fmt.Printf("%s %s", status, nameDesc)
+		fmt.Fprintf(os.Stdout, "%s %s", status, nameDesc)
 
 		if feat.State == fflag.DeprecatedState {
-			fmt.Printf("\n  %s %s", yellow("DEPRECATED"), feat.DeprecationMsg)
+			fmt.Fprintf(os.Stdout, "\n  %s %s", yellow("DEPRECATED"), feat.DeprecationMsg)
 		}
 
 		if feat.State == fflag.RetiredState {
-			fmt.Printf("\n  %s %s", magenta("RETIRED"), feat.DeprecationMsg)
+			fmt.Fprintf(os.Stdout, "\n  %s %s", magenta("RETIRED"), feat.DeprecationMsg)
 		}
 
-		fmt.Println()
+		fmt.Fprintln(os.Stdout)
 	}
 
 	feats := fflag.Crowdsec.GetAllFeatures()
@@ -63,53 +64,53 @@ func (cli *cliConfig) featureFlags(showRetired bool) error {
 	}
 
 	if len(enabled) > 0 {
-		fmt.Println(" --- Enabled features ---")
-		fmt.Println()
+		fmt.Fprintln(os.Stdout, " --- Enabled features ---")
+		fmt.Fprintln(os.Stdout)
 
 		for _, feat := range enabled {
 			printFeature(feat)
 		}
 
-		fmt.Println()
+		fmt.Fprintln(os.Stdout)
 	}
 
 	if len(disabled) > 0 {
-		fmt.Println(" --- Disabled features ---")
-		fmt.Println()
+		fmt.Fprintln(os.Stdout, " --- Disabled features ---")
+		fmt.Fprintln(os.Stdout)
 
 		for _, feat := range disabled {
 			printFeature(feat)
 		}
 
-		fmt.Println()
+		fmt.Fprintln(os.Stdout)
 	}
 
-	fmt.Println("To enable a feature you can: ")
-	fmt.Println("  - set the environment variable CROWDSEC_FEATURE_<uppercase_feature_name> to true")
+	fmt.Fprintln(os.Stdout, "To enable a feature you can: ")
+	fmt.Fprintln(os.Stdout, "  - set the environment variable CROWDSEC_FEATURE_<uppercase_feature_name> to true")
 
 	featurePath, err := filepath.Abs(csconfig.GetFeatureFilePath(cli.cfg().FilePath))
 	if err != nil {
 		// we already read the file, shouldn't happen
 		return err
 	}
 
-	fmt.Printf("  - add the line '- <feature_name>' to the file %s\n", featurePath)
-	fmt.Println()
+	fmt.Fprintf(os.Stdout, "  - add the line '- <feature_name>' to the file %s\n", featurePath)
+	fmt.Fprintln(os.Stdout)
 
 	if len(enabled) == 0 && len(disabled) == 0 {
-		fmt.Println("However, no feature flag is available in this release.")
-		fmt.Println()
+		fmt.Fprintln(os.Stdout, "However, no feature flag is available in this release.")
+		fmt.Fprintln(os.Stdout)
 	}
 
 	if showRetired && len(retired) > 0 {
-		fmt.Println(" --- Retired features ---")
-		fmt.Println()
+		fmt.Fprintln(os.Stdout, " --- Retired features ---")
+		fmt.Fprintln(os.Stdout)
 
 		for _, feat := range retired {
 			printFeature(feat)
 		}
 
-		fmt.Println()
+		fmt.Fprintln(os.Stdout)
 	}
 
 	return nil

cmd/crowdsec-cli/cliconfig/showyaml.go

@@ -2,6 +2,7 @@ package cliconfig
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/spf13/cobra"
 
@@ -15,7 +16,7 @@ func (*cliConfig) newShowYAMLCmd(mergedConfigGetter mergedConfigGetter) *cobra.C
 		Args:              args.NoArgs,
 		DisableAutoGenTag: true,
 		RunE: func(_ *cobra.Command, _ []string) error {
-			fmt.Println(mergedConfigGetter())
+			fmt.Fprintln(os.Stdout, mergedConfigGetter())
 			return nil
 		},
 	}

cmd/crowdsec-cli/clihub/items.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"os"
 	"slices"
 	"strings"
 
@@ -69,7 +70,7 @@ func ListItems(out io.Writer, wantColor string, itemTypes []string, items map[st
 		}
 
 		if nothingToDisplay {
-			fmt.Println("No items to display")
+			fmt.Fprintln(os.Stdout, "No items to display")
 		}
 	case "json":
 		type itemHubStatus struct {

cmd/crowdsec-cli/climachine/add.go

@@ -116,7 +116,7 @@ func (cli *cliMachines) add(ctx context.Context, args []string, machinePassword
 
 		fmt.Fprintf(os.Stderr, "API credentials written to '%s'.\n", dumpFile)
 	} else {
-		fmt.Print(string(apiConfigDump))
+		fmt.Fprint(os.Stderr, string(apiConfigDump))
 	}
 
 	return nil

cmd/crowdsec-cli/climachine/inspect.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"sort"
 
 	"github.com/fatih/color"
@@ -45,7 +46,7 @@ func (cli *cliMachines) inspectHubHuman(out io.Writer, machine *ent.Machine) {
 	state := machine.Hubstate
 
 	if len(state) == 0 {
-		fmt.Println("No hub items found for this machine")
+		fmt.Fprintln(os.Stdout, "No hub items found for this machine")
 		return
 	}
 

cmd/crowdsec-cli/climachine/prune.go

@@ -25,7 +25,7 @@ func (cli *cliMachines) prune(ctx context.Context, duration time.Duration, notVa
 				"This can break installations if the machines are only temporarily disconnected. Continue?", false); err != nil {
 			return err
 		} else if !yes {
-			fmt.Println("User aborted prune. No changes were made.")
+			fmt.Fprintln(os.Stdout, "User aborted prune. No changes were made.")
 			return nil
 		}
 	}
@@ -42,7 +42,7 @@ func (cli *cliMachines) prune(ctx context.Context, duration time.Duration, notVa
 	}
 
 	if len(machines) == 0 {
-		fmt.Println("No machines to prune.")
+		fmt.Fprintln(os.Stdout, "No machines to prune.")
 		return nil
 	}
 
@@ -54,7 +54,7 @@ func (cli *cliMachines) prune(ctx context.Context, duration time.Duration, notVa
 				"These will NOT be recoverable. Continue?", false); err != nil {
 			return err
 		} else if !yes {
-			fmt.Println("User aborted prune. No changes were made.")
+			fmt.Fprintln(os.Stdout, "User aborted prune. No changes were made.")
 			return nil
 		}
 	}