python-trio / flake8-async

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

reportUnusedCoroutine / unused-coroutine *is* in fact raised for trio #155

Closed jakkdl closed 1 year ago

jakkdl commented 1 year ago

if trio_typing is installed, and the call is reachable. I had no clue that both mypy and pyright will straight up not check unreachable statements, but after moving out sleep_forever and the serve functions in https://github.com/Zac-HD/flake8-trio/blob/main/tests/eval_files/trio105.py to separate functions:

$ mypy tests/eval_files/trio105.py | grep -c unused-coroutine
21
$ pyright tests/eval_files/trio105.py | grep -c reportUnusedCoroutine
4

the disparity is due to pyright not raising it if there's other problems with the call (missing parameters etc), when I fixed that and switch to a normal way of opening nurseries they both jump up to 26. The total number of TRIO105 errors defined in the file is 29, and the differences are all logical (1. open_process only exists on win32 and trio-typing currently doesn't export it in the stubs at all, so mypy/pyright says it doesn't exist, 2. async with trio.open_file(...) complains about missing __aenter__ instead, and 3. k = trio.open_file(par) being an error is a limitation in the flake8-trio implementation).

tl;dr: turns out that I due to a combination of confusions was wrong! So if end users are running with type-checking and has stubs from trio_typing, TRIO105 should be fully redundant both for trio and anyio, and can be removed.

Without trio_typing ~no errors are raised since trio doesn't have py.typed marker, if it had I think it would work fully as well.

Zac-HD commented 1 year ago

I’d actually like to keep what we already have for now; it’s unfortunately pretty common to disable typechecking so a simple lint isn’t quite redundant.

I'm also hoping that the static-typing-for-Trio situation will improve over the coming months, e.g. by merging trio-typing into Trio proper and adding tests that ensure it's all up to date (e.g. fixing [1] and [2] above). Once/if that's in, we probably can remove TRIO105 🙂

jakkdl commented 1 year ago

No need to hope, it's just a matter of prioritizing trio static typing and I'll get it done :grin: Sounds good!