Skip to content

Commit 98d27c4

Browse files
danparizherntBre
danparizher
and
ntBre
authored
[flake8-pyi] Fix operator precedence by adding parentheses when needed (PYI061) (#20508)
## Summary Fixes #20265 --------- Co-authored-by: Brent Westbrook <[email protected]>
1 parent c06c3f9 commit 98d27c4

File tree

4 files changed

+317
-1
lines changed

4 files changed

+317
-1
lines changed

‎crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,11 @@ def good_func(arg1: Literal[int] | None):
7878
c: (None | Literal[None]) | None
7979
d: None | (Literal[None] | None)
8080
e: None | ((None | Literal[None]) | None) | None
81+
82+
# Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
83+
print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
84+
print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
85+
print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
86+
print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
87+
print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
88+
print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__

‎crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use ruff_python_ast::{
44
self as ast, Expr, ExprBinOp, ExprContext, ExprNoneLiteral, Operator, PythonVersion,
55
helpers::{pep_604_union, typing_optional},
66
name::Name,
7+
operator_precedence::OperatorPrecedence,
8+
parenthesize::parenthesized_range,
79
};
810
use ruff_python_semantic::analyze::typing::{traverse_literal, traverse_union};
911
use ruff_text_size::{Ranged, TextRange};
@@ -238,7 +240,19 @@ fn create_fix(
238240
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
239241
});
240242
let union_expr = pep_604_union(&[new_literal_expr, none_expr]);
241-
let content = checker.generator().expr(&union_expr);
243+
244+
// Check if we need parentheses to preserve operator precedence
245+
let content = if needs_parentheses_for_precedence(
246+
semantic,
247+
literal_expr,
248+
checker.comment_ranges(),
249+
checker.source(),
250+
) {
251+
format!("({})", checker.generator().expr(&union_expr))
252+
} else {
253+
checker.generator().expr(&union_expr)
254+
};
255+
242256
let union_edit = Edit::range_replacement(content, literal_expr.range());
243257
Fix::applicable_edit(union_edit, applicability)
244258
}
@@ -256,3 +270,37 @@ enum UnionKind {
256270
TypingOptional,
257271
BitOr,
258272
}
273+
274+
/// Check if the union expression needs parentheses to preserve operator precedence.
275+
/// This is needed when the union is part of a larger expression where the `|` operator
276+
/// has lower precedence than the surrounding operations (like attribute access).
277+
fn needs_parentheses_for_precedence(
278+
semantic: &ruff_python_semantic::SemanticModel,
279+
literal_expr: &Expr,
280+
comment_ranges: &ruff_python_trivia::CommentRanges,
281+
source: &str,
282+
) -> bool {
283+
// Get the parent expression to check if we're in a context that needs parentheses
284+
let Some(parent_expr) = semantic.current_expression_parent() else {
285+
return false;
286+
};
287+
288+
// Check if the literal expression is already parenthesized
289+
if parenthesized_range(
290+
literal_expr.into(),
291+
parent_expr.into(),
292+
comment_ranges,
293+
source,
294+
)
295+
.is_some()
296+
{
297+
return false; // Already parenthesized, don't add more
298+
}
299+
300+
// Check if the parent expression has higher precedence than the `|` operator
301+
let union_precedence = OperatorPrecedence::BitOr;
302+
let parent_precedence = OperatorPrecedence::from(parent_expr);
303+
304+
// If the parent operation has higher precedence than `|`, we need parentheses
305+
parent_precedence > union_precedence
306+
}

