*: fix the fmtsafe linter and fix the fallout

The `fmtsafe` linter added in #48040 #48048 was actually
malfunctioning because it was not stripping the vendor prefix
properly.

This patch fixes it.

Fixing the linter uncovered a range of defects throughout the
remainder of the code, some benign and some outright bugs.

Examples:

```
  return pgerror.Wrapf(err, "while running %s", stmt)
```
(The second argument should be the pgcode, not the error string!)

```
 return errors.Errorf(`no consistency checks are defined for %s` + gen.Meta().Name)
```
(Incompatible use of `%s` and string concatenation.)

```
  errors.New(fmt.Sprintf("foo %s", blah))
```
(Should use `Newf()` instead to capture more data in telemetry.)

Release note: None

pkg/acceptance/localcluster/tc/tc.go

@@ -98,16 +98,13 @@ func (c *Controller) AddLatency(srcIP, dstIP string, latency time.Duration) erro
 
 // CleanUp resets all interfaces back to their default tc policies.
 func (c *Controller) CleanUp() error {
-	var errs []string
+	var err error
 	for _, ifce := range c.interfaces {
-		out, err := exec.Command("sudo", strings.Split(fmt.Sprintf("tc qdisc del dev %s root", ifce), " ")...).Output()
+		out, thisErr := exec.Command("sudo", strings.Split(fmt.Sprintf("tc qdisc del dev %s root", ifce), " ")...).Output()
 		if err != nil {
-			errs = append(errs, errors.Wrapf(
-				err, "failed to remove tc rules for %q -- you may have to remove them manually: %s", ifce, out).Error())
+			err = errors.CombineErrors(err, errors.Wrapf(
+				thisErr, "failed to remove tc rules for %q -- you may have to remove them manually: %s", ifce, out))
 		}
 	}
-	if len(errs) > 0 {
-		return errors.New(strings.Join(errs, "; "))
-	}
-	return nil
+	return err
 }

pkg/ccl/changefeedccl/avro_test.go

@@ -86,7 +86,7 @@ func parseValues(tableDesc *sqlbase.TableDescriptor, values string) ([]sqlbase.E
 			}
 			datum, err := typedExpr.Eval(evalCtx)
 			if err != nil {
-				return nil, errors.Wrap(err, typedExpr.String())
+				return nil, errors.Wrapf(err, "evaluating %s", typedExpr)
 			}
 			row = append(row, sqlbase.DatumToEncDatum(col.Type, datum))
 		}

pkg/ccl/changefeedccl/cdctest/testfeed.go

@@ -255,7 +255,7 @@ func (f *jobFeed) fetchJobError() error {
 		return err
 	}
 	if len(errorStr.String) > 0 {
-		f.jobErr = errors.New(errorStr.String)
+		f.jobErr = errors.Newf("%s", errorStr.String)
 	}
 	return nil
 }

pkg/ccl/changefeedccl/cdctest/validator.go

@@ -216,13 +216,14 @@ func (v *beforeAfterValidator) checkRowAt(
 	}
 
 	var valid bool
-	if err := v.sqlDB.QueryRow(stmtBuf.String(), args...).Scan(&valid); err != nil {
-		return errors.Wrap(err, stmtBuf.String())
+	stmt := stmtBuf.String()
+	if err := v.sqlDB.QueryRow(stmt, args...).Scan(&valid); err != nil {
+		return errors.Wrapf(err, "while executing %s", stmt)
 	}
 	if !valid {
 		v.failures = append(v.failures, fmt.Sprintf(
 			"%q field did not agree with row at %s: %s %v",
-			field, ts.AsOfSystemTime(), stmtBuf.String(), args))
+			field, ts.AsOfSystemTime(), stmt, args))
 	}
 	return nil
 }

pkg/ccl/changefeedccl/schemafeed/schema_feed_test.go

