Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-pyi] Improve autofix safety for redundant-none-literal (PYI061) #14583

Merged
merged 8 commits into from
Nov 25, 2024
Merged

[flake8-pyi] Improve autofix safety for redundant-none-literal (PYI061) #14583

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9a55a6b
Avoid syntax errors on fix.
sbrugman Nov 25, 2024
70f1f06
comment nit
AlexWaygood Nov 25, 2024
7ee67f6
Update crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_l…
AlexWaygood Nov 25, 2024
78bf0cf
Update crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_l…
AlexWaygood Nov 25, 2024
175586f
simplify slightly
AlexWaygood Nov 25, 2024
df2683a
fmt
AlexWaygood Nov 25, 2024
6a05263
Abstain from fixing when union contains bare None literal.
sbrugman Nov 25, 2024
5a3c341
small cleanups
AlexWaygood Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py
View file
Open in desktop
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
Open in desktop
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]
33 changes: 31 additions & 2 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_none_literal.rs
View file
Open in desktop
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,6 +31,9 @@ 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)
#[violation]
Expand Down Expand Up @@ -87,8 +90,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
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
// `None | None`. Instead fix to `None`. No action needed from `typing.Union`,
// as `Union[None, None]` is valid syntax.
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
// 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
Open in desktop
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
Loading