From 28ab61d8850af9abe7f5b3f0406008b8abde4f8c Mon Sep 17 00:00:00 2001 From: Robsdedude Date: Mon, 30 Jun 2025 21:22:23 +0000 Subject: [PATCH] [`pyupgrade`] Avoid PEP-604 unions with `typing.NamedTuple` (`UP007`, `UP045`) (#18682) ## Summary Make `UP045` ignore `Optional[NamedTuple]` as `NamedTuple` is a function (not a proper type). Rewriting it to `NamedTuple | None` breaks at runtime. While type checkers currently accept `NamedTuple` as a type, they arguably shouldn't. Therefore, we outright ignore it and don't touch or lint on it. For a more detailed discussion, see the linked issue. ## Test Plan Added examples to the existing tests. ## Related Issues Fixes: https://github.com/astral-sh/ruff/issues/18619 --- .../test/fixtures/pyupgrade/UP007.py | 49 +++++++++++++++++++ .../test/fixtures/pyupgrade/UP045.py | 22 +++++++++ .../pyupgrade/rules/use_pep604_annotation.rs | 30 ++++++++++++ ...er__rules__pyupgrade__tests__UP007.py.snap | 2 + 4 files changed, 103 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007.py index cbc3e79b29..dd60d4c833 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007.py @@ -90,3 +90,52 @@ class AClass: def myfunc(param: "tuple[Union[int, 'AClass', None], str]"): print(param) + + +from typing import NamedTuple, Union + +import typing_extensions +from typing_extensions import ( + NamedTuple as NamedTupleTE, + Union as UnionTE, +) + +# Regression test for https://github.com/astral-sh/ruff/issues/18619 +# Don't emit lint for `NamedTuple` +a_plain_1: Union[NamedTuple, int] = None +a_plain_2: Union[int, NamedTuple] = None +a_plain_3: Union[NamedTuple, None] = None +a_plain_4: Union[None, NamedTuple] = None +a_plain_te_1: UnionTE[NamedTupleTE, int] = None +a_plain_te_2: UnionTE[int, NamedTupleTE] = None +a_plain_te_3: UnionTE[NamedTupleTE, None] = None +a_plain_te_4: UnionTE[None, NamedTupleTE] = None +a_plain_typing_1: UnionTE[typing.NamedTuple, int] = None +a_plain_typing_2: UnionTE[int, typing.NamedTuple] = None +a_plain_typing_3: UnionTE[typing.NamedTuple, None] = None +a_plain_typing_4: UnionTE[None, typing.NamedTuple] = None +a_string_1: "Union[NamedTuple, int]" = None +a_string_2: "Union[int, NamedTuple]" = None +a_string_3: "Union[NamedTuple, None]" = None +a_string_4: "Union[None, NamedTuple]" = None +a_string_te_1: "UnionTE[NamedTupleTE, int]" = None +a_string_te_2: "UnionTE[int, NamedTupleTE]" = None +a_string_te_3: "UnionTE[NamedTupleTE, None]" = None +a_string_te_4: "UnionTE[None, NamedTupleTE]" = None +a_string_typing_1: "typing.Union[typing.NamedTuple, int]" = None +a_string_typing_2: "typing.Union[int, typing.NamedTuple]" = None +a_string_typing_3: "typing.Union[typing.NamedTuple, None]" = None +a_string_typing_4: "typing.Union[None, typing.NamedTuple]" = None + +b_plain_1: Union[NamedTuple] = None +b_plain_2: Union[NamedTuple, None] = None +b_plain_te_1: UnionTE[NamedTupleTE] = None +b_plain_te_2: UnionTE[NamedTupleTE, None] = None +b_plain_typing_1: UnionTE[typing.NamedTuple] = None +b_plain_typing_2: UnionTE[typing.NamedTuple, None] = None +b_string_1: "Union[NamedTuple]" = None +b_string_2: "Union[NamedTuple, None]" = None +b_string_te_1: "UnionTE[NamedTupleTE]" = None +b_string_te_2: "UnionTE[NamedTupleTE, None]" = None +b_string_typing_1: "typing.Union[typing.NamedTuple]" = None +b_string_typing_2: "typing.Union[typing.NamedTuple, None]" = None diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP045.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP045.py index 13be285aa0..de2b18b52d 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP045.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP045.py @@ -47,3 +47,25 @@ class ServiceRefOrValue: # Test for: https://github.com/astral-sh/ruff/issues/18508 # Optional[None] should not be offered a fix foo: Optional[None] = None + + +from typing import NamedTuple, Optional + +import typing_extensions +from typing_extensions import ( + NamedTuple as NamedTupleTE, + Optional as OptionalTE, +) + +# Regression test for https://github.com/astral-sh/ruff/issues/18619 +# Don't emit lint for `NamedTuple` +a1: Optional[NamedTuple] = None +a2: typing.Optional[NamedTuple] = None +a3: OptionalTE[NamedTuple] = None +a4: typing_extensions.Optional[NamedTuple] = None +a5: Optional[typing.NamedTuple] = None +a6: typing.Optional[typing.NamedTuple] = None +a7: OptionalTE[typing.NamedTuple] = None +a8: typing_extensions.Optional[typing.NamedTuple] = None +a9: "Optional[NamedTuple]" = None +a10: Optional[NamedTupleTE] = None diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs index 54c923e768..a6a5b7187a 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs @@ -132,6 +132,17 @@ pub(crate) fn non_pep604_annotation( slice: &Expr, operator: Pep604Operator, ) { + // `NamedTuple` is not a type; it's a type constructor. Using it in a type annotation doesn't + // make much sense. But since type checkers will currently (incorrectly) _not_ complain about it + // being used in a type annotation, we just ignore `Optional[typing.NamedTuple]` and + // `Union[...]` containing `NamedTuple`. + // + if is_optional_named_tuple(checker, operator, slice) + || is_union_with_named_tuple(checker, operator, slice) + { + return; + } + // Avoid fixing forward references, types not in an annotation, and expressions that would // lead to invalid syntax. let fixable = checker.semantic().in_type_definition() @@ -273,6 +284,25 @@ fn is_allowed_value(expr: &Expr) -> bool { } } +/// Return `true` if this is an `Optional[typing.NamedTuple]` annotation. +fn is_optional_named_tuple(checker: &Checker, operator: Pep604Operator, slice: &Expr) -> bool { + matches!(operator, Pep604Operator::Optional) && is_named_tuple(checker, slice) +} + +/// Return `true` if this is a `Union[...]` annotation containing `typing.NamedTuple`. +fn is_union_with_named_tuple(checker: &Checker, operator: Pep604Operator, slice: &Expr) -> bool { + matches!(operator, Pep604Operator::Union) + && (is_named_tuple(checker, slice) + || slice + .as_tuple_expr() + .is_some_and(|tuple| tuple.elts.iter().any(|elt| is_named_tuple(checker, elt)))) +} + +/// Return `true` if this is a `typing.NamedTuple` annotation. +fn is_named_tuple(checker: &Checker, expr: &Expr) -> bool { + checker.semantic().match_typing_expr(expr, "NamedTuple") +} + /// Return `true` if this is an `Optional[None]` annotation. fn is_optional_none(operator: Pep604Operator, slice: &Expr) -> bool { matches!(operator, Pep604Operator::Optional) && matches!(slice, Expr::NoneLiteral(_)) diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP007.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP007.py.snap index a2b50cedc1..d61828d4c4 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP007.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP007.py.snap @@ -314,3 +314,5 @@ UP007.py:91:26: UP007 [*] Use `X | Y` for type annotations 91 |-def myfunc(param: "tuple[Union[int, 'AClass', None], str]"): 91 |+def myfunc(param: "tuple[int | 'AClass' | None, str]"): 92 92 | print(param) +93 93 | +94 94 |