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

fmt: error verb rune 'w', overriding/breaking fmt.formatter implementation #56562

Open
splace opened this issue Nov 4, 2022 · 6 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@splace
Copy link

splace commented Nov 4, 2022

https://go.dev/play/p/eM3d8y2L768

What did you expect to see?

s
w

What did you see instead?

s
%!w(main.T2={})
@splace splace changed the title affected/package:fmt error verb rune 'w', overriding/breaking fmt.formatter implementation fmt error verb rune 'w', overriding/breaking fmt.formatter implementation Nov 4, 2022
@ianlancetaylor ianlancetaylor changed the title fmt error verb rune 'w', overriding/breaking fmt.formatter implementation fmt: error verb rune 'w', overriding/breaking fmt.formatter implementation Nov 4, 2022
@robpike
Copy link
Contributor

robpike commented Nov 5, 2022

This looks like a bug in the checking of 'w', which occurs before seeing if the type implements Formatter.

@neild
Copy link
Contributor

neild commented Nov 5, 2022

fmt.Errorf will convert 'w' to 'v' before calling a Format method, since 'w' is intended to be a synonym for 'v' other than indicating that an operand be wrapped. We didn't want existing errors that implement Formatter to need to check for 'w'.

There's also a vet check that verifies that the operand to 'w' is an error, and that '%w' is not used with any formatting function other than Errorf.

Perhaps the bug is that we don't document that it is not valid to use '%w' with any function other than Errorf.

@splace
Copy link
Author

splace commented Nov 5, 2022

This looks like a bug in the checking of 'w', which occurs before seeing if the type implements Formatter.

reported because seemed like it had to be that, because its hidden by its rarity and also because it didn't seem too 'invasive' to fix.

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 7, 2022
@mknyszek mknyszek added this to the Backlog milestone Nov 7, 2022
@ericlagergren
Copy link
Contributor

Perhaps the bug is that we don't document that it is not valid to use '%w' with any function other than Errorf.

Why is '%w' special in this regard?

@neild
Copy link
Contributor

neild commented Nov 7, 2022

'%w' is the error wrapping verb. It indicates that fmt.Errorf should return an error that unwraps to the target of '%w', and is otherwise a synonym for '%v'.

Passing '%w' to any formatting function other than fmt.Errorf is generally indicative of a mistake; either the user intended to wrap the error (in which case they need to use Errorf) or they didn't (in which case '%v' is a clearer indication of intent). There's a vet check for passing '%w' to a formatter other than Errorf, so I'd expect this to occur rarely if ever in practice.)

@splace
Copy link
Author

splace commented May 22, 2023

just tried to use rune 'p' in a bespoke formatter (for percentage)

also seems to be blocked from working because of its use in the non-overridden default code. as pointer type formatter!

why implementing fmt.Formatter doesn't simply override the default is surprising.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants