search

python-trio / flake8-async

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

reimplement trio100 & trio101 with libcst #142

Closed jakkdl closed 1 year ago

jakkdl commented 1 year ago

Also changed how trio100 works to avoid missed cases where a function is defined inside the with statement. 100 & 101 moved into their own files.

I spent some time trying to get batching to work, BatchedVisitor is not compatible with MatcherDecoratableVisitor - and especially not with transformers. Tried dynamically creating a class that inherited from all enabled visitors, but ran into so many minor headaches with that approach I abandoned it. Might revisit once all visitors are changed to cst, but for now it's certainly not worth the effort for the sake of speeding it up. There might be a case of wanting to do the transforms simultaneously in case one transform unlocks another, and stuff like that. I thought batching would be required, since the utility visitors are separate classes, but I think that can be made to work by having error classes that require the functionality of a utility visitor to inherit from it. But neither 100 nor 101 require any of the utility ones so didn't bother with it too much.

I'm waiting with enabling transforming for trio100 for a later PR, since that will require a bunch of UI changes / flags / etc.

Ended up opening a bunch of issues in pyright/libcst/mypy during the process, https://github.com/Instagram/LibCST/issues/870 is probably the most relevant one - which is probably gonna be somewhat of a pain going forward if not fixed. (I didn't bother trying to stop shed from removing Union, but if possible that'd make it less painful.)

jakkdl commented 1 year ago

Ah, looks like the CI failure is due to shed not fixing everything in one go. I've noticed that happening frequently which is somewhat annoying, will track down the cause and open an issue in shed.

jakkdl commented 1 year ago

Fixed the CI, and cribbed your try/finally solution for LIBCST_PARSER_TYPE - except switched out pop() for del to avoid a reportUnusedCallResult

jakkdl commented 1 year ago

This is looking nice! I think I was anticipating a considerably gnarlier migration...

you keep underestimating how good the [infra]structure/patterns I've built up are :grin: Though I did spend a lot of time on polishing this, and there's quite a bit of potential future gnarl left.

Zac-HD commented 1 year ago

you keep underestimating how good the [infra]structure/patterns I've built up are 😁

Hofstadter's law has earned my respect, but you're right - kudos!

jakkdl commented 1 year ago

Tweaked and ready to be shipped! And good call, this is nicer.