python-trio / flake8-async

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

implement autofix for 91x #158

Closed jakkdl closed 1 year ago

jakkdl commented 1 year ago

does still not finish #70 :sweat_smile: It "merely" inserts checkpoints wherever needed.

TODO:

And added a couple more tasks to #124 before this is reasonably usable by an end user. That includes checks for whether autofix toggling actually is toggled, which I kinda know isn't done properly by this anyway (but atm it mostly doesn't matter cause the new file doesn't get written without the autofix flag). But I'll leave those for another PR.

Zac-HD commented 1 year ago

Exciting!

jakkdl commented 1 year ago

Exciting indeed! Although not sure how you can be excited about reviewing modifications to a visitor that's up to 800 lines :scream: And that's before adding remove-unnecessary-checkpoint-logic!!! Thankfully 1850 of the 2365 added lines are in generated autofix files, so you "only" have to review a diff of +501/-58 :sweat_smile:

jakkdl commented 1 year ago

fixed comments

jakkdl commented 1 year ago

except I had untracked files ... updated my aliases so that won't happen again ^^ This is so handy it's crazy:

alias untracked="git ls-files -o --directory --exclude-standard | sed q1 > /dev/null"
alias gfpush='untracked && pytest --quiet -x && git commit -a --amend --no-edit && git push -f'
jakkdl commented 1 year ago

Oh and I realized that the pass # AUTOFIX_INSERT was a big brainfart yesterday, autofixing is all about inserting lines - I'm not explicitly marking each checkpoint that should be inserted, they're just added/generated to the autofix file. So completely removed that. Still need the isort: skip_file though since autofix doesn't insert an empty line before the added import, and trying to worry about that isn't really viable.

jakkdl commented 1 year ago

ahahaha, trio.lowlevel.checkpoint() sure doesn't checkpoint - you kiiiinda need to do await trio.lowlevel.checkpoint()

jakkdl commented 1 year ago

I'm writing the extra tests now, and this implementation still does some dumb stuff (yield inside boolops and stuff), but I'll leave (not) handling those for the next PR.

Zac-HD commented 1 year ago

alias gfpush='untracked && pytest --quiet -x && git commit -a --amend --no-edit && git push -f'

--force-with-lease!

ahahaha, trio.lowlevel.checkpoint() sure doesn't checkpoint - you kiiiinda need to do await trio.lowlevel.checkpoint()

lint all the things 😜

jakkdl commented 1 year ago

Ooh, --force-with-lease is nice!

Well, there's no linter - other than flake8-trio itself, that catches unawaited trio.lowlevel.checkpoint()! Unless we change the pyright dependency from trio to trio-typing. That might definitely be good though, but I'm getting 59 errors when doing that - so needs a separate PR.

jakkdl commented 1 year ago

I'll try to get the diffs to be smaller, but it's hard to avoid at this stage - and especially when working with the monster that is 91x :sweat_smile:

Zac-HD commented 1 year ago

Oh yeah, that's not a criticism! Just an expression of relief that I made it through 😋

jakkdl commented 1 year ago

Haha, just meant it's worth trying for. If I don't keep it in the back of my mind the scope creep of PR's is very real :innocent: