[refurb] Implement if-expr-instead-of-or-operator (FURB110)

crates/ruff_linter/resources/test/fixtures/refurb/FURB110.py

@@ -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
+    )
+)

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1363,6 +1363,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::IfExprMinMax) {
                 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(
             comp @ ast::ExprListComp {

crates/ruff_linter/src/codes.rs

@@ -1037,6 +1037,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         // refurb
         (Refurb, "101") => (RuleGroup::Preview, rules::refurb::rules::ReadWholeFile),
         (Refurb, "105") => (RuleGroup::Preview, rules::refurb::rules::PrintEmptyString),
+        (Refurb, "110") => (RuleGroup::Preview, rules::refurb::rules::IfExpInsteadOfOrOperator),
         #[allow(deprecated)]
         (Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),
         (Refurb, "118") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedOperator),

crates/ruff_linter/src/rules/refurb/mod.rs

@@ -16,6 +16,7 @@ mod tests {
 
     #[test_case(Rule::ReadWholeFile, Path::new("FURB101.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::ReadlinesInFor, Path::new("FURB129.py"))]
     #[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]

crates/ruff_linter/src/rules/refurb/rules/if_exp_instead_of_or_operator.rs

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

crates/ruff_linter/src/rules/refurb/rules/mod.rs

@@ -3,6 +3,7 @@ pub(crate) use check_and_remove_from_set::*;
 pub(crate) use delete_full_slice::*;
 pub(crate) use for_loop_set_mutations::*;
 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 implicit_cwd::*;
 pub(crate) use int_on_sliced_str::*;
@@ -30,6 +31,7 @@ mod check_and_remove_from_set;
 mod delete_full_slice;
 mod for_loop_set_mutations;
 mod hashlib_digest_hex;
+mod if_exp_instead_of_or_operator;
 mod if_expr_min_max;
 mod implicit_cwd;
 mod int_on_sliced_str;

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB110_FURB110.py.snap

@@ -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 |     )