Report slow string convertions using fmt

[Jacalz]

I think it would be very good if staticcheck could flag cases where people are using fmt for just simply converting to and from strings. It was found in my PR for fyne-io/fyne (fyne-io/fyne#2142 (comment)) that the strconv package generally is twice as fast as the fmt package for converting to and from strings. While it is very easy and simple to do so, I think it is a poor decision most of the time.

Below are a few examples of what I would suggest flagging:

// With a single format and equivalent format type to what is being passed.
fmt.Sprintf("%", val)

// Just passing one variable.
fmt.Sprint(val)
fmt.Sprintln(val)

// With a single format and equivalent format type to what is being passed.
fmt.Sscanf(str, "%", &val)

// Just passing one variable.
fmt.Scan(&val)
fmt.Scanln(&val)

All these cases can quite easily be changed to using strconv for pretty big performance improvements.

[Jacalz]

Similar to go-critic/go-critic#873.

[dominikh]

The problem with checks like these is that performance doesn't always matter. In a lot of contexts, consistency with surrounding code is more important.

[Jacalz]

Maybe, but I think this is an easy win in performance without loosing readability. I would almost argue that it, if anything, actually improves readability of the code in question. With using strconv the code is verbosely saying that you are converting to something else.

[TomOnTime]

I frequently see fmt.Sprintf("%d", i) when strconv.Itoa(i) would be better.

If staticcheck does S1025 - Don’t use fmt.Sprintf("%s", x) unnecessarily , it should do this too.

[dominikh]

I disagree. I see two differences between that and S1025:

• Changing to strconv may require importing a package that wasn't previously imported.
• S1025 objectively simplifies code. It replaces calls to fmt.Sprintf with more primitive operations, such as conversions. In contrast, this check would switch from one library function to another, neither of which is necessarily simpler than the other.

[ainar-g]

Sounds like this could be an aggressive check though? This definitely sounds like something I tend to notice during code reviews.

[TomOnTime]

• Changing to strconv may require importing a package that wasn't previously imported.

fmt.Sprintf imports strconv. I don't see the downside here but I'm interested in learning what the difference would be.

• S1025 objectively simplifies code. It replaces calls to fmt.Sprintf with more primitive operations, such as conversions. In contrast, this check would switch from one library function to another, neither of which is necessarily simpler than the other.

Are we talking about the same thing? The change from from fmt.Sprintf("%d", i) to strconv.Itoa(i) seems to be the same kind of simplification.

[dominikh]

I don't consider one to be simpler than the other. Faster, maybe, but not simpler. Especially in code that is concerned with output, not computation, using fmt seems much more natural to me.

[Jacalz]

I'd suggest adding another one to the list here. I've seen a lot of usages of fmt.Sprintf("%x", someKindOfHash) when it's both a lot clearer (in my opinion) and about 30% faster to use hex.EncodeToString(someKindOfHash).

[Antonboom]

Covered by golangci/golangci-lint#3714