mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:25:17 +00:00
[refurb
] Implement if-expr-instead-of-or-operator
(FURB110
) (#10687)
## Summary Add [`FURB110`](https://github.com/dosisod/refurb/blob/master/refurb/checks/logical/use_or.py) See: #1348 ## Test Plan `cargo test`
This commit is contained in:
parent
1dc93107dc
commit
44459f92ef
8 changed files with 289 additions and 0 deletions
30
crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py
vendored
Normal file
30
crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py
vendored
Normal file
|
@ -0,0 +1,30 @@
|
||||||
|
z = x if x else y # FURB110
|
||||||
|
|
||||||
|
z = x \
|
||||||
|
if x else y # FURB110
|
||||||
|
|
||||||
|
z = x if x \
|
||||||
|
else \
|
||||||
|
y # FURB110
|
||||||
|
|
||||||
|
z = x() if x() else y() # FURB110
|
||||||
|
|
||||||
|
# FURB110
|
||||||
|
z = x if (
|
||||||
|
# Test for x.
|
||||||
|
x
|
||||||
|
) else (
|
||||||
|
# Test for y.
|
||||||
|
y
|
||||||
|
)
|
||||||
|
|
||||||
|
# FURB110
|
||||||
|
z = (
|
||||||
|
x if (
|
||||||
|
# Test for x.
|
||||||
|
x
|
||||||
|
) else (
|
||||||
|
# Test for y.
|
||||||
|
y
|
||||||
|
)
|
||||||
|
)
|
|
@ -1358,6 +1358,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
|
||||||
if checker.enabled(Rule::IfExprMinMax) {
|
if checker.enabled(Rule::IfExprMinMax) {
|
||||||
refurb::rules::if_expr_min_max(checker, if_exp);
|
refurb::rules::if_expr_min_max(checker, if_exp);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::IfExpInsteadOfOrOperator) {
|
||||||
|
refurb::rules::if_exp_instead_of_or_operator(checker, if_exp);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Expr::ListComp(
|
Expr::ListComp(
|
||||||
comp @ ast::ExprListComp {
|
comp @ ast::ExprListComp {
|
||||||
|
|
|
@ -1037,6 +1037,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
// refurb
|
// refurb
|
||||||
(Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile),
|
(Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile),
|
||||||
(Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString),
|
(Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString),
|
||||||
|
(Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::IfExpInsteadOfOrOperator),
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
|
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
|
||||||
(Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator),
|
(Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator),
|
||||||
|
|
|
@ -16,6 +16,7 @@ mod tests {
|
||||||
|
|
||||||
#[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))]
|
#[test_case(Rule::ReadWholeFile, Path::new("FURB101.py"))]
|
||||||
#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
|
#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
|
||||||
|
#[test_case(Rule::IfExpInsteadOfOrOperator, Path::new("FURB110.py"))]
|
||||||
#[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))]
|
#[test_case(Rule::ReimplementedOperator, Path::new("FURB118.py"))]
|
||||||
#[test_case(Rule::ReadlinesInFor, Path::new("FURB129.py"))]
|
#[test_case(Rule::ReadlinesInFor, Path::new("FURB129.py"))]
|
||||||
#[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
|
#[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
|
||||||
|
|
|
@ -0,0 +1,101 @@
|
||||||
|
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_text_size::Ranged;
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for ternary `if` expressions that can be replaced with the `or`
|
||||||
|
/// operator.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// Ternary `if` expressions are more verbose than `or` expressions while
|
||||||
|
/// providing the same functionality.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// z = x if x else y
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// z = x or y
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// ## Fix safety
|
||||||
|
/// This rule's fix is marked as unsafe in the event that the body of the
|
||||||
|
/// `if` expression contains side effects.
|
||||||
|
///
|
||||||
|
/// For example, `foo` will be called twice in `foo() if foo() else bar()`
|
||||||
|
/// (assuming `foo()` returns a truthy value), but only once in
|
||||||
|
/// `foo() or bar()`.
|
||||||
|
#[violation]
|
||||||
|
pub struct IfExpInsteadOfOrOperator;
|
||||||
|
|
||||||
|
impl Violation for IfExpInsteadOfOrOperator {
|
||||||
|
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
|
||||||
|
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
format!("Replace ternary `if` expression with `or` operator")
|
||||||
|
}
|
||||||
|
|
||||||
|
fn fix_title(&self) -> Option<String> {
|
||||||
|
Some(format!("Replace with `or` operator"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// FURB110
|
||||||
|
pub(crate) fn if_exp_instead_of_or_operator(checker: &mut Checker, if_expr: &ast::ExprIf) {
|
||||||
|
let ast::ExprIf {
|
||||||
|
test,
|
||||||
|
body,
|
||||||
|
orelse,
|
||||||
|
range,
|
||||||
|
} = if_expr;
|
||||||
|
|
||||||
|
if ComparableExpr::from(test) != ComparableExpr::from(body) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
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),
|
||||||
|
),
|
||||||
|
if_expr.range(),
|
||||||
|
),
|
||||||
|
if contains_effect(body, |id| checker.semantic().is_builtin(id)) {
|
||||||
|
Applicability::Unsafe
|
||||||
|
} else {
|
||||||
|
Applicability::Safe
|
||||||
|
},
|
||||||
|
));
|
||||||
|
|
||||||
|
checker.diagnostics.push(diagnostic);
|
||||||
|
}
|
|
@ -3,6 +3,7 @@ pub(crate) use check_and_remove_from_set::*;
|
||||||
pub(crate) use delete_full_slice::*;
|
pub(crate) use delete_full_slice::*;
|
||||||
pub(crate) use for_loop_set_mutations::*;
|
pub(crate) use for_loop_set_mutations::*;
|
||||||
pub(crate) use hashlib_digest_hex::*;
|
pub(crate) use hashlib_digest_hex::*;
|
||||||
|
pub(crate) use if_exp_instead_of_or_operator::*;
|
||||||
pub(crate) use if_expr_min_max::*;
|
pub(crate) use if_expr_min_max::*;
|
||||||
pub(crate) use implicit_cwd::*;
|
pub(crate) use implicit_cwd::*;
|
||||||
pub(crate) use int_on_sliced_str::*;
|
pub(crate) use int_on_sliced_str::*;
|
||||||
|
@ -30,6 +31,7 @@ mod check_and_remove_from_set;
|
||||||
mod delete_full_slice;
|
mod delete_full_slice;
|
||||||
mod for_loop_set_mutations;
|
mod for_loop_set_mutations;
|
||||||
mod hashlib_digest_hex;
|
mod hashlib_digest_hex;
|
||||||
|
mod if_exp_instead_of_or_operator;
|
||||||
mod if_expr_min_max;
|
mod if_expr_min_max;
|
||||||
mod implicit_cwd;
|
mod implicit_cwd;
|
||||||
mod int_on_sliced_str;
|
mod int_on_sliced_str;
|
||||||
|
|
|
@ -0,0 +1,150 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/refurb/mod.rs
|
||||||
|
---
|
||||||
|
FURB110.py:1:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
||||||
|
|
|
||||||
|
1 | z = x if x else y # FURB110
|
||||||
|
| ^^^^^^^^^^^^^ FURB110
|
||||||
|
2 |
|
||||||
|
3 | z = x \
|
||||||
|
|
|
||||||
|
= help: Replace with `or` operator
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
1 |-z = x if x else y # FURB110
|
||||||
|
1 |+z = x or y # FURB110
|
||||||
|
2 2 |
|
||||||
|
3 3 | z = x \
|
||||||
|
4 4 | if x else y # FURB110
|
||||||
|
|
||||||
|
FURB110.py:3:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
||||||
|
|
|
||||||
|
1 | z = x if x else y # FURB110
|
||||||
|
2 |
|
||||||
|
3 | z = x \
|
||||||
|
| _____^
|
||||||
|
4 | | if x else y # FURB110
|
||||||
|
| |_______________^ FURB110
|
||||||
|
5 |
|
||||||
|
6 | z = x if x \
|
||||||
|
|
|
||||||
|
= help: Replace with `or` operator
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
1 1 | z = x if x else y # FURB110
|
||||||
|
2 2 |
|
||||||
|
3 |-z = x \
|
||||||
|
4 |- if x else y # FURB110
|
||||||
|
3 |+z = x or y # FURB110
|
||||||
|
5 4 |
|
||||||
|
6 5 | z = x if x \
|
||||||
|
7 6 | else \
|
||||||
|
|
||||||
|
FURB110.py:6:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
||||||
|
|
|
||||||
|
4 | if x else y # FURB110
|
||||||
|
5 |
|
||||||
|
6 | z = x if x \
|
||||||
|
| _____^
|
||||||
|
7 | | else \
|
||||||
|
8 | | y # FURB110
|
||||||
|
| |_________^ FURB110
|
||||||
|
9 |
|
||||||
|
10 | z = x() if x() else y() # FURB110
|
||||||
|
|
|
||||||
|
= help: Replace with `or` operator
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
3 3 | z = x \
|
||||||
|
4 4 | if x else y # FURB110
|
||||||
|
5 5 |
|
||||||
|
6 |-z = x if x \
|
||||||
|
7 |- else \
|
||||||
|
8 |- y # FURB110
|
||||||
|
6 |+z = x or y # FURB110
|
||||||
|
9 7 |
|
||||||
|
10 8 | z = x() if x() else y() # FURB110
|
||||||
|
11 9 |
|
||||||
|
|
||||||
|
FURB110.py:10:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
||||||
|
|
|
||||||
|
8 | y # FURB110
|
||||||
|
9 |
|
||||||
|
10 | z = x() if x() else y() # FURB110
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^ FURB110
|
||||||
|
11 |
|
||||||
|
12 | # FURB110
|
||||||
|
|
|
||||||
|
= help: Replace with `or` operator
|
||||||
|
|
||||||
|
ℹ Unsafe fix
|
||||||
|
7 7 | else \
|
||||||
|
8 8 | y # FURB110
|
||||||
|
9 9 |
|
||||||
|
10 |-z = x() if x() else y() # FURB110
|
||||||
|
10 |+z = x() or y() # FURB110
|
||||||
|
11 11 |
|
||||||
|
12 12 | # FURB110
|
||||||
|
13 13 | z = x if (
|
||||||
|
|
||||||
|
FURB110.py:13:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
||||||
|
|
|
||||||
|
12 | # FURB110
|
||||||
|
13 | z = x if (
|
||||||
|
| _____^
|
||||||
|
14 | | # Test for x.
|
||||||
|
15 | | x
|
||||||
|
16 | | ) else (
|
||||||
|
17 | | # Test for y.
|
||||||
|
18 | | y
|
||||||
|
19 | | )
|
||||||
|
| |_^ FURB110
|
||||||
|
20 |
|
||||||
|
21 | # FURB110
|
||||||
|
|
|
||||||
|
= help: Replace with `or` operator
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
10 10 | z = x() if x() else y() # FURB110
|
||||||
|
11 11 |
|
||||||
|
12 12 | # FURB110
|
||||||
|
13 |-z = x if (
|
||||||
|
13 |+z = (
|
||||||
|
14 14 | # Test for x.
|
||||||
|
15 15 | x
|
||||||
|
16 |-) else (
|
||||||
|
16 |+) or (
|
||||||
|
17 17 | # Test for y.
|
||||||
|
18 18 | y
|
||||||
|
19 19 | )
|
||||||
|
|
||||||
|
FURB110.py:23:5: FURB110 [*] Replace ternary `if` expression with `or` operator
|
||||||
|
|
|
||||||
|
21 | # FURB110
|
||||||
|
22 | z = (
|
||||||
|
23 | x if (
|
||||||
|
| _____^
|
||||||
|
24 | | # Test for x.
|
||||||
|
25 | | x
|
||||||
|
26 | | ) else (
|
||||||
|
27 | | # Test for y.
|
||||||
|
28 | | y
|
||||||
|
29 | | )
|
||||||
|
| |_____^ FURB110
|
||||||
|
30 | )
|
||||||
|
|
|
||||||
|
= help: Replace with `or` operator
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
20 20 |
|
||||||
|
21 21 | # FURB110
|
||||||
|
22 22 | z = (
|
||||||
|
23 |- x if (
|
||||||
|
23 |+ (
|
||||||
|
24 24 | # Test for x.
|
||||||
|
25 25 | x
|
||||||
|
26 |- ) else (
|
||||||
|
26 |+ ) or (
|
||||||
|
27 27 | # Test for y.
|
||||||
|
28 28 | y
|
||||||
|
29 29 | )
|
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
@ -3043,6 +3043,7 @@
|
||||||
"FURB101",
|
"FURB101",
|
||||||
"FURB105",
|
"FURB105",
|
||||||
"FURB11",
|
"FURB11",
|
||||||
|
"FURB110",
|
||||||
"FURB113",
|
"FURB113",
|
||||||
"FURB118",
|
"FURB118",
|
||||||
"FURB12",
|
"FURB12",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue