Skip to content

Commit 9d5ecac

Browse files
soundsonacidntBre
andauthored
[flake8-pyi] Skip fix if all Union members are None (PYI016) (#19416)
patches #19403 --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
1 parent 9af8597 commit 9d5ecac

File tree

4 files changed

+53
-1
lines changed

4 files changed

+53
-1
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI016.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,7 @@ def func2() -> str | str: # PYI016: Duplicate union member `str`
142142
# avoid reporting twice
143143
field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
144144
field49: typing.Optional[complex | complex] | complex
145+
146+
# Regression test for https://github.com/astral-sh/ruff/issues/19403
147+
# Should throw duplicate union member but not fix
148+
isinstance(None, typing.Union[None, None])

crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_union_member.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ pub(crate) fn duplicate_union_member<'a>(checker: &Checker, expr: &'a Expr) {
6464
let mut diagnostics = Vec::new();
6565

6666
let mut union_type = UnionKind::TypingUnion;
67+
let mut optional_present = false;
6768
// Adds a member to `literal_exprs` if it is a `Literal` annotation
6869
let mut check_for_duplicate_members = |expr: &'a Expr, parent: &'a Expr| {
6970
if matches!(parent, Expr::BinOp(_)) {
@@ -74,6 +75,7 @@ pub(crate) fn duplicate_union_member<'a>(checker: &Checker, expr: &'a Expr) {
7475
&& is_optional_type(checker, expr)
7576
{
7677
// If the union member is an `Optional`, add a virtual `None` literal.
78+
optional_present = true;
7779
&VIRTUAL_NONE_LITERAL
7880
} else {
7981
expr
@@ -87,7 +89,7 @@ pub(crate) fn duplicate_union_member<'a>(checker: &Checker, expr: &'a Expr) {
8789
DuplicateUnionMember {
8890
duplicate_name: checker.generator().expr(virtual_expr),
8991
},
90-
// Use the real expression's range for diagnostics,
92+
// Use the real expression's range for diagnostics.
9193
expr.range(),
9294
));
9395
}
@@ -104,6 +106,13 @@ pub(crate) fn duplicate_union_member<'a>(checker: &Checker, expr: &'a Expr) {
104106
return;
105107
}
106108

109+
// Do not reduce `Union[None, ... None]` to avoid introducing a `TypeError` unintentionally
110+
// e.g. `isinstance(None, Union[None, None])`, if reduced to `isinstance(None, None)`, causes
111+
// `TypeError: isinstance() arg 2 must be a type, a tuple of types, or a union` to throw.
112+
if unique_nodes.iter().all(|expr| expr.is_none_literal_expr()) && !optional_present {
113+
return;
114+
}
115+
107116
// Mark [`Fix`] as unsafe when comments are in range.
108117
let applicability = if checker.comment_ranges().intersects(expr.range()) {
109118
Applicability::Unsafe

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI016_PYI016.py.snap

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,13 +974,17 @@ PYI016.py:143:61: PYI016 [*] Duplicate union member `complex`
974974
143 |-field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
975975
143 |+field48: typing.Union[typing.Optional[complex], complex]
976976
144 144 | field49: typing.Optional[complex | complex] | complex
977+
145 145 |
978+
146 146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
977979

978980
PYI016.py:144:36: PYI016 [*] Duplicate union member `complex`
979981
|
980982
142 | # avoid reporting twice
981983
143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
982984
144 | field49: typing.Optional[complex | complex] | complex
983985
| ^^^^^^^ PYI016
986+
145 |
987+
146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
984988
|
985989
= help: Remove duplicate union member `complex`
986990

@@ -990,3 +994,15 @@ PYI016.py:144:36: PYI016 [*] Duplicate union member `complex`
990994
143 143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
991995
144 |-field49: typing.Optional[complex | complex] | complex
992996
144 |+field49: typing.Optional[complex] | complex
997+
145 145 |
998+
146 146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
999+
147 147 | # Should throw duplicate union member but not fix
1000+
1001+
PYI016.py:148:37: PYI016 Duplicate union member `None`
1002+
|
1003+
146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
1004+
147 | # Should throw duplicate union member but not fix
1005+
148 | isinstance(None, typing.Union[None, None])
1006+
| ^^^^ PYI016
1007+
|
1008+
= help: Remove duplicate union member `None`

crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__preview__PYI016_PYI016.py.snap

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,8 @@ PYI016.py:143:61: PYI016 [*] Duplicate union member `complex`
11621162
143 |-field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
11631163
143 |+field48: typing.Union[None, complex]
11641164
144 144 | field49: typing.Optional[complex | complex] | complex
1165+
145 145 |
1166+
146 146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
11651167

11661168
PYI016.py:143:72: PYI016 [*] Duplicate union member `complex`
11671169
|
@@ -1179,13 +1181,17 @@ PYI016.py:143:72: PYI016 [*] Duplicate union member `complex`
11791181
143 |-field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
11801182
143 |+field48: typing.Union[None, complex]
11811183
144 144 | field49: typing.Optional[complex | complex] | complex
1184+
145 145 |
1185+
146 146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
11821186

11831187
PYI016.py:144:36: PYI016 [*] Duplicate union member `complex`
11841188
|
11851189
142 | # avoid reporting twice
11861190
143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
11871191
144 | field49: typing.Optional[complex | complex] | complex
11881192
| ^^^^^^^ PYI016
1193+
145 |
1194+
146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
11891195
|
11901196
= help: Remove duplicate union member `complex`
11911197

@@ -1195,13 +1201,18 @@ PYI016.py:144:36: PYI016 [*] Duplicate union member `complex`
11951201
143 143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
11961202
144 |-field49: typing.Optional[complex | complex] | complex
11971203
144 |+field49: None | complex
1204+
145 145 |
1205+
146 146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
1206+
147 147 | # Should throw duplicate union member but not fix
11981207

11991208
PYI016.py:144:47: PYI016 [*] Duplicate union member `complex`
12001209
|
12011210
142 | # avoid reporting twice
12021211
143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
12031212
144 | field49: typing.Optional[complex | complex] | complex
12041213
| ^^^^^^^ PYI016
1214+
145 |
1215+
146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
12051216
|
12061217
= help: Remove duplicate union member `complex`
12071218

@@ -1211,3 +1222,15 @@ PYI016.py:144:47: PYI016 [*] Duplicate union member `complex`
12111222
143 143 | field48: typing.Union[typing.Optional[typing.Union[complex, complex]], complex]
12121223
144 |-field49: typing.Optional[complex | complex] | complex
12131224
144 |+field49: None | complex
1225+
145 145 |
1226+
146 146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
1227+
147 147 | # Should throw duplicate union member but not fix
1228+
1229+
PYI016.py:148:37: PYI016 Duplicate union member `None`
1230+
|
1231+
146 | # Regression test for https://github.com/astral-sh/ruff/issues/19403
1232+
147 | # Should throw duplicate union member but not fix
1233+
148 | isinstance(None, typing.Union[None, None])
1234+
| ^^^^ PYI016
1235+
|
1236+
= help: Remove duplicate union member `None`

0 commit comments

Comments
 (0)