Avoid flagging __future__ annotations as required for non-evaluated type annotations

[charliermarsh]

Summary

If an annotation won't be evaluated at runtime, we don't need to flag from __future__ import annotations as required. This applies both to quoted annotations and annotations outside of runtime-evaluated positions, like:

def main() -> None:
    a_list: list[str] | None = []
    a_list.append("hello")

Closes #11397.

[AlexWaygood]

Hmm, I wouldn't do this for quoted annotations. I think it was definitely intentional in the original flake8 plugin for PEP-585 annotations in quotes to trigger this check, because otherwise UP037 won't automatically remove the quotes for you without the __future__ annotations import. I like the idea of ignoring annotations inside functions, though.

[github-actions]

ruff-ecosystem results

Linter (stable)

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

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

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

- corporate/lib/stripe.py:3017:29: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
- corporate/lib/stripe.py:3115:48: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
- corporate/lib/stripe.py:915:40: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
FA102	3	0	3	0	0

Linter (preview)

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

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

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

- corporate/lib/stripe.py:3017:29: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
- corporate/lib/stripe.py:3115:48: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
- corporate/lib/stripe.py:915:40: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
FA102	3	0	3	0	0

[charliermarsh]

The diagnostic and documentation aren't really correct or consistent with that use-case though, at least as-written. The messaging suggests you need this to avoid runtime errors. Almost feels like it needs an explicit message (e.g., you could remove quotes by adding this), but in that case, isn't it always true that adding from __future__ import annotations would let you remove quotes? Regardless of the syntax you're using?

[charliermarsh]

I'm now wondering if that should be a separate rule or part of FA100? Since the type can be written more succinctly if you import from __future__ import annotations, at the very least by removing the quotes?

[AlexWaygood]

The messaging suggests you need this to avoid runtime errors.

Huh, that's not how I've ever interpreted that message. The message is "Missing from __future__ import annotations, but uses PEP 585 collection" -- I always interpreted it as "Missing from __future__ import annotations, but uses PEP 585 collection [which could be written more simply if you added this `future import]".

The original flake8 plugin explicitly mentions pyupgrade in its README as one of the motivations for the plugin: https://github.com/TylerYep/flake8-future-annotations?tab=readme-ov-file#flake8-future-annotations.

And I don't think this rule is nearly adequate to prevent all runtime errors arising from using newer typing syntax on older versions, nor should it try to do that.

isn't it always true that adding from __future__ import annotations would let you remove quotes? Regardless of the syntax you're using?

Not necessarily... eg. removing quotes from this annotation would cause a syntax error:

def foo() -> "Some kind of string":
    pass

[AlexWaygood]

I'm now wondering if that should be a separate rule or part of FA100? Since the type can be written more succinctly if you import from __future__ import annotations, at the very least by removing the quotes?

IDK, because I've always thought of "enabling other tools to auto-upgrade your annotations" as being the primary motivation of the rule 😄

[charliermarsh]

Are you mixing up FA100 and FA102? Or am I mixing them up? FA100 is the rule that tells you "you can write this more succinctly if you import from __future__ import annotations." This PR does not modify FA100.

[charliermarsh]

This rule modifies FA102, which tells you that you're using an unsupported type annotation that will work if you add from __future__ import annotations.

[AlexWaygood]

Argh, I should have checked more thoroughly. Yikes, and sorry :(

Yes, I didn't realise there was a distinction between FA100 and FA102. In that case, we should indeed make this change, as long as we still emit FA100 on all these snippets. (But maybe we should also update FA100 so that it's not emitted for annotations inside functions, as well, since the __future__ import is irrelevant in that context?)

It would be good if the rule summary at https://docs.astral.sh/ruff/rules/#flake8-future-annotations-fa made the distinction between the rules here a bit clearer.

[charliermarsh]

No need to apologize, I was also unsure. My guess is that we don't emit FA100 on the quoted ones. I'll have to check...

[inoa-jboliveira]

LGTM! Thank you for such swift reply

[dhruvmanila]

I think this makes sense.

[charliermarsh]

Confirmed that we still emit FA100 on these.

[AlexWaygood]

Confirmed that we still emit FA100 on these.

TYVM!

[TylerYep]

If an annotation won't be evaluated at runtime, we don't need to flag from future import annotations as required.

@charliermarsh This is only accurate in Python versions 3.10+ I believe:

Python 3.8.16 (default, Dec 20 2022, 18:52:29) 
[Clang 14.0.3 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> x: list[int]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'type' object is not subscriptable
>>> from __future__ import annotations
>>> x: list[int]
>>> 

Python 3.10.13 (main, Aug 24 2023, 22:36:46) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> x: list[int]
>>>

[TylerYep]

But quoted ones will work regardless of the Python version:

Python 3.8.16 (default, Dec 20 2022, 18:52:29) 
[Clang 14.0.3 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> x: "list[int]"
>>> 

I didn't read the PR thoroughly, but wanted to make sure the lint rule is only changed for the quoted version but still flags non-quoted lowercase types without future annotations?

[charliermarsh]

@TylerYep - the logic here only applies to annotated assignments within function bodies, which I believe works on all Python versions:

def f():
    x: list[int] = 1

f()

[TylerYep]

Ah, makes sense, thank you for clarifying!