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

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

@@ -0,0 +1,74 @@
+
+# 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 []
+
+# No emit if parent is `if` statement
+if (task_id in task_dict and task_dict[task_id] is not task) \
+        or task_id in used_group_ids:
+    pass
+
+no_target, is_x64, target = True, False, 'target'
+if (no_target and not is_x64) or target == 'ARM_APPL_RUST_TARGET':
+    pass
+
+# No emit if parent is bool_op expression
+isinstance(val, str) and ((len(val) == 7 and val[0] == "#") or val in enums.NamedColor)
+
+# 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
+
+[x for x in l if a and b or c]
+
+{x: y for x in l if a and b or c}
+
+{x for x in l if a and b or c}
+
+new_list = [
+    x
+    for sublist in all_lists
+    if a and b or c
+    for x in sublist
+    if (isinstance(operator, list) and x in operator) or x != operator
+]

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/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,124 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{
+    BoolOp, Expr, ExprBoolOp, ExprDictComp, ExprIfExp, ExprListComp, ExprSetComp,
+};
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+
+/// ## 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}`"))
+    }
+}
+
+/// Returns `Some((condition, true_value, false_value))`
+/// if `bool_op` is `condition and true_value or false_value` form.
+fn parse_and_or_ternary(bool_op: &ExprBoolOp) -> Option<(Expr, Expr, Expr)> {
+    if bool_op.op != BoolOp::Or {
+        return None;
+    }
+    let [expr, false_value] = bool_op.values.as_slice() else {
+        return None;
+    };
+    let Some(and_op) = expr.as_bool_op_expr() else {
+        return None;
+    };
+    if and_op.op != BoolOp::And {
+        return None;
+    }
+    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
+}
+
+/// Returns `true` if expr is used as comprehension-if.
+fn is_comprehension_if(parent: Option<&Expr>, expr: &ExprBoolOp) -> bool {
+    let comprehensions = match parent {
+        Some(Expr::ListComp(ExprListComp { generators, .. })) => generators,
+        Some(Expr::SetComp(ExprSetComp { generators, .. })) => generators,
+        Some(Expr::DictComp(ExprDictComp { generators, .. })) => generators,
+        _ => {
+            return false;
+        }
+    };
+    comprehensions
+        .iter()
+        .any(|comp| comp.ifs.iter().any(|ifs| ifs.range() == expr.range()))
+}
+
+pub(crate) fn and_or_ternary(checker: &mut Checker, bool_op: &ExprBoolOp) {
+    if checker.semantic().current_statement().is_if_stmt() {
+        return;
+    }
+    let parent_expr = checker.semantic().current_expression_parent();
+    if parent_expr.is_some_and(Expr::is_bool_op_expr) {
+        return;
+    }
+    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 = if is_comprehension_if(parent_expr, bool_op) {
+        format!("({})", checker.generator().expr(&if_expr))
+    } else {
+        checker.generator().expr(&if_expr)
+    };
+
+    let mut diagnostic = Diagnostic::new(
+        AndOrTernary {
+            ternary: ternary.clone(),
+        },
+        bool_op.range,
+    );
+    if checker.enabled(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;