[pylint] Add fix for collapsible-else-if (PLR5501)

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

@@ -49,6 +49,17 @@ def not_ok1():
             pass
 
 
+def not_ok_with_comments():
+    if 1:
+        pass
+    else:
+        # inner comment
+        if 2:
+            pass
+        else:
+            pass  # final pass comment
+
+
 # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737
 def not_ok2():
     if True:

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

@@ -1069,13 +1069,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 ruff::rules::sort_dunder_all_aug_assign(checker, aug_assign);
             }
         }
-        Stmt::If(
-            if_ @ ast::StmtIf {
-                test,
-                elif_else_clauses,
-                ..
-            },
-        ) => {
+        Stmt::If(if_ @ ast::StmtIf { test, .. }) => {
             if checker.enabled(Rule::EmptyTypeCheckingBlock) {
                 if typing::is_type_checking_block(if_, &checker.semantic) {
                     flake8_type_checking::rules::empty_type_checking_block(checker, if_);
@@ -1117,9 +1111,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 pyupgrade::rules::outdated_version_block(checker, if_);
             }
             if checker.enabled(Rule::CollapsibleElseIf) {
-                if let Some(diagnostic) = pylint::rules::collapsible_else_if(elif_else_clauses) {
-                    checker.diagnostics.push(diagnostic);
-                }
+                pylint::rules::collapsible_else_if(checker, stmt);
             }
             if checker.enabled(Rule::CheckAndRemoveFromSet) {
                 refurb::rules::check_and_remove_from_set(checker, if_);

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

@@ -193,6 +193,7 @@ mod tests {
     }
 
     #[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"))]
+    #[test_case(Rule::CollapsibleElseIf, Path::new("collapsible_else_if.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",

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

@@ -1,9 +1,13 @@
-use ruff_python_ast::{ElifElseClause, Stmt};
+use ast::whitespace::indentation;
+use ruff_python_ast::{self as ast, ElifElseClause, Stmt};
 use ruff_text_size::{Ranged, TextRange};
 
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
 
+use crate::checkers::ast::Checker;
+use crate::rules::pyupgrade::fixes::adjust_indentation;
+
 /// ## What it does
 /// Checks for `else` blocks that consist of a single `if` statement.
 ///
@@ -40,27 +44,72 @@ use ruff_macros::{derive_message_formats, violation};
 pub struct CollapsibleElseIf;
 
 impl Violation for CollapsibleElseIf {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
     #[derive_message_formats]
     fn message(&self) -> String {
         format!("Use `elif` instead of `else` then `if`, to reduce indentation")
     }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Use `elif` instead of `else` then `if`, to reduce indentation".to_string())
+    }
 }
 
 /// PLR5501
-pub(crate) fn collapsible_else_if(elif_else_clauses: &[ElifElseClause]) -> Option<Diagnostic> {
-    let Some(ElifElseClause {
-        body,
-        test: None,
-        range,
-    }) = elif_else_clauses.last()
+pub(crate) fn collapsible_else_if(checker: &mut Checker, stmt: &Stmt) {
+    let Stmt::If(ast::StmtIf {
+        elif_else_clauses, ..
+    }) = stmt
     else {
-        return None;
+        return;
+    };
+
+    let Some(
+        else_clause @ ElifElseClause {
+            body,
+            test: None,
+            range,
+        },
+    ) = elif_else_clauses.last()
+    else {
+        return;
+    };
+    let [first @ Stmt::If(ast::StmtIf { .. })] = body.as_slice() else {
+        return;
     };
-    if let [first @ Stmt::If(_)] = body.as_slice() {
-        return Some(Diagnostic::new(
-            CollapsibleElseIf,
-            TextRange::new(range.start(), first.start()),
+
+    let mut diagnostic = Diagnostic::new(
+        CollapsibleElseIf,
+        TextRange::new(range.start(), first.start()),
+    );
+
+    if checker.settings.preview.is_enabled() {
+        let inner_if_line_start = checker.locator().line_start(first.start());
+        let inner_if_line_end = checker.locator().line_end(first.end());
+
+        // Identify the indentation of the loop itself (e.g., the `while` or `for`).
+        let desired_indentation = indentation(checker.locator(), else_clause).unwrap_or("");
+
+        // Dedent the content from the end of the `else` to the end of the `if`.
+        let indented = adjust_indentation(
+            TextRange::new(inner_if_line_start, inner_if_line_end),
+            desired_indentation,
+            checker.locator(),
+            checker.stylist(),
+        )
+        .unwrap();
+
+        // Unindent the first line (which is the `if` and add `el` to the start)
+        let fixed_indented = format!("el{}", indented.strip_prefix(desired_indentation).unwrap());
+
+        diagnostic.set_fix(Fix::applicable_edit(
+            Edit::range_replacement(
+                fixed_indented,
+                TextRange::new(else_clause.start(), inner_if_line_end),
+            ),
+            Applicability::Safe,
         ));
     }
-    None
+
+    checker.diagnostics.push(diagnostic);
 }

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

@@ -11,6 +11,7 @@ collapsible_else_if.py:37:5: PLR5501 Use `elif` instead of `else` then `if`, to
    | |________^ PLR5501
 39 |               pass
    |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
 
 collapsible_else_if.py:45:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
    |
@@ -23,17 +24,33 @@ collapsible_else_if.py:45:5: PLR5501 Use `elif` instead of `else` then `if`, to
 47 |               pass
 48 |           else:
    |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
 
-collapsible_else_if.py:58:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
+collapsible_else_if.py:55:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
    |
-56 |       elif True:
-57 |           print(2)
-58 |       else:
+53 |       if 1:
+54 |           pass
+55 |       else:
    |  _____^
-59 | |         if True:
+56 | |         # inner comment
+57 | |         if 2:
    | |________^ PLR5501
-60 |               print(3)
-61 |           else:
+58 |               pass
+59 |           else:
    |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
+
+collapsible_else_if.py:69:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+67 |       elif True:
+68 |           print(2)
+69 |       else:
+   |  _____^
+70 | |         if True:
+   | |________^ PLR5501
+71 |               print(3)
+72 |           else:
+   |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
 
 

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

@@ -0,0 +1,116 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+collapsible_else_if.py:37:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+35 |       if 1:
+36 |           pass
+37 |       else:
+   |  _____^
+38 | |         if 2:
+   | |________^ PLR5501
+39 |               pass
+   |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
+
+ℹ Safe fix
+34 34 | def not_ok0():
+35 35 |     if 1:
+36 36 |         pass
+37    |-    else:
+38    |-        if 2:
+39    |-            pass
+   37 |+    elif 2:
+   38 |+        pass
+40 39 | 
+41 40 | 
+42 41 | def not_ok1():
+
+collapsible_else_if.py:45:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+43 |       if 1:
+44 |           pass
+45 |       else:
+   |  _____^
+46 | |         if 2:
+   | |________^ PLR5501
+47 |               pass
+48 |           else:
+   |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
+
+ℹ Safe fix
+42 42 | def not_ok1():
+43 43 |     if 1:
+44 44 |         pass
+   45 |+    elif 2:
+   46 |+        pass
+45 47 |     else:
+46    |-        if 2:
+47    |-            pass
+48    |-        else:
+49    |-            pass
+   48 |+        pass
+50 49 | 
+51 50 | 
+52 51 | def not_ok_with_comments():
+
+collapsible_else_if.py:55:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+53 |       if 1:
+54 |           pass
+55 |       else:
+   |  _____^
+56 | |         # inner comment
+57 | |         if 2:
+   | |________^ PLR5501
+58 |               pass
+59 |           else:
+   |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
+
+ℹ Safe fix
+52 52 | def not_ok_with_comments():
+53 53 |     if 1:
+54 54 |         pass
+   55 |+    elif 2:
+   56 |+        pass
+55 57 |     else:
+56    |-        # inner comment
+57    |-        if 2:
+58    |-            pass
+59    |-        else:
+60    |-            pass  # final pass comment
+   58 |+        pass  # final pass comment
+61 59 | 
+62 60 | 
+63 61 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737
+
+collapsible_else_if.py:69:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+67 |       elif True:
+68 |           print(2)
+69 |       else:
+   |  _____^
+70 | |         if True:
+   | |________^ PLR5501
+71 |               print(3)
+72 |           else:
+   |
+   = help: Use `elif` instead of `else` then `if`, to reduce indentation
+
+ℹ Safe fix
+66 66 |         print(1)
+67 67 |     elif True:
+68 68 |         print(2)
+   69 |+    elif True:
+   70 |+        print(3)
+69 71 |     else:
+70    |-        if True:
+71    |-            print(3)
+72    |-        else:
+73    |-            print(4)
+   72 |+        print(4)
+74 73 | 
+
+