[skip-changelog] Removed unnecessary fmt.*printf before tr(...) / Fixed i18n early static initialization problem (#1425)

* Removed unnecessary fmt.*printf

* removed unused variable

* Fixed i18n early static initialization problem

#1425 (comment)

these particular strings are not correctly handled by the i18n package because
they are declared at package level before the call to i18n.Init(), and so are
just returned as-is.

* Fix lint errors and use error.Is instead of direct comparison

* Revert "guard" in Tr function

otherwise the unit-tests will fail:

=== RUN   TestBoardOptions
--- FAIL: TestBoardOptions (0.00s)
panic: i18n.Tr called before i18n.Init() [recovered]
	panic: i18n.Tr called before i18n.Init()

goroutine 9 [running]:
testing.tRunner.func1.2(0xc56e00, 0xeaee20)
	/opt/hostedtoolcache/go/1.16.7/x64/src/testing/testing.go:1143 +0x332
testing.tRunner.func1(0xc0002a2f00)
	/opt/hostedtoolcache/go/1.16.7/x64/src/testing/testing.go:1146 +0x4b6
panic(0xc56e00, 0xeaee20)
	/opt/hostedtoolcache/go/1.16.7/x64/src/runtime/panic.go:965 +0x1b9
github.com/arduino/arduino-cli/i18n.Tr(0xdceacd, 0x28, 0x0, 0x0, 0x0, 0x0, 0x1)
	/home/runner/work/arduino-cli/arduino-cli/i18n/i18n.go:54 +0xd9
github.com/arduino/arduino-cli/arduino/cores.(*Board).GetBuildProperties(0x13bf060, 0xc0003b6150, 0xc000339400, 0x0, 0x0)
	/home/runner/work/arduino-cli/arduino-cli/arduino/cores/board.go:109 +0x69d
github.com/arduino/arduino-cli/arduino/cores.(*Board).GeneratePropertiesForConfiguration(0x13bf060, 0xdb5f54, 0xe, 0xc000309ef0, 0xc911e0, 0xc0003b6000)
	/home/runner/work/arduino-cli/arduino-cli/arduino/cores/board.go:141 +0x28f
github.com/arduino/arduino-cli/arduino/cores.TestBoardOptions(0xc0002a2f00)
	/home/runner/work/arduino-cli/arduino-cli/arduino/cores/board_test.go:298 +0x4bd
testing.tRunner(0xc0002a2f00, 0xe052a8)
	/opt/hostedtoolcache/go/1.16.7/x64/src/testing/testing.go:1193 +0xef
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.16.7/x64/src/testing/testing.go:1238 +0x2b3
FAIL	github.com/arduino/arduino-cli/arduino/cores	0.021s
=== RUN   TestIndexParsing
--- FAIL: TestIndexParsing (0.00s)
panic: i18n.Tr called before i18n.Init() [recovered]
	panic: i18n.Tr called before i18n.Init()

* Apply suggestions from code review

Co-authored-by: per1234 <[email protected]>

Co-authored-by: per1234 <[email protected]>

Author: cmaglie

arduino/builder/compilation_database.go

@@ -60,10 +60,10 @@ func LoadCompilationDatabase(file *paths.Path) (*CompilationDatabase, error) {
 // see https://clang.llvm.org/docs/JSONCompilationDatabase.html
 func (db *CompilationDatabase) SaveToFile() {
 	if jsonContents, err := json.MarshalIndent(db.Contents, "", " "); err != nil {
-		fmt.Printf(tr("Error serializing compilation database: %s"), err)
+		fmt.Println(tr("Error serializing compilation database: %s", err))
 		return
 	} else if err := db.File.WriteFile(jsonContents); err != nil {
-		fmt.Printf(tr("Error writing compilation database: %s"), err)
+		fmt.Println(tr("Error writing compilation database: %s", err))
 	}
 }
 
@@ -75,7 +75,7 @@ func dirForCommand(command *exec.Cmd) string {
 	}
 	dir, err := os.Getwd()
 	if err != nil {
-		fmt.Printf(tr("Error getting current directory for compilation database: %s"), err)
+		fmt.Println(tr("Error getting current directory for compilation database: %s", err))
 		return ""
 	}
 	return dir

arduino/discovery/discovery.go

@@ -71,16 +71,16 @@ type discoveryMessage struct {
 func (msg discoveryMessage) String() string {
 	s := fmt.Sprintf("type: %s", msg.EventType)
 	if msg.Message != "" {
-		s = fmt.Sprintf(tr("%[1]s, message: %[2]s"), s, msg.Message)
+		s = tr("%[1]s, message: %[2]s", s, msg.Message)
 	}
 	if msg.ProtocolVersion != 0 {
-		s = fmt.Sprintf(tr("%[1]s, protocol version: %[2]d"), s, msg.ProtocolVersion)
+		s = tr("%[1]s, protocol version: %[2]d", s, msg.ProtocolVersion)
 	}
 	if len(msg.Ports) > 0 {
-		s = fmt.Sprintf(tr("%[1]s, ports: %[2]s"), s, msg.Ports)
+		s = tr("%[1]s, ports: %[2]s", s, msg.Ports)
 	}
 	if msg.Port != nil {
-		s = fmt.Sprintf(tr("%[1]s, port: %[2]s"), s, msg.Port)
+		s = tr("%[1]s, port: %[2]s", s, msg.Port)
 	}
 	return s
 }

arduino/libraries/librariesmanager/install.go

@@ -17,7 +17,6 @@ package librariesmanager
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -32,10 +31,16 @@ import (
 	"gopkg.in/src-d/go-git.v4"
 )
 
+type alreadyInstalledError struct{}
+
+func (e *alreadyInstalledError) Error() string {
+	return tr("library already installed")
+}
+
 var (
 	// ErrAlreadyInstalled is returned when a library is already installed and task
 	// cannot proceed.
-	ErrAlreadyInstalled = errors.New(tr("library already installed"))
+	ErrAlreadyInstalled = &alreadyInstalledError{}
 )
 
 // InstallPrerequisiteCheck performs prequisite checks to install a library. It returns the

arduino/libraries/librariesmanager/librariesmanager.go

@@ -62,7 +62,7 @@ var tr = i18n.Tr
 // Add adds a library to the alternatives
 func (alts *LibraryAlternatives) Add(library *libraries.Library) {
 	if len(alts.Alternatives) > 0 && alts.Alternatives[0].Name != library.Name {
-		panic(fmt.Sprintf(tr("the library name is different from the set (%[1]s != %[2]s)"), alts.Alternatives[0].Name, library.Name))
+		panic(fmt.Sprintf("the library name is different from the set (%[1]s != %[2]s)", alts.Alternatives[0].Name, library.Name))
 	}
 	alts.Alternatives = append(alts.Alternatives, library)
 }

arduino/serialutils/serialutils.go

@@ -130,8 +130,7 @@ func Reset(portToTouch string, wait bool, cb *ResetProgressCallbacks, dryRun boo
 			// do nothing!
 		} else {
 			if err := TouchSerialPortAt1200bps(portToTouch); err != nil {
-				fmt.Printf(tr("TOUCH: error during reset: %s"), err)
-				fmt.Println()
+				fmt.Println(tr("TOUCH: error during reset: %s", err))
 			}
 		}
 	}

arduino/sketch/sketch.go

@@ -260,7 +260,7 @@ type InvalidSketchFolderNameError struct {
 }
 
 func (e *InvalidSketchFolderNameError) Error() string {
-	return fmt.Sprintf(tr("no valid sketch found in %[1]s: missing %[2]s"), e.SketchFolder, e.SketchFile)
+	return tr("no valid sketch found in %[1]s: missing %[2]s", e.SketchFolder, e.SketchFile)
 }
 
 // CheckForPdeFiles returns all files ending with .pde extension

cli/board/attach.go

@@ -42,7 +42,7 @@ func initAttachCommand() *cobra.Command {
 		Run:  runAttachCommand,
 	}
 	attachCommand.Flags().StringVar(&attachFlags.searchTimeout, "timeout", "5s",
-		fmt.Sprintf(tr("The connected devices search timeout, raise it if your board doesn't show up (e.g. to %s)."), "10s"))
+		tr("The connected devices search timeout, raise it if your board doesn't show up (e.g. to %s).", "10s"))
 	return attachCommand
 }
 