@@ -26,7 +26,7 @@ func TestTableHistoryIngestionTracking(t *testing.T) {
 	ts := func(wt int64) hlc.Timestamp { return hlc.Timestamp{WallTime: wt} }
 	validateFn := func(_ context.Context, desc *sqlbase.TableDescriptor) error {
 		if desc.Name != `` {
-			return errors.New(desc.Name)
+			return errors.Newf("descriptor: %s", desc.Name)
 		}
 		return nil
 	}
@@ -108,11 +108,11 @@ func TestTableHistoryIngestionTracking(t *testing.T) {
 	// does not validate, high-water does not change
 	require.EqualError(t, m.ingestDescriptors(ctx, ts(7), ts(10), []*sqlbase.TableDescriptor{
 		{ID: 0, Name: `whoops!`},
-	}, validateFn), `whoops!`)
+	}, validateFn), `descriptor: whoops!`)
 	require.Equal(t, ts(7), m.highWater())
 
 	// ts 10 has errored, so validate can return its error without blocking
-	require.EqualError(t, m.waitForTS(ctx, ts(10)), `whoops!`)
+	require.EqualError(t, m.waitForTS(ctx, ts(10)), `descriptor: whoops!`)
 
 	// ts 8 and 9 are still unknown
 	errCh8 = make(chan error, 1)
@@ -125,16 +125,16 @@ func TestTableHistoryIngestionTracking(t *testing.T) {
 	// turns out ts 10 is not a tight bound. ts 9 also has an error
 	require.EqualError(t, m.ingestDescriptors(ctx, ts(7), ts(9), []*sqlbase.TableDescriptor{
 		{ID: 0, Name: `oh no!`},
-	}, validateFn), `oh no!`)
+	}, validateFn), `descriptor: oh no!`)
 	require.Equal(t, ts(7), m.highWater())
-	require.EqualError(t, <-errCh9, `oh no!`)
+	require.EqualError(t, <-errCh9, `descriptor: oh no!`)
 
 	// ts 8 is still unknown
 	requireChannelEmpty(t, errCh8)
 
 	// always return the earlist error seen (so waiting for ts 10 immediately
 	// returns the 9 error now, it returned the ts 10 error above)
-	require.EqualError(t, m.waitForTS(ctx, ts(9)), `oh no!`)
+	require.EqualError(t, m.waitForTS(ctx, ts(9)), `descriptor: oh no!`)
 
 	// something earlier than ts 10 can still be okay
 	require.NoError(t, m.ingestDescriptors(ctx, ts(7), ts(8), nil, validateFn))

pkg/ccl/importccl/exportcsv.go

@@ -235,7 +235,7 @@ func (sp *csvWriter) Run(ctx context.Context) {
 				break
 			}
 			if err := writer.Flush(); err != nil {
-				return errors.New(fmt.Sprintf("failed to flush csv writer, error %s", err))
+				return errors.Wrap(err, "failed to flush csv writer")
 			}
 
 			conf, err := cloud.ExternalStorageConfFromURI(sp.spec.Destination)
@@ -259,7 +259,7 @@ func (sp *csvWriter) Run(ctx context.Context) {
 			// Close writer to ensure buffer and any compression footer is flushed.
 			err = writer.Close()
 			if err != nil {
-				return errors.New(fmt.Sprintf("failed to close exporting writer, error %s", err))
+				return errors.Wrapf(err, "failed to close exporting writer")
 			}
 
 			size := writer.Len()

pkg/ccl/importccl/read_import_base.go

@@ -236,11 +236,11 @@ func readInputFiles(
 				})
 
 				if err := grp.Wait(); err != nil {
-					return errors.Wrap(err, dataFile)
+					return errors.Wrapf(err, "%s", dataFile)
 				}
 			} else {
 				if err := fileFunc(ctx, src, dataFileIndex, resumePos[dataFileIndex], nil /* rejected */); err != nil {
-					return errors.Wrap(err, dataFile)
+					return errors.Wrapf(err, "%s", dataFile)
 				}
 			}
 			return nil

pkg/ccl/partitionccl/partition.go

