Skip to content

x/tools/go/analysis/printf: doc: explain "possible formatting directive" message in documentation #66733

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
adonovan opened this issue Apr 8, 2024 · 8 comments
Assignees
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Apr 8, 2024

The printf checker reports a diagnostic for calls like this:

fmt.Print("hello, %s") // "%s call has possible Printf formatting directive %s"

(Aside: though it's easy to conceive of false positives for this heuristic, in practice it seems to be very reliable.)

In Google issue 302359716, a number of users tell us that the message makes them think the problem is in the format string, when really it's in the choice of formatting function. Perhaps the error message could give them a hint:

fmt.Print("hello, %s") // "fmt.Print call has possible formatting directive %s; use fmt.Printf(...) or fmt.Printf("%s", ...)"

The error message could vary depending on whether the corresponding 'f' variant function exists. The main challenge is deciding the exact wording; these messages have been agonized over for a decade.

@robpike

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Apr 8, 2024
@gopherbot gopherbot added this to the Unreleased milestone Apr 8, 2024
@adonovan adonovan added Analysis Issues related to static analysis (vet, x/tools/go/analysis) and removed Tools This label describes issues relating to any tools in the x/tools repository. labels Apr 8, 2024
@robpike
Copy link
Contributor

robpike commented Apr 8, 2024

Print call has Printf formatting directive.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 9, 2024
@adonovan
Copy link
Member Author

adonovan commented Apr 9, 2024

Print call has Printf formatting directive.

Do you mean that you would prefer the message to say even less than it does currently, by removing the "possible" qualifier?

@rsc
Copy link
Contributor

rsc commented Apr 10, 2024

It would be good to keep the message short. Let's work on making the documentation link that accompanies this message do a better job of calling out the mistake.

@adonovan adonovan changed the title x/tools/go/analysis/printf: improve "possible formatting directive" message x/tools/go/analysis/printf: doc: explain "possible formatting directive" message in documentation Apr 10, 2024
@adonovan
Copy link
Member Author

OK, I'll treat this as a documentation problem and update the printf docs.

@adonovan adonovan added Documentation Issues describing a change to documentation. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 10, 2024
@adonovan adonovan self-assigned this Apr 10, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/578035 mentions this issue: go/analysis/passes/printf: elaborate the documentation

@findleyr
Copy link
Member

See also #47872.

@findleyr
Copy link
Member

...and, somewhat related, https://go.dev/cl/422854, derived from the google internal b/227741360.

While the printf analyzer is among the most useful vet checks, it does tend to be confusing for new users.

@nharper
Copy link

nharper commented Apr 10, 2024

As someone who ran into difficulty with this message, the improvement that would help most (either in the message or the linked documentation) is a reminder that the caller probably meant to call a different function - the one that takes a formatting string instead of the one that doesn't, e.g. "Print call has Printf formatting directive. Call Printf instead of Print".

@golang golang locked and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

7 participants