polyfloyd / go-errorlint

A source code linter that can be used to find code that will cause problems with Go's error wrapping scheme
MIT License
248 stars 18 forks source link

Extend allowed with some std funcs and external via funcopts #69

Closed psydvl closed 10 months ago

psydvl commented 10 months ago

Add some missed stdlib functions Speed up allowed check using maps Extend NewAnalyzer with funcopts to add custom allowed func-err pairs

polyfloyd commented 10 months ago

Thanks! This is really cool to have :)

Do you already have thought about how adding custom allowed errors/functions would work in practice? Most people use errorlint as part of golangci-lint I think.

Did you benchmark the map performance improvement?

psydvl commented 10 months ago

I'm trying to add errorlint at company monorepo, and we use here custom analyzer instead of golangci-lint for PRs stylecheck. So there will be at least ~500 (go dev) users of custom allowed errors/functions :) Also, I think, this variant will allow to add errorlint cli config file with errfunc pairs, so anyone can use it with their own io.Reader embedded interface for example. But this idea I left for future

Benchmarks (I'll push commit with it in few minutes, it seems better to have it):

Code

```go func Benchmark_isAllowedErrAndFunc(b *testing.B) { var benchCases = []struct { desc string err string fun string }{ { desc: "empty", err: "", fun: "", }, { desc: "short", err: "x", fun: "x", }, { desc: "long not existed", // should pass strings.HasPrefix length check, 30 symbols here err: "xxxx_xxxx_yyyy_yyyy_zzzz_zzzz_", fun: "xxxx_xxxx_yyyy_yyyy_zzzz_zzzz_", }, { desc: "existed, not wildcard", err: "io.EOF", fun: "(io.Reader).Read", }, { desc: "existed, wildcard", err: "golang.org/x/sys/unix.Exx", fun: "golang.org/x/sys/unix.xxx", }, } for _, bb := range benchCases { b.Run(bb.desc, func(b *testing.B) { err, fun := bb.err, bb.fun b.ResetTimer() for i := 0; i < b.N; i++ { isAllowedErrAndFunc(err, fun) } }) } } ```

Old

``` goos: linux goarch: amd64 pkg: github.com/polyfloyd/go-errorlint/errorlint cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz Benchmark_isAllowedErrAndFunc Benchmark_isAllowedErrAndFunc/empty Benchmark_isAllowedErrAndFunc/empty-12 15565950 75.92 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/short Benchmark_isAllowedErrAndFunc/short-12 15465111 67.71 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/long_not_existed Benchmark_isAllowedErrAndFunc/long_not_existed-12 16938921 77.14 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/existed,_not_wildcard Benchmark_isAllowedErrAndFunc/existed,_not_wildcard-12 32858502 36.40 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/existed,_wildcard Benchmark_isAllowedErrAndFunc/existed,_wildcard-12 89847319 13.65 ns/op 0 B/op 0 allocs/op PASS ok github.com/polyfloyd/go-errorlint/errorlint 6.246s ```

New

``` goos: linux goarch: amd64 pkg: github.com/polyfloyd/go-errorlint/errorlint cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz Benchmark_isAllowedErrAndFunc Benchmark_isAllowedErrAndFunc/empty Benchmark_isAllowedErrAndFunc/empty-12 71153664 16.23 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/short Benchmark_isAllowedErrAndFunc/short-12 91582710 12.45 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/long_not_existed Benchmark_isAllowedErrAndFunc/long_not_existed-12 71206419 17.43 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/existed,_not_wildcard Benchmark_isAllowedErrAndFunc/existed,_not_wildcard-12 72211255 16.00 ns/op 0 B/op 0 allocs/op Benchmark_isAllowedErrAndFunc/existed,_wildcard Benchmark_isAllowedErrAndFunc/existed,_wildcard-12 47858254 22.94 ns/op 0 B/op 0 allocs/op PASS ok github.com/polyfloyd/go-errorlint/errorlint 6.765s ```

I've reorder exact match and wildcards checks and old order with map seems run faster, but i'm not sure that wildcard pairs list will not raise So, I've tested on 10 items in allowedErrorWildcards

wildcard check Second(curr) First(old) Old code
empty 25.74 ns/op 23.49 ns/op 84.32 ns/op
short 20.79 ns/op 20.71 ns/op 76.51 ns/op
long_not_existed 44.17 ns/op 46.62 ns/op 96.53 ns/op
_not_wildcard 16.12 ns/op 51.05 ns/op 64.77 ns/op
_wildcard 50.15 ns/op 44.11 ns/op 42.84 ns/op
polyfloyd commented 10 months ago

Awesome, nice work!