Skip to content

cmd/vet, gopls: report function value printf wrappers #71398

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

Closed
cwedgwood opened this issue Jan 22, 2025 · 6 comments
Closed

cmd/vet, gopls: report function value printf wrappers #71398

cwedgwood opened this issue Jan 22, 2025 · 6 comments
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@cwedgwood
Copy link
Contributor

Go version

go1.23.4 linux/amd64

Output of go env in your module/workspace:

""

What did you do?

Using gopls (devel) version from a couple of days ago.

Possibly related: #53131

gopls will detect problems with fmt.Printf and similar functions, but if the function is declared via a variable this doesn't seem to occur.

This is a simple demonstration (the real-life use case is more complicated):

package main

import "fmt"

func main() {
        p := fmt.Printf
        fmt.Printf("%d") // gopls will report this
        p("%d")          // not reported
}

What did you see happen?

No check performed for the call using p.

What did you expect to see?

I would like to see the function via pointer for p checked as fmt.Printf is.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 22, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 22, 2025
@findleyr findleyr changed the title x/tools/gopls: Printf wrappers not checking if a variable cmd/vet, gopls: report function value printf wrappers Jan 22, 2025
@findleyr findleyr added the FeatureRequest Issues asking for a new feature that does not need a proposal. label Jan 22, 2025
@cwedgwood
Copy link
Contributor Author

(@findleyr thanks, I realized after pressing submit I created the issue for vet when it was originally intended for gopls)

@findleyr
Copy link
Member

findleyr commented Jan 23, 2025

#53131 is not really related; the current behavior is expected, as the analyzer doesn't consider function values. Tracking function values is a different and significantly more complicated problem.

This could be rather difficult to implement, depending on how sophisticated the analysis. We could do a limited pointer analysis within function bodies, but that wouldn't be feasible for e.g. function parameters or package variables that could be set outside the current package. The latter two patterns are probably most common, such as when setting or passing in a logger.

This may be an area where annotations could be useful. For example: if a function documents that its logf func(string, ...any) parameter is used to format log messages, it is really a property of the API that logf is a printf wrapper.

CC @adonovan

@findleyr
Copy link
Member

Quick take: this is probably simultaneously less common and more complicated than other forms of printf wrapping, and therefore may be hard to prioritize or commit to supporting. However, we've discussed various forms of annotation in the past, and that would both address this problem and allow for establishing invariants that can't be statically derived--IMO that may be a more promising approach to consider. (I'm not saying that we should necessarily add annotations, just that they may be a better solution for this specific problem).

@adonovan
Copy link
Member

As a workaround, if you can arrange your code so that you mostly don't call p directly but via a wrapper function, you can contrive for that wrapper to be treated as printf-like by the analyzer like so:

var p = ...

func wrapperf(format string, args...any) {
   if false { fmt.Sprintf(format, args...) } //  has the effect of annotating wrapperf as "printf-like"
   p(format, args...) //  does the actual work
}

Of course, you have to be aware of the issue.

@findleyr findleyr removed the gopls Issues related to the Go language server, gopls. label Jan 23, 2025
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jan 23, 2025
@adonovan
Copy link
Member

@adonovan
Copy link
Member

We are not going to attempt to track the flow of function values through variables (we once had a pointer analysis but it was not efficient, precise enough, or sustainable), so I think this issue can be reduced to a request for a way to annotate certain func types or interface methods as printf like. Therefore I will close this issue as a dup of #58340.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants