Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/vet, x/tools/go/analysis: flag superfluous fmt.Errorf (without format specifiers) calls to instead use errors.New and save binary size #52696

Closed
odeke-em opened this issue May 3, 2022 · 10 comments
Closed

cmd/vet, x/tools/go/analysis: flag superfluous fmt.Errorf (without format specifiers) calls to instead use errors.New and save binary size #52696

odeke-em opened this issue May 3, 2022 · 10 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@odeke-em
Copy link
Member

odeke-em commented May 3, 2022

What version of Go are you using (go version)?

$ go version
go version devel go1.19-9a53b472b5 Thu Mar 31 21:39:51 2022 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Not applicable

What did you do?

Per https://twitter.com/odeke_et/status/1520929877104332801, over the weekend while working on some code for an embedded system, I examined the binary size and noticed the use of fmt.Errorf without format specifiers or just constant strings was permitted and could instead use errors.New of which the results of swapping the change caused a 600KiB reduction in binary size on Linux AMD64, and 700KiB reduction in binary size on Darwin AMD64!

Just compiled this simple program https://go.dev/play/p/Ec6Wy1sUpF_e or inlined below

package main

import "fmt"

func main() {
	panic(fmt.Errorf("This is a call!"))
}

On Linux it results in a 1.8MiB binary on both Linux AMD64 and Darwin AMD64

go build -o fmt-binary main.go && ls -lh
-rwxrwxr-x 1 emmanuel emmanuel 1.8M May  1 17:41 fmt-binary

On changing it to use errors.New

package main

import "errors"

func main() {
	panic(errors.New("This is a call!"))
}

results in a 1.2MiB binary on Linux AMD64 and a 1.1MiB binary on Darwin AMD64

What did you expect to see?

Running go vet or go test should flag such calls and make suggestions. I expected this

fmt.Errorf has no formatting directives, please instead use "errors.New"

What did you see instead?

Importing the fmt package unnecessarily results in that bloat, and we should do our best to avoid this problem. Unfortunately even our standard library has 374 such superfluous calls if I run

git grep 'fmt.Errorf.\`\|fmt.Errorf."' | grep -v "%"

with the results in the depressible details below

