Repository navigation
[flake8-tidy-imports] Allow lazy imports to be used in deferred annotations (TID255) - #28767
Conversation
…otations (`TID255`) Summary -- This PR fixes an issue I saw in the ecosystem report for the stabilization PR (#28761 (comment)). In short, we were flagging annotations like this: ```py __lazy_modules__ = ["pathlib"] from pathlib import Path class C: path: Path ``` on Python versions before 3.14, even though they are not actually resolved immediately when `__lazy_modules__` makes the imports lazy (3.15 > 3.14, as it turns out). Because the semantic model checks the target version when setting `TYPING_ONLY_ANNOTATION`, we instead check the other two annotation flags: `RUNTIME_EVALUATED_ANNOTATION` and `RUNTIME_REQUIRED_ANNOTATION`. If an annotation is runtime-evaluated and not runtime-required, and we didn't already bail out at the `semantic.execution_context().is_runtime()` check earlier in the same function, then we know that we're in an annotation that _would_ have been typing-only on 3.14+. Test Plan -- New mdtests
ntBre
added
the
bug
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| lazy-import-immediately-resolved | 12 | 0 | 12 | 0 | 0 |
| annotated: Annotated[Path, Path(".")] # both ok | ||
| ``` | ||
|
|
||
| #### Runtime-required annotations |
There was a problem hiding this comment.
Can you also add a regression test for dataclass, to acknowledge that this may require user configuration
__lazy_modules__ = ["pathlib"]
from pathlib import Path
from dataclasses import dataclass
@dataclass
class C:
path: PathHere, Path is evaluated immediately. Users could fix this by adding dataclass to runtime required decorators, but Codex points out that this isn't correct.
Dataclasses support quoted annotations whose imports can live under TYPE_CHECKING, so globally marking all dataclass annotations runtime-required could unnecessarily suppress those rules.
I think this is a fine enough limitation for now, but we should document it under known limitations. Unless you can think of a fix (nothing obvious came to my mind)
There was a problem hiding this comment.
Sure, I'll add a test and a note to the docs! I think there are many ways that lazy imports could end up being immediately resolved that Ruff can't follow (e.g. any decorator that uses annotations), but this may be one of the more common cases.
There was a problem hiding this comment.
Yeah, I'd mainly document that this rule is known to have false positives in cases where annotations are resolved immediately that Ruff does not understand and that we use the required runtime types settings as an approximation of that
There was a problem hiding this comment.
The default case causes false negatives rather than false positives in TID255 (we treat the annotation as deferred and not immediately resolved, disabling the rule, despite the decorator resolving it). It also seemed better to me not to document runtime-required-decorators here, as I think using that setting for this rule will give incorrect results in other rules that use it, like the TC rules. That's what I took away from your earlier message at least.
I documented the false negative limitation in the most recent commit.
Summary
This PR fixes an issue I saw in the ecosystem report for the stabilization PR (#28761 (comment)). In short, we were flagging annotations like this:
on Python versions before 3.14, even though they are not actually resolved immediately when
__lazy_modules__makes the imports lazy (3.15 > 3.14, as it turns out).Because the semantic model checks the target version when setting
TYPING_ONLY_ANNOTATION, weinstead check the other two annotation flags:
RUNTIME_EVALUATED_ANNOTATIONandRUNTIME_REQUIRED_ANNOTATION. If an annotation is runtime-evaluated and not runtime-required, andwe didn't already bail out at the
semantic.execution_context().is_runtime()check earlier in thesame function, then we know that we're in an annotation that would have been typing-only on 3.14+.
Test Plan
New mdtests and the ecosystem check here. I expect -12 scikit-build-core diagnostics compared to main.