Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Feb 4, 2024

Conversation

psydvl
Copy link
Contributor

@psydvl psydvl commented Feb 1, 2024

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

@polyfloyd
Copy link
Owner

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
Copy link
Contributor Author

psydvl commented Feb 2, 2024

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

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 polyfloyd merged commit c5cc3fb into polyfloyd:main Feb 4, 2024
2 checks passed
@polyfloyd
Copy link
Owner

Awesome, nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants