Consider to support more strconv functions

[Antonboom]

Hi!

Maybe you will be interested in:

1. https://pkg.go.dev/strconv#FormatComplex
2. https://pkg.go.dev/strconv#FormatFloat
3. https://pkg.go.dev/strconv#Unquote
4. https://pkg.go.dev/strconv#Quote (and other Quote*)

[catenacyber]

As mentioned in #2 this linter is more about fmt.Sprint than strconv

Where do you see strconv.Quote or strconv.Unquote being faster than some current code ?

FormatComplex and FormatFloat looked more powerful functions with their precision argument than something optimized to be fast.
Do you have examples ?

[Antonboom]

Do you have examples ?

The first ones that came to mind:

fmt.Sprintf(`"%s"`, "hello")
fmt.Sprintf("%q", "hello")
strconv.Quote("hello")
`"` + "hello" + `"`

fmt.Sprintf("`%s`", "hello")
fmt.Sprintf("%#q", "hello")
"`" + "hello" + "`"

fmt.Sprintf("%q", 91)
"'['"

strings.Trim(`"hello"`, `"`)
strconv.Unquote(`"hello"`)

fmt.Sprintf("%f", 42.42) // And a lot of other verbs.
strconv.FormatFloat(42.42, 'f', 6, 64)

fmt.Println(fmt.Sprintf("%e", 42i+42)) // And other verbs.
fmt.Println(strconv.FormatComplex(42i+42, 'e', 6, 64))

[catenacyber]

Not all are equivalent cf https://go.dev/play/p/9s4GhmIBEz_J

What I see in some codebase is fmt.Sprintf("constantstring%s" like 0x%x

[catenacyber]

What about ea8b325 ?

[Antonboom]

Offtop:

Please, make a PR in the next time :)
It is more suitable for review

[ldez]

Offtopic:

The latest version (v0.2.2) contains features and breaking changes but the version was increased as a bug fix.
Your linter is used as a library, so all the exported elements are a part of the public API and should not changed without clear information about them.

It's better for us if you follow semver:

• breaking changes -> major (vX.y.y)
• new features -> minor (vy.X.y)
• bug fixes -> patch (vy.y.X)

[catenacyber]

Thanks @ldez
I will do a 0.3 with some more features

I did 0.2.1 as suggested here
#7 (comment)
but @Antonboom did not have the full context I guess

[Antonboom]

Yes, sorry for the confusion, I suggested a patch version because I didn't consider the flag as a new feature (with a default value equal to the existing behavior). I didn't know about other changes.

[Antonboom]

@catenacyber about ea8b325

1. I left some comments
2. It looks like thing that is not related to your linter

• The change is questionable, because sometimes people use fmt.Errorf in any case for consistency. But set of linters checks is not configurable.
• Usually error path is not performance bottleneck, because it happens much less often.

Also we already have the similar checks:

• https://staticcheck.dev/docs/checks/#S1028
• https://go-critic.com/overview.html#dynamicfmtstring (propose to add "%s" oO)
• https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#errorf
• https://github.com/Djarvur/go-err113#reports (do not define dynamic errors, use wrapped static errors instead)

And this is another one reason for pull requests instead of commits.

Anyway, as u wish.

[catenacyber]

The change is questionable, because sometimes people use fmt.Errorf in any case for consistency. But set of linters checks is not configurable.

Ok, adding an option to configure this

Usually error path is not performance bottleneck, because it happens much less often.

Indeed, and I hope fmt.Sprintf("%d" is neither a performance bottleneck. But 0.5% CPU improvement is always good to take

[catenacyber]

https://staticcheck.dev/docs/checks/#S1028
https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#errorf

Eheh, perfsprint will optimize the fmt.Sprintf inside errors.New first

https://go-critic.com/overview.html#dynamicfmtstring

Interesting

https://github.com/Djarvur/go-err113#reports

Looks a different class

And this is another one reason for pull requests instead of commits.

I have been doing that since

[catenacyber]

Is there something clear to do her e?

[Antonboom]

Thanks for feedback and related changes 🙏