[pylint] Add fix for useless-else-on-loop (PLW0120)

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

@@ -135,3 +135,14 @@ def test_break_in_match():
     else:
         return True
     return False
+
+
+def test_retain_comment():
+    """Retain the comment within the `else` block"""
+    for j in range(10):
+        pass
+    else:
+        # [useless-else-on-loop]
+        print("fat chance")
+        for j in range(10):
+            break

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

@@ -12,12 +12,13 @@ mod tests {
     use rustc_hash::FxHashSet;
     use test_case::test_case;
 
-    use crate::assert_messages;
     use crate::registry::Rule;
     use crate::rules::pylint;
+    use crate::settings::types::PreviewMode;
     use crate::settings::types::PythonVersion;
     use crate::settings::LinterSettings;
     use crate::test::test_path;
+    use crate::{assert_messages, settings};
 
     #[test_case(Rule::AndOrTernary, Path::new("and_or_ternary.py"))]
     #[test_case(Rule::AssertOnStringLiteral, Path::new("assert_on_string_literal.py"))]
@@ -190,6 +191,24 @@ mod tests {
         Ok(())
     }
 
+    #[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"))]
+    fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!(
+            "preview__{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
+        let diagnostics = test_path(
+            Path::new("pylint").join(path).as_path(),
+            &settings::LinterSettings {
+                preview: PreviewMode::Enabled,
+                ..settings::LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
+
     #[test]
     fn repeated_isinstance_calls() -> Result<()> {
         let diagnostics = test_path(

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

@@ -1,10 +1,16 @@
-use ruff_python_ast::{self as ast, ExceptHandler, MatchCase, Stmt};
+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::identifier;
+use ruff_python_ast::{self as ast, ExceptHandler, MatchCase, 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` clauses on loops without a `break` statement.
@@ -42,15 +48,50 @@ use crate::checkers::ast::Checker;
 pub struct UselessElseOnLoop;
 
 impl Violation for UselessElseOnLoop {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
     #[derive_message_formats]
     fn message(&self) -> String {
         format!(
-            "`else` clause on loop without a `break` statement; remove the `else` and de-indent all the \
-             code inside it"
+            "`else` clause on loop without a `break` statement; remove the `else` and dedent its contents"
         )
     }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Remove `else`".to_string())
+    }
 }
 
+/// PLW0120
+pub(crate) fn useless_else_on_loop(
+    checker: &mut Checker,
+    stmt: &Stmt,
+    body: &[Stmt],
+    orelse: &[Stmt],
+) {
+    if orelse.is_empty() || loop_exits_early(body) {
+        return;
+    }
+
+    let else_range = identifier::else_(stmt, checker.locator().contents()).expect("else clause");
+
+    let mut diagnostic = Diagnostic::new(UselessElseOnLoop, else_range);
+
+    if checker.settings.preview.is_enabled() {
+        diagnostic.try_set_fix(|| {
+            remove_else(
+                stmt,
+                orelse,
+                else_range,
+                checker.locator(),
+                checker.stylist(),
+            )
+        });
+    }
+
+    checker.diagnostics.push(diagnostic);
+}
+
+/// Returns `true` if the given body contains a `break` statement.
 fn loop_exits_early(body: &[Stmt]) -> bool {
     body.iter().any(|stmt| match stmt {
         Stmt::If(ast::StmtIf {
@@ -91,17 +132,50 @@ fn loop_exits_early(body: &[Stmt]) -> bool {
     })
 }
 
-/// PLW0120
-pub(crate) fn useless_else_on_loop(
-    checker: &mut Checker,
+/// Generate a [`Fix`] to remove the `else` clause from the given statement.
+fn remove_else(
     stmt: &Stmt,
-    body: &[Stmt],
     orelse: &[Stmt],
-) {
-    if !orelse.is_empty() && !loop_exits_early(body) {
-        checker.diagnostics.push(Diagnostic::new(
-            UselessElseOnLoop,
-            identifier::else_(stmt, checker.locator().contents()).unwrap(),
-        ));
+    else_range: TextRange,
+    locator: &Locator,
+    stylist: &Stylist,
+) -> Result<Fix> {
+    let Some(start) = orelse.first() else {
+        return Err(anyhow::anyhow!("Empty `else` clause"));
+    };
+    let Some(end) = orelse.last() else {
+        return Err(anyhow::anyhow!("Empty `else` clause"));
+    };
+
+    let start_indentation = indentation(locator, start);
+    if start_indentation.is_none() {
+        // Inline `else` block (e.g., `else: x = 1`).
+        Ok(Fix::safe_edit(Edit::deletion(
+            else_range.start(),
+            start.start(),
+        )))
+    } else {
+        // Identify the indentation of the loop itself (e.g., the `while` or `for`).
+        let Some(desired_indentation) = indentation(locator, stmt) else {
+            return Err(anyhow::anyhow!("Compound statement cannot be inlined"));
+        };
+
+        // Dedent the content from the end of the `else` to the end of the loop.
+        let indented = adjust_indentation(
+            TextRange::new(
+                locator.full_line_end(else_range.start()),
+                locator.full_line_end(end.end()),
+            ),
+            desired_indentation,
+            locator,
+            stylist,
+        )?;
+
+        // Replace the content from the start of the `else` to the end of the loop.
+        Ok(Fix::safe_edit(Edit::replacement(
+            indented,
+            locator.line_start(else_range.start()),
+            locator.full_line_end(end.end()),
+        )))
     }
 }

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

@@ -1,7 +1,7 @@
 ---
 source: crates/ruff_linter/src/rules/pylint/mod.rs
 ---
-useless_else_on_loop.py:9:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:9:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
    |
  7 |         if i % 2:
  8 |             return i
@@ -10,8 +10,9 @@ useless_else_on_loop.py:9:5: PLW0120 `else` clause on loop without a `break` sta
 10 |         print("math is broken")
 11 |     return None
    |
+   = help: Remove `else`
 
-useless_else_on_loop.py:18:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:18:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
    |
 16 |     while True:
 17 |         return 1
@@ -20,26 +21,29 @@ useless_else_on_loop.py:18:5: PLW0120 `else` clause on loop without a `break` st
 19 |         print("math is broken")
 20 |     return None
    |
+   = help: Remove `else`
 
-useless_else_on_loop.py:30:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:30:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
    |
 28 |             break
 29 | 
 30 | else:  # [useless-else-on-loop]
    | ^^^^ PLW0120
 31 |     print("or else!")
    |
+   = help: Remove `else`
 
-useless_else_on_loop.py:37:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:37:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
    |
 35 |     while False:
 36 |         break
 37 | else:  # [useless-else-on-loop]
    | ^^^^ PLW0120
 38 |     print("or else!")
    |
+   = help: Remove `else`
 
-useless_else_on_loop.py:42:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:42:1: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
    |
 40 | for j in range(10):
 41 |     pass
@@ -48,8 +52,9 @@ useless_else_on_loop.py:42:1: PLW0120 `else` clause on loop without a `break` st
 43 |     print("fat chance")
 44 |     for j in range(10):
    |
+   = help: Remove `else`
 
-useless_else_on_loop.py:88:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:88:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
    |
 86 |             else:
 87 |                 print("all right")
@@ -58,8 +63,9 @@ useless_else_on_loop.py:88:5: PLW0120 `else` clause on loop without a `break` st
 89 |         return True
 90 |     return False
    |
+   = help: Remove `else`
 
-useless_else_on_loop.py:98:9: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and de-indent all the code inside it
+useless_else_on_loop.py:98:9: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
     |
  96 |         for _ in range(3):
  97 |             pass
@@ -68,5 +74,17 @@ useless_else_on_loop.py:98:9: PLW0120 `else` clause on loop without a `break` st
  99 |             if 1 < 2:  # pylint: disable=comparison-of-constants
 100 |                 break
     |
+    = help: Remove `else`
+
+useless_else_on_loop.py:144:5: PLW0120 `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
+    |
+142 |     for j in range(10):
+143 |         pass
+144 |     else:
+    |     ^^^^ PLW0120
+145 |         # [useless-else-on-loop]
+146 |         print("fat chance")
+    |
+    = help: Remove `else`