search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.59k stars 17.61k forks source link

cmd/vet: check against fmt.Errorf uses with more than one %w #34062

Closed ainar-g closed 5 years ago

ainar-g commented 5 years ago 
fmt.Println(fmt.Errorf("bad: %w %w", errors.New(""), errors.New("")))

https://play.golang.org/p/Tk4meXDxL7b

I expected Go 1.13's vet to mark this as a bad format, but it doesn't seem to do that. I think it should.

katiehockman commented 5 years ago

/cc @alandonovan

alandonovan commented 5 years ago

Why? Each verb is accompanied by a value of the appropriate type.

ainar-g commented 5 years ago

Are you asking, why should this be marked as an error at all or why it should be in vet? If the former, because only one %w is allowed, per the fmt docs. Some people were already confused, see #34060. As for the latter, vet already checks format strings, so it seems like an appropriate place.

alandonovan commented 5 years ago

Ah, I didn't realize fmt permitted only one. Then yes, that's an easy and appropriate thing for vet to catch.

hasitpbhatt commented 5 years ago

If it's not super critical, I'd like to take a stab at it.

mtricht commented 5 years ago

The following CL already provides this functionality with tests included: https://go-review.googlesource.com/c/tools/+/177601 The vendored version of golang.org/x/tools in src/cmd refers to the version 25a4f137592f which does not contain this functionality. Updating golang.org/x/tools in there should solve this.

Hopefully this helps anyone, I'm rather new to this repository. I tried updating the vendored x/tools with:

cd src/cmd
go get -d golang.org/x/tools@latest
go mod tidy
go mod vendor

as README.vendor suggests me to, but this only resulted in the error no dependencies to vendor, the vendor directory being deleted and thus the build failing. I'm sure someone smarter than me will solve this in no time.

hasitpbhatt commented 5 years ago

@alandonovan Due to https://github.com/golang/go/issues/34191, could not check for this specific test "TestScript". Change can be found in https://polymer2-go-review.googlesource.com/c/go/+/196843

gopherbot commented 5 years ago

Change https://golang.org/cl/196843 mentions this issue: cmd: update golang.org/x/tools

gopherbot commented 5 years ago

Change https://golang.org/cl/197338 mentions this issue: go/analysis: fix vet errors