Skip to content

Commit

Permalink
[flake8-pyi] Improve autofix safety for redundant-none-literal (P…
Browse files Browse the repository at this point in the history
…YI061) (#14583)

Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
sbrugman and AlexWaygood authored Nov 25, 2024
1 parent e8fce20 commit c606bf0
Show file tree
Hide file tree
Showing 5 changed files with 480 additions and 112 deletions.
29 changes: 25 additions & 4 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.py
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 All @@ -17,21 +17,29 @@ def func4(arg1: Literal[int, None, float]):
...


def func5(arg1: Literal[None, None]):
def func5(arg1: Literal[None, None]):
...


def func6(arg1: Literal[
"hello",
None # Comment 1
, "world"
]):
]):
...


def func7(arg1: Literal[
None # Comment 1
]):
]):
...


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


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


Expand All @@ -58,3 +66,16 @@ 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]

a: int | Literal[None] | None
b: None | Literal[None] | None
c: (None | Literal[None]) | None
d: None | (Literal[None] | None)
e: None | ((None | Literal[None]) | None) | None
f: Literal[None] | Literal[None]
21 changes: 20 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI061.pyi
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,29 @@ 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]

a: int | Literal[None] | None
b: None | Literal[None] | None
c: (None | Literal[None]) | None
d: None | (Literal[None] | None)
e: None | ((None | Literal[None]) | None) | None
f: Literal[None] | Literal[None]
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
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_semantic::analyze::typing::traverse_literal;
use ruff_python_ast::{Expr, ExprBinOp, ExprNoneLiteral, ExprSubscript, Operator};
use ruff_python_semantic::{
analyze::typing::{traverse_literal, traverse_union},
SemanticModel,
};
use ruff_text_size::Ranged;

use smallvec::SmallVec;
Expand Down Expand Up @@ -31,6 +34,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,14 +93,16 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'
let fix = if other_literal_elements_seen {
None
} else {
Some(Fix::applicable_edit(
Edit::range_replacement("None".to_string(), literal_expr.range()),
if checker.comment_ranges().intersects(literal_expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
},
))
create_fix_edit(checker.semantic(), literal_expr).map(|edit| {
Fix::applicable_edit(
edit,
if checker.comment_ranges().intersects(literal_expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
},
)
})
};

for none_expr in none_exprs {
Expand All @@ -110,3 +118,47 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'
checker.diagnostics.push(diagnostic);
}
}

fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option<Edit> {
let mut enclosing_union = None;
for expr in semantic.current_expressions().skip(1).take_while(|expr| {
matches!(
expr,
Expr::BinOp(ExprBinOp {
op: Operator::BitOr,
..
})
)
}) {
enclosing_union = Some(expr);
}

let mut is_fixable = true;
if let Some(enclosing_union) = enclosing_union {
traverse_union(
&mut |expr, _| {
if matches!(expr, Expr::NoneLiteral(_)) {
is_fixable = false;
}
if expr != literal_expr {
if let Expr::Subscript(ExprSubscript { value, slice, .. }) = expr {
if semantic.match_typing_expr(value, "Literal")
&& matches!(**slice, Expr::NoneLiteral(_))
{
is_fixable = false;
}
}
}
},
semantic,
enclosing_union,
);
}

// Avoid producing code that would raise an exception when
// `Literal[None] | None` would be fixed to `None | None`.
// Instead do not provide a fix. No action needed for `typing.Union`,
// as `Union[None, None]` is valid Python.
// See https://github.com/astral-sh/ruff/issues/14567.
is_fixable.then(|| Edit::range_replacement("None".to_string(), literal_expr.range()))
}
Loading

0 comments on commit c606bf0

Please sign in to comment.