python-trio / flake8-async

Highly opinionated linter for Trio code
https://flake8-async.readthedocs.io
MIT License
17 stars 2 forks source link

Merge visitors? Plugin run speed? #36

Closed jakkdl closed 2 years ago

jakkdl commented 2 years ago

Having written all the visitors, is the plugin being slow enough on large files it's worth trying to optimize it? Merging all the visitors might get somewhat messy, but could lead to a large speedup. Otherwise I'm not aware of any big things I could do to speed it up, but there's probably a few spots. (the recently added lists/stacks that get copied for resetting at exit can likely instead just pop the element they pushed (if they pushed one)

Zac-HD commented 2 years ago

From some quick benchmarks at work, we're right around the normal runtime of our flake8 plugins, so I don't think it's worth optimizing for performance at the expense of our time (or worse, maintainability of the code).

jakkdl commented 2 years ago

good to know! :+1:

And yeah I suspect it'd hamper maintainability quite a bit unless done with care. I have trouble keeping all the visit* functions apart myself in the MiscVisitor already so I'd have to spend some time coming up with a class structure that lets the logic between codes be separated still (e.g. a visitor superclass iterates through all subclasses visit functions in turn for each node). Fun problem, but low priority.

jakkdl commented 2 years ago

Actually maybe better to split MiscVisitor and have a dedicated visitor for each error, unless they're tightly related (103+104, 107+108). That should make for better readability&maintainability, and with some cleverness only iterate through the tree once.