Skip to content

Commit 28ab61d

Browse files
authored
[pyupgrade] Avoid PEP-604 unions with typing.NamedTuple (UP007, UP045) (#18682)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## 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: #18619
1 parent 4963835 commit 28ab61d

File tree

4 files changed

+103
-0
lines changed

4 files changed

+103
-0
lines changed

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,52 @@ class AClass:
9090

9191
def myfunc(param: "tuple[Union[int, 'AClass', None], str]"):
9292
print(param)
93+
94+
95+
from typing import NamedTuple, Union
96+
97+
import typing_extensions
98+
from typing_extensions import (
99+
NamedTuple as NamedTupleTE,
100+
Union as UnionTE,
101+
)
102+
103+
# Regression test for https://github.com/astral-sh/ruff/issues/18619
104+
# Don't emit lint for `NamedTuple`
105+
a_plain_1: Union[NamedTuple, int] = None
106+
a_plain_2: Union[int, NamedTuple] = None
107+
a_plain_3: Union[NamedTuple, None] = None
108+
a_plain_4: Union[None, NamedTuple] = None
109+
a_plain_te_1: UnionTE[NamedTupleTE, int] = None
110+
a_plain_te_2: UnionTE[int, NamedTupleTE] = None
111+
a_plain_te_3: UnionTE[NamedTupleTE, None] = None
112+
a_plain_te_4: UnionTE[None, NamedTupleTE] = None
113+
a_plain_typing_1: UnionTE[typing.NamedTuple, int] = None
114+
a_plain_typing_2: UnionTE[int, typing.NamedTuple] = None
115+
a_plain_typing_3: UnionTE[typing.NamedTuple, None] = None
116+
a_plain_typing_4: UnionTE[None, typing.NamedTuple] = None
117+
a_string_1: "Union[NamedTuple, int]" = None
118+
a_string_2: "Union[int, NamedTuple]" = None
119+
a_string_3: "Union[NamedTuple, None]" = None
120+
a_string_4: "Union[None, NamedTuple]" = None
121+
a_string_te_1: "UnionTE[NamedTupleTE, int]" = None
122+
a_string_te_2: "UnionTE[int, NamedTupleTE]" = None
123+
a_string_te_3: "UnionTE[NamedTupleTE, None]" = None
124+
a_string_te_4: "UnionTE[None, NamedTupleTE]" = None
125+
a_string_typing_1: "typing.Union[typing.NamedTuple, int]" = None
126+
a_string_typing_2: "typing.Union[int, typing.NamedTuple]" = None
127+
a_string_typing_3: "typing.Union[typing.NamedTuple, None]" = None
128+
a_string_typing_4: "typing.Union[None, typing.NamedTuple]" = None
129+
130+
b_plain_1: Union[NamedTuple] = None
131+
b_plain_2: Union[NamedTuple, None] = None
132+
b_plain_te_1: UnionTE[NamedTupleTE] = None
133+
b_plain_te_2: UnionTE[NamedTupleTE, None] = None
134+
b_plain_typing_1: UnionTE[typing.NamedTuple] = None
135+
b_plain_typing_2: UnionTE[typing.NamedTuple, None] = None
136+
b_string_1: "Union[NamedTuple]" = None
137+
b_string_2: "Union[NamedTuple, None]" = None
138+
b_string_te_1: "UnionTE[NamedTupleTE]" = None
139+
b_string_te_2: "UnionTE[NamedTupleTE, None]" = None
140+
b_string_typing_1: "typing.Union[typing.NamedTuple]" = None
141+
b_string_typing_2: "typing.Union[typing.NamedTuple, None]" = None

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP045.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,25 @@ class ServiceRefOrValue:
4747
# Test for: https://github.com/astral-sh/ruff/issues/18508
4848
# Optional[None] should not be offered a fix
4949
foo: Optional[None] = None
50+
51+
52+
from typing import NamedTuple, Optional
53+
54+
import typing_extensions
55+
from typing_extensions import (
56+
NamedTuple as NamedTupleTE,
57+
Optional as OptionalTE,
58+
)
59+
60+
# Regression test for https://github.com/astral-sh/ruff/issues/18619
61+
# Don't emit lint for `NamedTuple`
62+
a1: Optional[NamedTuple] = None
63+
a2: typing.Optional[NamedTuple] = None
64+
a3: OptionalTE[NamedTuple] = None
65+
a4: typing_extensions.Optional[NamedTuple] = None
66+
a5: Optional[typing.NamedTuple] = None
67+
a6: typing.Optional[typing.NamedTuple] = None
68+
a7: OptionalTE[typing.NamedTuple] = None
69+
a8: typing_extensions.Optional[typing.NamedTuple] = None
70+
a9: "Optional[NamedTuple]" = None
71+
a10: Optional[NamedTupleTE] = None

crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ pub(crate) fn non_pep604_annotation(
132132
slice: &Expr,
133133
operator: Pep604Operator,
134134
) {
135+
// `NamedTuple` is not a type; it's a type constructor. Using it in a type annotation doesn't
136+
// make much sense. But since type checkers will currently (incorrectly) _not_ complain about it
137+
// being used in a type annotation, we just ignore `Optional[typing.NamedTuple]` and
138+
// `Union[...]` containing `NamedTuple`.
139+
// <https://github.com/astral-sh/ruff/issues/18619>
140+
if is_optional_named_tuple(checker, operator, slice)
141+
|| is_union_with_named_tuple(checker, operator, slice)
142+
{
143+
return;
144+
}
145+
135146
// Avoid fixing forward references, types not in an annotation, and expressions that would
136147
// lead to invalid syntax.
137148
let fixable = checker.semantic().in_type_definition()
@@ -273,6 +284,25 @@ fn is_allowed_value(expr: &Expr) -> bool {
273284
}
274285
}
275286

287+
/// Return `true` if this is an `Optional[typing.NamedTuple]` annotation.
288+
fn is_optional_named_tuple(checker: &Checker, operator: Pep604Operator, slice: &Expr) -> bool {
289+
matches!(operator, Pep604Operator::Optional) && is_named_tuple(checker, slice)
290+
}
291+
292+
/// Return `true` if this is a `Union[...]` annotation containing `typing.NamedTuple`.
293+
fn is_union_with_named_tuple(checker: &Checker, operator: Pep604Operator, slice: &Expr) -> bool {
294+
matches!(operator, Pep604Operator::Union)
295+
&& (is_named_tuple(checker, slice)
296+
|| slice
297+
.as_tuple_expr()
298+
.is_some_and(|tuple| tuple.elts.iter().any(|elt| is_named_tuple(checker, elt))))
299+
}
300+
301+
/// Return `true` if this is a `typing.NamedTuple` annotation.
302+
fn is_named_tuple(checker: &Checker, expr: &Expr) -> bool {
303+
checker.semantic().match_typing_expr(expr, "NamedTuple")
304+
}
305+
276306
/// Return `true` if this is an `Optional[None]` annotation.
277307
fn is_optional_none(operator: Pep604Operator, slice: &Expr) -> bool {
278308
matches!(operator, Pep604Operator::Optional) && matches!(slice, Expr::NoneLiteral(_))

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP007.py.snap

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,3 +314,5 @@ UP007.py:91:26: UP007 [*] Use `X | Y` for type annotations
314314
91 |-def myfunc(param: "tuple[Union[int, 'AClass', None], str]"):
315315
91 |+def myfunc(param: "tuple[int | 'AClass' | None, str]"):
316316
92 92 | print(param)
317+
93 93 |
318+
94 94 |

0 commit comments

Comments
 (0)