catenacyber / perfsprint

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

Please, rename project to `nosprintfconv` (or similar) #2

Closed Antonboom closed 9 months ago

Antonboom commented 9 months ago

Hello!

The current name:

  1. Do not reflect linter nature (what is linter checks, allow, forbid..?)
  2. Do not reflect real situation, because suggested fixes contains not only strconv
  3. Do not consistent with existing similar linters (e.g. nosprintfhostport)

Any other ideas? 🙂

catenacyber commented 9 months ago

I am bad at naming indeed...

And this POC has grown from when I started (I did not know then that fmt.Sprintf("%x" was used instead of hex.EncodeToString)

Do not reflect linter nature (what is linter checks, allow, forbid..?)

For me, this linter is meant for faster performance. It is not meant to forbid something bad, just suggest something faster. Do you know other such linters ?

Do not reflect real situation, because suggested fixes contains not only strconv

Indeed, it is more centered around fmt.Sprint*

Do not consistent with existing similar linters (e.g. nosprintfhostport)

So, what about perfsprint ?

Antonboom commented 9 months ago

Do you know other such linters ?

$ grep 'PresetPerformance' pkg/lint/lintersdb/manager.go -B3 | grep NewConfig
                linter.NewConfig(golinters.NewBodyclose()).
                linter.NewConfig(golinters.NewMaligned(malignedCfg)).
                linter.NewConfig(golinters.NewNoctx()).
                linter.NewConfig(golinters.NewPreAlloc(preallocCfg)).

Also https://go-critic.com/overview#checkers-from-the-performance-group

appendCombine
equalFold
hugeParam
indexAlloc
preferDecodeRune
preferFprint
preferStringWriter
preferWriteByte
rangeExprCopy
rangeValCopy
sliceClear
stringXbytes

So, what about perfsprint ?

Sounds cool. But seems to oblige you to use fmt.Sprintf, but in a different, special way 😅

Looks worse than your version 👍

catenacyber commented 9 months ago

https://github.com/catenacyber/perfsprint + https://github.com/catenacyber/perfsprint/commit/59344202c9ddf082ac768fa357527241d1477b4e

Ok for you ?

Antonboom commented 9 months ago

@catenacyber, you are an owner :)

I accept any decision. Looks great! 👍

P.S. Probably need change repository settings. I have no idea why CI was not started 🤔

Screenshot 2023-10-18 at 21 44 56
catenacyber commented 9 months ago

The repository is new, so there is no run yet

catenacyber commented 9 months ago

Red because of https://github.com/catenacyber/perfsprint/actions/runs/6565343804/job/17833666127

golangci/golangci-lint crit unable to find '1.54.2' - use 'latest' or see https://github.com/golangci/golangci-lint/releases for details

Antonboom commented 9 months ago

Oh, sorry for typo

v1.54.2

of course

catenacyber commented 9 months ago

Indeed, just found it and pushed the fix