disnake icon indicating copy to clipboard operation
disnake copied to clipboard

Reconsider `TC001`-`TC003` lint rules

Open shiftinv opened this issue 2 months ago • 0 comments

Regarding #1384 - indiscriminately moving type annotations that are unused at runtime into TYPE_CHECKING is fundamentally incompatible with a library that heavily relies on runtime parsing of signatures, imho. I've seen these three rules be a bit of a nuisance with other annotation-parsing libraries like pydantic or sqlalchemy already, they seem very situational and not automatically applicable everywhere.

bugs

To be clear, there aren't any immediate issues with it, but it's made for some pesky and hard to uncover bugs in the past. Namely #384 (fun how that's exactly 1000 issues prior), which would again be happening here, since #1384 moved the AnyContext import in commands/converters.py back into TYPE_CHECKING. The only reason it's not happening now is #946, which inadvertently skips parsing the signature of builtin converters, making this a bug that'll likely surface at some other point in a completely unrelated refactor c:

advantages

Given all that, to also take into account possible advantages of moving these imports -

Import time is essentially unaffected (fairly unsurprising), the difference is somewhere between incredibly negligible to nonexistent: Image (screenshot because fancy colors, command is hyperfine --warmup 5 -L sha 603e5c8f,5f431644 -p 'git checkout {sha}' 'uv run python -c "import disnake"' if anyone wants to try for themselves. this was in a tmpfs/ramdisk to avoid io overhead)

Less potential for import cycles is good, but ultimately I don't think it's really all that important. They're quite rare already, and if we do hit them, they've usually been either unavoidable or easily fixable by hand.

workarounds

A workaround/compromise could be using https://docs.astral.sh/ruff/settings/#lint_flake8-type-checking_runtime-evaluated-base-classes to exclude classes inheriting from commands.Converter from the TC001 rule, but it does feel a little bit like a bandaid fix.

tl;dr

I'm sort of inclined to vote for a revert on this, at least a partial one, since a very core part of the library is processing annotations at runtime. Thanks for coming to my ted talk, I guess.

shiftinv avatar Nov 19 '25 10:11 shiftinv