@@ -103,7 +103,7 @@ func valueEncodePartitionTuple(
 		}
 		datum, err := typedExpr.Eval(evalCtx)
 		if err != nil {
-			return nil, errors.Wrap(err, typedExpr.String())
+			return nil, errors.Wrapf(err, "evaluating %s", typedExpr)
 		}
 		if err := sqlbase.CheckDatumTypeFitsColumnType(&cols[i], datum.ResolvedType()); err != nil {
 			return nil, err

pkg/ccl/partitionccl/partition_test.go

@@ -1113,14 +1113,15 @@ func verifyScansOnNode(
 		}
 	}
 	if len(scansWrongNode) > 0 {
-		var err bytes.Buffer
-		fmt.Fprintf(&err, "expected to scan on %s: %s\n%s\nfull trace:",
-			node, query, strings.Join(scansWrongNode, "\n"))
+		err := errors.Newf("expected to scan on %s: %s", node, query)
+		err = errors.WithDetailf(err, "scans:\n%s", strings.Join(scansWrongNode, "\n"))
+		var trace strings.Builder
 		for _, traceLine := range traceLines {
-			err.WriteString("\n  ")
-			err.WriteString(traceLine)
+			trace.WriteString("\n  ")
+			trace.WriteString(traceLine)
 		}
-		return errors.New(err.String())
+		err = errors.WithDetailf(err, "trace:%s", trace.String())
+		return err
 	}
 	return nil
 }

pkg/ccl/storageccl/engineccl/rocksdb.go

@@ -139,7 +139,7 @@ func statusToError(s C.DBStatus) error {
 	if s.data == nil {
 		return nil
 	}
-	return errors.New(cStringToGoString(s))
+	return errors.Newf("%s", cStringToGoString(s))
 }
 
 func cStatsToGoStats(stats C.MVCCStatsResult, nowNanos int64) (enginepb.MVCCStats, error) {

pkg/cli/cli_test.go

@@ -1882,11 +1882,11 @@ func TestJunkPositionalArguments(t *testing.T) {
 		line := test + " " + junk
 		out, err := c.RunWithCapture(line)
 		if err != nil {
-			t.Fatal(errors.Wrap(err, strconv.Itoa(i)))
+			t.Fatalf("%d: %v", i, err)
 		}
 		exp := fmt.Sprintf("%s\nERROR: unknown command %q for \"cockroach %s\"\n", line, junk, test)
 		if exp != out {
-			t.Errorf("expected:\n%s\ngot:\n%s", exp, out)
+			t.Errorf("%d: expected:\n%s\ngot:\n%s", i, exp, out)
 		}
 	}
 }

pkg/cli/debug_check_store.go

