[pylint] - add fix for useless-else-on-loop / PLW0120

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,8 +1,11 @@
+use ast::whitespace::indentation;
 use ruff_python_ast::{self as ast, ExceptHandler, MatchCase, Stmt};
 
-use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::identifier;
+use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
+use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
 
@@ -42,13 +45,18 @@ 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"
         )
     }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Remove redundant `else` clause".to_string())
+    }
 }
 
 fn loop_exits_early(body: &[Stmt]) -> bool {
@@ -98,10 +106,72 @@ pub(crate) fn useless_else_on_loop(
     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(),
-        ));
+    if orelse.is_empty() || loop_exits_early(body) {
+        return;
     }
+
+    let else_range = identifier::else_(stmt, checker.locator().contents()).unwrap();
+
+    let mut diagnostic = Diagnostic::new(UselessElseOnLoop, else_range);
+
+    if checker.settings.preview.is_enabled() {
+        let start = orelse.first().unwrap();
+        let end = orelse.last().unwrap();
+        let start_indentation = indentation(checker.locator(), start);
+        if start_indentation.is_none() {
+            // Inline `else` block (e.g., `else: x = 1`).
+            diagnostic.set_fix(Fix::applicable_edit(
+                Edit::range_replacement(
+                    "".to_string(),
+                    TextRange::new(else_range.start(), start.start()),
+                ),
+                Applicability::Safe,
+            ));
+        } else {
+            let start_indentation = start_indentation.unwrap();
+
+            let desired_indentation = indentation(checker.locator(), stmt).unwrap_or("");
+            let mut indented = String::new();
+
+            checker
+                .locator()
+                .lines(TextRange::new(start.start(), end.end()))
+                .split(checker.stylist().line_ending().as_str())
+                .for_each(|line| {
+                    if let Some(stripped_line) = line.strip_prefix(start_indentation) {
+                        indented.push_str(desired_indentation);
+                        indented.push_str(stripped_line);
+                    } else {
+                        indented.push_str(line);
+                    }
+                    indented.push_str(checker.stylist().line_ending().as_str());
+                });
+
+            // we'll either delete the whole "else" line, or preserve the comment if there is one
+            let else_line_range = checker.locator().full_line_range(else_range.start());
+            let else_deletion_range = if let Some(comment_token) =
+                SimpleTokenizer::starts_at(else_range.start(), checker.locator().contents())
+                    .find(|token| token.kind == SimpleTokenKind::Comment)
+            {
+                TextRange::new(else_range.start(), comment_token.start())
+            } else {
+                else_line_range
+            };
+
+            let final_string = indented
+                .trim_end_matches(checker.stylist().line_ending().as_str())
+                .to_string();
+
+            diagnostic.set_fix(Fix::applicable_edits(
+                Edit::range_replacement(
+                    final_string,
+                    TextRange::new(else_line_range.end(), end.end()),
+                ),
+                [Edit::range_deletion(else_deletion_range)],
+                Applicability::Safe,
+            ));
+        }
+    }
+
+    checker.diagnostics.push(diagnostic);
 }

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

@@ -10,6 +10,7 @@ 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 redundant `else` clause
 
 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
    |
@@ -20,6 +21,7 @@ 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 redundant `else` clause
 
 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
    |
@@ -29,6 +31,7 @@ useless_else_on_loop.py:30:1: PLW0120 `else` clause on loop without a `break` st
    | ^^^^ PLW0120
 31 |     print("or else!")
    |
+   = help: Remove redundant `else` clause
 
 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
    |
@@ -38,6 +41,7 @@ useless_else_on_loop.py:37:1: PLW0120 `else` clause on loop without a `break` st
    | ^^^^ PLW0120
 38 |     print("or else!")
    |
+   = help: Remove redundant `else` clause
 
 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
    |
@@ -48,6 +52,7 @@ 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 redundant `else` clause
 
 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
    |
@@ -58,6 +63,7 @@ useless_else_on_loop.py:88:5: PLW0120 `else` clause on loop without a `break` st
 89 |         return True
 90 |     return False
    |
+   = help: Remove redundant `else` clause
 
 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
     |
@@ -68,5 +74,6 @@ 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 redundant `else` clause
 
 

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

