[flake8-type-checking] Improve flexibility of runtime-evaluated-decorators

[Daverball]

This is an alternative proposal towards accomplishing the use-case #15060 is trying to provide, which doesn't require adding a new setting.

Summary

This slightly changes semantics of the setting to support a common pattern used by e.g. FastAPI. We will resolve assignments so you can do things like this:

from __future__ import annotations

import fastapi

from mymodule import Foo

app = fastapi.FastAPI("My App")

@app.get("/home")
def home() -> Foo: ...

And add fastapi.FastAPI.get to runtime-evaluated-decorators in order to prevent a TC001 violation for from mymodule import Foo.

Test Plan

cargo nextest run

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[MichaReiser]

It seems I was a few minutes too slow. I replied on #15060

[Daverball]

@MichaReiser No worries, the glob part is honestly the smaller part of this proposal. You can make it work without the glob support. It just seemed really annoying to me having to manually list all the decorators. With some frameworks you may very well be looking at multiple dozens of methods. The decorator as application configuration hook style is quite popular.

So I can take it back out if we're sure we really don't want it.

[MichaReiser]

Thanks @Daverball My preference would be to start without it and add it if users run into it. We then also have the option to a) allow regex or b) do a prefix match

[Daverball]

The only thing I dislike about this approach, is that it forces you to write two separate entries for the same module and separate module case.

I also considered adding an option to resolve_qualified_name that would return a Some(QualifiedName) for Assignment and AnnAssignment as a band-aid for not having full cross-module type inference. That would bring it back down to one entry, but each instance would need to be ignored separately.

[MichaReiser]

The only thing I dislike about this approach, is that it forces you to write two separate entries for the same module and separate module case.

Can you expand on what you mean by this

[Daverball]

Can you expand on what you mean by this

Sure, if you have a look at the new tests, you'll notice that I'm adding an entry for fastapi.FastAPI.get for the case that uses resolve_assignment but also module.app.app.get for the regular import case from another module, which assumes that whatever you're importing is something that would either be a class or function defined in the module you're importing from, even though it could just be regular assignment as well.

So fastapi.FastAPI.get works for decorators in the same file. but if you import app into another file you need a second entry in order for ruff to do the correct thing.

[MichaReiser]

Sure, if you have a look at the new tests, you'll notice that I'm adding an entry for fastapi.FastAPI.get for the case that uses resolve_assignment but also module.app.app.get for the regular import case from another module, which assumes that whatever you're importing is something that would either be a class or function defined in the module you're importing from, even though it could just be regular assignment as well.

Thanks, that makes sense, considering that Ruff can't see what module.app.App resolves to. The problem is similar to why we have the typing-modules, logger_objects, and gettext.extend-function-names settings. This makes me wonder if we need a more generic approach that allows mapping qualified names, e.g., module.app.App -> fastapi.FastAPI`.

[MichaReiser]

Nice, thank you. I think it would be great if we could extend the setting documentation to cover both using a framework decorator but also an instance method from a symbol exported from a user module

[MichaReiser]

Nit: We could consider adding an extend method` to reduce the code

[Daverball]

I struggled a bit to get the lifetimes correct. If there's a more idiomatic way to write the function, feel free to go ahead and fix it. But I definitely like how it simplified resolve_assignment, I also noticed that the two match arms were redundant, so I consolidated them.

[MichaReiser]

The trick was to use into_iterator so that the items have the type &'a str and not &&'a str. The alternative would have been to use iter().copied(), I think, which dereferences &&'a str to &'a str

[MichaReiser]

Thanks