```shell archive/tar/common.go: return nil, fmt.Errorf("archive/tar: sockets not supported") archive/tar/reader_test.go:func (rbs *readBadSeeker) Seek(int64, int) (int64, error) { return 0, fmt.Errorf("illegal seek") } cmd/compile/internal/importer/exportdata.go: err = fmt.Errorf("invalid archive header") cmd/compile/internal/importer/exportdata.go: err = fmt.Errorf("go archive is missing __.PKGDEF") cmd/compile/internal/importer/exportdata.go: err = fmt.Errorf("not a Go object file") cmd/compile/internal/ssa/poset.go: return fmt.Errorf("non-empty noneq map") cmd/compile/internal/ssa/stmtlines_test.go: return nil, fmt.Errorf("unrecognized executable format") cmd/compile/internal/staticdata/data.go: return nil, 0, fmt.Errorf("not a regular file") cmd/compile/internal/staticdata/data.go: return nil, 0, fmt.Errorf("file changed between reads") cmd/compile/internal/staticdata/data.go: return nil, 0, fmt.Errorf("file changed between reads") cmd/compile/internal/types2/check_test.go: return fmt.Errorf("flags comment line too long") cmd/compile/internal/types2/resolver.go: return "", fmt.Errorf("empty string") cmd/compile/internal/types2/resolver.go: err = fmt.Errorf("Config.Importer not installed") cmd/cover/cover.go: return fmt.Errorf("too many options") cmd/cover/cover.go: return fmt.Errorf("too many options") cmd/cover/cover.go: return fmt.Errorf("missing source file") cmd/cover/cover.go: return fmt.Errorf("too many arguments") cmd/dist/buildtag_test.go: {"syntax(error", false, fmt.Errorf("parsing //go:build line: unexpected (")}, cmd/dist/buildtag_test.go: {"(gc", false, fmt.Errorf("parsing //go:build line: missing )")}, cmd/dist/buildtag_test.go: {"gc gc", false, fmt.Errorf("parsing //go:build line: unexpected tag")}, cmd/dist/buildtag_test.go: {"(gc))", false, fmt.Errorf("parsing //go:build line: unexpected )")}, cmd/dist/test.go: return fmt.Errorf("'go env GOEXE GOTMPDIR' output contains <2 lines") cmd/go/internal/cache/cache.go: return nil, &fs.PathError{Op: "open", Path: dir, Err: fmt.Errorf("not a directory")} cmd/go/internal/cache/cache.go: return fmt.Errorf("file content changed underfoot") cmd/go/internal/cache/default.go: defaultDirErr = fmt.Errorf("GOCACHE is not an absolute path") cmd/go/internal/cfg/cfg.go: return "", fmt.Errorf("GOENV=off") cmd/go/internal/cfg/cfg.go: return "", fmt.Errorf("missing user-config dir") cmd/go/internal/fsys/fsys.go: return fmt.Errorf("empty string key in overlay file Replace map") cmd/go/internal/generate/generate.go:var stop = fmt.Errorf("error in generation") cmd/go/internal/get/get.go: return fmt.Errorf("cannot download, $GOPATH not set. For more details see: 'go help gopath'") cmd/go/internal/get/get.go: return fmt.Errorf("cannot download, $GOPATH must not be set to $GOROOT. For more details see: 'go help gopath'") cmd/go/internal/imports/scan.go:var ErrNoGo = fmt.Errorf("no Go source files") cmd/go/internal/load/flag.go: return fmt.Errorf("missing = in =") cmd/go/internal/load/flag.go: return fmt.Errorf("missing in =") cmd/go/internal/load/pkg.go: return nil, nil, fmt.Errorf("invalid pattern syntax") cmd/go/internal/load/pkg.go: return nil, nil, fmt.Errorf("no matching files found") cmd/go/internal/modfetch/cache.go:var errNotCached = fmt.Errorf("not in cache") cmd/go/internal/modfetch/cache.go: return fmt.Errorf("module cache not found: neither GOMODCACHE nor GOPATH is set") cmd/go/internal/modfetch/codehost/codehost.go: return "", "", fmt.Errorf("neither GOPATH nor GOMODCACHE are set") cmd/go/internal/modfetch/codehost/codehost.go: return "", "", fmt.Errorf("codehost.WorkDir: type cannot contain colon") cmd/go/internal/modfetch/codehost/git.go: return nil, fmt.Errorf("git remote cannot use host:path syntax") cmd/go/internal/modfetch/codehost/git.go: return nil, fmt.Errorf("git remote must not be local directory") cmd/go/internal/modfetch/coderepo.go: return fmt.Errorf("major version without preceding tag must be v0, not v1") cmd/go/internal/modfetch/coderepo.go: return fmt.Errorf("downloaded zip file too large") cmd/go/internal/modfetch/coderepo.go: return fmt.Errorf("missing top-level directory prefix") cmd/go/internal/modfetch/coderepo.go: return fmt.Errorf("zip file contains more than one top-level directory") cmd/go/internal/modfetch/proxy.go: proxyOnce.err = fmt.Errorf("GOPROXY list is not the empty string, but contains no entries") cmd/go/internal/modfetch/proxy.go: return nil, p.versionError(version, fmt.Errorf("internal error: version passed to GoMod is not canonical")) cmd/go/internal/modfetch/proxy.go: return p.versionError(version, fmt.Errorf("internal error: version passed to Zip is not canonical")) cmd/go/internal/modfetch/proxy.go: return p.versionError(version, fmt.Errorf("downloaded zip file too large")) cmd/go/internal/modfetch/sumdb.go: return "", nil, fmt.Errorf("missing GOSUMDB") cmd/go/internal/modfetch/sumdb.go: return "", nil, fmt.Errorf("invalid GOSUMDB: too many fields") cmd/go/internal/modfetch/sumdb.go: return fmt.Errorf("cannot write key") cmd/go/internal/modget/get.go: return errSet(fmt.Errorf("no package to get in current directory")) cmd/go/internal/modload/init.go: return nil, fmt.Errorf("existing contents have changed since last read") cmd/go/internal/test/testflag.go: return fmt.Errorf("-vet argument cannot contain equal signs") cmd/go/internal/test/testflag.go: return fmt.Errorf("-vet argument is comma-separated list, cannot contain spaces") cmd/go/internal/test/testflag.go: return fmt.Errorf("-vet argument contains empty list element") cmd/go/internal/vcs/vcs.go: return "", fmt.Errorf("unable to parse output of bzr info") cmd/go/internal/vcs/vcs.go: return "", fmt.Errorf("unable to parse output of bzr info") cmd/go/internal/vcs/vcs.go: return "", fmt.Errorf("unable to parse output of svn info") cmd/go/internal/vcs/vcs.go: return "", fmt.Errorf("unable to parse output of svn info") cmd/go/internal/vcs/vcs.go: return nil, fmt.Errorf("empty entry in GOVCS") cmd/go/internal/vcs/vcs.go: return nil, fmt.Errorf("no modules on example.net") cmd/go/internal/vcs/vcs.go: return nil, fmt.Errorf("rsc.io is not a module") cmd/go/internal/web/http.go: return nil, fmt.Errorf("no such host localhost.localdev") cmd/go/internal/work/exec.go: return fmt.Errorf("vet config not found") cmd/go/internal/work/gc.go: err = fmt.Errorf("file larger than size reported by stat") cmd/go/internal/work/gccgo.go: return fmt.Errorf("gccgo -importcfg does not support shared libraries") cmd/go/testdata/script/test_fuzz_mutator.txt: return fmt.Errorf("coordinator: did not test FuzzB seed") cmd/go/testdata/script/test_fuzz_mutator.txt: return fmt.Errorf("worker: did not test any mutants") cmd/gofmt/gofmt.go: s.AddReport(fmt.Errorf("error: cannot use -w with standard input")) cmd/internal/archive/archive.go: r.error(fmt.Errorf("debug/goobj: internal error: misuse of skip")) cmd/internal/buildid/buildid.go: errBuildIDMalformed = fmt.Errorf("malformed object file") cmd/internal/buildid/note.go: return "", &fs.PathError{Path: name, Op: "parse", Err: fmt.Errorf("cannot find __text section")} cmd/internal/buildid/rewrite.go: return nil, [32]byte{}, fmt.Errorf("buildid.FindAndHash: no id specified") cmd/internal/buildid/rewrite.go: return nil, [32]byte{}, fmt.Errorf("buildid.FindAndHash: buffer too small") cmd/internal/obj/riscv/obj.go: return 0, fmt.Errorf("fixme") cmd/internal/objfile/disasm.go: return nil, fmt.Errorf("unsupported architecture") cmd/internal/objfile/elf.go: return 0, nil, fmt.Errorf("text section not found") cmd/internal/objfile/elf.go: return 0, fmt.Errorf("unknown load address") cmd/internal/objfile/goobj.go: return 0, nil, nil, fmt.Errorf("pcln not available in go object file") cmd/internal/objfile/goobj.go: return 0, fmt.Errorf("unknown load address") cmd/internal/objfile/macho.go: return 0, nil, fmt.Errorf("text section not found") cmd/internal/objfile/macho.go: return 0, fmt.Errorf("unknown load address") cmd/internal/objfile/pe.go: return nil, fmt.Errorf("invalid section number in symbol table") cmd/internal/objfile/pe.go: return 0, nil, nil, fmt.Errorf("pe file format not recognized") cmd/internal/objfile/pe.go: return 0, nil, fmt.Errorf("pe file format not recognized") cmd/internal/objfile/pe.go: return 0, nil, fmt.Errorf("text section not found") cmd/internal/objfile/pe.go: return 0, fmt.Errorf("unknown load address") cmd/internal/objfile/plan9obj.go: return 0, nil, fmt.Errorf("text section not found") cmd/internal/objfile/plan9obj.go: return 0, fmt.Errorf("unknown load address") cmd/internal/objfile/xcoff.go: return nil, fmt.Errorf("invalid section number in symbol table") cmd/internal/objfile/xcoff.go: return 0, nil, fmt.Errorf("text section not found") cmd/internal/objfile/xcoff.go: return 0, fmt.Errorf("unknown load address") cmd/internal/osinfo/os_js.go: return "", fmt.Errorf("unimplemented") cmd/link/internal/ld/macho.go: return fmt.Errorf("unexpected content after code signature") cmd/link/internal/ld/macho_combine_dwarf.go: return fmt.Errorf("missing __LINKEDIT segment") cmd/link/internal/ld/macho_combine_dwarf.go: return fmt.Errorf("missing __DWARF segment") cmd/link/internal/ld/macho_combine_dwarf.go: return fmt.Errorf("missing __text section") cmd/link/internal/loadelf/ldelf.go: return false, 0, fmt.Errorf("corrupt .ARM.attributes (section name not NUL-terminated)\n") cmd/link/internal/loadelf/ldelf.go: return false, 0, fmt.Errorf("could not parse .ARM.attributes\n") cmd/link/internal/loadelf/ldelf.go: err = fmt.Errorf("elf section past end of file") cmd/link/internal/loadelf/ldelf.go: err = fmt.Errorf("invalid elf symbol index") cmd/link/internal/loadelf/ldelf.go: return fmt.Errorf("readym: read null symbol!") cmd/pprof/pprof.go: return nil, fmt.Errorf("printing assembly in Intel syntax is not supported") cmd/trace/annotations.go: return annotationAnalysisResult{}, fmt.Errorf("empty trace") cmd/vendor/github.com/google/pprof/internal/binutils/binutils.go: return nil, fmt.Errorf("could not find local addr2liner") cmd/vendor/github.com/google/pprof/internal/driver/driver.go: return fmt.Errorf("unexpected granularity") cmd/vendor/github.com/google/pprof/internal/driver/driver.go: return nil, fmt.Errorf("zero divisor specified") cmd/vendor/github.com/google/pprof/internal/driver/driver.go: return nil, nil, nil, fmt.Errorf("profile has no samples") cmd/vendor/github.com/google/pprof/internal/driver/fetch.go: return nil, nil, nil, nil, false, fmt.Errorf("failed to fetch any source profiles") cmd/vendor/github.com/google/pprof/internal/driver/fetch.go: return nil, nil, nil, nil, false, fmt.Errorf("failed to fetch any base profiles") cmd/vendor/github.com/google/pprof/internal/driver/fetch.go: return "", fmt.Errorf("failed to identify temp dir") cmd/vendor/github.com/google/pprof/internal/driver/interactive.go: return nil, config{}, fmt.Errorf("unexpected end of line after >") cmd/vendor/github.com/google/pprof/internal/driver/settings.go: return fmt.Errorf("invalid config name") cmd/vendor/github.com/google/pprof/internal/elfexec/elfexec.go: return nil, fmt.Errorf("multiple build ids found, don't know which to use") cmd/vendor/github.com/google/pprof/internal/elfexec/elfexec.go: return 0, fmt.Errorf("don't know how to handle mapping.Offset") cmd/vendor/github.com/google/pprof/internal/report/report.go: return fmt.Errorf("unexpected output format") cmd/vendor/github.com/google/pprof/internal/transport/transport.go: return fmt.Errorf("-tls_key is specified, so -tls_cert must also be specified") cmd/vendor/github.com/google/pprof/internal/transport/transport.go: return fmt.Errorf("-tls_cert is specified, so -tls_key must also be specified") cmd/vendor/github.com/google/pprof/profile/merge.go: return nil, fmt.Errorf("no profiles to merge") cmd/vendor/github.com/google/pprof/profile/profile.go:var errUnrecognized = fmt.Errorf("unrecognized profile format") cmd/vendor/github.com/google/pprof/profile/profile.go:var errMalformed = fmt.Errorf("malformed profile format") cmd/vendor/github.com/google/pprof/profile/profile.go:var errNoData = fmt.Errorf("empty input file") cmd/vendor/github.com/google/pprof/profile/profile.go:var errConcatProfile = fmt.Errorf("concatenated profiles detected") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("missing sample type information") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("profile has nil sample") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("sample has nil location") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("profile has nil mapping") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("found mapping with reserved ID=0") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("profile has nil function") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("found function with reserved ID=0") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("profile has nil location") cmd/vendor/github.com/google/pprof/profile/profile.go: return fmt.Errorf("found location with reserved id=0") cmd/vendor/golang.org/x/arch/arm/armasm/decode.go: errMode = fmt.Errorf("unsupported execution mode") cmd/vendor/golang.org/x/arch/arm/armasm/decode.go: errShort = fmt.Errorf("truncated instruction") cmd/vendor/golang.org/x/arch/arm/armasm/decode.go: errUnknown = fmt.Errorf("unknown instruction") cmd/vendor/golang.org/x/arch/arm64/arm64asm/decode.go: errShort = fmt.Errorf("truncated instruction") cmd/vendor/golang.org/x/arch/arm64/arm64asm/decode.go: errUnknown = fmt.Errorf("unknown instruction") cmd/vendor/golang.org/x/arch/ppc64/ppc64asm/decode.go: errShort = fmt.Errorf("truncated instruction") cmd/vendor/golang.org/x/arch/ppc64/ppc64asm/decode.go: errUnknown = fmt.Errorf("unknown instruction") cmd/vendor/golang.org/x/arch/ppc64/ppc64asm/gnu.go: panic(fmt.Errorf("wrong table: offset not followed by register")) cmd/vendor/golang.org/x/arch/ppc64/ppc64asm/plan9.go: panic(fmt.Errorf("wrong table: offset not followed by register")) cmd/vendor/golang.org/x/mod/modfile/rule.go: return VersionInterval{}, fmt.Errorf("expected '[' or version") cmd/vendor/golang.org/x/mod/modfile/rule.go: return VersionInterval{}, fmt.Errorf("expected version after '['") cmd/vendor/golang.org/x/mod/modfile/rule.go: return VersionInterval{}, fmt.Errorf("expected ',' after version") cmd/vendor/golang.org/x/mod/modfile/rule.go: return VersionInterval{}, fmt.Errorf("expected version after ','") cmd/vendor/golang.org/x/mod/modfile/rule.go: return VersionInterval{}, fmt.Errorf("expected ']' after version") cmd/vendor/golang.org/x/mod/modfile/rule.go: return "", fmt.Errorf("unquoted string cannot contain quote") cmd/vendor/golang.org/x/mod/modfile/rule.go: return "", fmt.Errorf("invalid module path") cmd/vendor/golang.org/x/mod/modfile/rule.go: Err: fmt.Errorf("must be of the form v1.2.3"), cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("leading slash") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("missing dot in first path element") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("leading dash in first path element") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("invalid version") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("invalid UTF-8") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("empty string") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("leading dash") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("double slash") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("trailing slash") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("empty path element") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("leading dot in path element") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("trailing dot in path element") cmd/vendor/golang.org/x/mod/module/module.go: return fmt.Errorf("trailing tilde and digits in path element") cmd/vendor/golang.org/x/mod/module/module.go: Err: fmt.Errorf("disallowed version string"), cmd/vendor/golang.org/x/mod/module/module.go: return "", fmt.Errorf("internal error: inconsistency in EscapePath") cmd/vendor/golang.org/x/mod/sumdb/client.go: return fmt.Errorf("cannot authenticate record data in server response") cmd/vendor/golang.org/x/mod/sumdb/test.go: return nil, fmt.Errorf("missing records") cmd/vendor/golang.org/x/mod/sumdb/tlog/tile.go: return nil, fmt.Errorf("indexes not in tree") cmd/vendor/golang.org/x/mod/sumdb/tlog/tile.go: return nil, fmt.Errorf("downloaded inconsistent tile") cmd/vendor/golang.org/x/mod/sumdb/tlog/tile.go: return nil, fmt.Errorf("downloaded inconsistent tile") cmd/vendor/golang.org/x/mod/sumdb/tlog/tlog.go: return Hash{}, fmt.Errorf("malformed hash") cmd/vendor/golang.org/x/mod/sumdb/tlog/tlog.go: return nil, fmt.Errorf("tlog: invalid inputs in ProveRecord") cmd/vendor/golang.org/x/mod/sumdb/tlog/tlog.go: return fmt.Errorf("tlog: invalid inputs in CheckRecord") cmd/vendor/golang.org/x/mod/sumdb/tlog/tlog.go: return nil, fmt.Errorf("tlog: invalid inputs in ProveTree") cmd/vendor/golang.org/x/mod/sumdb/tlog/tlog.go: return fmt.Errorf("tlog: invalid inputs in CheckTree") cmd/vendor/golang.org/x/mod/zip/zip.go: addError(zf, fmt.Errorf("go.mod file not in module root directory")) cmd/vendor/golang.org/x/sys/unix/syscall_solaris.go: return 0, fmt.Errorf("need to request at least one event or use Pending() instead") cmd/vendor/golang.org/x/tools/go/analysis/internal/analysisflags/flags.go: return fmt.Errorf("want true or false") cmd/vendor/golang.org/x/tools/go/analysis/passes/buildtag/buildtag_old.go: return fmt.Errorf("possible malformed +build comment") cmd/vendor/golang.org/x/tools/go/analysis/passes/buildtag/buildtag_old.go: return fmt.Errorf("+build comment must appear before package clause and be followed by a blank line") cmd/vendor/golang.org/x/tools/go/analysis/passes/buildtag/buildtag_old.go: return fmt.Errorf("possible malformed +build comment") cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go: return fmt.Errorf("empty string") cmd/vendor/golang.org/x/tools/go/analysis/validate.go: return fmt.Errorf("nil *Analyzer") cmd/vendor/golang.org/x/tools/go/types/objectpath/objectpath.go: return nil, fmt.Errorf("empty path") cmd/vet/testdata/unused/unused.go: fmt.Errorf("") // ERROR "result of fmt.Errorf call not used" cmd/vet/vet_test.go: errs = append(errs, fmt.Errorf("Unmatched Errors:")) compress/flate/deflate_test.go: report("", fmt.Errorf("bytes differ after round-tripping")) crypto/tls/auth.go: return 0, 0, fmt.Errorf("tls: Ed25519 public keys are not supported before TLS 1.2") crypto/tls/auth.go: return fmt.Errorf("tls: unsupported certificate: private key is *ed25519.PrivateKey, expected ed25519.PrivateKey") crypto/tls/auth.go: return fmt.Errorf("tls: certificate RSA key size too small for supported signature algorithms") crypto/tls/auth.go: return fmt.Errorf("tls: peer doesn't support the certificate custom signature algorithms") crypto/tls/handshake_client_test.go: return fmt.Errorf("server: got len(VerifiedChains) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(VerifiedChains) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(OCSPResponse) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(SignedCertificateTimestamps) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(OCSPResponse) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(SignedCertificateTimestamps) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(VerifiedChains) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(OCSPResponse) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("client: got len(SignedCertificateTimestamps) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("got len(VerifiedChains) = 0, wanted non-zero") crypto/tls/handshake_client_test.go: return fmt.Errorf("got len(OCSPResponse) = 0, wanted non-zero") crypto/tls/handshake_server_test.go: return fmt.Errorf("expected SessionTicketKey to be set") crypto/tls/handshake_server_test.go: return fmt.Errorf("expected SessionTicketKey to be set") crypto/tls/prf.go: return nil, fmt.Errorf("crypto/tls: ExportKeyingMaterial context too long") crypto/x509/parser.go: return nil, nil, nil, nil, fmt.Errorf("x509: invalid NameConstraints extension") crypto/x509/verify_test.go: hintErr: fmt.Errorf("empty"), database/sql/convert_test.go: return fmt.Errorf("coefficient too large") database/sql/convert_test.go: return fmt.Errorf("coefficient too large") database/sql/driver/types.go: return nil, fmt.Errorf("nil value not allowed") database/sql/driver/types.go: return nil, fmt.Errorf("uint64 values with high bit set are not supported") debug/buildinfo/buildinfo.go: return nil, fmt.Errorf("address not mapped") debug/pe/file.go: return nil, fmt.Errorf("optional header size is less than optional header magic size") debug/pe/symbol.go: return rv, fmt.Errorf("invalid symbol index") debug/pe/symbol.go: return rv, fmt.Errorf("incorrect symbol storage class") debug/pe/symbol.go: return rv, fmt.Errorf("aux symbol unavailable") encoding/asn1/marshal.go: return nil, fmt.Errorf("asn1: cannot marshal nil value") encoding/gob/error.go: error_(fmt.Errorf("gob: "+format, args...)) encoding/gob/gobencdec_test.go: err2 := fmt.Errorf("foo") encoding/json/decode_test.go: return fmt.Errorf("bad quoted string") encoding/json/decode_test.go: return fmt.Errorf("bad hex") encoding/json/decode_test.go: return fmt.Errorf("bad quoted string") encoding/json/decode_test.go: return fmt.Errorf("bad hex") encoding/json/decode_test.go: return fmt.Errorf("bad quoted string") encoding/json/decode_test.go: return fmt.Errorf("bad hex") encoding/json/decode_test.go: return fmt.Errorf("bad quoted string") encoding/json/decode_test.go: return fmt.Errorf("bad hex") encoding/json/decode_test.go: {in: `{"x": 1}`, ptr: new(tx), err: fmt.Errorf("json: unknown field \"x\""), disallowUnknownFields: true}, encoding/json/decode_test.go: {in: `{"Y": 1, "Z": 2}`, ptr: new(T), err: fmt.Errorf("json: unknown field \"Z\""), disallowUnknownFields: true}, encoding/json/decode_test.go: {in: `{"alpha": "abc", "alphabet": "xyz"}`, ptr: new(U), err: fmt.Errorf("json: unknown field \"alphabet\""), disallowUnknownFields: true}, encoding/json/decode_test.go: {in: `{"alphabet": "xyz"}`, ptr: new(U), err: fmt.Errorf("json: unknown field \"alphabet\""), disallowUnknownFields: true}, encoding/json/decode_test.go: err: fmt.Errorf("json: unknown field \"X\""), encoding/json/decode_test.go: err: fmt.Errorf("json: unknown field \"X\""), encoding/json/decode_test.go: err: fmt.Errorf("json: unknown field \"extra\""), encoding/json/decode_test.go: err: fmt.Errorf("json: unknown field \"extra\""), encoding/json/decode_test.go: err: fmt.Errorf("json: cannot set embedded pointer to unexported struct: json.embed1"), encoding/json/decode_test.go: err: fmt.Errorf("json: cannot set embedded pointer to unexported struct: json.embed3"), encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeToken of Comment containing --> marker") encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeToken of ProcInst xml target only valid for xml declaration, first token encoded") encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeToken of ProcInst with invalid Target") encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeToken of ProcInst containing ?> marker") encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeToken of Directive containing wrong < or > markers") encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeToken of invalid token type") encoding/xml/marshal.go: return fmt.Errorf("xml: EncodeElement of StartElement with missing name") encoding/xml/marshal.go: return fmt.Errorf("xml: start tag with no name") encoding/xml/marshal.go: return fmt.Errorf("xml: end tag with no name") encoding/xml/xml_test.go:func (errWriter) Write(p []byte) (n int, err error) { return 0, fmt.Errorf("unwritable") } flag/flag_test.go: return fmt.Errorf("test error") go/doc/doc.go: panic(fmt.Errorf("doc.NewFromFiles: no token.FileSet provided (fset == nil)")) go/doc/doc.go: panic(fmt.Errorf("doc.NewFromFiles: option argument type must be doc.Mode")) go/doc/doc.go: panic(fmt.Errorf("doc.NewFromFiles: there must not be more than 1 option argument")) go/internal/gccgoimporter/ar.go: return nil, fmt.Errorf(".go_export not found in this archive") go/internal/gcimporter/exportdata.go: err = fmt.Errorf("invalid archive header") go/internal/gcimporter/exportdata.go: err = fmt.Errorf("go archive is missing __.PKGDEF") go/internal/gcimporter/exportdata.go: err = fmt.Errorf("not a Go object file") go/types/check_test.go: return fmt.Errorf("flags comment line too long") go/types/resolver.go: return "", fmt.Errorf("empty string") go/types/resolver.go: err = fmt.Errorf("Config.Importer not installed") html/template/template.go:var escapeOK = fmt.Errorf("template escaped correctly") html/template/template.go: return fmt.Errorf("html/template: cannot Parse after Execute") html/template/template.go: return nil, fmt.Errorf("html/template: no files named in call to ParseFiles") image/gif/reader.go: return fmt.Errorf("gif: missing image data") internal/buildcfg/cfg.go: Error = fmt.Errorf("invalid GOAMD64: must be v1, v2, v3, v4") internal/buildcfg/cfg.go: Error = fmt.Errorf("invalid GOARM: must be 5, 6, 7") internal/buildcfg/cfg.go: Error = fmt.Errorf("invalid GOMIPS: must be hardfloat, softfloat") internal/buildcfg/cfg.go: Error = fmt.Errorf("invalid GOMIPS64: must be hardfloat, softfloat") internal/buildcfg/cfg.go: Error = fmt.Errorf("invalid GOPPC64: must be power8, power9") internal/buildcfg/exp.go: return nil, fmt.Errorf("GOEXPERIMENT regabiargs requires regabiwrappers") internal/fuzz/encoding.go: return nil, fmt.Errorf("cannot unmarshal empty string") internal/fuzz/encoding.go: return nil, fmt.Errorf("must include version and at least one value") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected call expression") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected []byte or primitive type") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected []byte") internal/fuzz/encoding.go: return nil, fmt.Errorf("string literal required for type []byte") internal/fuzz/encoding.go: return nil, fmt.Errorf("invalid selector type") internal/fuzz/encoding.go: return nil, fmt.Errorf("invalid selector type") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected []byte or primitive type") internal/fuzz/encoding.go: return nil, fmt.Errorf("malformed bool") internal/fuzz/encoding.go: return nil, fmt.Errorf("true or false required for type bool") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected operation on int or float type") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected operation on int or float type") internal/fuzz/encoding.go: return nil, fmt.Errorf("literal value required for primitive type") internal/fuzz/encoding.go: return nil, fmt.Errorf("literal value required for primitive type") internal/fuzz/encoding.go: return nil, fmt.Errorf("string literal value required for type string") internal/fuzz/encoding.go: return nil, fmt.Errorf("character literal required for byte/rune types") internal/fuzz/encoding.go: return nil, fmt.Errorf("malformed character literal, missing single quotes") internal/fuzz/encoding.go: return nil, fmt.Errorf("can only encode single byte to a byte type") internal/fuzz/encoding.go: return nil, fmt.Errorf("integer literal required for int types") internal/fuzz/encoding.go: return nil, fmt.Errorf("integer literal required for uint types") internal/fuzz/encoding.go: return nil, fmt.Errorf("float or integer literal required for float32 type") internal/fuzz/encoding.go: return nil, fmt.Errorf("float or integer literal required for float64 type") internal/fuzz/encoding.go: return nil, fmt.Errorf("integer literal required for math.Float32frombits type") internal/fuzz/encoding.go: return nil, fmt.Errorf("integer literal required for math.Float64frombits type") internal/fuzz/encoding.go: return nil, fmt.Errorf("expected []byte or primitive type") internal/fuzz/sys_posix.go: return workerComm{}, fmt.Errorf("fuzz temp file exceeds maximum size") internal/fuzz/sys_windows.go: return workerComm{}, fmt.Errorf("GO_TEST_FUZZ_WORKER_HANDLES not set") internal/fuzz/sys_windows.go: return workerComm{}, fmt.Errorf("fuzz temp file exceeds maximum size") internal/fuzz/worker.go: return fuzzResult{}, fmt.Errorf("attempted to minimize a crash but could not reproduce") internal/profile/merge.go: return nil, fmt.Errorf("no profiles to merge") internal/profile/profile.go:var errUnrecognized = fmt.Errorf("unrecognized profile format") internal/profile/profile.go:var errMalformed = fmt.Errorf("malformed profile format") internal/profile/profile.go: return fmt.Errorf("missing sample type information") internal/profile/profile.go: return fmt.Errorf("found mapping with reserved ID=0") internal/profile/profile.go: return fmt.Errorf("found function with reserved ID=0") internal/profile/profile.go: return fmt.Errorf("found location with reserved id=0") internal/testenv/testenv.go: gorootErr = fmt.Errorf("failed to locate GOROOT/src in any parent directory") internal/trace/order.go: return nil, fmt.Errorf("no consistent ordering of events possible") internal/trace/order.go: return nil, fmt.Errorf("stray syscall exit") internal/trace/parser.go: return ParseResult{}, fmt.Errorf("for traces produced by go 1.6 or below, the binary argument must be provided") internal/trace/parser.go: return 0, fmt.Errorf("bad header length") internal/trace/parser.go: return 0, fmt.Errorf("not a trace file") internal/trace/parser.go: return 0, fmt.Errorf("not a trace file") internal/trace/parser.go: err = fmt.Errorf("trace is empty") internal/trace/parser.go: err = fmt.Errorf("no EvFrequency event") internal/trace/parser.go:var ErrTimeOrder = fmt.Errorf("time stamps out of order") internal/xcoff/ar.go: return nil, fmt.Errorf("small AIX archive not supported") internal/xcoff/ar.go: return nil, fmt.Errorf("AIAFMAG not found after member header") internal/xcoff/file.go: return nil, fmt.Errorf("no symbol table") io/io_test.go: rb.err = fmt.Errorf("fake error") math/big/floatconv.go: err = fmt.Errorf("exponent overflow") mime/multipart/multipart.go: return nil, fmt.Errorf("multipart: boundary is empty") net/dnsclient_unix_test.go: return dnsmessage.Message{}, &OpError{Op: "write", Err: fmt.Errorf("socket on fire")} net/http/client_test.go: return nil, fmt.Errorf("Read: n=0 with err=nil") net/http/client_test.go: CheckRedirect: func(*Request, []*Request) error { return fmt.Errorf("no redirects!") }, net/http/clientserver_test.go: return fmt.Errorf("wanted a stack trace logged; got nothing") net/http/h2_bundle.go: return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher (need at least one of TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 or TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)") net/http/httputil/reverseproxy.go: p.getErrorHandler()(rw, req, fmt.Errorf("internal error: 101 switching protocols response with non-writable body")) net/http/httputil/reverseproxy_test.go: return fmt.Errorf("tried to by-pass proxy") net/http/internal/chunked_test.go: readErr := fmt.Errorf("chunk end read error") net/http/request.go: return nil, fmt.Errorf("too many Host headers") net/http/serve_test.go: return fmt.Errorf("http2 Get #2 expected error, got nil") net/sendfile_test.go: err = fmt.Errorf("expected ErrDeadlineExceeded, but got nil") net/textproto/reader.go: return nil, fmt.Errorf("missing validateFirstLine func") net/url/url.go: err = fmt.Errorf("invalid semicolon separator in query") runtime/debug/mod.go: return nil, fmt.Errorf("replacement with no module on previous line") runtime/debug/mod.go: return nil, fmt.Errorf("build line missing '='") runtime/debug/mod.go: return nil, fmt.Errorf("build line with missing key") runtime/debug/mod.go: return nil, fmt.Errorf("invalid quoted key in build line") runtime/debug/mod.go: return nil, fmt.Errorf("build line missing '=' after quoted key") runtime/debug/mod.go: return nil, fmt.Errorf("build line missing '=' after key") runtime/debug/mod.go: return nil, fmt.Errorf("invalid quoted value in build line") runtime/malloc_test.go: return fmt.Errorf("zero value") runtime/pprof/pprof.go: return fmt.Errorf("cpu profiling already in use") runtime/pprof/pprof_test.go: return fmt.Errorf("Expected both time reports to be positive") runtime/pprof/proto.go: return fmt.Errorf("truncated profile") runtime/pprof/proto.go: return fmt.Errorf("malformed profile") runtime/pprof/proto.go: return fmt.Errorf("truncated profile") runtime/pprof/proto.go: return fmt.Errorf("malformed profile") runtime/pprof/proto.go: return fmt.Errorf("mismatched profile records and tags") runtime/pprof/proto.go: return fmt.Errorf("mismatched profile records and tags") runtime/race/race_test.go: return out, fmt.Errorf("runtime fatal error") runtime/syscall_windows_test.go: return 0, fmt.Errorf("cpu affinity mask is empty") runtime/testdata/testprog/numcpu_freebsd.go: return fmt.Errorf("could not check against an empty CPU list") strings/replace_test.go: return 0, fmt.Errorf("unwritable") testing/benchmark.go: return fmt.Errorf("invalid count") testing/benchmark.go: return fmt.Errorf("invalid duration") text/template/funcs.go: return 0, fmt.Errorf("cannot index slice/array with nil") text/template/funcs.go: return reflect.Value{}, fmt.Errorf("index of untyped nil") text/template/funcs.go: return reflect.Value{}, fmt.Errorf("index of nil pointer") text/template/funcs.go: return reflect.Value{}, fmt.Errorf("slice of untyped nil") text/template/funcs.go: return reflect.Value{}, fmt.Errorf("cannot 3-index slice a string") text/template/funcs.go: return 0, fmt.Errorf("len of nil pointer") text/template/funcs.go: return reflect.Value{}, fmt.Errorf("call of nil") text/template/helper.go: return nil, fmt.Errorf("template: no files named in call to ParseFiles") time/zoneinfo_test.go: panic(fmt.Errorf("zoneinfo initialized before first LoadLocation")) vendor/golang.org/x/crypto/curve25519/curve25519.go: return nil, fmt.Errorf("bad input point: low order point") vendor/golang.org/x/text/unicode/bidi/core.go: return fmt.Errorf("types is null") vendor/golang.org/x/text/unicode/bidi/core.go: return fmt.Errorf("pairTypes is null") vendor/golang.org/x/text/unicode/bidi/core.go: return fmt.Errorf("pairValues is null") vendor/golang.org/x/text/unicode/bidi/core.go: return fmt.Errorf("pairTypes is different length from pairValues") vendor/golang.org/x/text/unicode/norm/iter.go: return 0, fmt.Errorf("norm: invalid whence") vendor/golang.org/x/text/unicode/norm/iter.go: return 0, fmt.Errorf("norm: negative position") ```

At Orijtech Inc we've got a patch to add this functionality for Go1.19
@odeke-em odeke-em added this to the Go1.19 milestone May 3, 2022
@robpike
Copy link
Contributor

robpike commented May 4, 2022

Since this is an efficiency issue and not a correctness one, vet is not the place to address this, but doing the check in another tool - or just grepping, as you point out - would be a fine solution.

odeke-em added a commit to orijtech/otils that referenced this issue May 4, 2022
@odeke-em
fmterrorfix: tooling to automatically rewrite fmt.Errorf to errors.Ne…
9d89b37 
…w if needed

Implements a CLI tool that'll traverse a directory, find all patterns
matching fmt.Errorf without format specifiers and replace in-place such
with errors.New. For example for the Go standard library

```shell
go run main.go $GOPATH/src/go.googlesource.com
```

Updates golang/go#52696
@odeke-em odeke-em mentioned this issue May 4, 2022
Merged
odeke-em added a commit to orijtech/otils that referenced this issue May 4, 2022
@odeke-em
fmterrorfix: tooling to automatically rewrite fmt.Errorf to errors.Ne…
619f3d0 
…w if needed

Implements a CLI tool that'll traverse a directory, find all patterns
matching fmt.Errorf without format specifiers and replace in-place such
with errors.New. For example for the Go standard library

