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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading