Skip to content

Commit

Permalink
[flake8-pyi]: More autofixes for redundant-none-literal (PYI061) (#…
Browse files Browse the repository at this point in the history 
…14872)

Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
@kiran-4444 @AlexWaygood
kiran-4444 and AlexWaygood authored Dec 12, 2024
1 parent 68e8496 commit e5cb4d6
Show file tree
Hide file tree
Showing 3 changed files with 203 additions and 34 deletions.
107 changes: 83 additions & 24 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,15 +1,14 @@
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{Expr, ExprBinOp, ExprNoneLiteral, ExprSubscript, Operator};
use ruff_python_semantic::{
analyze::typing::{traverse_literal, traverse_union},
SemanticModel,
use ruff_python_ast::{
self as ast, Expr, ExprBinOp, ExprContext, ExprNoneLiteral, ExprSubscript, Operator,
};
use ruff_text_size::Ranged;
use ruff_python_semantic::analyze::typing::{traverse_literal, traverse_union};
use ruff_text_size::{Ranged, TextRange};

use smallvec::SmallVec;

use crate::checkers::ast::Checker;
use crate::{checkers::ast::Checker, settings::types::PythonVersion};

/// ## What it does
/// Checks for redundant `Literal[None]` annotations.
Expand All @@ -34,9 +33,14 @@ use crate::checkers::ast::Checker;
/// Literal[1, 2, 3, "foo", 5] | None
/// ```
///
/// ## Fix safety
/// ## Fix safety and availability
/// This rule's fix is marked as safe unless the literal contains comments.
///
/// There is currently no fix available if there are other elements in the `Literal` slice aside
/// from `None` and [`target-version`] is set to Python 3.9 or lower, as the fix always uses the
/// `|` syntax to create unions rather than `typing.Union`, and the `|` syntax for unions was added
/// in Python 3.10.
///
/// ## 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)
#[derive(ViolationMetadata)]
Expand Down Expand Up @@ -65,35 +69,44 @@ impl Violation for RedundantNoneLiteral {
}
}

