Minor stylistic improvements for functions detecting PEP-604 unions (#14633)

This commit is contained in:
Alex Waygood 2024-11-27 11:29:37 +00:00 committed by GitHub
parent 8a860b89b4
commit 4fb1416bf4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 63 additions and 46 deletions

View file

@ -119,22 +119,32 @@ pub(crate) fn redundant_none_literal<'a>(checker: &mut Checker, literal_expr: &'
}
}
/// If possible, return a [`Fix`] for a violation of this rule.
///
/// Avoid producing code that would raise an exception when
/// `Literal[None] | None` would be fixed to `None | None`.
/// Instead, do not provide a fix. We don't need to worry about unions
/// that use [`typing.Union`], as `Union[None, None]` is valid Python.
/// 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> {
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 enclosing_pep604_union = semantic
.current_expressions()
.skip(1)
.take_while(|expr| {
matches!(
expr,
Expr::BinOp(ExprBinOp {
op: Operator::BitOr,
..
})
)
})
.last();
let mut is_fixable = true;
if let Some(enclosing_union) = enclosing_union {
if let Some(enclosing_pep604_union) = enclosing_pep604_union {
traverse_union(
&mut |expr, _| {
if matches!(expr, Expr::NoneLiteral(_)) {
@ -151,14 +161,9 @@ fn create_fix_edit(semantic: &SemanticModel, literal_expr: &Expr) -> Option<Edit
}
},
semantic,
enclosing_union,
enclosing_pep604_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()))
}

View file

@ -89,7 +89,7 @@ pub(crate) fn never_union(checker: &mut Checker, expr: &Expr) {
// 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.
if !in_union_with_bare_none(checker.semantic()) {
if !is_pep604_union_with_bare_none(checker.semantic()) {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
checker.locator().slice(right.as_ref()).to_string(),
expr.range(),
@ -107,7 +107,7 @@ pub(crate) fn never_union(checker: &mut Checker, expr: &Expr) {
},
right.range(),
);
if !in_union_with_bare_none(checker.semantic()) {
if !is_pep604_union_with_bare_none(checker.semantic()) {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
checker.locator().slice(left.as_ref()).to_string(),
expr.range(),
@ -219,31 +219,43 @@ impl std::fmt::Display for NeverLike {
}
}
fn in_union_with_bare_none(semantic: &SemanticModel) -> bool {
let mut enclosing_union = None;
let mut expression_ancestors = semantic.current_expressions().skip(1);
let mut parent_expr = expression_ancestors.next();
while let Some(Expr::BinOp(ExprBinOp {
op: Operator::BitOr,
..
})) = parent_expr
{
enclosing_union = parent_expr;
parent_expr = expression_ancestors.next();
}
/// Return `true` if this union is a [PEP 604 union] that contains `None`,
/// e.g. `int | Never | None`.
///
/// Autofixing these unions can be dangerous,
/// as `None | None` results in a runtime exception in Python.
///
/// [PEP 604 union]: https://docs.python.org/3/library/stdtypes.html#types-union
fn is_pep604_union_with_bare_none(semantic: &SemanticModel) -> bool {
let enclosing_pep604_union = semantic
.current_expressions()
.skip(1)
.take_while(|expr| {
matches!(
expr,
Expr::BinOp(ExprBinOp {
op: Operator::BitOr,
..
})
)
})
.last();
let mut is_union_with_bare_none = false;
if let Some(enclosing_union) = enclosing_union {
traverse_union(
&mut |expr, _| {
if matches!(expr, Expr::NoneLiteral(_)) {
is_union_with_bare_none = true;
}
},
semantic,
enclosing_union,
);
}
let Some(enclosing_pep604_union) = enclosing_pep604_union else {
return false;
};
is_union_with_bare_none
let mut union_contains_bare_none = false;
traverse_union(
&mut |expr, _| {
if matches!(expr, Expr::NoneLiteral(_)) {
union_contains_bare_none = true;
}
},
semantic,
enclosing_pep604_union,
);
union_contains_bare_none
}