Repository navigation
Add TRIO232: blocking sync call on file objects - #104
Conversation
0e2bf9a to
21e2cc5
Compare
|
Approach seems promising, agree that |
21e2cc5 to
bc0e54e
Compare
|
Rebased on top of main (which wasn't trivial since it was forked from an outdated #103 that was later amended - fun stuff!). |
bc0e54e to
3c92aa0
Compare
|
I'll refactor it in a different PR as I write the next object sync checker, as this one should work well as a standalone. Ready to merge! |
Zac-HD
left a comment
There was a problem hiding this comment.
Tiny comment on tests, otherwise looks great.
| if f: | ||
| f.read() # TRIO232: 8, 'read', 'f' |
There was a problem hiding this comment.
Would this still give a lint error without the if, or in an else? It'd be nice to demonstrate that with tests.
There was a problem hiding this comment.
Yes, it currently does not care at all that it could be a None - but I had type-checking turned on in my editor when writing this one hence the if's. You're completely right it should be shown in tests though - and will fix in followup PR 👍
|
For the future refactored version, one annoying (to lint, it's fortunately rare) pattern is defining a |
oh and it should work fine with global variables as long as they're defined in the same file. Will add tests to show that. I'll make a PR just with these two fixes + another pattern I just realized will be useful: async def type_assign():
f: TextIOWrapper = ...
f.read() |
A first draft at tracking types for file/path/httpx/etc and warning on sync calls. Except this one just does it for
open()and the variosio.*types it can return as a start.This and others that build on top of it should finish off #58
Branched from on top of #103 - but you can just view the latest commit to see the contents of this one.
Flake8TrioVisitor.super().register('open', 'io.TextIOWrapper')in the__init__io.TextIOWrapperobjects to functions, but that's going to be much more common with Path and httpx objects, so I implemented it for this as well just to show the logic.Optional[Path]andPath|Nonewill be done.list[Path]and similar cases can be done if it makes sense but I don't think will be common of any of the types we care about.