Skip to content

Commit

Permalink
Consolidate the two fix functions and offer no fix on Python <=3.9
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexWaygood
AlexWaygood committed Dec 12, 2024
1 parent 8663352 commit c9150d4
Showing 1 changed file with 53 additions and 87 deletions.
140 changes: 53 additions & 87 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
Expand Up @@ -3,15 +3,12 @@ use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{
self as ast, Expr, ExprBinOp, ExprContext, ExprNoneLiteral, ExprSubscript, Operator,
};
use ruff_python_semantic::{
analyze::typing::{traverse_literal, traverse_union},
SemanticModel,
};
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 @@ -36,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 @@ -67,55 +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 mut none_exprs: SmallVec<[&ExprNoneLiteral; 1]> = SmallVec::new();
let mut other_literal_elements_seen = false;
let mut literal_elements: Vec<&Expr> = Vec::new();
let mut literal_subscript = None;
if let Expr::Subscript(ast::ExprSubscript { value, .. }) = literal_expr {
if checker.semantic().match_typing_expr(value, "Literal") {
literal_subscript = Some(value.as_ref());
}
let Expr::Subscript(ast::ExprSubscript {
value: literal_subscript,
..
}) = literal_expr
else {
return;
};

let mut find_literal_elements = |expr: &'a Expr, _parent: &'a Expr| {
let mut none_exprs: SmallVec<[&ExprNoneLiteral; 1]> = SmallVec::new();
let mut literal_elements = vec![];

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);
}
};

let Some(literal_subscript) = literal_subscript else {
return;
};
traverse_literal(&mut partition_literal_elements, semantic, literal_expr);

traverse_literal(&mut find_literal_elements, checker.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 {
create_fix_edit_2(checker, literal_expr, &literal_elements, literal_subscript).map(|edit| {
Fix::applicable_edit(
edit,
if checker.comment_ranges().intersects(literal_expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
},
)
})
} 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 @@ -124,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 @@ -150,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 @@ -165,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 @@ -185,52 +183,21 @@ fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option<Edit
semantic,
enclosing_pep604_union,
);
}

is_fixable.then(|| Edit::range_replacement("None".to_string(), literal_expr.range()))
}
if !is_fixable {
return None;
}
}

fn create_fix_edit_2(
checker: &mut Checker,
literal_expr: &Expr,
literal_elements: &[&Expr],
literal_subscript: &Expr,
) -> Option<Edit> {
let enclosing_pep604_union = checker
.semantic()
.current_expressions()
.skip(1)
.take_while(|expr| {
matches!(
expr,
Expr::BinOp(ExprBinOp {
op: Operator::BitOr,
..
})
)
})
.last();
if literal_elements.is_empty() {
return Some(Edit::range_replacement(
"None".to_string(),
literal_expr.range(),
));
}

let mut is_fixable = true;
if let Some(enclosing_pep604_union) = enclosing_pep604_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 checker.semantic().match_typing_expr(value, "Literal")
&& matches!(**slice, Expr::NoneLiteral(_))
{
is_fixable = false;
}
}
}
},
checker.semantic(),
enclosing_pep604_union,
);
if checker.settings.target_version < PythonVersion::Py310 {
return None;
}

let bin_or = Expr::BinOp(ExprBinOp {
Expand All @@ -241,7 +208,7 @@ fn create_fix_edit_2(
ctx: ExprContext::Load,
slice: Box::new(if literal_elements.len() > 1 {
Expr::Tuple(ast::ExprTuple {
elts: literal_elements.iter().copied().cloned().collect(),
elts: literal_elements.into_iter().cloned().collect(),
range: TextRange::default(),
ctx: ExprContext::Load,
parenthesized: true,
Expand All @@ -257,6 +224,5 @@ fn create_fix_edit_2(
});

let content = checker.generator().expr(&bin_or);

is_fixable.then(|| Edit::range_replacement(content, literal_expr.range()))
Some(Edit::range_replacement(content, literal_expr.range()))
}

0 comments on commit c9150d4

Please sign in to comment.