```shell
go run main.go $GOPATH/src/go.googlesource.com
```

Updates golang/go#52696
@gopherbot
Copy link

Change https://go.dev/cl/403938 mentions this issue: all: replace superflous fmt.Errorf with errors.New

@gopherbot
Copy link

Change https://go.dev/cl/403955 mentions this issue: go/analysis/printf: recommend superfluous fmt.Errorf with errors.New

@odeke-em
Copy link
Member Author

odeke-em commented May 4, 2022

Since this is an efficiency issue and not a correctness one, vet is not the place to address this, but doing the check in another tool - or just grepping, as you point out - would be a fine solution.

I did write such a tool indeed at https://github.com/orijtech/otils/blob/master/fmterrorffix/main.go to find and replace the patterns.

However, @robpike the problem with not adding this pass to the standard Go tool chain is that letting our users actually invoke fmt.Errorf("<CONSTANT>") without a way to enforce it in the biggest distribution of Go toolchain promotes writing inefficient code and bloat; code reviews can't easily catch such issues as evidenced by the fact that even the Go project has lots of such calls. Making the modification to the printf static analyzer is minimal yet provides a high return on investment as per my CL https://go-review.googlesource.com/c/tools/+/403955

@leitzler
Copy link
Contributor

leitzler commented May 4, 2022

