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

cmd/vet: Always errors if printf-style function from an imported package is used #14754

Closed
spenczar opened this issue Mar 10, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@spenczar
Copy link
Contributor

commented Mar 10, 2016

This is a regression pointed out by @valyala in #12294 (comment).

https://golang.org/cl/20163 does not work. vet is unable to find the signature of any functions or methods which aren't defined in the current package being vetted, so it flags these functions/methods as always having 'no formatting directive'.

The ideal answer, from a vet user's perspective, would seem to be to have vet import those packages and look up their function signature, but (as far as I can understand) that would expand what vet does pretty dramatically and could make it slower. Right now, it doesn't seem to have a build context, so importing isn't straightforward. I would be happy to be wrong about this, though!

If I'm not wrong and importing is not in the cards, then the right thing might be to just revert the changes introduced in https://golang.org/cl/20163, or we could just assume sane defaults if we're unable to find the function signature, using the old, pre-CL20163 printfList which hardcoded the expected position of the format param.

@spenczar spenczar changed the title cmd/vet: Always errors if printf-style function from an external package is used cmd/vet: Always errors if printf-style function from an imported package is used Mar 10, 2016

@spenczar

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2016

As I look at this, it seems that reverting is the only sane way forward. Calculating the signatures of the functions (as we do now) makes the code more complex with very little benefit, since the original change was attempting to support libraries which use custom printf-style functions, like gRPC's Errorf. If we can't effectively vet imported functions, we can't vet those, and we may as well return to a hardcoded list.

So, I'd like to revert, but I want someone with more experience with the ast and vet packages to weigh in to correct me if it's possible to follow imports simply.

@valyala

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2016

Vet could inspect function arguments by searching for the fisrt string literal and assume this is format string. There is no need in function type knowledge in this case.

@valyala

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2016

@spenczar , I prepared a new CL, which scans call arguments for printf-like format string instead of scanning function type arguments.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 22, 2016

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

@gopherbot gopherbot closed this in ee1b90a Mar 29, 2016

@golang golang locked and limited conversation to collaborators Mar 30, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.