[Daverball]

This is a partial solution for #16412

I would still like to add something like runtime-evaluated-generic-subscripts, but that will require further consideration about how the presence of such subscripts should affect the rest of the type expression, or in more extreme cases, the other annotations in the function/class definition.

Summary

For things like SQLAlchemy's Mapped the evaluation of the annotations only requires a subset of all the symbols to be resolvable at runtime, this is sometimes necessary to make forward references to other models work without causing import cycles.

But this means we can neither assume that the annotations are runtime-required, nor can we assume, that they are typing-only. So we should assume the referenced binding is already in its correct execution context, i.e. we should neither emit TC001-003, nor TC004 violations.

Instead of adding two additional settings now and potentially two more settings later, the existing two settings for runtime-required annotations now accept a table in addition to the list to flag individual imports with individual runtime requirements.

For example:

[lint.flake8-type-checking.runtime-evaluated-base-classes]
"pydantic.BaseModel" = "required"
"sqlalchemy.orm.DeclarativeBase" = "ambiguous"

[lint.flake8-type-checking.runtime-evaluated-decorators]
"pydantic.validate_call" = "required"
"sqlalchemy.orm.declared_attr" = "ambiguous"

Test Plan

cargo nextest run

[Daverball]

@ntBre In looking at potential interactions of runtime-ambiguous contexts with other rules I noticed that future-annotation: True only allows triggering UP037 for in_runtime_evaluated_annotation, is that intentional?

In the vast majority of cases it should be safe to remove quotes in a runtime-required annotation context with the presence of from __future__ import annotations, since the runtime portion was already able to deal with at least partially quoted annotations and adding the future import is the same as fully quoting all annotations.

The same should also be true for runtime-ambiguous annotation contexts.

With Python 3.14+ the situation is a little bit more complicated, if the runtime portion makes use of typing.get_type_hints then the quotes should make as little difference as they do with the future import, if it uses annotationlib.get_annotations there is a difference, although there is probably something pretty esoteric going on at that point and you wouldn't want UP037 to be active on any annotations that aren't purely syntactical and disappear at runtime, regardless of Python version or the presence of the future import, unless you specifically used runtime-evaluated-base-classes/runtime-evaluated-decorators as an escape hatch.

I can certainly see the argument for being as conservative as possible with UP037 when these settings are involved, but I'm also not entirely sure if it was conscious decision or if it just turned out this way by happenstance, since there are already so many other semantic flags to consider.

[astral-sh-bot]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[Daverball]

@ntBre Any chance you could take a look at this? I'd like to get a sense for how this smaller scoped change is received, before I start working on runtime-evaluated-generic-subscripts, but I also don't want to delay working on that too much, in case I get busy with other things again.

[ntBre]

Sorry, this is in my review queue, but I've been preparing for the 0.16 release this week. From a very quick read this sounds reasonable to me.

I don't really love the idea of the ~ prefix. Should we just expose the new arrays to users? Or maybe use a table like:

[lint.flake8-type-checking.runtime-evaluated-base-classes]
required = []
ambiguous = []

However, that's a pretty superficial complaint.

My guess on UP037 is that it was not intentional, but I'd have to look back and try to remember for sure.

I'll try to prioritize a deeper look here next week after the minor release!

[Daverball]

No, worries and thanks for taking a quick look already anyways.

FWIW I'm not super stoked on the prefix idea either, but it seemed better in the short-term than the churn caused by changing the schema of an existing setting, even if we accept and auto-translate the old value during the deprecation period.

If we dislike the prefix idea enough and still want to avoid causing unnecessary churn, we could just split it into four settings, like originally proposed. But it seems a little overwhelming if we end up with six distinct settings at the end and the documentation would likely get pretty messy.

If we're fine with causing churn to improve the ergonomics of the settings, we should maybe consider combining runtime-evaluated-base-classes and runtime-evaluated-decorators into a single runtime-evaluated-annotations setting, with sub-settings for the two (and later three) different contexts, that will make the transition easier, since we can just deprecate entire settings, instead of deprecating specific value shapes of existing settings. It also puts all the documentation for how this works in a single place, instead of having to repeat/cross reference information between the two/three settings.

[MichaReiser]

I don't mind using a symbol like ~ in itself. One question that popped up for me is how this would work once we change Ruff's table inheritance to always merge, the same as we do today (instead of replacing). We would then need a syntax (or table structure) that allows removing entries again, similar as to what you're used from gitinogre (where a !path undoes a previous ignore). My concern with ~ is that we use ~ as not in ty and it's also the bitwise not operator.

I, unfortunately, don't have a much better suggestion than a (inline) table, or ? which I don't like much also

[Daverball]

If we're fine with switching to a new setting name and value schema, with the looming rule re-categorization, we should probably also consider moving the setting up the tree to lint.

While the setting's effects are mostly limited to TC rules, it does still have some effect on a couple of different rules, like UP037, and it might affect more rules in the future, if those rules decide to start looking at the relevant semantic flags as well for some reason.

This way we would avoid causing churn twice for the same setting. Even if the later change would be a simple redirect to the new setting location, so it's less work than the initial change.

[Daverball]

@ntBre I refactored the settings according to our discussion. It would be great if you could take another look at this.

[ntBre]

Will do, thank you. This has been in my inbox since last time I said I'd take a look (sorry!), but I've been focused on #1774. I'm hoping actually to catch up on reviews late this week and into next week.

[ntBre]

Thank you for your work here, and sorry again for the delay!

This overall still looks reasonable to me, I just had some minor suggestions and one larger idea about the configuration structure itself. Exactly how we want to represent this and the other settings is really the only open question to me, but what's already implemented here may ultimately end up as the best option.

[Daverball]

@ntBre I fixed the two obvious things. The rest hinges on if and how we decide to change the structure of the settings. I think my personal preference would be to keep the table, but I could be convinced that splitting it into two tables, one for runtime required annotations and one for runtime ambiguous ones, is better for documentation purposes and keeping the inheritance logic more simple. Having separate lists for decorators, base classes and eventually generic subscripts still makes sense to me, to avoid both unnecessary work during semantic analysis and weird edge cases that could not be avoided with a single list.

[ntBre]

Thank you! And sorry for the delay once again. This looks good to me besides a couple of mostly very small questions. The one about default is maybe slightly larger.

[ntBre]

Thank you!