Repository navigation
[flake8-type-checking] TC004 is too strict for SQLAlchemy's Mapped #16412
Description
Summary
Given something like:
https://play.ruff.rs/da4c76c4-92d0-4083-99c1-022152398a50
from __future__ import annotations
from sqlalchemy.orm import BaseModel, Mapped, relationship
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from .bar import Bar
class Foo(BaseModel):
id: Mapped[int]
bar_id: Mapped[int | None]
bar: Mapped[Bar | None] = relationship()
Ruff will flag the import inside the TYPE_CHECKING block, which very well may be required, due to circular dependencies between the two models.
Additionally runtime-required-base-classes is technically too strict for sqlalchemy.orm.BaseModel, since only the annotations that use a Mapped class are runtime required and furthermore only non-model types are runtime required, any forward references to other models can be resolved during mapping.
We could handle the latter using an additional setting runtime-required-generic-subscripts. But the former would still be a problem.
To handle the former we would need something like runtime-ambiguous instead of runtime-required, so we emit neither TC001-003 nor TC004 for uses in that context, since the use will have to be considered neither runtime-only nor typing-only.
But turning two settings, into six settings doesn't seem particularly elegant either. Maybe someone can think of a more elegant way to structure these settings.
I think this is the final major hurdle for covering all the use-cases the flake8 plugin currently covers (beyond implementing the currently still missing TC009 rule).
Eventually type inference could let us do something more targeted for SQLAlchemy that knows exactly which symbols aren't runtime-required, but that's still a long way off and not as flexible for other libraries that may do something similar.
Version
0.9.7
Activity
Daverball commentedon Jun 25, 2026
Thinking about this again, we could probably get away with only one additional setting, if we allow prefixing each name in those three settings with a
~to signify it's ambiguous whether or not annotations will be runtime required and splitting those out during parsing.The bigger challenge will be making sure all of the rules and places that currently look at
ResolvedReference.in_runtime_evaluated_annotationare aware of the newResolvedReference.in_runtime_ambiguous_annotationand adjusting their behavior accordingly, so that we never flag issues in either direction, i.e. we go from making an assumption about the runtime context of a reference, to making no assumptions and assuming things are already correct, the way they are now. So this might mean we also have to look at all of the places that look atin_runtime_contextorin_typing_context, since they may also need to consider whether the context is ambiguous.Daverball commentedon Jul 2, 2026
Looks like the actual number of potentially affected rules is pretty low and mostly localized to
TC001-003andTC004.UP037andPLC2701are the only other rules that look at the relevant flags from what I can tell. Neither of which necessarily need to special-case runtime-ambiguous references, although it might be slightly nicer if they do,ntBre commentedon Sep 4, 2026
I was rereading this issue today before reviewing #26508, and I think it might be worth expanding either the docs on
runtime-evaluated-base-classesitself or on the reference to it in theTC004docs. It wasn't obvious to me why that setting was affecting the analysis of fields of the derived class when the docs focus on exempting the class itself, from what I understood.[`flake8-type-checking`] Add the notion of runtime-ambiguous referenc…