Respect operator precedence in `FURB110` (#11464)

## Summary

Ensures that we parenthesize expressions (if necessary) to preserve
operator precedence in `FURB110`.

Closes https://github.com/astral-sh/ruff/issues/11398.
This commit is contained in:
Charlie Marsh 2024-05-18 23:17:11 -04:00 committed by GitHub
parent 24899efe50
commit 48b0660228
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 73 additions and 18 deletions

View File

@ -38,3 +38,12 @@ z = (
else
y
)
# FURB110
z = (
x
if x
else y
if y > 0
else None
)

View File

@ -1,9 +1,14 @@
use std::borrow::Cow;
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::helpers::contains_effect;
use ruff_python_ast::parenthesize::parenthesized_range;
use ruff_python_ast::Expr;
use ruff_python_index::Indexer;
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -64,29 +69,13 @@ pub(crate) fn if_exp_instead_of_or_operator(checker: &mut Checker, if_expr: &ast
let mut diagnostic = Diagnostic::new(IfExpInsteadOfOrOperator, *range);
// Grab the range of the `test` and `orelse` expressions.
let left = parenthesized_range(
test.into(),
if_expr.into(),
checker.indexer().comment_ranges(),
checker.locator().contents(),
)
.unwrap_or(test.range());
let right = parenthesized_range(
orelse.into(),
if_expr.into(),
checker.indexer().comment_ranges(),
checker.locator().contents(),
)
.unwrap_or(orelse.range());
// Replace with `{test} or {orelse}`.
diagnostic.set_fix(Fix::applicable_edit(
Edit::range_replacement(
format!(
"{} or {}",
checker.locator().slice(left),
checker.locator().slice(right),
parenthesize_test(test, if_expr, checker.indexer(), checker.locator()),
parenthesize_test(orelse, if_expr, checker.indexer(), checker.locator()),
),
if_expr.range(),
),
@ -99,3 +88,30 @@ pub(crate) fn if_exp_instead_of_or_operator(checker: &mut Checker, if_expr: &ast
checker.diagnostics.push(diagnostic);
}
/// Parenthesize an expression for use in an `or` operator (e.g., parenthesize `x` in `x or y`),
/// if it's required to maintain the correct order of operations.
///
/// If the expression is already parenthesized, it will be returned as-is regardless of whether
/// the parentheses are required.
///
/// See: <https://docs.python.org/3/reference/expressions.html#operator-precedence>
fn parenthesize_test<'a>(
expr: &Expr,
if_expr: &ast::ExprIf,
indexer: &Indexer,
locator: &Locator<'a>,
) -> Cow<'a, str> {
if let Some(range) = parenthesized_range(
expr.into(),
if_expr.into(),
indexer.comment_ranges(),
locator.contents(),
) {
Cow::Borrowed(locator.slice(range))
} else if matches!(expr, Expr::If(_) | Expr::Lambda(_) | Expr::Named(_)) {
Cow::Owned(format!("({})", locator.slice(expr.range())))
} else {
Cow::Borrowed(locator.slice(expr.range()))
}
}

View File

@ -177,3 +177,33 @@ FURB110.py:34:5: FURB110 [*] Replace ternary `if` expression with `or` operator
39 |- y
34 |+ x or y
40 35 | )
41 36 |
42 37 | # FURB110
FURB110.py:44:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
42 | # FURB110
43 | z = (
44 | x
| _____^
45 | | if x
46 | | else y
47 | | if y > 0
48 | | else None
| |_____________^ FURB110
49 | )
|
= help: Replace with `or` operator
Safe fix
41 41 |
42 42 | # FURB110
43 43 | z = (
44 |- x
45 |- if x
46 |- else y
44 |+ x or (y
47 45 | if y > 0
48 |- else None
46 |+ else None)
49 47 | )