From 9a957b3ab09a5f6d5b9cb1fae5d81801d505d8f3 Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Mon, 9 Oct 2023 10:30:17 +0200 Subject: [PATCH] Add a linter to disable fmt.Printf in code Printf is often used while developping to inspect or debug code. It happens that these statements are forgotten and commited. We can prevent that by using a linter to disable them. We have some cases where fmt.Printf was used to print useful stuff, like in the CLI. They have been rewritten in fmt.Fprintf to avoid been flagged by the linter. --- .golangci.yaml | 6 ++++++ client/instances.go | 2 +- cmd/apps.go | 14 +++++++------- cmd/check.go | 6 +++--- cmd/config.go | 9 +++++---- cmd/feature.go | 19 ++++++++++--------- cmd/files.go | 24 ++++++++++++------------ cmd/fix.go | 8 ++++---- cmd/instances.go | 30 +++++++++++++++--------------- cmd/jobs.go | 9 +++++---- cmd/root.go | 2 +- cmd/settings.go | 3 ++- cmd/tools.go | 4 ++-- pkg/statik/statik.go | 4 ++-- tests/swift/swifttest.go | 3 ++- web/server.go | 2 +- 16 files changed, 78 insertions(+), 67 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index d46fe20739b..c9e68de55a3 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -47,6 +47,7 @@ linters: - bidichk - exportloopref - errname + - forbidigo - gocritic - gofmt - govet @@ -60,6 +61,11 @@ linters: # Default: false fast: false linters-settings: + forbidigo: + # Forbid fmt.Printf, as they are often used for debugging and they should + # not be commited. The good cases can be written fmt.Fprintf(os.Stdout, ). + forbid: + - fmt.Printf gocritic: disabled-checks: - appendAssign diff --git a/client/instances.go b/client/instances.go index bec78d8a6af..05afdfaa58e 100644 --- a/client/instances.go +++ b/client/instances.go @@ -548,7 +548,7 @@ func (ac *AdminClient) Export(opts *ExportOptions) error { filename = params["filename"] } - fmt.Printf("Exporting archive %d/%d (%s)... ", i+1, partsCount, filename) + fmt.Fprintf(os.Stdout, "Exporting archive %d/%d (%s)... ", i+1, partsCount, filename) filepath := path.Join(opts.LocalPath, filename) f, err := os.OpenFile(filepath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) diff --git a/cmd/apps.go b/cmd/apps.go index b8159d97c20..c2f107625c9 100644 --- a/cmd/apps.go +++ b/cmd/apps.go @@ -310,7 +310,7 @@ var activateMaintenanceKonnectorsCmd = &cobra.Command{ break } if len(locale) > 5 { - fmt.Printf("Invalid locale name: %q\n", locale) + fmt.Fprintf(os.Stdout, "Invalid locale name: %q\n", locale) continue } shortMessage := prompt("Short message:") @@ -366,7 +366,7 @@ func installApp(cmd *cobra.Command, args []string, appType string) error { } return err } - fmt.Printf("Application installed successfully on %s\n", in.Attrs.Domain) + fmt.Fprintf(os.Stdout, "Application installed successfully on %s\n", in.Attrs.Domain) return nil }) } @@ -393,7 +393,7 @@ func installApp(cmd *cobra.Command, args []string, appType string) error { if err != nil { return err } - fmt.Printf("%s (%s) has been installed on %s\n", slug, manifest.Attrs.Version, flagDomain) + fmt.Fprintf(os.Stdout, "%s (%s) has been installed on %s\n", slug, manifest.Attrs.Version, flagDomain) return nil } @@ -420,7 +420,7 @@ func updateApp(cmd *cobra.Command, args []string, appType string) error { } return err } - fmt.Printf("Application updated successfully on %s\n", in.Attrs.Domain) + fmt.Fprintf(os.Stdout, "Application updated successfully on %s\n", in.Attrs.Domain) return nil }) } @@ -457,7 +457,7 @@ func updateApp(cmd *cobra.Command, args []string, appType string) error { } else if app.IsMoreRecent(newManifest.Attrs.Version, manifest.Attrs.Version) { msg = "%s has been downgraded to %s\n" } - fmt.Printf(msg, args[0], newManifest.Attrs.Version) + fmt.Fprintf(os.Stdout, msg, args[0], newManifest.Attrs.Version) return nil } @@ -478,7 +478,7 @@ func uninstallApp(cmd *cobra.Command, args []string, appType string) error { if err != nil { return err } - fmt.Printf("%s has been uninstalled\n", manifest.Attrs.Slug) + fmt.Fprintf(os.Stdout, "%s has been uninstalled\n", manifest.Attrs.Slug) return nil } @@ -525,7 +525,7 @@ func showWebAppTriggers(cmd *cobra.Command, args []string, appType string) error var triggerIDs []string if manifest.Attrs.Services == nil { - fmt.Printf("No triggers\n") + fmt.Fprintf(os.Stdout, "No triggers\n") return nil } for _, service := range *manifest.Attrs.Services { diff --git a/cmd/check.go b/cmd/check.go index 5f488a2c2e0..96b4190760b 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -127,7 +127,7 @@ triggers of the same type, for the same worker, and with the same arguments. if len(result) > 0 { for _, r := range result { j, _ := json.Marshal(r) - fmt.Printf("%s\n", j) + fmt.Fprintf(os.Stdout, "%s\n", j) } os.Exit(1) } @@ -167,7 +167,7 @@ generation smaller than their generation. if len(result) > 0 { for _, r := range result { j, _ := json.Marshal(r) - fmt.Printf("%s\n", j) + fmt.Fprintf(os.Stdout, "%s\n", j) } os.Exit(1) } @@ -220,7 +220,7 @@ check via the flags. if len(result) > 0 { for _, r := range result { j, _ := json.Marshal(r) - fmt.Printf("%s\n", j) + fmt.Fprintf(os.Stdout, "%s\n", j) } os.Exit(1) } diff --git a/cmd/config.go b/cmd/config.go index 9c089d9ed70..f748181e53a 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -170,7 +170,7 @@ $ bmFjbNFjY+XZkS26YtVPUIKKm/JdnAGwG30n6A4ypS1p1dHev8hOtaRbW+lGneoO7PS9JCW8U5GSXh return err } data := base64.StdEncoding.EncodeToString(dataEncrypted) - fmt.Printf("%s\n", data) + fmt.Fprintf(os.Stdout, "%s\n", data) return nil }, @@ -198,7 +198,7 @@ var decryptCredentialsDataCmd = &cobra.Command{ return err } - fmt.Printf("%s\n", decrypted) + fmt.Fprintf(os.Stdout, "%s\n", decrypted) return nil }, @@ -222,7 +222,7 @@ var encryptCredentialsCmd = &cobra.Command{ if err != nil { return err } - fmt.Printf("Encrypted credentials: %s\n", encryptedCreds) + fmt.Fprintf(os.Stdout, "Encrypted credentials: %s\n", encryptedCreds) return nil }, } @@ -251,10 +251,11 @@ var decryptCredentialsCmd = &cobra.Command{ return fmt.Errorf("Could not decrypt cipher text: %s", err) } - fmt.Printf(`Decrypted credentials: + fmt.Fprintf(os.Stdout, `Decrypted credentials: login: %q password: %q `, login, password) + return nil }, } diff --git a/cmd/feature.go b/cmd/feature.go index e5300efd212..7d66cd1dbe8 100644 --- a/cmd/feature.go +++ b/cmd/feature.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/url" + "os" "strings" "github.com/cozy/cozy-stack/client/request" @@ -58,14 +59,14 @@ cozy-stack feature show displays the feature flags that are shown by apps. return err } for k, v := range obj.Data.Attributes { - fmt.Printf("- %s: %s\n", k, string(v)) + fmt.Fprintf(os.Stdout, "- %s: %s\n", k, string(v)) } if len(obj.Included) > 0 { - fmt.Printf("\nSources:\n") + fmt.Fprintf(os.Stdout, "\nSources:\n") for _, source := range obj.Included { - fmt.Printf("- %s\n", source.ID) + fmt.Fprintf(os.Stdout, "- %s\n", source.ID) for k, v := range source.Attributes { - fmt.Printf("\t- %s: %s\n", k, string(v)) + fmt.Fprintf(os.Stdout, "\t- %s: %s\n", k, string(v)) } } } @@ -109,7 +110,7 @@ If you give a null value, the flag will be removed. return err } for k, v := range obj { - fmt.Printf("- %s: %s\n", k, string(v)) + fmt.Fprintf(os.Stdout, "- %s: %s\n", k, string(v)) } return nil }, @@ -159,7 +160,7 @@ All the sets can be removed by setting an empty list (''). return err } for _, set := range sets { - fmt.Printf("- %v\n", set) + fmt.Fprintf(os.Stdout, "- %v\n", set) } return nil }, @@ -200,7 +201,7 @@ To remove a flag, set it to an empty array (or null). return err } for k, v := range obj { - fmt.Printf("- %s: %s\n", k, string(v)) + fmt.Fprintf(os.Stdout, "- %s: %s\n", k, string(v)) } return nil }, @@ -234,7 +235,7 @@ These flags are read only and can only be updated by changing configuration and return err } for k, v := range obj { - fmt.Printf("- %s: %s\n", k, string(v)) + fmt.Fprintf(os.Stdout, "- %s: %s\n", k, string(v)) } return nil }, @@ -271,7 +272,7 @@ If you give a null value, the flag will be removed. return err } for k, v := range obj { - fmt.Printf("- %s: %s\n", k, string(v)) + fmt.Fprintf(os.Stdout, "- %s: %s\n", k, string(v)) } return nil }, diff --git a/cmd/files.go b/cmd/files.go index 68eb5a216b0..f6d234d8b65 100644 --- a/cmd/files.go +++ b/cmd/files.go @@ -121,29 +121,29 @@ var usageFilesCmd = &cobra.Command{ if err != nil { return err } - fmt.Printf("Usage: %v\n", info["used"]) + fmt.Fprintf(os.Stdout, "Usage: %v\n", info["used"]) if files, ok := info["files"]; ok { - fmt.Printf(" Including latest version of files: %v\n", files) + fmt.Fprintf(os.Stdout, " Including latest version of files: %v\n", files) } if versions, ok := info["versions"]; ok { - fmt.Printf(" Including older versions of files: %v\n", versions) + fmt.Fprintf(os.Stdout, " Including older versions of files: %v\n", versions) } if flagIncludeTrash { if trashed, ok := info["trashed"]; ok { - fmt.Printf(" Including trashed files: %v\n", trashed) + fmt.Fprintf(os.Stdout, " Including trashed files: %v\n", trashed) } } if quota, ok := info["quota"]; ok { - fmt.Printf("Quota: %v\n", quota) + fmt.Fprintf(os.Stdout, "Quota: %v\n", quota) } if count, ok := info["doc_count"]; ok { - fmt.Printf("Documents count: %v\n", count) + fmt.Fprintf(os.Stdout, "Documents count: %v\n", count) } if count, ok := info["versions_count"]; ok { - fmt.Printf("Versions Documents count: %v\n", count) + fmt.Fprintf(os.Stdout, "Versions Documents count: %v\n", count) } return nil }, @@ -338,7 +338,7 @@ func treeCmd(c *client.Client, root string, w io.Writer, verbose bool) error { return err } if verbose { - fmt.Printf("%s ", doc.ID) + fmt.Fprintf(os.Stdout, "%s ", doc.ID) } attrs := doc.Attrs @@ -461,11 +461,11 @@ func importFiles(c *client.Client, from, to string, match *regexp.Regexp) error return err } if !fromInfos.IsDir() { - fmt.Printf("Importing file %s to cozy://%s\n", from, to) + fmt.Fprintf(os.Stdout, "Importing file %s to cozy://%s\n", from, to) return i.upload(from, to) } - fmt.Printf("Importing from %s to cozy://%s\n", from, to) + fmt.Fprintf(os.Stdout, "Importing from %s to cozy://%s\n", from, to) return filepath.Walk(from, func(localname string, f os.FileInfo, err error) error { if err != nil { @@ -485,14 +485,14 @@ func importFiles(c *client.Client, from, to string, match *regexp.Regexp) error distname := path.Join(to, strings.TrimPrefix(localname, from)) if f.IsDir() { - fmt.Printf("create dir %s\n", distname) + fmt.Fprintf(os.Stdout, "create dir %s\n", distname) if !flagImportDryRun { if _, err = i.mkdir(distname); err != nil { return err } } } else { - fmt.Printf("copying file %s to %s\n", localname, distname) + fmt.Fprintf(os.Stdout, "copying file %s to %s\n", localname, distname) if !flagImportDryRun { return i.upload(localname, distname) } diff --git a/cmd/fix.go b/cmd/fix.go index 0de5af6d276..92413f25c62 100644 --- a/cmd/fix.go +++ b/cmd/fix.go @@ -55,7 +55,7 @@ var mimeFixerCmd = &cobra.Command{ if class == attrs.Class { return nil } - fmt.Printf("Fix %s: %s -> %s\n", attrs.Name, attrs.Class, class) + fmt.Fprintf(os.Stdout, "Fix %s: %s -> %s\n", attrs.Name, attrs.Class, class) _, err = c.UpdateAttrsByID(doc.ID, &client.FilePatch{ Rev: doc.Rev, Attrs: client.FilePatchAttrs{ @@ -91,7 +91,7 @@ var jobsFixer = &cobra.Command{ return err } - fmt.Printf("Cleaned %d jobs on %s\n", result.Deleted, args[0]) + fmt.Fprintf(os.Stdout, "Cleaned %d jobs on %s\n", result.Deleted, args[0]) return nil }, } @@ -227,11 +227,11 @@ var contactEmailsFixer = &cobra.Command{ address = strings.TrimSpace(address) _, err := mail.ParseAddress(address) if err == nil { - fmt.Printf(" Email fixed: \"%s\" → \"%s\"\n", old, address) + fmt.Fprintf(os.Stdout, " Email fixed: \"%s\" → \"%s\"\n", old, address) changed = true email["address"] = address } else { - fmt.Printf(" Invalid email: \"%s\" → \"%s\"\n", old, address) + fmt.Fprintf(os.Stdout, " Invalid email: \"%s\" → \"%s\"\n", old, address) } } } diff --git a/cmd/instances.go b/cmd/instances.go index 0b1bb21f65b..9ea756ba1c5 100644 --- a/cmd/instances.go +++ b/cmd/instances.go @@ -138,7 +138,7 @@ It will also show the couch_cluster if it is not the default one. fmt.Println(couchdb.EscapeCouchdbName(in.Attrs.Domain)) } if in.Attrs.CouchCluster != 0 { - fmt.Printf("couch_cluster: %d\n", in.Attrs.CouchCluster) + fmt.Fprintf(os.Stdout, "couch_cluster: %d\n", in.Attrs.CouchCluster) } return nil }, @@ -206,14 +206,14 @@ be used as the error message. return err } - fmt.Printf("Instance created with success for domain %s\n", in.Attrs.Domain) + fmt.Fprintf(os.Stdout, "Instance created with success for domain %s\n", in.Attrs.Domain) myProtocol := "https" if build.IsDevRelease() { myProtocol = "http" } if in.Attrs.RegisterToken != nil { - fmt.Printf("Registration token: \"%s\"\n", hex.EncodeToString(in.Attrs.RegisterToken)) - fmt.Printf("Define your password by visiting %s://%s/?registerToken=%s\n", myProtocol, in.Attrs.Domain, hex.EncodeToString(in.Attrs.RegisterToken)) + fmt.Fprintf(os.Stdout, "Registration token: \"%s\"\n", hex.EncodeToString(in.Attrs.RegisterToken)) + fmt.Fprintf(os.Stdout, "Define your password by visiting %s://%s/?registerToken=%s\n", myProtocol, in.Attrs.Domain, hex.EncodeToString(in.Attrs.RegisterToken)) } if len(flagApps) == 0 { return nil @@ -229,7 +229,7 @@ be used as the error message. } } if !found { - fmt.Printf("/!\\ Application %s has not been installed\n", slug) + fmt.Fprintf(os.Stdout, "/!\\ Application %s has not been installed\n", slug) } } } @@ -424,9 +424,9 @@ specific domain. debug = false } if debug { - fmt.Printf("Debug is enabled on %s\n", domain) + fmt.Fprintf(os.Stdout, "Debug is enabled on %s\n", domain) } else { - fmt.Printf("Debug is disabled on %s\n", domain) + fmt.Fprintf(os.Stdout, "Debug is disabled on %s\n", domain) } return err }, @@ -443,9 +443,9 @@ var countInstanceCmd = &cobra.Command{ return err } if count == 1 { - fmt.Printf("%d instance\n", count) + fmt.Fprintf(os.Stdout, "%d instance\n", count) } else { - fmt.Printf("%d instances\n", count) + fmt.Fprintf(os.Stdout, "%d instances\n", count) } return nil }, @@ -635,14 +635,14 @@ and all its data. return err } - fmt.Printf("Instance for domain %s has been destroyed with success\n", domain) + fmt.Fprintf(os.Stdout, "Instance for domain %s has been destroyed with success\n", domain) return nil }, } func confirmDomain(action, domain string) error { reader := bufio.NewReader(os.Stdin) - fmt.Printf(`Are you sure you want to %s instance for domain %s? + fmt.Fprintf(os.Stdout, `Are you sure you want to %s instance for domain %s? All data associated with this domain will be permanently lost. Type again the domain to confirm: `, action, domain) @@ -885,11 +885,11 @@ updated.`, logs := make(chan *client.JobLog) go func() { for log := range logs { - fmt.Printf("[%s][time:%s]", log.Level, log.Time.Format(time.RFC3339)) + fmt.Fprintf(os.Stdout, "[%s][time:%s]", log.Level, log.Time.Format(time.RFC3339)) for k, v := range log.Data { - fmt.Printf("[%s:%s]", k, v) + fmt.Fprintf(os.Stdout, "[%s:%s]", k, v) } - fmt.Printf(" %s\n", log.Message) + fmt.Fprintf(os.Stdout, " %s\n", log.Message) } }() return ac.Updates(&client.UpdatesOptions{ @@ -1061,7 +1061,7 @@ var setAuthModeCmd = &cobra.Command{ return err } if res.StatusCode == http.StatusNoContent { - fmt.Printf("Auth mode has been changed for %s\n", domain) + fmt.Fprintf(os.Stdout, "Auth mode has been changed for %s\n", domain) } else { resBody, err := io.ReadAll(res.Body) if err != nil { diff --git a/cmd/jobs.go b/cmd/jobs.go index 107b50e8471..b16ba70af50 100644 --- a/cmd/jobs.go +++ b/cmd/jobs.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/url" + "os" "strings" "time" @@ -49,14 +50,14 @@ var jobsRunCmd = &cobra.Command{ o.Logs = make(chan *client.JobLog) go func() { for log := range o.Logs { - fmt.Printf("[%s]", log.Level) + fmt.Fprintf(os.Stdout, "[%s]", log.Level) if flagJobPrintLogsVerbose { - fmt.Printf("[time:%s]", log.Time.Format(time.RFC3339)) + fmt.Fprintf(os.Stdout, "[time:%s]", log.Time.Format(time.RFC3339)) for k, v := range log.Data { - fmt.Printf("[%s:%s]", k, v) + fmt.Fprintf(os.Stdout, "[%s:%s]", k, v) } } - fmt.Printf(" %s\n", log.Message) + fmt.Fprintf(os.Stdout, " %s\n", log.Message) } }() } diff --git a/cmd/root.go b/cmd/root.go index 5730b31c0e7..720ab5558fa 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -100,7 +100,7 @@ func newAdminClient() *client.AdminClient { if !build.IsDevRelease() { if len(pass) == 0 { var err error - fmt.Printf("Password:") + fmt.Fprintf(os.Stdout, "Password:") pass, err = gopass.GetPasswdMasked() if err != nil { errFatalf("Could not get password from standard input: %s\n", err) diff --git a/cmd/settings.go b/cmd/settings.go index 42e7d74a4a7..fd7c4b6c06a 100644 --- a/cmd/settings.go +++ b/cmd/settings.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "os" "sort" "strings" @@ -69,7 +70,7 @@ func printSettings(obj map[string]interface{}) { } sort.Strings(keys) for _, k := range keys { - fmt.Printf("- %s: %v\n", k, attrs[k]) + fmt.Fprintf(os.Stdout, "- %s: %v\n", k, attrs[k]) } } diff --git a/cmd/tools.go b/cmd/tools.go index ba6efa8c197..78f2a5282c7 100644 --- a/cmd/tools.go +++ b/cmd/tools.go @@ -81,7 +81,7 @@ owner's instance. if err := json.NewDecoder(res.Body).Decode(&data); err != nil { return err } - fmt.Printf("ID: %q\n", data["id"]) + fmt.Fprintf(os.Stdout, "ID: %q\n", data["id"]) return nil }, } @@ -117,7 +117,7 @@ key) as inputs (both encoded in base64), and print on stdout the encrypted data if err != nil { return err } - fmt.Printf("4.%s", base64.StdEncoding.EncodeToString(encrypted)) + fmt.Fprintf(os.Stdout, "4.%s", base64.StdEncoding.EncodeToString(encrypted)) return nil }, } diff --git a/pkg/statik/statik.go b/pkg/statik/statik.go index 90fa6adb6a1..ce3db90a52a 100644 --- a/pkg/statik/statik.go +++ b/pkg/statik/statik.go @@ -340,12 +340,12 @@ func downloadExternals(filename string, currentAssets []*asset) (newAssets []*as if a, ok := currentAssetsMap[externalAsset.name]; ok && bytes.Equal(a.sha256, externalAsset.sha256) { newAsset = a } else { - fmt.Printf("downloading %q... ", externalAsset.name) + fmt.Fprintf(os.Stdout, "downloading %q... ", externalAsset.name) newAsset, err = downloadExternal(externalAsset) if err != nil { return } - fmt.Printf("ok (%s)\n", humanize.Bytes(uint64(newAsset.size))) + fmt.Fprintf(os.Stdout, "ok (%s)\n", humanize.Bytes(uint64(newAsset.size))) } newAssets = append(newAssets, newAsset) } diff --git a/tests/swift/swifttest.go b/tests/swift/swifttest.go index 028cfe9e345..d79df054b6d 100644 --- a/tests/swift/swifttest.go +++ b/tests/swift/swifttest.go @@ -24,7 +24,8 @@ func main() { if err != nil { panic(err) } - fmt.Printf("cozy-stack serve '--fs-url=swift://%s%s?UserName=swifttest&Password=swifttest&AuthURL=%s'\n", + fmt.Fprintf(os.Stdout, + "cozy-stack serve '--fs-url=swift://%s%s?UserName=swifttest&Password=swifttest&AuthURL=%s'\n", u.Host, u.Path, srv.AuthURL) // Wait for CTRL-C diff --git a/web/server.go b/web/server.go index 4fb15d998f1..7d160355d16 100644 --- a/web/server.go +++ b/web/server.go @@ -203,7 +203,7 @@ func (s *Servers) Start(handler http.Handler, name string, addr string) error { return err } - fmt.Printf("http server %s started on %q\n", name, addr) + fmt.Fprintf(os.Stdout, "http server %s started on %q\n", name, addr) switch host { case "localhost": addrs = append(addrs, net.JoinHostPort("127.0.0.1", port))