[flake8-type-checking] Stabilize quoted-type-alias (TC008)

[ntBre]

Summary

Stabilizes TC008. The test was already in the right place.

Test Plan

No open issues or PRs.

[codspeed-hq]

CodSpeed Performance Report

Merging #16639 will degrade performances by 12.88%

_{Comparing brent/tc008-0.10 (174758b) with micha/ruff-0.10 (5ec7c13)}

Summary

❌ 1 regressions
✅ 31 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
❌	red_knot_check_file[incremental]	4.8 ms	5.5 ms	-12.88%

[github-actions]

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+9 -0 violations, +0 -0 fixes in 3 projects; 52 projects unchanged)

ibis-project/ibis (+1 -0 violations, +0 -0 fixes)

+ ibis/common/typing.py:28:47: TC008 [*] Remove quotes from type alias

latchbio/latch (+3 -0 violations, +0 -0 fixes)

+ src/latch/ldata/_transfer/upload.py:30:32: TC008 [*] Remove quotes from type alias
+ src/latch/ldata/_transfer/upload.py:31:35: TC008 [*] Remove quotes from type alias
+ src/latch_cli/snakemake/config/utils.py:13:33: TC008 [*] Remove quotes from type alias

zulip/zulip (+5 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --no-preview --select ALL

+ analytics/views/stats.py:341:14: TC008 [*] Remove quotes from type alias
+ analytics/views/stats.py:343:16: TC008 [*] Remove quotes from type alias
+ confirmation/models.py:76:5: TC008 [*] Remove quotes from type alias
+ confirmation/models.py:77:5: TC008 [*] Remove quotes from type alias
+ zerver/lib/push_notifications.py:76:49: TC008 [*] Remove quotes from type alias

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
TC008	9	9	0	0	0

Linter (preview)

✅ ecosystem check detected no linter changes.

[ntBre]

These all look like false positives except the latchbio cases.

The Zulip cases require an import that is conditional on some settings type other than TYPE_CHECKING, so I think unconditionally unquoting those could be a problem.

And the ibis case is a recursive type alias, which I think must be quoted? Possibly the rule doesn't descend into the Union because it handles the ... | ... version fine.

[Daverball]

The Zulip cases require an import that is conditional on some settings type other than TYPE_CHECKING, so I think unconditionally unquoting those could be a problem.

Indeed, it would be a problem, although in order to detect this type of stuff we would need to start tracking all conditional bindings separately, not just ConditionalDeletion and I'm not fully convinced it's worth the extra complexity. Although we could probably come up with a more simple heuristic to exclude some bindings, like e.g. if the parent statement is an if/try statement without a corresponding shadowing binding in the other branch, then we reject that binding.

And the ibis case is a recursive type alias, which I think must be quoted? Possibly the rule doesn't descend into the Union because it handles the ... | ... version fine.

It's because the if branch before it also defines the same symbol, so our naïve approach to bindings assumes the else branch has access to the symbol, since it was defined before our type alias definition. If recursive type aliases were broken, the line in the other branch would also yield a false positive.

So this is a similar problem to the other one, although it seems slightly easier to fix, e.g. by walking the parent statements of the the binding and reference in order to determine whether or not they are part of the same if statement, and if they are, whether or not they're part of the same branch. We would also need to do the same thing for match statements.

Red knot might be better suited for detecting these types of things though, since control flow analysis could tell us which bindings are unreachable starting from our reference.

[MichaReiser]

Let's keep this rule in preview then and create a follow up issue. @dylwil3 has also done some work on control flow analysis in Ruff that might become useful here, once it's ready for prime time.

[ntBre]

Sounds good, I'll close this for now.

@Daverball thank you for the additional context here and on some of these other PRs. I really appreciate it!