Fix use of format string functions with constant string

[NathanChase22]

In accordance with recent updates to the Go lang vet checker which flags whenever printf statements and their variations are formatted like this fmt.printf(var) where var is a non-constant string variable.

Source code is changed such that in these cases the statement is changed to: fmt.printf("%s",var)

I believe this helps communicate that the variable in the statement is a no-arg string variable.

Result: calling make unit-test with Go 1.24.1 no longer fails to build. However for the sake of maintaining compatibility the go.mod file still has the original version requirement

[stgraber]

Hmm, I see a lot of those that could be handled in a more efficient way.

For the cases that look like fmt.Sprintf("%s\r\n", i18n.G("something")), doing fmt.Print(i18n.G("something")+"\r\n") will be more efficient.

Same goes for the error handling case, instead of doing fmt.Errorf("%s", "Fixed error") or similar, you can do errors.New("Fixed error").

The goal of the new vet warning is to limit calls to otherwise expensive Printf style functions when not strictly needed. Using a work around to keep using the slow path is counterproductive. If we're going to be updating all those function calls, we may as well switch to the more efficient format.

[NathanChase22]

For the cases that look like fmt.Sprintf("%s\r\n", i18n.G("something")), doing fmt.Print(i18n.G("something")+"\r\n") will be more efficient.

This is because Sprintf() has formatting overhead that is unnecessary for the given use case?

Same goes for the error handling case, instead of doing fmt.Errorf("%s", "Fixed error") or similar, you can do errors.New("Fixed error").

Likewise using the errors package is preferable because we are doing a static string, meaning the formatting overhead of Errorf is unneeded?

The goal of the new vet warning is to limit calls to otherwise expensive Printf style functions when not strictly needed. Using a work around to keep using the slow path is counterproductive. If we're going to be updating all those function calls, we may as well switch to the more efficient format.

With this is in mind I can make those changes

EDIT: then cases such as api.StatusErrorf(http.StatusLocked, err.Error()) where it's a wrapper for Sprintf() I believe making it api.StatusErrorf(http.StatusLocked, "%s", err.Error()) is okay right? Since the error message is non-static but the compiler wants the formatting to be constant.

[stgraber]

This is because Sprintf() has formatting overhead that is unnecessary for the given use case?

Correct. When we're passing it a fixed string without any formatted arguments, it causes Go to look through the string for any % type tokens that it needs to replace, effectively wasting CPU time.

Likewise using the errors package is preferable because we are doing a static string, meaning the formatting overhead of Errorf is unneeded?

Yep, same thing, fmt.Errorf needs to parse character by character to handle format strings. That's a waste of CPU time when there is none, so for any fixed strings, errors.New is to be preferred.

[stgraber]

api.StatusErrorf(http.StatusLocked, err.Error()) is triggering the go vet check too?

I would have expect it to be limited to Go's own functions, so fmt.Printf, fmt.Errorf, ..., not our own functions.

If go vet goes deep enough to see that api.StatusErrorf is a wrapper around fmt.Errorf, then we'd probably want to introduce a api.StatusError which doesn't take a format string and instead only takes fixed errors.

[NathanChase22]

api.StatusErrorf(http.StatusLocked, err.Error()) is triggering the go vet check too?

I would have expect it to be limited to Go's own functions, so fmt.Printf, fmt.Errorf, ..., not our own functions.

If go vet goes deep enough to see that api.StatusErrorf is a wrapper around fmt.Errorf, then we'd probably want to introduce a api.StatusError which doesn't take a format string and instead only takes fixed errors.

Looking at the actual implementation of api.StatusErrorf it checks whether there aren't any arguments and circumvents the call to fmt.Sprintf() , meaning in runtime the static strings aren't going through unnecessary formatting steps. Though this isn't caught by go vet

EDIT: also if we were to create a new function like StatusError() that will conflict with the struct that's called the same thing. I wasn't sure whether we should be renaming struct names in order to make this minor change

[stgraber]

Thanks for this. It took me a little while to review all of it but I've now done so and also took the opportunity to slice it into smaller commits which will help with backporting to the LTS branch.