[pylint] Implement consider-using-ternary (R1706)

crates/ruff_linter/resources/test/fixtures/pylint/and_or_ternary.py

@@ -0,0 +1,48 @@
+
+# No Ternary (No Error)
+
+1<2 and 'b' and 'c'
+
+1<2 or 'a' and 'b'
+
+1<2 and 'a'
+
+1<2 or 'a'
+
+2>1
+
+1<2 and 'a' or 'b' and 'c'
+
+1<2 and 'a' or 'b' or 'c'
+
+1<2 and 'a' or 'b' or 'c' or (lambda x: x+1)
+
+1<2 and 'a' or 'b' or (lambda x: x+1) or 'c'
+
+default = 'default'
+if (not isinstance(default, bool) and isinstance(default, int)) \
+        or (isinstance(default, str) and default):
+    pass
+
+docid, token = None, None
+(docid is None and token is None) or (docid is not None and token is not None)
+
+vendor, os_version = 'darwin', '14'
+vendor == "debian" and os_version in ["12"] or vendor == "ubuntu" and os_version in []
+
+# Ternary (Error)
+
+1<2 and 'a' or 'b'
+
+(lambda x: x+1) and 'a' or 'b'
+
+'a' and (lambda x: x+1) or 'orange'
+
+val = '#0000FF'
+(len(val) == 7 and val[0] == "#") or val in {'green'}
+
+marker = 'marker'
+isinstance(marker, dict) and 'field' in marker or marker in {}
+
+def has_oranges(oranges, apples=None) -> bool:
+    return apples and False or oranges

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

@@ -1412,6 +1412,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::RepeatedEqualityComparison) {
                 pylint::rules::repeated_equality_comparison(checker, bool_op);
             }
+            if checker.enabled(Rule::AndOrTernary) {
+                pylint::rules::and_or_ternary(checker, bool_op);
+            }
         }
         Expr::NamedExpr(..) => {
             if checker.enabled(Rule::AssignmentInAssert) {

crates/ruff_linter/src/codes.rs

@@ -249,6 +249,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "R1701") => (RuleGroup::Unspecified, rules::pylint::rules::RepeatedIsinstanceCalls),
         (Pylint, "R1711") => (RuleGroup::Unspecified, rules::pylint::rules::UselessReturn),
         (Pylint, "R1714") => (RuleGroup::Unspecified, rules::pylint::rules::RepeatedEqualityComparison),
+        (Pylint, "R1706") => (RuleGroup::Unspecified, rules::pylint::rules::AndOrTernary),
         (Pylint, "R1722") => (RuleGroup::Unspecified, rules::pylint::rules::SysExitAlias),
         (Pylint, "R2004") => (RuleGroup::Unspecified, rules::pylint::rules::MagicValueComparison),
         (Pylint, "R5501") => (RuleGroup::Unspecified, rules::pylint::rules::CollapsibleElseIf),

crates/ruff_linter/src/rules/pylint/helpers.rs

@@ -1,7 +1,7 @@
 use std::fmt;
 
 use ruff_python_ast as ast;
-use ruff_python_ast::{Arguments, CmpOp, Constant, Expr};
+use ruff_python_ast::{Arguments, BoolOp, CmpOp, Constant, Expr, ExprBoolOp};
 use ruff_python_semantic::analyze::function_type;
 use ruff_python_semantic::{ScopeKind, SemanticModel};
 
@@ -82,3 +82,26 @@ impl fmt::Display for CmpOpExt {
         write!(f, "{representation}")
     }
 }
+
+/// Returns `Some([condition, true_value, false_value])`
+/// if `bool_op` is `condition and true_value or false_value` form.
+pub(super) fn parse_and_or_ternary(bool_op: &ExprBoolOp) -> Option<[Expr; 3]> {
+    if bool_op.op != BoolOp::Or {
+        return None;
+    }
+    let [expr, false_value] = bool_op.values.as_slice() else {
+        return None;
+    };
+    match expr.as_bool_op_expr() {
+        Some(and_op) if and_op.op == BoolOp::And => {
+            let [condition, true_value] = and_op.values.as_slice() else {
+                return None;
+            };
+            if !false_value.is_bool_op_expr() && !true_value.is_bool_op_expr() {
+                return Some([condition.clone(), true_value.clone(), false_value.clone()]);
+            }
+        }
+        _ => {}
+    }
+    None
+}

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

@@ -18,6 +18,7 @@ mod tests {
     use crate::settings::LinterSettings;
     use crate::test::test_path;
 
+    #[test_case(Rule::AndOrTernary, Path::new("and_or_ternary.py"))]
     #[test_case(Rule::AssertOnStringLiteral, Path::new("assert_on_string_literal.py"))]
     #[test_case(Rule::AwaitOutsideAsync, Path::new("await_outside_async.py"))]
     #[test_case(

crates/ruff_linter/src/rules/pylint/rules/and_or_ternary.rs

@@ -0,0 +1,72 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{Expr, ExprBoolOp, ExprIfExp};
+use ruff_text_size::TextRange;
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+use crate::rules::pylint::helpers::parse_and_or_ternary;
+
+/// ## What it does
+/// Checks if pre-python 2.5 ternary syntax is used.
+///
+/// ## Why is this bad?
+/// If-expressions are more readable than logical ternary expressions.
+///
+/// ## Example
+/// ```python
+/// x, y = 1, 2
+/// maximum = x >= y and x or y
+/// ```
+///
+/// Use instead:
+/// ```python
+/// x, y = 1, 2
+/// maximum = x if x >= y else y
+/// ```
+#[violation]
+pub struct AndOrTernary {
+    ternary: String,
+}
+
+impl Violation for AndOrTernary {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Pre-python 2.5 ternary syntax used")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        let AndOrTernary { ternary } = self;
+        Some(format!("Use `{ternary}`"))
+    }
+}
+
+pub(crate) fn and_or_ternary(checker: &mut Checker, bool_op: &ExprBoolOp) {
+    let Some([condition, true_value, false_value]) = parse_and_or_ternary(bool_op) else {
+        return;
+    };
+
+    let if_expr = Expr::IfExp(ExprIfExp {
+        test: Box::new(condition),
+        body: Box::new(true_value),
+        orelse: Box::new(false_value),
+        range: TextRange::default(),
+    });
+    let ternary = checker.generator().expr(&if_expr);
+
+    let mut diagnostic = Diagnostic::new(
+        AndOrTernary {
+            ternary: ternary.clone(),
+        },
+        bool_op.range,
+    );
+    if checker.patch(diagnostic.kind.rule()) {
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+            ternary,
+            bool_op.range,
+        )));
+    }
+    checker.diagnostics.push(diagnostic);
+}

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

@@ -1,3 +1,4 @@
+pub(crate) use and_or_ternary::*;
 pub(crate) use assert_on_string_literal::*;
 pub(crate) use await_outside_async::*;
 pub(crate) use bad_dunder_method_name::*;
@@ -57,6 +58,7 @@ pub(crate) use useless_return::*;
 pub(crate) use yield_from_in_async_function::*;
 pub(crate) use yield_in_init::*;
 
+mod and_or_ternary;
 mod assert_on_string_literal;
 mod await_outside_async;
 mod bad_dunder_method_name;

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1706_and_or_ternary.py.snap

@@ -0,0 +1,122 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+and_or_ternary.py:35:1: PLR1706 [*] Pre-python 2.5 ternary syntax used
+   |
+33 | # Ternary (Error)
+34 | 
+35 | 1<2 and 'a' or 'b'
+   | ^^^^^^^^^^^^^^^^^^ PLR1706
+36 | 
+37 | (lambda x: x+1) and 'a' or 'b'
+   |
+   = help: Use `'a' if 1 < 2 else 'b'`
+
+ℹ Fix
+32 32 | 
+33 33 | # Ternary (Error)
+34 34 | 
+35    |-1<2 and 'a' or 'b'
+   35 |+'a' if 1 < 2 else 'b'
+36 36 | 
+37 37 | (lambda x: x+1) and 'a' or 'b'
+38 38 | 
+
+and_or_ternary.py:37:1: PLR1706 [*] Pre-python 2.5 ternary syntax used
+   |
+35 | 1<2 and 'a' or 'b'
+36 | 
+37 | (lambda x: x+1) and 'a' or 'b'
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+38 | 
+39 | 'a' and (lambda x: x+1) or 'orange'
+   |
+   = help: Use `'a' if (lambda x: x + 1) else 'b'`
+
+ℹ Fix
+34 34 | 
+35 35 | 1<2 and 'a' or 'b'
+36 36 | 
+37    |-(lambda x: x+1) and 'a' or 'b'
+   37 |+'a' if (lambda x: x + 1) else 'b'
+38 38 | 
+39 39 | 'a' and (lambda x: x+1) or 'orange'
+40 40 | 
+
+and_or_ternary.py:39:1: PLR1706 [*] Pre-python 2.5 ternary syntax used
+   |
+37 | (lambda x: x+1) and 'a' or 'b'
+38 | 
+39 | 'a' and (lambda x: x+1) or 'orange'
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+40 | 
+41 | val = '#0000FF'
+   |
+   = help: Use `(lambda x: x + 1) if 'a' else 'orange'`
+
+ℹ Fix
+36 36 | 
+37 37 | (lambda x: x+1) and 'a' or 'b'
+38 38 | 
+39    |-'a' and (lambda x: x+1) or 'orange'
+   39 |+(lambda x: x + 1) if 'a' else 'orange'
+40 40 | 
+41 41 | val = '#0000FF'
+42 42 | (len(val) == 7 and val[0] == "#") or val in {'green'}
+
+and_or_ternary.py:42:1: PLR1706 [*] Pre-python 2.5 ternary syntax used
+   |
+41 | val = '#0000FF'
+42 | (len(val) == 7 and val[0] == "#") or val in {'green'}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+43 | 
+44 | marker = 'marker'
+   |
+   = help: Use `val[0] == '#' if len(val) == 7 else val in {'green'}`
+
+ℹ Fix
+39 39 | 'a' and (lambda x: x+1) or 'orange'
+40 40 | 
+41 41 | val = '#0000FF'
+42    |-(len(val) == 7 and val[0] == "#") or val in {'green'}
+   42 |+val[0] == '#' if len(val) == 7 else val in {'green'}
+43 43 | 
+44 44 | marker = 'marker'
+45 45 | isinstance(marker, dict) and 'field' in marker or marker in {}
+
+and_or_ternary.py:45:1: PLR1706 [*] Pre-python 2.5 ternary syntax used
+   |
+44 | marker = 'marker'
+45 | isinstance(marker, dict) and 'field' in marker or marker in {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+46 | 
+47 | def has_oranges(oranges, apples=None) -> bool:
+   |
+   = help: Use `'field' in marker if isinstance(marker, dict) else marker in {}`
+
+ℹ Fix
+42 42 | (len(val) == 7 and val[0] == "#") or val in {'green'}
+43 43 | 
+44 44 | marker = 'marker'
+45    |-isinstance(marker, dict) and 'field' in marker or marker in {}
+   45 |+'field' in marker if isinstance(marker, dict) else marker in {}
+46 46 | 
+47 47 | def has_oranges(oranges, apples=None) -> bool:
+48 48 |     return apples and False or oranges
+
+and_or_ternary.py:48:12: PLR1706 [*] Pre-python 2.5 ternary syntax used
+   |
+47 | def has_oranges(oranges, apples=None) -> bool:
+48 |     return apples and False or oranges
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+   |
+   = help: Use `False if apples else oranges`
+
+ℹ Fix
+45 45 | isinstance(marker, dict) and 'field' in marker or marker in {}
+46 46 | 
+47 47 | def has_oranges(oranges, apples=None) -> bool:
+48    |-    return apples and False or oranges
+   48 |+    return False if apples else oranges
+
+