cli/cli.go

@@ -75,7 +75,7 @@ func NewCommand() *cobra.Command {
 		PersistentPostRun: postRun,
 	}
 
-	arduinoCli.SetUsageTemplate(usageTemplate)
+	arduinoCli.SetUsageTemplate(getUsageTemplate())
 
 	createCliCommandTree(arduinoCli)
 
@@ -103,10 +103,10 @@ func createCliCommandTree(cmd *cobra.Command) {
 	cmd.AddCommand(version.NewCommand())
 
 	cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, tr("Print the logs on the standard output."))
-	cmd.PersistentFlags().String("log-level", "", fmt.Sprintf(tr("Messages with this level and above will be logged. Valid levels are: %s, %s, %s, %s, %s, %s, %s"), "trace", "debug", "info", "warn", "error", "fatal", "panic"))
+	cmd.PersistentFlags().String("log-level", "", tr("Messages with this level and above will be logged. Valid levels are: %s", "trace, debug, info, warn, error, fatal, panic"))
 	cmd.PersistentFlags().String("log-file", "", tr("Path to the file where logs will be written."))
-	cmd.PersistentFlags().String("log-format", "", fmt.Sprintf(tr("The output format for the logs, can be {%s|%s}."), "text", "json"))
-	cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", fmt.Sprintf(tr("The output format, can be {%s|%s}."), "text", "json"))
+	cmd.PersistentFlags().String("log-format", "", tr("The output format for the logs, can be: %s", "text, json"))
+	cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", tr("The output format for the logs, can be: %s", "text, json"))
 	cmd.PersistentFlags().StringVar(&configFile, "config-file", "", tr("The custom config file (if not specified the default will be used)."))
 	cmd.PersistentFlags().StringSlice("additional-urls", []string{}, tr("Comma-separated list of additional URLs for the Boards Manager."))
 	cmd.PersistentFlags().Bool("no-color", false, "Disable colored output.")
