catenacyber / perfsprint

Golang linter to use strconv
MIT License
20 stars 2 forks source link

Explain the rules #28

Open AndersBennedsgaard opened 3 months ago

AndersBennedsgaard commented 3 months ago

It would be nice with a short explanation of what each rule is, why it is useful, and how an example solution could be. Since the number of checks are quite small, it could just be a hardcoded list in the README.

catenacyber commented 3 months ago

make bench shows all the transformations, and justifies them...

If you want them in the README in another format, a PR is welcome :-)

AndersBennedsgaard commented 2 months ago

No it doesn't? It runs Go benchmarks:

└> make bench                      
go test -bench=Bench ./...
?       github.com/catenacyber/perfsprint   [no test files]
goos: linux
goarch: amd64
pkg: github.com/catenacyber/perfsprint/analyzer
cpu: 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
BenchmarkStringFormatting/fmt.Sprint-16     25345364            41.99 ns/op        5 B/op          1 allocs/op
BenchmarkStringFormatting/fmt.Sprintf-16            24310509            46.64 ns/op        5 B/op          1 allocs/op
BenchmarkStringFormatting/just_string-16            1000000000           0.2542 ns/op       0 B/op         0 allocs/op
BenchmarkErrorFormatting/fmt.Sprint-16              12080151           105.8 ns/op        32 B/op          1 allocs/op
BenchmarkErrorFormatting/fmt.Sprintf-16             10296699           118.9 ns/op        32 B/op          1 allocs/op
BenchmarkErrorFormatting/Error()-16                 796787808            1.443 ns/op           0 B/op          0 allocs/op
BenchmarkFormattingError/fmt.Errorf-16              15397543           101.6 ns/op        32 B/op          2 allocs/op
BenchmarkFormattingError/errors.New-16              1000000000           0.2474 ns/op       0 B/op         0 allocs/op
BenchmarkBoolFormatting/fmt.Sprint-16               26476066            40.09 ns/op        4 B/op          1 allocs/op
BenchmarkBoolFormatting/fmt.Sprintf-16              29117131            41.23 ns/op        4 B/op          1 allocs/op
BenchmarkBoolFormatting/strconv.FormatBool-16       1000000000           0.2371 ns/op       0 B/op         0 allocs/op
BenchmarkHexEncoding/fmt.Sprintf-16                 11472624           124.9 ns/op        34 B/op          3 allocs/op
BenchmarkHexEncoding/hex.EncodeToString-16          43786639            27.65 ns/op        8 B/op          1 allocs/op
BenchmarkHexArrayEncoding/fmt.Sprintf-16             8901759           134.6 ns/op        16 B/op          3 allocs/op
BenchmarkHexArrayEncoding/hex.EncodeToString-16     45421173            25.56 ns/op        8 B/op          1 allocs/op
BenchmarkIntFormatting/fmt.Sprint-16                10068175           103.8 ns/op        24 B/op          1 allocs/op
BenchmarkIntFormatting/fmt.Sprintf-16               11987208           105.4 ns/op        24 B/op          1 allocs/op
BenchmarkIntFormatting/strconv.Itoa-16              21441700            62.10 ns/op       24 B/op          1 allocs/op
BenchmarkIntConversionFormatting/fmt.Sprint-16      14688165            77.34 ns/op       16 B/op          2 allocs/op
BenchmarkIntConversionFormatting/fmt.Sprintf-16     14185489            76.67 ns/op       16 B/op          2 allocs/op
BenchmarkIntConversionFormatting/strconv.FormatInt-16           22949540            47.18 ns/op       16 B/op          1 allocs/op
BenchmarkUintFormatting/fmt.Sprint-16                           10403521           104.5 ns/op        24 B/op          1 allocs/op
BenchmarkUintFormatting/fmt.Sprintf-16                          10714227           103.7 ns/op        24 B/op          1 allocs/op
BenchmarkUintFormatting/strconv.FormatUint-16                   16311451            62.97 ns/op       24 B/op          1 allocs/op
BenchmarkUintHexFormatting/fmt.Sprintf-16                       14271810            71.43 ns/op       16 B/op          1 allocs/op
BenchmarkUintHexFormatting/strconv.FormatUint-16                26732412            52.73 ns/op       16 B/op          1 allocs/op
BenchmarkStringAdditionFormatting/fmt.Sprintf-16                16425195            73.39 ns/op       16 B/op          1 allocs/op
BenchmarkStringAdditionFormatting/string_concatenation-16       1000000000           0.3022 ns/op          0 B/op          0 allocs/op
PASS
ok      github.com/catenacyber/perfsprint/analyzer  34.842s

I see no explanations here

catenacyber commented 2 months ago

I am sorry, but I do not know what you expect as an "explanation". Feel free to open a PR if you want something more in the README

AndersBennedsgaard commented 2 months ago

Something similar to what Go-critic does: https://go-critic.com/overview#argorder:


# strconv.FormatUint

Use strconv.FormatUint

Before:

```go
a := fmt.Sprint(uint(42))
fmt.Println(a)

After:

a := strconv.FormatUint(42, 10)
fmt.Println(a)


However, if you don't think this is necessary, then feel free to close this issue.

My reason for opening this issue was that I found it hard to know what messages like `fmt.Sprint can be replaced with faster strconv.FormatUint` actually means, and having a small explanation for each rule with an easy to follow example is very useful for us noobs
catenacyber commented 2 months ago

What about https://github.com/catenacyber/perfsprint?tab=readme-ov-file#replacements ?

ccoVeille commented 2 months ago

Apparently everything is fine for you.

I think the best for us who complain about the lack of explanation is to open a PR with suggestions. Then the discussion could continue.

Respectfully.

AndersBennedsgaard commented 2 months ago

Tbh. I didn't really understand https://github.com/catenacyber/perfsprint?tab=readme-ov-file#replacements when I first saw it, so I sort of ignored it.. It might be enough to close this issue, but the explanation could probably be a little more clear if I couldn't understand it coming with no knowledge of perfspring :thinking: But as I said above, feel free to close this if you disagree.

I think the best for us who complain about the lack of explanation is to open a PR with suggestions. Then the discussion could continue.

I know something like this could be a bit easier to discuss with more practical examples (like a PR with changes to the README), but the author and community might not agree with my changes for whatever reason and close it, which would make my work completely wasted.

For that reason I rarely open PRs before having the discussion in an issue, since I've had my PRs closed before simply because the author didn't agree with it.

catenacyber commented 2 months ago

I think the best for us who complain about the lack of explanation is to open a PR with suggestions.

That is most welcome :-)

But as I said above, feel free to close this if you disagree.

I think documentation can be improved, and I do not think I am the best to do it as I do not have a fresh perspective