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

This is my first PR. Please feel free to give me any feedback for even
small drawbacks.

## Summary

Checks if pre-python 2.5 ternary syntax is used.

Before
```python
x, y = 1, 2
maximum = x >= y and x or y  # [consider-using-ternary]
```

After
```python
x, y = 1, 2
maximum = x if x >= y else y
```

References: 

[pylint](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/consider-using-ternary.html)
#970 
[and_or_ternary distinction
logic](https://github.com/pylint-dev/pylint/blob/main/pylint/checkers/refactoring/refactoring_checker.py#L1813)

## Test Plan

Unit test, python file, snapshot added.

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

@@ -0,0 +1,73 @@
+# OK
+
+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 []
+
+# Don't emit if the parent is an `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
+
+# Don't emit if the parent is a `bool_op` expression.
+isinstance(val, str) and ((len(val) == 7 and val[0] == "#") or val in enums.NamedColor)
+
+# Errors
+
+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);
+            }
             if checker.enabled(Rule::UnnecessaryKeyCheck) {
                 ruff::rules::unnecessary_key_check(checker, expr);
             }

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::Preview, 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,133 @@
+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::fix::snippet::SourceCodeSnippet;
+use crate::registry::AsRule;
+
+/// ## What it does
+/// Checks for uses of the known pre-Python 2.5 ternary syntax.
+///
+/// ## Why is this bad?
+/// Prior to the introduction of the if-expression (ternary) operator in Python
+/// 2.5, the only way to express a conditional expression was to use the `and`
+/// and `or` operators.
+///
+/// The if-expression construct is clearer and more explicit, and should be
+/// preferred over the use of `and` and `or` for 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: SourceCodeSnippet,
+}
+
+impl Violation for AndOrTernary {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        if let Some(ternary) = self.ternary.full_display() {
+            format!("Consider using if-else expression (`{ternary}`)")
+        } else {
+            format!("Consider using if-else expression")
+        }
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some(format!("Convert to if-else expression"))
+    }
+}
+
+/// Returns `Some((condition, true_value, false_value))`, if `bool_op` is of the form `condition and true_value or false_value`.
+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 None;
+    }
+    Some((condition, true_value, false_value))
+}
+
+/// Returns `true` if the expression is used within a comprehension.
+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()))
+}
+
+/// PLR1706
+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.clone()),
+        body: Box::new(true_value.clone()),
+        orelse: Box::new(false_value.clone()),
+        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: SourceCodeSnippet::new(ternary.clone()),
+        },
+        bool_op.range,
+    );
+    if checker.enabled(diagnostic.kind.rule()) {
+        diagnostic.set_fix(Fix::unsafe_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,229 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+and_or_ternary.py:46:1: PLR1706 [*] Consider using if-else expression (`'a' if 1 < 2 else 'b'`)
+   |
+44 | # Errors
+45 | 
+46 | 1<2 and 'a' or 'b'
+   | ^^^^^^^^^^^^^^^^^^ PLR1706
+47 | 
+48 | (lambda x: x+1) and 'a' or 'b'
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+43 43 | 
+44 44 | # Errors
+45 45 | 
+46    |-1<2 and 'a' or 'b'
+   46 |+'a' if 1 < 2 else 'b'
+47 47 | 
+48 48 | (lambda x: x+1) and 'a' or 'b'
+49 49 | 
+
+and_or_ternary.py:48:1: PLR1706 [*] Consider using if-else expression (`'a' if (lambda x: x + 1) else 'b'`)
+   |
+46 | 1<2 and 'a' or 'b'
+47 | 
+48 | (lambda x: x+1) and 'a' or 'b'
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+49 | 
+50 | 'a' and (lambda x: x+1) or 'orange'
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+45 45 | 
+46 46 | 1<2 and 'a' or 'b'
+47 47 | 
+48    |-(lambda x: x+1) and 'a' or 'b'
+   48 |+'a' if (lambda x: x + 1) else 'b'
+49 49 | 
+50 50 | 'a' and (lambda x: x+1) or 'orange'
+51 51 | 
+
+and_or_ternary.py:50:1: PLR1706 [*] Consider using if-else expression (`(lambda x: x + 1) if 'a' else 'orange'`)
+   |
+48 | (lambda x: x+1) and 'a' or 'b'
+49 | 
+50 | 'a' and (lambda x: x+1) or 'orange'
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+51 | 
+52 | val = '#0000FF'
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+47 47 | 
+48 48 | (lambda x: x+1) and 'a' or 'b'
+49 49 | 
+50    |-'a' and (lambda x: x+1) or 'orange'
+   50 |+(lambda x: x + 1) if 'a' else 'orange'
+51 51 | 
+52 52 | val = '#0000FF'
+53 53 | (len(val) == 7 and val[0] == "#") or val in {'green'}
+
+and_or_ternary.py:53:1: PLR1706 [*] Consider using if-else expression
+   |
+52 | val = '#0000FF'
+53 | (len(val) == 7 and val[0] == "#") or val in {'green'}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+54 | 
+55 | marker = 'marker'
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+50 50 | 'a' and (lambda x: x+1) or 'orange'
+51 51 | 
+52 52 | val = '#0000FF'
+53    |-(len(val) == 7 and val[0] == "#") or val in {'green'}
+   53 |+val[0] == '#' if len(val) == 7 else val in {'green'}
+54 54 | 
+55 55 | marker = 'marker'
+56 56 | isinstance(marker, dict) and 'field' in marker or marker in {}
+
+and_or_ternary.py:56:1: PLR1706 [*] Consider using if-else expression
+   |
+55 | marker = 'marker'
+56 | isinstance(marker, dict) and 'field' in marker or marker in {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+57 | 
+58 | def has_oranges(oranges, apples=None) -> bool:
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+53 53 | (len(val) == 7 and val[0] == "#") or val in {'green'}
+54 54 | 
+55 55 | marker = 'marker'
+56    |-isinstance(marker, dict) and 'field' in marker or marker in {}
+   56 |+'field' in marker if isinstance(marker, dict) else marker in {}
+57 57 | 
+58 58 | def has_oranges(oranges, apples=None) -> bool:
+59 59 |     return apples and False or oranges
+
+and_or_ternary.py:59:12: PLR1706 [*] Consider using if-else expression (`False if apples else oranges`)
+   |
+58 | def has_oranges(oranges, apples=None) -> bool:
+59 |     return apples and False or oranges
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+60 | 
+61 | [x for x in l if a and b or c]
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+56 56 | isinstance(marker, dict) and 'field' in marker or marker in {}
+57 57 | 
+58 58 | def has_oranges(oranges, apples=None) -> bool:
+59    |-    return apples and False or oranges
+   59 |+    return False if apples else oranges
+60 60 | 
+61 61 | [x for x in l if a and b or c]
+62 62 | 
+
+and_or_ternary.py:61:18: PLR1706 [*] Consider using if-else expression (`(b if a else c)`)
+   |
+59 |     return apples and False or oranges
+60 | 
+61 | [x for x in l if a and b or c]
+   |                  ^^^^^^^^^^^^ PLR1706
+62 | 
+63 | {x: y for x in l if a and b or c}
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+58 58 | def has_oranges(oranges, apples=None) -> bool:
+59 59 |     return apples and False or oranges
+60 60 | 
+61    |-[x for x in l if a and b or c]
+   61 |+[x for x in l if (b if a else c)]
+62 62 | 
+63 63 | {x: y for x in l if a and b or c}
+64 64 | 
+
+and_or_ternary.py:63:21: PLR1706 [*] Consider using if-else expression (`(b if a else c)`)
+   |
+61 | [x for x in l if a and b or c]
+62 | 
+63 | {x: y for x in l if a and b or c}
+   |                     ^^^^^^^^^^^^ PLR1706
+64 | 
+65 | {x for x in l if a and b or c}
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+60 60 | 
+61 61 | [x for x in l if a and b or c]
+62 62 | 
+63    |-{x: y for x in l if a and b or c}
+   63 |+{x: y for x in l if (b if a else c)}
+64 64 | 
+65 65 | {x for x in l if a and b or c}
+66 66 | 
+
+and_or_ternary.py:65:18: PLR1706 [*] Consider using if-else expression (`(b if a else c)`)
+   |
+63 | {x: y for x in l if a and b or c}
+64 | 
+65 | {x for x in l if a and b or c}
+   |                  ^^^^^^^^^^^^ PLR1706
+66 | 
+67 | new_list = [
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+62 62 | 
+63 63 | {x: y for x in l if a and b or c}
+64 64 | 
+65    |-{x for x in l if a and b or c}
+   65 |+{x for x in l if (b if a else c)}
+66 66 | 
+67 67 | new_list = [
+68 68 |     x
+
+and_or_ternary.py:70:8: PLR1706 [*] Consider using if-else expression (`(b if a else c)`)
+   |
+68 |     x
+69 |     for sublist in all_lists
+70 |     if a and b or c
+   |        ^^^^^^^^^^^^ PLR1706
+71 |     for x in sublist
+72 |     if (isinstance(operator, list) and x in operator) or x != operator
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+67 67 | new_list = [
+68 68 |     x
+69 69 |     for sublist in all_lists
+70    |-    if a and b or c
+   70 |+    if (b if a else c)
+71 71 |     for x in sublist
+72 72 |     if (isinstance(operator, list) and x in operator) or x != operator
+73 73 | ]
+
+and_or_ternary.py:72:8: PLR1706 [*] Consider using if-else expression
+   |
+70 |     if a and b or c
+71 |     for x in sublist
+72 |     if (isinstance(operator, list) and x in operator) or x != operator
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1706
+73 | ]
+   |
+   = help: Convert to if-else expression
+
+ℹ Suggested fix
+69 69 |     for sublist in all_lists
+70 70 |     if a and b or c
+71 71 |     for x in sublist
+72    |-    if (isinstance(operator, list) and x in operator) or x != operator
+   72 |+    if (x in operator if isinstance(operator, list) else x != operator)
+73 73 | ]
+
+

crates/ruff_workspace/src/configuration.rs

@@ -1076,6 +1076,8 @@ mod tests {
     ];
 
     const PREVIEW_RULES: &[Rule] = &[
+        Rule::AndOrTernary,
+        Rule::AssignmentInAssert,
         Rule::DirectLoggerInstantiation,
         Rule::InvalidGetLoggerArgument,
         Rule::ManualDictComprehension,
@@ -1085,7 +1087,6 @@ mod tests {
         Rule::TooManyPublicMethods,
         Rule::UndocumentedWarn,
         Rule::UnnecessaryEnumerate,
-        Rule::AssignmentInAssert,
     ];
 
     #[allow(clippy::needless_pass_by_value)]

ruff.schema.json

@@ -2960,6 +2960,7 @@
         "PLR17",
         "PLR170",
         "PLR1701",
+        "PLR1706",
         "PLR171",
         "PLR1711",
         "PLR1714",