@@ -196,7 +196,7 @@ func preRun(cmd *cobra.Command, args []string) {
 	if logFile != "" {
 		file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
 		if err != nil {
-			fmt.Printf(tr("Unable to open file for logging: %s"), logFile)
+			fmt.Println(tr("Unable to open file for logging: %s", logFile))
 			os.Exit(errorcodes.ErrBadCall)
 		}
 

cli/compile/compile.go

@@ -19,7 +19,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"fmt"
 	"os"
 
 	"github.com/arduino/arduino-cli/arduino/discovery"
@@ -94,7 +93,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().StringArrayVar(&buildProperties, "build-property", []string{},
 		tr("Override a build property with a custom value. Can be used multiple times for multiple properties."))
 	command.Flags().StringVar(&warnings, "warnings", "none",
-		fmt.Sprintf(tr(`Optional, can be "%[1]s", "%[2]s", "%[3]s" and "%[4]s". Defaults to "%[1]s". Used to tell gcc which warning level to use (-W flag).`), "none", "default", "more", "all"))
+		tr(`Optional, can be: %s. Used to tell gcc which warning level to use (-W flag).`, "none, default, more, all"))
 	command.Flags().BoolVarP(&verbose, "verbose", "v", false, tr("Optional, turns on verbose mode."))
 	command.Flags().BoolVar(&quiet, "quiet", false, tr("Optional, suppresses almost every output."))
 	command.Flags().BoolVarP(&uploadAfterCompile, "upload", "u", false, tr("Upload the binary after the compilation."))
@@ -215,7 +214,7 @@ func run(cmd *cobra.Command, args []string) {
 
 		fields := map[string]string{}
 		if len(userFieldRes.UserFields) > 0 {
-			feedback.Printf(tr("Uploading to specified board using %s protocol requires the following info:"), discoveryPort.Protocol)
+			feedback.Print(tr("Uploading to specified board using %s protocol requires the following info:", discoveryPort.Protocol))
 			fields = arguments.AskForUserFields(userFieldRes.UserFields)
 		}
 

cli/config/init.go

@@ -16,7 +16,6 @@
 package config
 
 import (
-	"fmt"
 	"os"
 
 	"github.com/arduino/arduino-cli/cli/errorcodes"
@@ -107,7 +106,7 @@ func runInitCommand(cmd *cobra.Command, args []string) {
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
-	msg := fmt.Sprintf(tr("Config file written to: %s"), configFileAbsPath.String())
+	msg := tr("Config file written to: %s", configFileAbsPath.String())
 	logrus.Info(msg)
 	feedback.Print(msg)
 }

cli/core/download.go

@@ -33,12 +33,12 @@ import (
 
 func initDownloadCommand() *cobra.Command {
 	downloadCommand := &cobra.Command{
-		Use:   fmt.Sprintf(tr("download [%s:%s[@%s]]..."), tr("PACKAGER"), tr("ARCH"), tr("VERSION")),
+		Use:   fmt.Sprintf("download [%s:%s[@%s]]...", tr("PACKAGER"), tr("ARCH"), tr("VERSION")),
 		Short: tr("Downloads one or more cores and corresponding tool dependencies."),
 		Long:  tr("Downloads one or more cores and corresponding tool dependencies."),
 		Example: "" +
-			"  " + os.Args[0] + " core download arduino:samd       # " + tr("to download the latest version of Arduino SAMD core.\n") +
-			"  " + os.Args[0] + " core download arduino:[email protected] # " + tr("for a specific version (in this case 1.6.9)."),
+			"  " + os.Args[0] + " core download arduino:samd       # " + tr("download the latest version of Arduino SAMD core.") + "\n" +
+			"  " + os.Args[0] + " core download arduino:[email protected] # " + tr("download a specific version (in this case 1.6.9)."),
 		Args: cobra.MinimumNArgs(1),
 		Run:  runDownloadCommand,
 	}

cli/core/list.go

@@ -85,7 +85,7 @@ func (ir installedResult) String() string {
 	for _, p := range ir.platforms {
 		name := p.Name
 		if p.Deprecated {
-			name = fmt.Sprintf(tr("[DEPRECATED] %s"), name)
+			name = fmt.Sprintf("[%s] %s", tr("DEPRECATED"), name)
 		}
 		t.AddRow(p.Id, p.Installed, p.Latest, name)
 	}

cli/core/search.go

@@ -115,7 +115,7 @@ func (sr searchResults) String() string {
 		for _, item := range sr.platforms {
 			name := item.GetName()
 			if item.Deprecated {
-				name = fmt.Sprintf(tr("[DEPRECATED] %s"), name)
+				name = fmt.Sprintf("[%s] %s", tr("DEPRECATED"), name)
 			}
 			t.AddRow(item.GetId(), item.GetLatest(), name)
 		}

cli/daemon/daemon.go

@@ -47,7 +47,7 @@ var tr = i18n.Tr
 func NewCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "daemon",
-		Short:   fmt.Sprintf(tr("Run as a daemon on port %s"), configuration.Settings.GetString("daemon.port")),
+		Short:   tr("Run as a daemon on port: %s", configuration.Settings.GetString("daemon.port")),
 		Long:    tr("Running as a daemon the initialization of cores and libraries is done only once."),
 		Example: "  " + os.Args[0] + " daemon",
 		Args:    cobra.NoArgs,

cli/debug/debug.go

@@ -17,7 +17,6 @@ package debug
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"os/signal"
 	"sort"
@@ -62,7 +61,7 @@ func NewCommand() *cobra.Command {
 	debugCommand.Flags().StringVarP(&fqbn, "fqbn", "b", "", tr("Fully Qualified Board Name, e.g.: arduino:avr:uno"))
 	port.AddToCommand(debugCommand)
 	debugCommand.Flags().StringVarP(&programmer, "programmer", "P", "", tr("Programmer to use for debugging"))
-	debugCommand.Flags().StringVar(&interpreter, "interpreter", "console", fmt.Sprintf(tr("Debug interpreter e.g.: %s, %s, %s, %s, %s"), "console", "mi", "mi1", "mi2", "mi3"))
+	debugCommand.Flags().StringVar(&interpreter, "interpreter", "console", tr("Debug interpreter e.g.: %s", "console, mi, mi1, mi2, mi3"))
 	debugCommand.Flags().StringVarP(&importDir, "input-dir", "", "", tr("Directory containing binaries for debug."))
 	debugCommand.Flags().BoolVarP(&printInfo, "info", "I", false, tr("Show metadata about the debug session instead of starting the debugger."))
 
@@ -151,7 +150,7 @@ func (r *debugInfoResult) String() string {
 		conf := properties.NewFromHashmap(r.info.GetServerConfiguration())
 		keys := conf.Keys()
 		sort.Strings(keys)
-		t.AddRow(fmt.Sprintf(tr("%s custom configurations"), r.info.GetServer()))
+		t.AddRow(tr("Configuration options for %s", r.info.GetServer()))
 		for _, k := range keys {
 			t.AddRow(table.NewCell(" - "+k, dimGreen), table.NewCell(conf.Get(k), dimGreen))
 		}

cli/lib/check_deps.go

@@ -87,15 +87,16 @@ func outputDep(dep *rpc.LibraryDependencyStatus) string {
 	red := color.New(color.FgRed)
 	yellow := color.New(color.FgYellow)
 	if dep.GetVersionInstalled() == "" {
-		res += fmt.Sprintf(tr("%s must be installed.")+"\n",
+		res += tr("%s must be installed.",
 			red.Sprintf("✕ %s %s", dep.GetName(), dep.GetVersionRequired()))
 	} else if dep.GetVersionInstalled() == dep.GetVersionRequired() {
-		res += fmt.Sprintf(tr("%s is already installed.")+"\n",
+		res += tr("%s is already installed.",
 			green.Sprintf("✓ %s %s", dep.GetName(), dep.GetVersionRequired()))
 	} else {
-		res += fmt.Sprintf(tr("%s is required but %s is currently installed.")+"\n",
+		res += tr("%[1]s is required but %[2]s is currently installed.",
 			yellow.Sprintf("✕ %s %s", dep.GetName(), dep.GetVersionRequired()),
 			yellow.Sprintf("%s", dep.GetVersionInstalled()))
 	}
+	res += "\n"
 	return res
 }

cli/lib/examples.go

@@ -115,7 +115,7 @@ func (ir libraryExamplesResult) String() string {
 		} else if lib.Library.Location != rpc.LibraryLocation_LIBRARY_LOCATION_USER {
 			name += " (" + lib.Library.GetLocation().String() + ")"
 		}
-		r := fmt.Sprintf(tr("Examples for library %s")+"\n", color.GreenString("%s", name))
+		r := tr("Examples for library %s", color.GreenString("%s", name)) + "\n"
 		sort.Slice(lib.Examples, func(i, j int) bool {
 			return strings.ToLower(lib.Examples[i]) < strings.ToLower(lib.Examples[j])
 		})

cli/lib/search.go

@@ -139,7 +139,7 @@ func (res result) String() string {
 
 	for _, lib := range results {
 		if res.results.GetStatus() == rpc.LibrarySearchStatus_LIBRARY_SEARCH_STATUS_SUCCESS {
-			out.WriteString(fmt.Sprintf(tr(`Name: "%s"`)+"\n", lib.Name))
+			out.WriteString(tr(`Name: "%s"`, lib.Name) + "\n")
 			if res.namesOnly {
 				continue
 			}

cli/output/rpc_progress.go

@@ -61,7 +61,7 @@ func NewDownloadProgressBarCB() func(*rpc.DownloadProgress) {
 		// fmt.Printf(">>> %v\n", curr)
 		if filename := curr.GetFile(); filename != "" {
 			if curr.GetCompleted() {
-				fmt.Printf(tr("%s already downloaded")+"\n", filename)
+				fmt.Println(tr("%s already downloaded", filename))
 				return
 			}
 			prefix = filename
@@ -73,7 +73,7 @@ func NewDownloadProgressBarCB() func(*rpc.DownloadProgress) {
 			bar.Set(int(curr.GetDownloaded()))
 		}
 		if curr.GetCompleted() {
-			bar.FinishPrintOver(fmt.Sprintf(tr("%s downloaded"), prefix))
+			bar.FinishPrintOver(tr("%s downloaded", prefix))
 		}
 	}
 }

cli/sketch/archive.go

@@ -48,7 +48,7 @@ func initArchiveCommand() *cobra.Command {
 		Run:  runArchiveCommand,
 	}
 
-	command.Flags().BoolVar(&includeBuildDir, "include-build-dir", false, fmt.Sprintf(tr("Includes %s directory in the archive."), "build"))
+	command.Flags().BoolVar(&includeBuildDir, "include-build-dir", false, tr("Includes %s directory in the archive.", "build"))
 
 	return command
 }

cli/sketch/new.go

@@ -65,5 +65,5 @@ func runNewCommand(cmd *cobra.Command, args []string) {
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
-	feedback.Printf(tr("Sketch created in: %s"), sketchDir)
+	feedback.Print(tr("Sketch created in: %s", sketchDir))
 }

cli/upload/upload.go

@@ -17,7 +17,6 @@ package upload
 
 import (
 	"context"
-	"fmt"
 	"os"
 
 	"github.com/arduino/arduino-cli/arduino/sketch"
@@ -69,7 +68,7 @@ func NewCommand() *cobra.Command {
 
 func checkFlagsConflicts(command *cobra.Command, args []string) {
 	if importFile != "" && importDir != "" {
-		feedback.Errorf(fmt.Sprintf(tr("error: %s and %s flags cannot be used together"), "--input-file", "--input-dir"))
+		feedback.Errorf(tr("error: %s and %s flags cannot be used together", "--input-file", "--input-dir"))
 		os.Exit(errorcodes.ErrBadArgument)
 	}
 }
@@ -121,7 +120,7 @@ func run(command *cobra.Command, args []string) {
 
 	fields := map[string]string{}
 	if len(userFieldRes.UserFields) > 0 {
-		feedback.Printf(tr("Uploading to specified board using %s protocol requires the following info:"), discoveryPort.Protocol)
+		feedback.Print(tr("Uploading to specified board using %s protocol requires the following info:", discoveryPort.Protocol))
 		fields = arguments.AskForUserFields(userFieldRes.UserFields)
 	}
 

cli/usage.go

@@ -19,20 +19,20 @@ import (
 	"github.com/arduino/arduino-cli/i18n"
 )
 
-// Declare ids used in usage
-var (
-	tr = i18n.Tr
-	_  = tr("Usage:")
-	_  = tr("Aliases:")
-	_  = tr("Examples:")
-	_  = tr("Available Commands:")
-	_  = tr("Flags:")
-	_  = tr("Global Flags:")
-	_  = tr("Additional help topics:")
-	_  = tr("Use %s for more information about a command.")
-)
-
-const usageTemplate = `{{tr "Usage:"}}{{if .Runnable}}
+var tr = i18n.Tr
+
+func getUsageTemplate() string {
+	// Force i18n to generate translation strings
+	_ = tr("Usage:")
+	_ = tr("Aliases:")
+	_ = tr("Examples:")
+	_ = tr("Available Commands:")
+	_ = tr("Flags:")
+	_ = tr("Global Flags:")
+	_ = tr("Additional help topics:")
+	_ = tr("Use %s for more information about a command.")
+
+	return `{{tr "Usage:"}}{{if .Runnable}}
   {{.UseLine}}{{end}}{{if .HasAvailableSubCommands}}
   {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}}
 
@@ -56,3 +56,4 @@ const usageTemplate = `{{tr "Usage:"}}{{if .Runnable}}
 
 {{tr "Use %s for more information about a command." (printf "%s %s" .CommandPath "[command] --help" | printf "%q")}}{{end}}
 `
+}