[flake8-simplify]: combine-if-conditions (#2823)

README.md

@@ -1259,6 +1259,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/) on Py
 | SIM110 | convert-loop-to-any | Use `{any}` instead of `for` loop | 🛠 |
 | SIM111 | convert-loop-to-all | Use `{all}` instead of `for` loop | 🛠 |
 | SIM112 | use-capital-environment-variables | Use capitalized environment variable `{expected}` instead of `{original}` | 🛠 |
+| SIM114 | [if-with-same-arms](https://github.com/charliermarsh/ruff/blob/main/docs/rules/if-with-same-arms.md) | Combine `if` statements using `or` |  |
 | SIM115 | open-file-with-context-handler | Use context handler for opening files |  |
 | SIM117 | multiple-with-statements | Use a single `with` statement with multiple contexts instead of nested `with` statements | 🛠 |
 | SIM118 | key-in-dict | Use `{key} in {dict}` instead of `{key} in {dict}.keys()` | 🛠 |

crates/ruff/resources/test/fixtures/flake8_simplify/SIM114.py

@@ -0,0 +1,79 @@
+# These SHOULD change
+if a:
+    b
+elif c:
+    b
+
+if x == 1:
+    for _ in range(20):
+        print("hello")
+elif x == 2:
+    for _ in range(20):
+        print("hello")
+
+if x == 1:
+    if True:
+        for _ in range(20):
+            print("hello")
+elif x == 2:
+    if True:
+        for _ in range(20):
+            print("hello")
+
+if x == 1:
+    if True:
+        for _ in range(20):
+            print("hello")
+    elif False:
+        for _ in range(20):
+            print("hello")
+elif x == 2:
+    if True:
+        for _ in range(20):
+            print("hello")
+    elif False:
+        for _ in range(20):
+            print("hello")
+
+if (
+    x == 1
+    and y == 2
+    and z == 3
+    and a == 4
+    and b == 5
+    and c == 6
+    and d == 7
+    and e == 8
+    and f == 9
+    and g == 10
+    and h == 11
+    and i == 12
+    and j == 13
+    and k == 14
+):
+    pass
+elif 1 == 2: pass
+
+# These SHOULD NOT change
+def complicated_calc(*arg, **kwargs):
+    return 42
+
+
+def foo(p):
+    if p == 2:
+        return complicated_calc(microsecond=0)
+    elif p == 3:
+        return complicated_calc(microsecond=0, second=0)
+    return None
+
+
+a = False
+b = True
+c = True
+
+if a:
+    z = 1
+elif b:
+    z = 2
+elif c:
+    z = 1

crates/ruff/src/checkers/ast.rs

@@ -1512,6 +1512,9 @@ where
                         self.current_stmt_parent().map(Into::into),
                     );
                 }
+                if self.settings.rules.enabled(&Rule::IfWithSameArms) {
+                    flake8_simplify::rules::if_with_same_arms(self, body, orelse);
+                }
                 if self.settings.rules.enabled(&Rule::NeedlessBool) {
                     flake8_simplify::rules::return_bool_condition_directly(self, stmt);
                 }

crates/ruff/src/registry.rs

