[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_ok1_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:
@@ -61,3 +72,28 @@ def not_ok2():
         else:
             print(4)
 
+
+def not_ok3():
+    if 1:
+        pass
+    else:
+        if 2: pass
+        else: pass
+
+
+def not_ok4():
+    if 1:
+        pass
+    else:
+        if 2: pass
+        else:
+            pass
+
+
+def not_ok5():
+    if 1:
+        pass
+    else:
+        if 2:
+            pass
+        else: pass

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,8 +1,15 @@
-use ruff_python_ast::{ElifElseClause, Stmt};
-use ruff_text_size::{Ranged, TextRange};
+use anyhow::Result;
 
-use ruff_diagnostics::{Diagnostic, Violation};
+use ast::whitespace::indentation;
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{self as ast, ElifElseClause, Stmt};
+use ruff_python_codegen::Stylist;
+use ruff_source_file::Locator;
+use ruff_text_size::{Ranged, TextRange};
+
+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 +47,85 @@ 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("Convert to `elif`".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;
     };
-    if let [first @ Stmt::If(_)] = body.as_slice() {
-        return Some(Diagnostic::new(
-            CollapsibleElseIf,
-            TextRange::new(range.start(), first.start()),
-        ));
+
+    let Some(
+        else_clause @ ElifElseClause {
+            body, test: None, ..
+        },
+    ) = elif_else_clauses.last()
+    else {
+        return;
+    };
+    let [first @ Stmt::If(ast::StmtIf { .. })] = body.as_slice() else {
+        return;
+    };
+
+    let mut diagnostic = Diagnostic::new(
+        CollapsibleElseIf,
+        TextRange::new(else_clause.start(), first.start()),
+    );
+
+    if checker.settings.preview.is_enabled() {
+        diagnostic.try_set_fix(|| {
+            convert_to_elif(first, else_clause, checker.locator(), checker.stylist())
+        });
     }
-    None
+
+    checker.diagnostics.push(diagnostic);
+}
+
+/// Generate [`Fix`] to convert an `else` block to an `elif` block.
+fn convert_to_elif(
+    first: &Stmt,
+    else_clause: &ElifElseClause,
+    locator: &Locator,
+    stylist: &Stylist,
+) -> Result<Fix> {
+    let inner_if_line_start = locator.line_start(first.start());
+    let inner_if_line_end = locator.line_end(first.end());
+
+    // Identify the indentation of the loop itself (e.g., the `while` or `for`).
+    let Some(indentation) = indentation(locator, else_clause) else {
+        return Err(anyhow::anyhow!("`else` is expected to be on its own line"));
+    };
+
+    // 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),
+        indentation,
+        locator,
+        stylist,
+    )?;
+
+    // Strip the indent from the first line of the `if` statement, and add `el` to the start.
+    let Some(unindented) = indented.strip_prefix(indentation) else {
+        return Err(anyhow::anyhow!("indented block to start with indentation"));
+    };
+    let indented = format!("{indentation}el{unindented}");
+
+    Ok(Fix::safe_edit(Edit::replacement(
+        indented,
+        locator.line_start(else_clause.start()),
+        inner_if_line_end,
+    )))
 }

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: Convert to `elif`
 
 collapsible_else_if.py:45:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
    |
@@ -23,17 +24,71 @@ collapsible_else_if.py:45:5: PLR5501 Use `elif` instead of `else` then `if`, to
 47 |               pass
 48 |           else:
    |
+   = help: Convert to `elif`
 
-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: Convert to `elif`
+
+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: Convert to `elif`
+
+collapsible_else_if.py:79:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+77 |       if 1:
+78 |           pass
+79 |       else:
+   |  _____^
+80 | |         if 2: pass
+   | |________^ PLR5501
+81 |           else: pass
+   |
+   = help: Convert to `elif`
+
+collapsible_else_if.py:87:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+85 |       if 1:
+86 |           pass
+87 |       else:
+   |  _____^
+88 | |         if 2: pass
+   | |________^ PLR5501
+89 |           else:
+90 |               pass
+   |
+   = help: Convert to `elif`
+
+collapsible_else_if.py:96:5: PLR5501 Use `elif` instead of `else` then `if`, to reduce indentation
+   |
+94 |       if 1:
+95 |           pass
+96 |       else:
+   |  _____^
+97 | |         if 2:
+   | |________^ PLR5501
+98 |               pass
+99 |           else: pass
+   |
+   = help: Convert to `elif`