@@ -99,26 +99,23 @@ type checkResult struct {
 }
 
 func (cr *checkResult) Error() error {
-	var errs []string
+	var err error
 	if cr.err != nil {
-		errs = append(errs, cr.err.Error())
+		err = cr.err
 	}
 	if !cr.actMS.Equal(enginepb.MVCCStats{}) && !cr.actMS.Equal(cr.claimMS) && cr.claimMS.ContainsEstimates <= 0 {
-		err := fmt.Sprintf("stats inconsistency:\n- stored:\n%+v\n- recomputed:\n%+v\n- diff:\n%s",
+		thisErr := errors.Newf(
+			"stats inconsistency:\n- stored:\n%+v\n- recomputed:\n%+v\n- diff:\n%s",
 			cr.claimMS, cr.actMS, strings.Join(pretty.Diff(cr.claimMS, cr.actMS), ","),
 		)
-		errs = append(errs, err)
+		err = errors.CombineErrors(err, thisErr)
 	}
-	if len(errs) > 0 {
+	if err != nil {
 		if cr.desc != nil {
-			prefix := cr.desc.String() + ": "
-			for i := range errs {
-				errs[i] = prefix + errs[i]
-			}
+			err = errors.Wrapf(err, "%s", cr.desc)
 		}
-		return errors.New(strings.Join(errs, "\n"))
 	}
-	return nil
+	return err
 }
 
 func worker(ctx context.Context, in checkInput) checkResult {

pkg/cli/debug_synctest.go

@@ -58,7 +58,7 @@ type scriptNemesis string
 func (sn scriptNemesis) exec(arg string) error {
 	b, err := exec.Command(string(sn), arg).CombinedOutput()
 	if err != nil {
-		return errors.Wrap(err, string(b))
+		return errors.WithDetailf(err, "command output:\n%s", string(b))
 	}
 	fmt.Fprintf(stderr, "%s %s: %s", sn, arg, b)
 	return nil

pkg/cli/demo.go

@@ -237,7 +237,8 @@ func checkDemoConfiguration(
 		}
 
 		// If geo-partition-replicas is requested, make sure the workload has a Partitioning step.
-		configErr := errors.New(fmt.Sprintf("workload %s is not configured to have a partitioning step", gen.Meta().Name))
+		configErr := errors.Newf(
+			"workload %s is not configured to have a partitioning step", gen.Meta().Name)
 		hookser, ok := gen.(workload.Hookser)
 		if !ok {
 			return nil, configErr

pkg/cli/error.go

@@ -229,7 +229,7 @@ func MaybeDecorateGRPCError(
 		connRefused := func() error {
 			extra := extraInsecureHint()
 			return errors.Errorf("server closed the connection.\n"+
-				"Is this a CockroachDB node?\n"+extra+"\n%v", err)
+				"Is this a CockroachDB node?\n%s\n%v", extra, err)
 		}
 
 		// Is this an "unable to connect" type of error?

pkg/cli/sql.go

@@ -1570,7 +1570,7 @@ func (c *cliState) serverSideParse(sql string) (helpText string, err error) {
 			hint = ""
 		}
 		// In any case report that there was an error while parsing.
-		err := errors.New(message)
+		err := errors.Newf("%s", message)
 		err = pgerror.WithCandidateCode(err, code)
 		if hint != "" {
 			err = errors.WithHint(err, hint)

pkg/cli/start.go

@@ -760,11 +760,11 @@ If problems persist, please see %s.`
 			// Attempt to start the server.
 			if err := s.Start(ctx); err != nil {
 				if le := (*server.ListenError)(nil); errors.As(err, &le) {
-					const errorPrefix = "consider changing the port via --"
+					const errorPrefix = "consider changing the port via --%s"
 					if le.Addr == serverCfg.Addr {
-						err = errors.Wrap(err, errorPrefix+cliflags.ListenAddr.Name)
+						err = errors.Wrapf(err, errorPrefix, cliflags.ListenAddr.Name)
 					} else if le.Addr == serverCfg.HTTPAddr {
-						err = errors.Wrap(err, errorPrefix+cliflags.ListenHTTPAddr.Name)
+						err = errors.Wrapf(err, errorPrefix, cliflags.ListenHTTPAddr.Name)
 					}
 				}
 

pkg/cli/zip.go

@@ -336,7 +336,7 @@ func runDebugZip(cmd *cobra.Command, args []string) (retErr error) {
 			selectClause = "*"
 		}
 		if err := dumpTableDataForZip(z, sqlConn, timeout, base, table, selectClause); err != nil {
-			return errors.Wrap(err, table)
+			return errors.Wrapf(err, "fetching %s", table)
 		}
 	}
 
@@ -432,7 +432,7 @@ func runDebugZip(cmd *cobra.Command, args []string) (retErr error) {
 					selectClause = "*"
 				}
 				if err := dumpTableDataForZip(z, curSQLConn, timeout, prefix, table, selectClause); err != nil {
-					return errors.Wrap(err, table)
+					return errors.Wrapf(err, "fetching %s", table)
 				}
 			}
 

pkg/cmd/cmpconn/compare.go

@@ -37,7 +37,7 @@ func CompareVals(a, b []interface{}) error {
 		return nil
 	}
 	if diff := cmp.Diff(a, b, cmpOptions...); diff != "" {
-		return errors.New(diff)
+		return errors.Newf("unexpected diff:\n%s", diff)
 	}
 	return nil
 }

pkg/cmd/github-post/main.go

@@ -162,7 +162,7 @@ func listFailures(
 				continue
 			}
 			if err := json.Unmarshal([]byte(line), &te); err != nil {
-				return errors.Wrap(err, line)
+				return errors.Wrapf(err, "unable to parse %q", line)
 			}
 		}
 		lastEvent = te

pkg/cmd/roachprod/main.go

@@ -135,19 +135,13 @@ func newCluster(name string) (*install.SyncedCluster, error) {
 
 	c, ok := install.Clusters[name]
 	if !ok {
-		// NB: We don't use fmt.Errorf due to a linter error about the error
-		// message containing capitals and punctuation. We don't use
-		// errors.New(fmt.Sprintf()) due to a linter error that we should use
-		// fmt.Errorf() instead. Sigh.
-		s := fmt.Sprintf(`unknown cluster: %s
-
+		err := errors.Newf(`unknown cluster: %s`, name)
+		err = errors.WithHintf(err, `
 Available clusters:
   %s
-
-Hint: use "roachprod sync" to update the list of available clusters.
-`,
-			name, strings.Join(sortedClusters(), "\n  "))
-		return nil, errors.New(s)
+`, strings.Join(sortedClusters(), "\n  "))
+		err = errors.WithHint(err, `Use "roachprod sync" to update the list of available clusters.`)
+		return nil, err
 	}
 
 	switch clusterType {
@@ -1114,21 +1108,17 @@ of nodes, outputting a line whenever a change is detected:
 		if err != nil {
 			return err
 		}
-		var errs []string
 		for msg := range c.Monitor(monitorIgnoreEmptyNodes, monitorOneShot) {
 			if msg.Err != nil {
 				msg.Msg += "error: " + msg.Err.Error()
 			}
-			s := fmt.Sprintf("%d: %s", msg.Index, msg.Msg)
+			thisError := errors.Newf("%d: %s", msg.Index, msg.Msg)
 			if msg.Err != nil || strings.Contains(msg.Msg, "dead") {
-				errs = append(errs, s)
+				err = errors.CombineErrors(err, thisError)
 			}
-			fmt.Println(s)
+			fmt.Println(thisError.Error())
 		}
-		if len(errs) != 0 {
-			return errors.New(strings.Join(errs, ", "))
-		}
-		return nil
+		return err
 	}),
 }
 

pkg/cmd/roachprod/vm/flagstub/flagstub.go

@@ -23,12 +23,12 @@ import (
 // implemented as no-op or no-value. All other operations will
 // return the provided error.
 func New(delegate vm.Provider, unimplemented string) vm.Provider {
-	return &provider{delegate: delegate, unimplemented: errors.New(unimplemented)}
+	return &provider{delegate: delegate, unimplemented: unimplemented}
 }
 
 type provider struct {
 	delegate      vm.Provider
-	unimplemented error
+	unimplemented string
 }
 
 // CleanSSH implements vm.Provider and is a no-op.
@@ -43,17 +43,17 @@ func (p *provider) ConfigSSH() error {
 
 // Create implements vm.Provider and returns Unimplemented.
 func (p *provider) Create(names []string, opts vm.CreateOpts) error {
-	return p.unimplemented
+	return errors.Newf("%s", p.unimplemented)
 }
 
 // Delete implements vm.Provider and returns Unimplemented.
 func (p *provider) Delete(vms vm.List) error {
-	return p.unimplemented
+	return errors.Newf("%s", p.unimplemented)
 }
 
 // Extend implements vm.Provider and returns Unimplemented.
 func (p *provider) Extend(vms vm.List, lifetime time.Duration) error {
-	return p.unimplemented
+	return errors.Newf("%s", p.unimplemented)
 }
 
 // FindActiveAccount implements vm.Provider and returns an empty account.