Update ci #202
Update ci #202
Conversation
…y312 testing, fix TRIO91X for empty-body-on-same-line found due to new black style
revert adding ipdb to tox (was briefly used when testing stuff)
… tags in setup.py to specify we support 3.12
| error_codes: dict[str, str] # pyright: ignore[reportUninitializedInstanceVariable] | ||
| error_codes: ClassVar[ | ||
| dict[str, str] | ||
| ] # pyright: ignore[reportUninitializedInstanceVariable] |
There was a problem hiding this comment.
Is that ignore still firing? I assume so :/
Also, why isn't line 163 changed (or other cases that look exactly like this)?
There was a problem hiding this comment.
It is not! And turns out reportUnnecessaryTypeIgnoreComment doesn't default to true with strict = true, so time to turn that on.
I did change this one hoping it would flow through and silence RUF012 ("Mutable class attribute should be annotated with typing.ClassVar") to the classes inheriting from it - but when that didn't happen I silenced the error rather than change it in the 25 other places. But maybe I'll bother doing the big search&replace
There was a problem hiding this comment.
oh hm, the reason it wasn't enabled is because there's no separate setting for pyright: ignore vs type: ignore (and unlikely to be added), and mypy doesn't have mypy: ignore
There was a problem hiding this comment.
Could perhaps enable mypy's type: ignore, but that'd involve adding a bunch of deps to its pre-commit env and/or go through a bunch of them and replace with pyright: ignore - and I don't think that's worth bothering with currently.
There was a problem hiding this comment.
If we annotate as Mapping[str, str] so that it's logically immutable maybe our typecheckers will be happier?
| T_EITHER = TypeVar( | ||
| "T_EITHER", bound=Union[Flake8TrioVisitor, Flake8TrioVisitor_cst] | ||
| ) |
There was a problem hiding this comment.
I didn't previously specify python_version in pyproject.toml for mypy, and now that I did it didn't like the old version which wouldn't work at runtime - and mypy doesn't seem to detect that we're inside an if TYPE_CHECKING block.
There was a problem hiding this comment.
although I never remember which tools want python_version to be set to max-version-you-support vs min-version-you-support, so maybe should set it to 3.12 instead of 3.9
There was a problem hiding this comment.
mypy wants you to run separate runs of all of them in parallel x)
python/mypy#12286
| def bar(*args) -> Any: ... | ||
|
|
||
|
|
||
| # mypy now treats `if ...` as `if True`, so we have another arbitrary function instead |
There was a problem hiding this comment.
Yeah, I was tempted for a second to report it to mypy until I realized that it's probably better this way in case it sneaks into production code...
| @@ -39,10 +38,10 @@ async def foo_yield(): # TRIO911: 0, "exit", Statement("yield", lineno+2) | |||
|
|
|||
|
|
|||
| async def foo_if(): | |||
| if ...: | |||
| if foo(): | |||
There was a problem hiding this comment.
since foo is async, should we if bar() in these places?
There was a problem hiding this comment.
huh, I'd expect that to trigger mypy's unused-coroutine but I guess the return value is technically used.
Reported it at python/mypy#16921 (comment)
For the test file it of course doesn't really matter, unless mypy starts treating if foo(): as if True: 😁, but yeah no reason not to.
| async for ( # error: 8, Statement("try/finally", lineno-3) | ||
| i | ||
| ) in trio.bypasslinters: |
There was a problem hiding this comment.
New version of Black now considers the line too long (90>88 after all) and splits it up, after which I move the comment back to line 123.
Not super pretty, maybe worth reporting upstream.
…I missed, add py313 support now that hypothesmith is updated
if ...:is now treated asif True:and it therefore thought tons of theeval_filescontained unreachable statements. Replaced it with an arbitrary function call.Statementbeing unhashable, which it technically should be, although I guess we're inheriting a__hash__fromNamedTupleand it currently works ™️. Need to change a bunch of equality checks in the code to make it less illegal, so leaving that for now.CodeRange | type[_UNDEFINED_DEFAULT]Instagram/LibCST#1107as an empty function, since it was matching against the indented body. This is now the black style, which is why I found it. So fixed that, and updated test file to have both.
There's only a few more instances of
IndentedBlockin the project, and none seem problematic, but it's possible this could be a problem in other places.I can split this across multiple PRs, but idk if it's warranted. Now that we moved and can set up pre-commit it should be more incremental going forward.