@@ -251,7 +251,6 @@ ruff_macros::define_rule_mapping!(
     YTT302 => rules::flake8_2020::rules::SysVersionCmpStr10,
     YTT303 => rules::flake8_2020::rules::SysVersionSlice1Referenced,
     // flake8-simplify
-    SIM115 => rules::flake8_simplify::rules::OpenFileWithContextHandler,
     SIM101 => rules::flake8_simplify::rules::DuplicateIsinstanceCall,
     SIM102 => rules::flake8_simplify::rules::CollapsibleIf,
     SIM103 => rules::flake8_simplify::rules::NeedlessBool,
@@ -262,6 +261,8 @@ ruff_macros::define_rule_mapping!(
     SIM110 => rules::flake8_simplify::rules::ConvertLoopToAny,
     SIM111 => rules::flake8_simplify::rules::ConvertLoopToAll,
     SIM112 => rules::flake8_simplify::rules::UseCapitalEnvironmentVariables,
+    SIM114 => rules::flake8_simplify::rules::IfWithSameArms,
+    SIM115 => rules::flake8_simplify::rules::OpenFileWithContextHandler,
     SIM117 => rules::flake8_simplify::rules::MultipleWithStatements,
     SIM118 => rules::flake8_simplify::rules::KeyInDict,
     SIM201 => rules::flake8_simplify::rules::NegateEqualOp,

crates/ruff/src/rules/flake8_simplify/mod.rs

@@ -37,6 +37,7 @@ mod tests {
     #[test_case(Rule::AndFalse, Path::new("SIM223.py"); "SIM223")]
     #[test_case(Rule::YodaConditions, Path::new("SIM300.py"); "SIM300")]
     #[test_case(Rule::DictGetWithDefault, Path::new("SIM401.py"); "SIM401")]
+    #[test_case(Rule::IfWithSameArms, Path::new("SIM114.py"); "SIM114")]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs

@@ -1,5 +1,7 @@
 use log::error;
 use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind};
+use rustpython_parser::lexer;
+use rustpython_parser::lexer::Tok;
 
 use ruff_macros::{define_violation, derive_message_formats};
 
@@ -13,6 +15,7 @@ use crate::checkers::ast::Checker;
 use crate::fix::Fix;
 use crate::registry::Diagnostic;
 use crate::rules::flake8_simplify::rules::fix_if;
+use crate::source_code::Locator;
 use crate::violation::{AutofixKind, Availability, Violation};
 
 define_violation!(
@@ -90,6 +93,36 @@ impl Violation for UseTernaryOperator {
     }
 }
 
+define_violation!(
+    /// ### What it does
+    /// Checks for `if` branches with identical arm bodies.
+    ///
+    /// ### Why is this bad?
+    /// If multiple arms of an `if` statement have the same body, using `or`
+    /// better signals the intent of the statement.
+    ///
+    /// ### Example
+    /// ```python
+    /// if x = 1:
+    ///     print("Hello")
+    /// elif x = 2:
+    ///     print("Hello")
+    /// ```
+    ///
+    /// Use instead:
+    /// ```python
+    /// if x = 1 or x = 2
+    ///     print("Hello")
+    /// ```
+    pub struct IfWithSameArms;
+);
+impl Violation for IfWithSameArms {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Combine `if` statements using `or`")
+    }
+}
+
 define_violation!(
     pub struct DictGetWithDefault {
         pub contents: String,
@@ -453,6 +486,84 @@ fn compare_expr(expr1: &ComparableExpr, expr2: &ComparableExpr) -> bool {
     expr1.eq(expr2)
 }
 
+fn get_if_body_pairs(orelse: &[Stmt], result: &mut Vec<Vec<Stmt>>) {
+    if orelse.is_empty() {
+        return;
+    }
+    let mut current_vec: Vec<Stmt> = vec![];
+    for if_line in orelse {
+        if let StmtKind::If {
+            body: orelse_body,
+            orelse: orelse_orelse,
+            ..
+        } = &if_line.node
+        {
+            if !current_vec.is_empty() {
+                result.push(current_vec.clone());
+                current_vec = vec![];
+            }
+            if !orelse_body.is_empty() {
+                result.push(orelse_body.clone());
+            }
+            get_if_body_pairs(orelse_orelse, result);
+        } else {
+            current_vec.push(if_line.clone());
+        }
+    }
+    if !current_vec.is_empty() {
+        result.push(current_vec.clone());
+    }
+}
+
+pub fn is_equal(locator: &Locator, stmts1: &[Stmt], stmts2: &[Stmt]) -> bool {
+    if stmts1.len() != stmts2.len() {
+        return false;
+    }
+    for (stmt1, stmt2) in stmts1.iter().zip(stmts2.iter()) {
+        let text1 = locator.slice_source_code_range(&Range::from_located(stmt1));
+        let text2 = locator.slice_source_code_range(&Range::from_located(stmt2));
+        let lexer1: Vec<Tok> = lexer::make_tokenizer(text1)
+            .flatten()
+            .map(|(_, tok, _)| tok)
+            .collect();
+        let lexer2: Vec<Tok> = lexer::make_tokenizer(text2)
+            .flatten()
+            .map(|(_, tok, _)| tok)
+            .collect();
+        if lexer1 != lexer2 {
+            return false;
+        }
+    }
+    true
+}
+
+/// SIM114
+pub fn if_with_same_arms(checker: &mut Checker, body: &[Stmt], orelse: &[Stmt]) {
+    if orelse.is_empty() {
+        return;
+    }
+
+    // It's not all combinations because of this:
+    // https://github.com/MartinThoma/flake8-simplify/issues/70#issuecomment-924074984
+    let mut final_stmts: Vec<Vec<Stmt>> = vec![body.to_vec()];
+    get_if_body_pairs(orelse, &mut final_stmts);
+    let if_statements = final_stmts.len();
+    if if_statements <= 1 {
+        return;
+    }
+
+    for i in 0..(if_statements - 1) {
+        if is_equal(checker.locator, &final_stmts[i], &final_stmts[i + 1]) {
+            let first = &final_stmts[i].first().unwrap();
+            let last = &final_stmts[i].last().unwrap();
+            checker.diagnostics.push(Diagnostic::new(
+                IfWithSameArms,
+                Range::new(first.location, last.end_location.unwrap()),
+            ));
+        }
+    }
+}
+
 /// SIM401
 pub fn use_dict_get_with_default(
     checker: &mut Checker,

crates/ruff/src/rules/flake8_simplify/rules/mod.rs

@@ -5,8 +5,9 @@ pub use ast_bool_op::{
 pub use ast_expr::{use_capital_environment_variables, UseCapitalEnvironmentVariables};
 pub use ast_for::{convert_for_loop_to_any_all, ConvertLoopToAll, ConvertLoopToAny};
 pub use ast_if::{
-    nested_if_statements, return_bool_condition_directly, use_dict_get_with_default,
-    use_ternary_operator, CollapsibleIf, DictGetWithDefault, NeedlessBool, UseTernaryOperator,
+    if_with_same_arms, nested_if_statements, return_bool_condition_directly,
+    use_dict_get_with_default, use_ternary_operator, CollapsibleIf, DictGetWithDefault,
+    IfWithSameArms, NeedlessBool, UseTernaryOperator,
 };
 pub use ast_ifexp::{
     explicit_false_true_in_ifexpr, explicit_true_false_in_ifexpr, twisted_arms_in_ifexpr,

crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM114_SIM114.py.snap

@@ -0,0 +1,75 @@
+---
+source: crates/ruff/src/rules/flake8_simplify/mod.rs
+expression: diagnostics
+---
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 3
+    column: 4
+  end_location:
+    row: 3
+    column: 5
+  fix: ~
+  parent: ~
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 8
+    column: 4
+  end_location:
+    row: 9
+    column: 22
+  fix: ~
+  parent: ~
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 15
+    column: 4
+  end_location:
+    row: 17
+    column: 26
+  fix: ~
+  parent: ~
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 24
+    column: 4
+  end_location:
+    row: 29
+    column: 26
+  fix: ~
+  parent: ~
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 25
+    column: 8
+  end_location:
+    row: 26
+    column: 26
+  fix: ~
+  parent: ~
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 32
+    column: 8
+  end_location:
+    row: 33
+    column: 26
+  fix: ~
+  parent: ~
+- kind:
+    IfWithSameArms: ~
+  location:
+    row: 54
+    column: 4
+  end_location:
+    row: 54
+    column: 8
+  fix: ~
+  parent: ~
+

docs/rules/combine-if-conditions.md

@@ -0,0 +1,23 @@
+# combine-if-conditions (SIM114)
+
+Derived from the **flake8-simplify** linter.
+
+### What it does
+Checks if consecutive `if` branches have the same body.
+
+### Why is this bad?
+These branches can be combine using the python `or` statement
+
+### Example
+```python
+if x = 1:
+    print("Hello")
+elif x = 2:
+    print("Hello")
+```
+
+Use instead:
+```python
+if x = 1 or x = 2
+    print("Hello")
+```

docs/rules/if-with-same-arms.md

@@ -0,0 +1,24 @@
+# if-with-same-arms (SIM114)
+
+Derived from the **flake8-simplify** linter.
+
+### What it does
+Checks for `if` branches with identical arm bodies.
+
+### Why is this bad?
+If multiple arms of an `if` statement have the same body, using `or`
+better signals the intent of the statement.
+
+### Example
+```python
+if x = 1:
+    print("Hello")
+elif x = 2:
+    print("Hello")
+```
+
+Use instead:
+```python
+if x = 1 or x = 2
+    print("Hello")
+```

ruff.schema.json

@@ -1911,6 +1911,7 @@
         "SIM110",
         "SIM111",
         "SIM112",
+        "SIM114",
         "SIM115",
         "SIM117",
         "SIM118",