‎crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.py.snap‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,5 +423,117 @@ PYI061 Use `None` rather than `Literal[None]`
423423
79 | d: None | (Literal[None] | None)
424424
80 | e: None | ((None | Literal[None]) | None) | None
425425
| ^^^^
426+
81 |
427+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
426428
|
427429
help: Replace with `None`
430+
431+
PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]`
432+
--> PYI061.py:83:18
433+
|
434+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
435+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
436+
| ^^^^
437+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
438+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
439+
|
440+
help: Replace with `Literal[...] | None`
441+
80 | e: None | ((None | Literal[None]) | None) | None
442+
81 |
443+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
444+
- print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
445+
83 + print((Literal[1] | None).__dict__) # Should become (Literal[1] | None).__dict__
446+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
447+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
448+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
449+
450+
PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]`
451+
--> PYI061.py:84:18
452+
|
453+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
454+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
455+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
456+
| ^^^^
457+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
458+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
459+
|
460+
help: Replace with `Literal[...] | None`
461+
81 |
462+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
463+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
464+
- print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
465+
84 + print((Literal[1] | None).method()) # Should become (Literal[1] | None).method()
466+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
467+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
468+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
469+
470+
PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]`
471+
--> PYI061.py:85:18
472+
|
473+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
474+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
475+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
476+
| ^^^^
477+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
478+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
479+
|
480+
help: Replace with `Literal[...] | None`
481+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
482+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
483+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
484+
- print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
485+
85 + print((Literal[1] | None)[0]) # Should become (Literal[1] | None)[0]
486+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
487+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
488+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
489+
490+
PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]`
491+
--> PYI061.py:86:18
492+
|
493+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
494+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
495+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
496+
| ^^^^
497+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
498+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
499+
|
500+
help: Replace with `Literal[...] | None`
501+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
502+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
503+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
504+
- print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
505+
86 + print((Literal[1] | None) + 1) # Should become (Literal[1] | None) + 1
506+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
507+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
508+
509+
PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]`
510+
--> PYI061.py:87:18
511+
|
512+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
513+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
514+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
515+
| ^^^^
516+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
517+
|
518+
help: Replace with `Literal[...] | None`
519+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
520+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
521+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
522+
- print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
523+
87 + print((Literal[1] | None) * 2) # Should become (Literal[1] | None) * 2
524+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
525+
526+
PYI061 [*] Use `Literal[...] | None` rather than `Literal[None, ...]`
527+
--> PYI061.py:88:19
528+
|
529+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
530+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
531+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
532+
| ^^^^
533+
|
534+
help: Replace with `Literal[...] | None`
535+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
536+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
537+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
538+
- print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
539+
88 + print((Literal[1] | None).__dict__) # Should become ((Literal[1] | None)).__dict__

‎crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__py38_PYI061_PYI061.py.snap‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,5 +465,153 @@ PYI061 Use `None` rather than `Literal[None]`
465465
79 | d: None | (Literal[None] | None)
466466
80 | e: None | ((None | Literal[None]) | None) | None
467467
| ^^^^
468+
81 |
469+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
468470
|
469471
help: Replace with `None`
472+
473+
PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]`
474+
--> PYI061.py:83:18
475+
|
476+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
477+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
478+
| ^^^^
479+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
480+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
481+
|
482+
help: Replace with `Optional[Literal[...]]`
483+
- from typing import Literal, Union
484+
1 + from typing import Literal, Union, Optional
485+
2 |
486+
3 |
487+
4 | def func1(arg1: Literal[None]):
488+
--------------------------------------------------------------------------------
489+
80 | e: None | ((None | Literal[None]) | None) | None
490+
81 |
491+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
492+
- print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
493+
83 + print(Optional[Literal[1]].__dict__) # Should become (Literal[1] | None).__dict__
494+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
495+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
496+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
497+
498+
PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]`
499+
--> PYI061.py:84:18
500+
|
501+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
502+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
503+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
504+
| ^^^^
505+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
506+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
507+
|
508+
help: Replace with `Optional[Literal[...]]`
509+
- from typing import Literal, Union
510+
1 + from typing import Literal, Union, Optional
511+
2 |
512+
3 |
513+
4 | def func1(arg1: Literal[None]):
514+
--------------------------------------------------------------------------------
515+
81 |
516+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
517+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
518+
- print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
519+
84 + print(Optional[Literal[1]].method()) # Should become (Literal[1] | None).method()
520+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
521+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
522+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
523+
524+
PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]`
525+
--> PYI061.py:85:18
526+
|
527+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
528+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
529+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
530+
| ^^^^
531+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
532+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
533+
|
534+
help: Replace with `Optional[Literal[...]]`
535+
- from typing import Literal, Union
536+
1 + from typing import Literal, Union, Optional
537+
2 |
538+
3 |
539+
4 | def func1(arg1: Literal[None]):
540+
--------------------------------------------------------------------------------
541+
82 | # Test cases for operator precedence issue (https://github.com/astral-sh/ruff/issues/20265)
542+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
543+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
544+
- print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
545+
85 + print(Optional[Literal[1]][0]) # Should become (Literal[1] | None)[0]
546+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
547+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
548+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
549+
550+
PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]`
551+
--> PYI061.py:86:18
552+
|
553+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
554+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
555+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
556+
| ^^^^
557+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
558+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
559+
|
560+
help: Replace with `Optional[Literal[...]]`
561+
- from typing import Literal, Union
562+
1 + from typing import Literal, Union, Optional
563+
2 |
564+
3 |
565+
4 | def func1(arg1: Literal[None]):
566+
--------------------------------------------------------------------------------
567+
83 | print(Literal[1, None].__dict__) # Should become (Literal[1] | None).__dict__
568+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
569+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
570+
- print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
571+
86 + print(Optional[Literal[1]] + 1) # Should become (Literal[1] | None) + 1
572+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
573+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
574+
575+
PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]`
576+
--> PYI061.py:87:18
577+
|
578+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
579+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
580+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
581+
| ^^^^
582+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
583+
|
584+
help: Replace with `Optional[Literal[...]]`
585+
- from typing import Literal, Union
586+
1 + from typing import Literal, Union, Optional
587+
2 |
588+
3 |
589+
4 | def func1(arg1: Literal[None]):
590+
--------------------------------------------------------------------------------
591+
84 | print(Literal[1, None].method()) # Should become (Literal[1] | None).method()
592+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
593+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
594+
- print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
595+
87 + print(Optional[Literal[1]] * 2) # Should become (Literal[1] | None) * 2
596+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
597+
598+
PYI061 [*] Use `Optional[Literal[...]]` rather than `Literal[None, ...]`
599+
--> PYI061.py:88:19
600+
|
601+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
602+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
603+
88 | print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
604+
| ^^^^
605+
|
606+
help: Replace with `Optional[Literal[...]]`
607+
- from typing import Literal, Union
608+
1 + from typing import Literal, Union, Optional
609+
2 |
610+
3 |
611+
4 | def func1(arg1: Literal[None]):
612+
--------------------------------------------------------------------------------
613+
85 | print(Literal[1, None][0]) # Should become (Literal[1] | None)[0]
614+
86 | print(Literal[1, None] + 1) # Should become (Literal[1] | None) + 1
615+
87 | print(Literal[1, None] * 2) # Should become (Literal[1] | None) * 2
616+
- print((Literal[1, None]).__dict__) # Should become ((Literal[1] | None)).__dict__
617+
88 + print((Optional[Literal[1]]).__dict__) # Should become ((Literal[1] | None)).__dict__

0 commit comments

Comments
 (0)