/// RUF037
/// PYI061
pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'a Expr) {
if !checker.semantic().seen_typing() {
let semantic = checker.semantic();

if !semantic.seen_typing() {
return;
}

let Expr::Subscript(ast::ExprSubscript {
value: literal_subscript,
..
}) = literal_expr
else {
return;
};

let mut none_exprs: SmallVec<[&ExprNoneLiteral; 1]> = SmallVec::new();
let mut other_literal_elements_seen = false;
let mut literal_elements = vec![];

let mut find_none = |expr: &'a Expr, _parent: &'a Expr| {
let mut partition_literal_elements = |expr: &'a Expr, _parent: &'a Expr| {
if let Expr::NoneLiteral(none_expr) = expr {
none_exprs.push(none_expr);
} else {
other_literal_elements_seen = true;
literal_elements.push(expr);
}
};

traverse_literal(&mut find_none, checker.semantic(), literal_expr);
traverse_literal(&mut partition_literal_elements, semantic, literal_expr);

if none_exprs.is_empty() {
return;
}

// Provide a [`Fix`] when the complete `Literal` can be replaced. Applying the fix
// can leave an unused import to be fixed by the `unused-import` rule.
let fix = if other_literal_elements_seen {
None
} else {
create_fix_edit(checker.semantic(), literal_expr).map(|edit| {
let other_literal_elements_seen = !literal_elements.is_empty();

// N.B. Applying the fix can leave an unused import to be fixed by the `unused-import` rule.
let fix =
create_fix_edit(checker, literal_expr, literal_subscript, literal_elements).map(|edit| {
Fix::applicable_edit(
edit,
if checker.comment_ranges().intersects(literal_expr.range()) {
Expand All @@ -102,8 +115,7 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'
Applicability::Safe
},
)
})
};
});

for none_expr in none_exprs {
let mut diagnostic = Diagnostic::new(
Expand All @@ -128,7 +140,14 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'
/// See <https://github.com/astral-sh/ruff/issues/14567>.
///
/// [`typing.Union`]: https://docs.python.org/3/library/typing.html#typing.Union
fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option<Edit> {
fn create_fix_edit(
checker: &Checker,
literal_expr: &Expr,
literal_subscript: &Expr,
literal_elements: Vec<&Expr>,
) -> Option<Edit> {
let semantic = checker.semantic();

let enclosing_pep604_union = semantic
.current_expressions()
.skip(1)
Expand All @@ -143,8 +162,9 @@ fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option<Edit
})
.last();

let mut is_fixable = true;
if let Some(enclosing_pep604_union) = enclosing_pep604_union {
let mut is_fixable = true;

traverse_union(
&mut |expr, _| {
if matches!(expr, Expr::NoneLiteral(_)) {
Expand All @@ -163,7 +183,46 @@ fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option<Edit
semantic,
enclosing_pep604_union,
);

if !is_fixable {
return None;
}
}

if literal_elements.is_empty() {
return Some(Edit::range_replacement(
"None".to_string(),
literal_expr.range(),
));
}

if checker.settings.target_version < PythonVersion::Py310 {
return None;
}

is_fixable.then(|| Edit::range_replacement("None".to_string(), literal_expr.range()))
let bin_or = Expr::BinOp(ExprBinOp {
range: TextRange::default(),
left: Box::new(Expr::Subscript(ast::ExprSubscript {
value: Box::new(literal_subscript.clone()),
range: TextRange::default(),
ctx: ExprContext::Load,
slice: Box::new(if literal_elements.len() > 1 {
Expr::Tuple(ast::ExprTuple {
elts: literal_elements.into_iter().cloned().collect(),
range: TextRange::default(),
ctx: ExprContext::Load,
parenthesized: true,
})
} else {
literal_elements[0].clone()
}),
})),
op: ruff_python_ast::Operator::BitOr,
right: Box::new(Expr::NoneLiteral(ExprNoneLiteral {
range: TextRange::default(),
})),
});

let content = checker.generator().expr(&bin_or);
Some(Edit::range_replacement(content, literal_expr.range()))
}
89 changes: 82 additions & 7 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
@@ -1,5 +1,6 @@
---
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
snapshot_kind: text
---
PYI061.py:4:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
Expand Down Expand Up @@ -55,14 +56,24 @@ PYI061.py:12:24: PYI061 [*] `Literal[None]` can be replaced with `None`
14 14 |
15 15 |

PYI061.py:16:30: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
PYI061.py:16:30: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
16 | def func4(arg1: Literal[int, None, float]):
| ^^^^ PYI061
17 | ...
|
= help: Replace with `Literal[...] | None`

ℹ Safe fix
13 13 | ...
14 14 |
15 15 |
16 |-def func4(arg1: Literal[int, None, float]):
16 |+def func4(arg1: Literal[int, float] | None):
17 17 | ...
18 18 |
19 19 |

PYI061.py:20:25: PYI061 [*] `Literal[None]` can be replaced with `None`
|
20 | def func5(arg1: Literal[None, None]):
Expand Down Expand Up @@ -99,7 +110,7 @@ PYI061.py:20:31: PYI061 [*] `Literal[None]` can be replaced with `None`
22 22 |
23 23 |

PYI061.py:26:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
PYI061.py:26:5: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
24 | def func6(arg1: Literal[
25 | "hello",
Expand All @@ -110,6 +121,20 @@ PYI061.py:26:5: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] |
|
= help: Replace with `Literal[...] | None`

ℹ Unsafe fix
21 21 | ...
22 22 |
23 23 |
24 |-def func6(arg1: Literal[
25 |- "hello",
26 |- None # Comment 1
27 |- , "world"
28 |- ]):
24 |+def func6(arg1: Literal["hello", "world"] | None):
29 25 | ...
30 26 |
31 27 |

PYI061.py:33:5: PYI061 [*] `Literal[None]` can be replaced with `None`
|
32 | def func7(arg1: Literal[
Expand Down Expand Up @@ -177,7 +202,7 @@ PYI061.py:52:9: PYI061 [*] `Literal[None]` can be replaced with `None`
54 54 |
55 55 | ###

PYI061.py:53:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
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"
Expand All @@ -188,6 +213,16 @@ PYI061.py:53:15: PYI061 `Literal[None, ...]` can be replaced with `Literal[...]
|
= help: Replace with `Literal[...] | None`

ℹ Safe fix
50 50 |
51 51 | # From flake8-pyi
52 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"
53 |+Literal[True] | None # Y061 None inside "Literal[]" expression. Replace with "Literal[True] | None"
54 54 |
55 55 | ###
56 56 | # The following rules here are slightly subtle,

PYI061.py:62:9: PYI061 [*] `Literal[None]` can be replaced with `None`
|
60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
Expand Down Expand Up @@ -228,7 +263,7 @@ PYI061.py:62:15: PYI061 [*] `Literal[None]` can be replaced with `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`
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"
Expand All @@ -239,7 +274,17 @@ PYI061.py:63:12: PYI061 `Literal[None, ...]` can be replaced with `Literal[...]
|
= help: Replace with `Literal[...] | None`

PYI061.py:63:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
ℹ Safe fix
60 60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
61 61 | # only emit Y061...
62 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"
63 |+Literal[1, "foo"] | None # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
64 64 |
65 65 | # ... but if Y061 and Y062 both apply
66 66 | # and there are no None members in the Literal[] slice,

PYI061.py:63:25: 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"
Expand All @@ -250,7 +295,17 @@ PYI061.py:63:25: PYI061 `Literal[None, ...]` can be replaced with `Literal[...]
|
= help: Replace with `Literal[...] | None`

PYI061.py:68:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
ℹ Safe fix
60 60 | # If Y061 and Y062 both apply, but all the duplicate members are None,
61 61 | # only emit Y061...
62 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"
63 |+Literal[1, "foo"] | None # Y061 None inside "Literal[]" expression. Replace with "Literal[1, 'foo'] | None"
64 64 |
65 65 | # ... but if Y061 and Y062 both apply
66 66 | # and there are no None members in the Literal[] slice,

PYI061.py:68:9: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
66 | # and there are no None members in the Literal[] slice,
67 | # only emit Y062:
Expand All @@ -259,7 +314,17 @@ PYI061.py:68:9: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] |
|
= help: Replace with `Literal[...] | None`

PYI061.py:68:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
ℹ Safe fix
65 65 | # ... but if Y061 and Y062 both apply
66 66 | # and there are no None members in the Literal[] slice,
67 67 | # only emit Y062:
68 |-Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
68 |+Literal[True, True] | None # Y062 Duplicate "Literal[]" member "True"
69 69 |
70 70 |
71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567

PYI061.py:68:21: PYI061 [*] `Literal[None, ...]` can be replaced with `Literal[...] | None`
|
66 | # and there are no None members in the Literal[] slice,
67 | # only emit Y062:
Expand All @@ -268,6 +333,16 @@ PYI061.py:68:21: PYI061 `Literal[None, ...]` can be replaced with `Literal[...]
|
= help: Replace with `Literal[...] | None`

ℹ Safe fix
65 65 | # ... but if Y061 and Y062 both apply
66 66 | # and there are no None members in the Literal[] slice,
67 67 | # only emit Y062:
68 |-Literal[None, True, None, True] # Y062 Duplicate "Literal[]" member "True"
68 |+Literal[True, True] | None # Y062 Duplicate "Literal[]" member "True"
69 69 |
70 70 |
71 71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567

PYI061.py:72:12: PYI061 `Literal[None]` can be replaced with `None`
|
71 | # Regression tests for https://github.com/astral-sh/ruff/issues/14567
Expand Down
Loading

0 comments on commit e5cb4d6

Please sign in to comment.