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: printf checker does not know about fmt.Formatter #17057

Closed
josharian opened this issue Sep 10, 2016 · 3 comments
Closed

cmd/vet: printf checker does not know about fmt.Formatter #17057

josharian opened this issue Sep 10, 2016 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9

Comments

@josharian
Copy link
Contributor

The printf checker should allow any verb for a value that implements fmt.Formatter. It does not.

package main

import "fmt"

type T int

func (t T) Format(f fmt.State, c rune) {
    f.Write([]byte{byte(c)})
}

func main() {
    var t T
    fmt.Printf("%z\n", t)
}
$ go run formatter.go 
z
$ go tool vet formatter.go 
formatter.go:13: unrecognized printf verb 'z'

As an aside, it looks like the implementation of fmt.Stringer detection in cmd/vet could be simplified by using go/types-based interface satisfaction analysis. (See the copylock check for an example.) I suspect the printf check was written before go/types had that functionality.

cc @valyala @robpike
@josharian josharian added this to the Go1.8 milestone Sep 10, 2016
@josharian
Copy link
Contributor Author

When this is fixed, we should revert CL 28959.

gopherbot pushed a commit that referenced this issue Sep 10, 2016
@josharian
cmd/vet: ignore printf failures in cmd
f15915a 
This is a temporary measure to work around #17057.
It will be reverted when #17057 is fixed.

Change-Id: I21c02f63f3530774c91065cfed5d9c566839ed9f
Reviewed-on: https://go-review.googlesource.com/28959
Reviewed-by: Rob Pike <r@golang.org>
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 6, 2016
@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 3, 2016
@josharian josharian mentioned this issue Nov 4, 2016
Closed
@gopherbot
Copy link

CL https://golang.org/cl/32851 mentions this issue.

gopherbot pushed a commit that referenced this issue Nov 13, 2016
@JayNakrani @robpike
cmd/vet: ignore unrecognized verbs for fmt.Formatter
662d253 
Updates #17057.

Change-Id: I54c838d3a44007d4023754e42971e91bfb5e8612
Reviewed-on: https://go-review.googlesource.com/32851
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/33166 mentions this issue.

gopherbot pushed a commit that referenced this issue Nov 13, 2016
@josharian @bradfitz
Revert "cmd/vet: ignore printf failures in cmd"
f8dc4f2 
This reverts commit f15915a.

CL 32851 fixed cmd/vet's handling of fmt.Formatter.

Updates #17057.

Change-Id: I3409100d16037645946fe7fe78fbb173e1648494
Reviewed-on: https://go-review.googlesource.com/33166
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@robpike robpike closed this as completed Nov 13, 2016
@golang golang locked and limited conversation to collaborators Nov 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
5 participants
@josharian @rsc @quentinmit @robpike @gopherbot