@@ -0,0 +1,168 @@
+---
+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
+   |
+ 7 |         if i % 2:
+ 8 |             return i
+ 9 |     else:  # [useless-else-on-loop]
+   |     ^^^^ PLW0120
+10 |         print("math is broken")
+11 |     return None
+   |
+   = help: Remove redundant `else` clause
+
+ℹ Safe fix
+6  6  |     for i in range(10):
+7  7  |         if i % 2:
+8  8  |             return i
+9     |-    else:  # [useless-else-on-loop]
+10    |-        print("math is broken")
+   9  |+    # [useless-else-on-loop]
+   10 |+    print("math is broken")
+11 11 |     return None
+12 12 | 
+13 13 | 
+
+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
+   |
+16 |     while True:
+17 |         return 1
+18 |     else:  # [useless-else-on-loop]
+   |     ^^^^ PLW0120
+19 |         print("math is broken")
+20 |     return None
+   |
+   = help: Remove redundant `else` clause
+
+ℹ Safe fix
+15 15 |     """else + return is not acceptable."""
+16 16 |     while True:
+17 17 |         return 1
+18    |-    else:  # [useless-else-on-loop]
+19    |-        print("math is broken")
+   18 |+    # [useless-else-on-loop]
+   19 |+    print("math is broken")
+20 20 |     return None
+21 21 | 
+22 22 | 
+
+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
+   |
+28 |             break
+29 | 
+30 | else:  # [useless-else-on-loop]
+   | ^^^^ PLW0120
+31 |     print("or else!")
+   |
+   = help: Remove redundant `else` clause
+
+ℹ Safe fix
+27 27 |         for _ in range(10):
+28 28 |             break
+29 29 | 
+30    |-else:  # [useless-else-on-loop]
+31    |-    print("or else!")
+   30 |+# [useless-else-on-loop]
+   31 |+print("or else!")
+32 32 | 
+33 33 | 
+34 34 | while True:
+
+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
+   |
+35 |     while False:
+36 |         break
+37 | else:  # [useless-else-on-loop]
+   | ^^^^ PLW0120
+38 |     print("or else!")
+   |
+   = help: Remove redundant `else` clause
+
+ℹ Safe fix
+34 34 | while True:
+35 35 |     while False:
+36 36 |         break
+37    |-else:  # [useless-else-on-loop]
+38    |-    print("or else!")
+   37 |+# [useless-else-on-loop]
+   38 |+print("or else!")
+39 39 | 
+40 40 | for j in range(10):
+41 41 |     pass
+
+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
+   |
+40 | for j in range(10):
+41 |     pass
+42 | else:  # [useless-else-on-loop]
+   | ^^^^ PLW0120
+43 |     print("fat chance")
+44 |     for j in range(10):
+   |
+   = help: Remove redundant `else` clause
+
+ℹ Safe fix
+39 39 | 
+40 40 | for j in range(10):
+41 41 |     pass
+42    |-else:  # [useless-else-on-loop]
+43    |-    print("fat chance")
+44    |-    for j in range(10):
+45    |-        break
+   42 |+# [useless-else-on-loop]
+   43 |+print("fat chance")
+   44 |+for j in range(10):
+   45 |+    break
+46 46 | 
+47 47 | 
+48 48 | def test_return_for2():
+
+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
+   |
+86 |             else:
+87 |                 print("all right")
+88 |     else:  # [useless-else-on-loop]
+   |     ^^^^ PLW0120
+89 |         return True
+90 |     return False
+   |
+   = help: Remove redundant `else` clause
+
+ℹ Safe fix
+85 85 |                     break
+86 86 |             else:
+87 87 |                 print("all right")
+88    |-    else:  # [useless-else-on-loop]
+89    |-        return True
+   88 |+    # [useless-else-on-loop]
+   89 |+    return True
+90 90 |     return False
+91 91 | 
+92 92 | 
+
+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
+    |
+ 96 |         for _ in range(3):
+ 97 |             pass
+ 98 |         else:
+    |         ^^^^ PLW0120
+ 99 |             if 1 < 2:  # pylint: disable=comparison-of-constants
+100 |                 break
+    |
+    = help: Remove redundant `else` clause
+
+ℹ Safe fix
+95  95  |     for _ in range(10):
+96  96  |         for _ in range(3):
+97  97  |             pass
+98      |-        else:
+99      |-            if 1 < 2:  # pylint: disable=comparison-of-constants
+100     |-                break
+    98  |+        if 1 < 2:  # pylint: disable=comparison-of-constants
+    99  |+            break
+101 100 |     else:
+102 101 |         return True
+103 102 |     return False
+
+