Isn't the increased binary size only due to importing fmt? If the package already uses fmt.Errorf() elsewhere the size should stay the same, right?

I've seen cases where fmt.Errorf is used even without formatting directives and errors isn't imported at all, purely for consistency. Adding this enforcement to the analyser also leaks into gopls that could get pretty chatty.

We should at least not flag cases where it doesn't matter imo, it feels more like an opinion than something that actually makes a difference.

@komuw
Copy link
Contributor

komuw commented May 4, 2022

related; #6853

@rsc
Copy link
Contributor

rsc commented May 4, 2022

Thanks for raising the issue, but please don't do this. Using fmt.Errorf("foo") is completely fine, especially in a program where all the errors are constructed with fmt.Errorf. Having to mentally switch between two functions based on the argument is unnecessary noise. Vet is for important correctness errors. fmt.Errorf("foo") is correct as is.

If you want to proceed with this vet change, it should be a proposal, but it's unlikely to be accepted.

@rsc
Copy link
Contributor

rsc commented May 4, 2022 •
edited

The binary size change is dramatic only because this is a trivial program that does nothing at all. So deleting the one reference to fmt let the binary not link fmt at all. But any real program is already linking in fmt because of one library or another. Removing a single call to fmt.Errorf in such a program will have no appreciable effect on binary size.

@rsc
Copy link
Contributor

rsc commented May 4, 2022

This was also discussed, but not for long, as #17173.

@zpavlinovic zpavlinovic added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label May 4, 2022
@dr2chase dr2chase modified the milestones: Go1.19, Backlog May 5, 2022
@heschi heschi added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 9, 2022
@adonovan
Copy link
Member

As others have pointed out, this is not actually an inefficiency in practice since every program links in the fmt package.

@golang golang locked and limited conversation to collaborators May 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
10 participants
@rsc @zpavlinovic @dr2chase @leitzler @robpike @odeke-em @komuw @adonovan @gopherbot @heschi