Skip to content

Commit

Permalink
Avoid syntax errors on fix.
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbrugman
sbrugman committed Nov 25, 2024
1 parent fa22bd6 commit 5680e20
Show file tree
Hide file tree
Showing 5 changed files with 326 additions and 91 deletions.
16 changes: 15 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Literal
from typing import Literal, Union


def func1(arg1: Literal[None]):
Expand Down Expand Up @@ -35,6 +35,14 @@ def func7(arg1: Literal[
...


def func8(arg1: Literal[None] | None):
...


def func9(arg1: Union[Literal[None], None]):
...


# OK
def good_func(arg1: Literal[int] | None):
...
Expand All @@ -58,3 +66,9 @@ def good_func(arg1: Literal[int] | None):
# and there are no None members in the Literal[] slice,
# only emit Y062:
Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"


# Regression tests for https://github.com/astral-sh/ruff/issues/14567
x: Literal[None] | None
y: None | Literal[None]
z: Union[Literal[None], None]
14 changes: 13 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Literal
from typing import Literal, Union


def func1(arg1: Literal[None]): ...
Expand Down Expand Up @@ -28,10 +28,22 @@ def func7(arg1: Literal[
]): ...


def func8(arg1: Literal[None] | None):...


def func9(arg1: Union[Literal[None], None]): ...


# OK
def good_func(arg1: Literal[int] | None): ...


# From flake8-pyi
Literal[None] # PYI061 None inside "Literal[]" expression. Replace with "None"
Literal[True, None] # PYI061 None inside "Literal[]" expression. Replace with "Literal[True] | None"


# Regression tests for https://github.com/astral-sh/ruff/issues/14567
x: Literal[None] | None
y: None | Literal[None]
z: Union[Literal[None], None]
35 changes: 33 additions & 2 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprNoneLiteral};
use ruff_python_ast::{Expr, ExprBinOp, ExprNoneLiteral, Operator};
use ruff_python_semantic::analyze::typing::traverse_literal;
use ruff_text_size::Ranged;

Expand Down Expand Up @@ -31,8 +31,13 @@ use crate::checkers::ast::Checker;
/// Literal[1, 2, 3, "foo", 5] | None
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as safe unless the literal contains comments.
///
/// ## References
/// - [Typing documentation: Legal parameters for `Literal` at type check time](https://typing.readthedocs.io/en/latest/spec/literal.html#legal-parameters-for-literal-at-type-check-time)
///
/// [#107271](https://github.com/python/cpython/issues/107271)
#[violation]
pub struct RedundantNoneLiteral {
other_literal_elements_seen: bool,
Expand Down Expand Up @@ -87,8 +92,34 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'
let fix = if other_literal_elements_seen {
None
} else {
// Avoid producing syntax errors when `Literal[None] | None` would be fixed to
// `None | None`. Instead fix to `None`. No action needed from `typing.Union`,
// as `Union[None, None]` is valid syntax.
// See https://github.com/astral-sh/ruff/issues/14567.
let replacement_range = if let Some(parent) = checker.semantic().current_expression_parent()
{
if let Expr::BinOp(ExprBinOp {
left,
op: Operator::BitOr,
right,
..
}) = parent
{
if matches!(**left, Expr::NoneLiteral(_)) || matches!(**right, Expr::NoneLiteral(_))
{
parent.range()
} else {
literal_expr.range()
}
} else {
literal_expr.range()
}
} else {
literal_expr.range()
};

Some(Fix::applicable_edit(
Edit::range_replacement("None".to_string(), literal_expr.range()),
Edit::range_replacement("None".to_string(), replacement_range),
if checker.comment_ranges().intersects(literal_expr.range()) {
Applicability::Unsafe
} else {
Expand Down
231 changes: 160 additions & 71 deletions ...c/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI061_PYI061.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ PYI061.py:4:25: PYI061 [*] `Literal[None]` can be replaced with `None`
= help: Replace with `None`

ℹ Safe fix
1 1 | from typing import Literal
1 1 | from typing import Literal, Union
2 2 |
3 3 |
4 |-def func1(arg1: Literal[None]):
Expand Down Expand Up @@ -132,112 +132,201 @@ PYI061.py:33:5: PYI061 [*] `Literal[None]` can be replaced with `None`
36 34 |
37 35 |

PYI061.py:44:9: PYI061 [*] `Literal[None]` can be replaced with `None`
PYI061.py:38:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
43 | # From flake8-pyi
44 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
45 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
38 | def func8(arg1: Literal[None] | None):
| ^^^^ PYI061
39 | ...
|
= help: Replace with `None`

ℹ Safe fix
35 35 | ...
36 36 |
37 37 |
38 |-def func8(arg1: Literal[None] | None):
38 |+def func8(arg1: None):
39 39 | ...
40 40 |
41 41 |
42 42 |
43 43 | # From flake8-pyi
44 |-Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
44 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
45 45 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
46 46 |
47 47 | ###

PYI061.py:45:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
43 | # From flake8-pyi
44 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
45 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"

PYI061.py:42:31: PYI061 [*] `Literal[None]` can be replaced with `None`
|
42 | def func9(arg1: Union[Literal[None], None]):
| ^^^^ PYI061
43 | ...
|
= help: Replace with `None`

ℹ Safe fix
39 39 | ...
40 40 |
41 41 |
42 |-def func9(arg1: Union[Literal[None], None]):
42 |+def func9(arg1: Union[None, None]):
43 43 | ...
44 44 |
45 45 |

PYI061.py:52:9: PYI061 [*] `Literal[None]` can be replaced with `None`
|
51 | # From flake8-pyi
52 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
53 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
|
= help: Replace with `None`

ℹ Safe fix
49 49 |
50 50 |
51 51 | # From flake8-pyi
52 |-Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
52 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
53 53 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
54 54 |
55 55 | ###

PYI061.py:53:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
51 | # From flake8-pyi
52 | Literal[None] # Y061 None inside "Literal[]" expression. Replace with "None"
53 | Literal[True, None] # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
| ^^^^ PYI061
46 |
47 | ###
54 |
55 | ###
|
= help: Replace with `Literal[...] | None`

PYI061.py:54:9: PYI061 [*] `Literal[None]` can be replaced with `None`
PYI061.py:62:9: PYI061 [*] `Literal[None]` can be replaced with `None`
|
52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
61 | # only emit Y061...
62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
63 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
|
= help: Replace with `None`

ℹ Safe fix
51 51 |
52 52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 53 | # only emit Y061...
54 |-Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
54 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
55 55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
56 56 |
57 57 | # ... but if Y061 and Y062 both apply

PYI061.py:54:15: PYI061 [*] `Literal[None]` can be replaced with `None`
|
52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
59 59 |
60 60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
61 61 | # only emit Y061...
62 |-Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
62 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
63 63 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
64 64 |
65 65 | # ... but if Y061 and Y062 both apply

PYI061.py:62:15: PYI061 [*] `Literal[None]` can be replaced with `None`
|
60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
61 | # only emit Y061...
62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
| ^^^^ PYI061
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
63 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
|
= help: Replace with `None`

ℹ Safe fix
51 51 |
52 52 | # If Y061 and Y062 both apply, but all the duplicate members are None,
53 53 | # only emit Y061...
54 |-Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
54 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
55 55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
56 56 |
57 57 | # ... but if Y061 and Y062 both apply

PYI061.py:55:12: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
59 59 |
60 60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
61 61 | # only emit Y061...
62 |-Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
62 |+None # Y061 None inside "Literal[]" expression. Replace with "None"
63 63 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
64 64 |
65 65 | # ... but if Y061 and Y062 both apply

PYI061.py:63:12: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
61 | # only emit Y061...
62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
63 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
| ^^^^ PYI061
56 |
57 | # ... but if Y061 and Y062 both apply
64 |
65 | # ... but if Y061 and Y062 both apply
|
= help: Replace with `Literal[...] | None`

PYI061.py:55:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
PYI061.py:63:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
53 | # only emit Y061...
54 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
55 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
61 | # only emit Y061...
62 | Literal[None, None] # Y061 None inside "Literal[]" expression. Replace with "None"
63 | Literal[1, None, "foo", None] # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
| ^^^^ PYI061
56 |
57 | # ... but if Y061 and Y062 both apply
64 |
65 | # ... but if Y061 and Y062 both apply
|
= help: Replace with `Literal[...] | None`

PYI061.py:60:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
PYI061.py:68:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
58 | # and there are no None members in the Literal[] slice,
59 | # only emit Y062:
60 | Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
66 | # and there are no None members in the Literal[] slice,
67 | # only emit Y062:
68 | Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
| ^^^^ PYI061
|
= help: Replace with `Literal[...] | None`

PYI061.py:60:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
PYI061.py:68:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
58 | # and there are no None members in the Literal[] slice,
59 | # only emit Y062:
60 | Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
66 | # and there are no None members in the Literal[] slice,
67 | # only emit Y062:
68 | Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
| ^^^^ PYI061
|
= help: Replace with `Literal[...] | None`

PYI061.py:72:12: PYI061 [*] `Literal[None]` can be replaced with `None`
|
71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567
72 | x: Literal[None] | None
| ^^^^ PYI061
73 | y: None | Literal[None]
74 | z: Union[Literal[None], None]
|
= help: Replace with `None`

ℹ Safe fix
69 69 |
70 70 |
71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567
72 |-x: Literal[None] | None
72 |+x: None
73 73 | y: None | Literal[None]
74 74 | z: Union[Literal[None], None]

PYI061.py:73:19: PYI061 [*] `Literal[None]` can be replaced with `None`
|
71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567
72 | x: Literal[None] | None
73 | y: None | Literal[None]
| ^^^^ PYI061
74 | z: Union[Literal[None], None]
|
= help: Replace with `None`

ℹ Safe fix
70 70 |
71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567
72 72 | x: Literal[None] | None
73 |-y: None | Literal[None]
73 |+y: None
74 74 | z: Union[Literal[None], None]

PYI061.py:74:18: PYI061 [*] `Literal[None]` can be replaced with `None`
|
72 | x: Literal[None] | None
73 | y: None | Literal[None]
74 | z: Union[Literal[None], None]
| ^^^^ PYI061
|
= help: Replace with `None`

ℹ Safe fix
71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567
72 72 | x: Literal[None] | None
73 73 | y: None | Literal[None]
74 |-z: Union[Literal[None], None]
74 |+z: Union[None, None]
Loading

0 comments on commit 5680e20

Please sign in to comment.