[flake8-return] - Add fixes for (RET505, RET506, RET507, RET508)

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

@@ -11,10 +11,11 @@ mod tests {
     use anyhow::Result;
     use test_case::test_case;
 
-    use crate::assert_messages;
     use crate::registry::Rule;
+    use crate::settings::types::PreviewMode;
     use crate::settings::LinterSettings;
     use crate::test::test_path;
+    use crate::{assert_messages, settings};
 
     #[test_case(Rule::UnnecessaryReturnNone, Path::new("RET501.py"))]
     #[test_case(Rule::ImplicitReturnValue, Path::new("RET502.py"))]
@@ -33,4 +34,25 @@ mod tests {
         assert_messages!(snapshot, diagnostics);
         Ok(())
     }
+
+    #[test_case(Rule::SuperfluousElseReturn, Path::new("RET505.py"))]
+    #[test_case(Rule::SuperfluousElseRaise, Path::new("RET506.py"))]
+    #[test_case(Rule::SuperfluousElseContinue, Path::new("RET507.py"))]
+    #[test_case(Rule::SuperfluousElseBreak, Path::new("RET508.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("flake8_return").join(path).as_path(),
+            &settings::LinterSettings {
+                preview: PreviewMode::Enabled,
+                ..settings::LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
 }

crates/ruff_linter/src/rules/flake8_return/rules/function.rs

@@ -1,9 +1,10 @@
+use anyhow::Result;
 use std::ops::Add;
 
 use ruff_python_ast::{self as ast, ElifElseClause, Expr, Stmt};
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
-use ruff_diagnostics::{AlwaysFixableViolation, Violation};
+use ruff_diagnostics::{AlwaysFixableViolation, FixAvailability, Violation};
 use ruff_diagnostics::{Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
 
@@ -18,6 +19,7 @@ use crate::checkers::ast::Checker;
 use crate::fix::edits;
 use crate::registry::{AsRule, Rule};
 use crate::rules::flake8_return::helpers::end_of_last_statement;
+use crate::rules::pyupgrade::fixes::adjust_indentation;
 
 use super::super::branch::Branch;
 use super::super::helpers::result_exists;
@@ -210,11 +212,17 @@ pub struct SuperfluousElseReturn {
 }
 
 impl Violation for SuperfluousElseReturn {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
     #[derive_message_formats]
     fn message(&self) -> String {
         let SuperfluousElseReturn { branch } = self;
         format!("Unnecessary `{branch}` after `return` statement")
     }
+
+    fn fix_title(&self) -> Option<String> {
+        let SuperfluousElseReturn { branch } = self;
+        Some(format!("Remove unnecessary `{branch}`"))
+    }
 }
 
 /// ## What it does
@@ -248,11 +256,17 @@ pub struct SuperfluousElseRaise {
 }
 
 impl Violation for SuperfluousElseRaise {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
     #[derive_message_formats]
     fn message(&self) -> String {
         let SuperfluousElseRaise { branch } = self;
         format!("Unnecessary `{branch}` after `raise` statement")
     }
+
+    fn fix_title(&self) -> Option<String> {
+        let SuperfluousElseRaise { branch } = self;
+        Some(format!("Remove unnecessary `{branch}`"))
+    }
 }
 
 /// ## What it does
@@ -288,11 +302,17 @@ pub struct SuperfluousElseContinue {
 }
 
 impl Violation for SuperfluousElseContinue {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
     #[derive_message_formats]
     fn message(&self) -> String {
         let SuperfluousElseContinue { branch } = self;
         format!("Unnecessary `{branch}` after `continue` statement")
     }
+
+    fn fix_title(&self) -> Option<String> {
+        let SuperfluousElseContinue { branch } = self;
+        Some(format!("Remove unnecessary `{branch}`"))
+    }
 }
 
 /// ## What it does
@@ -328,11 +348,17 @@ pub struct SuperfluousElseBreak {
 }
 
 impl Violation for SuperfluousElseBreak {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
     #[derive_message_formats]
     fn message(&self) -> String {
         let SuperfluousElseBreak { branch } = self;
         format!("Unnecessary `{branch}` after `break` statement")
     }
+
+    fn fix_title(&self) -> Option<String> {
+        let SuperfluousElseBreak { branch } = self;
+        Some(format!("Remove unnecessary `{branch}`"))
+    }
 }
 
 /// RET501
@@ -575,42 +601,54 @@ fn superfluous_else_node(
     };
     for child in if_elif_body {
         if child.is_return_stmt() {
-            let diagnostic = Diagnostic::new(
+            let mut diagnostic = Diagnostic::new(
                 SuperfluousElseReturn { branch },
                 elif_else_range(elif_else, checker.locator().contents())
                     .unwrap_or_else(|| elif_else.range()),
             );
             if checker.enabled(diagnostic.kind.rule()) {
+                if checker.settings.preview.is_enabled() {
+                    diagnostic.try_set_fix(|| remove_else(checker, elif_else));
+                }
                 checker.diagnostics.push(diagnostic);
             }
             return true;
         } else if child.is_break_stmt() {
-            let diagnostic = Diagnostic::new(
+            let mut diagnostic = Diagnostic::new(
                 SuperfluousElseBreak { branch },
                 elif_else_range(elif_else, checker.locator().contents())
                     .unwrap_or_else(|| elif_else.range()),
             );
             if checker.enabled(diagnostic.kind.rule()) {
+                if checker.settings.preview.is_enabled() {
+                    diagnostic.try_set_fix(|| remove_else(checker, elif_else));
+                }
                 checker.diagnostics.push(diagnostic);
             }
             return true;
         } else if child.is_raise_stmt() {
-            let diagnostic = Diagnostic::new(
+            let mut diagnostic = Diagnostic::new(
                 SuperfluousElseRaise { branch },
                 elif_else_range(elif_else, checker.locator().contents())
                     .unwrap_or_else(|| elif_else.range()),
             );
             if checker.enabled(diagnostic.kind.rule()) {
+                if checker.settings.preview.is_enabled() {
+                    diagnostic.try_set_fix(|| remove_else(checker, elif_else));
+                }
                 checker.diagnostics.push(diagnostic);
             }
             return true;
         } else if child.is_continue_stmt() {
-            let diagnostic = Diagnostic::new(
+            let mut diagnostic = Diagnostic::new(
                 SuperfluousElseContinue { branch },
                 elif_else_range(elif_else, checker.locator().contents())
                     .unwrap_or_else(|| elif_else.range()),
             );
             if checker.enabled(diagnostic.kind.rule()) {
+                if checker.settings.preview.is_enabled() {
+                    diagnostic.try_set_fix(|| remove_else(checker, elif_else));
+                }
                 checker.diagnostics.push(diagnostic);
             }
             return true;
@@ -688,3 +726,34 @@ pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Ex
         }
     }
 }
+
+fn remove_else(checker: &mut Checker, elif_else: &ElifElseClause) -> Result<Fix> {
+    if elif_else.test.is_some() {
+        // it's an elif, so we can just make it an if
+
+        // delete "el" from "elif"
+        Ok(Fix::safe_edit(Edit::deletion(
+            elif_else.start(),
+            elif_else.start() + TextSize::from(2),
+        )))
+    } else {
+        let else_line_range = checker.locator().full_line_range(elif_else.start());
+
+        let Some(desired_indentation) = indentation(checker.locator(), elif_else) else {
+            return Err(anyhow::anyhow!("Compound statement cannot be inlined"));
+        };
+
+        let indented = adjust_indentation(
+            TextRange::new(else_line_range.end(), elif_else.end()),
+            desired_indentation,
+            checker.locator(),
+            checker.stylist(),
+        )?;
+
+        Ok(Fix::safe_edit(Edit::replacement(
+            indented,
+            else_line_range.start(),
+            elif_else.end(),
+        )))
+    }
+}

crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET505_RET505.py.snap

@@ -10,6 +10,7 @@ RET505.py:8:5: RET505 Unnecessary `elif` after `return` statement
  9 |         b = 2
 10 |         return w
    |
+   = help: Remove unnecessary `elif`
 
 RET505.py:23:5: RET505 Unnecessary `elif` after `return` statement
    |
@@ -20,6 +21,7 @@ RET505.py:23:5: RET505 Unnecessary `elif` after `return` statement
 24 |         c = 2
 25 |     else:
    |
+   = help: Remove unnecessary `elif`
 
 RET505.py:41:5: RET505 Unnecessary `elif` after `return` statement
    |
@@ -30,6 +32,7 @@ RET505.py:41:5: RET505 Unnecessary `elif` after `return` statement
 42 |         b = 2
 43 |         return w
    |
+   = help: Remove unnecessary `elif`
 
 RET505.py:53:5: RET505 Unnecessary `else` after `return` statement
    |
@@ -40,6 +43,7 @@ RET505.py:53:5: RET505 Unnecessary `else` after `return` statement
 54 |         b = 2
 55 |         return z
    |
+   = help: Remove unnecessary `else`
 
 RET505.py:64:9: RET505 Unnecessary `else` after `return` statement
    |
@@ -50,6 +54,7 @@ RET505.py:64:9: RET505 Unnecessary `else` after `return` statement
 65 |             c = 3
 66 |             return x
    |
+   = help: Remove unnecessary `else`
 
 RET505.py:79:5: RET505 Unnecessary `else` after `return` statement
    |
@@ -60,6 +65,7 @@ RET505.py:79:5: RET505 Unnecessary `else` after `return` statement
 80 |         c = 3
 81 |     return
    |
+   = help: Remove unnecessary `else`
 
 RET505.py:89:9: RET505 Unnecessary `else` after `return` statement
    |
@@ -70,6 +76,7 @@ RET505.py:89:9: RET505 Unnecessary `else` after `return` statement
 90 |             b = 2
 91 |     else:
    |
+   = help: Remove unnecessary `else`
 
 RET505.py:99:5: RET505 Unnecessary `else` after `return` statement
     |
@@ -80,5 +87,6 @@ RET505.py:99:5: RET505 Unnecessary `else` after `return` statement
 100 |         try:
 101 |             return False
     |
+    = help: Remove unnecessary `else`
 
 

crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET506_RET506.py.snap

@@ -10,6 +10,7 @@ RET506.py:8:5: RET506 Unnecessary `elif` after `raise` statement
  9 |         b = 2
 10 |         raise Exception(w)
    |
+   = help: Remove unnecessary `elif`
 
 RET506.py:23:5: RET506 Unnecessary `elif` after `raise` statement
    |
@@ -20,6 +21,7 @@ RET506.py:23:5: RET506 Unnecessary `elif` after `raise` statement
 24 |         raise Exception(y)
 25 |     else:
    |
+   = help: Remove unnecessary `elif`
 
 RET506.py:34:5: RET506 Unnecessary `else` after `raise` statement
    |
@@ -30,6 +32,7 @@ RET506.py:34:5: RET506 Unnecessary `else` after `raise` statement
 35 |         b = 2
 36 |         raise Exception(z)
    |
+   = help: Remove unnecessary `else`
 
 RET506.py:45:9: RET506 Unnecessary `else` after `raise` statement
    |
@@ -40,6 +43,7 @@ RET506.py:45:9: RET506 Unnecessary `else` after `raise` statement
 46 |             c = 3
 47 |             raise Exception(x)
    |
+   = help: Remove unnecessary `else`
 
 RET506.py:60:5: RET506 Unnecessary `else` after `raise` statement
    |
@@ -50,6 +54,7 @@ RET506.py:60:5: RET506 Unnecessary `else` after `raise` statement
 61 |         c = 3
 62 |     raise Exception(y)
    |
+   = help: Remove unnecessary `else`
 
 RET506.py:70:9: RET506 Unnecessary `else` after `raise` statement
    |
@@ -60,6 +65,7 @@ RET506.py:70:9: RET506 Unnecessary `else` after `raise` statement
 71 |             b = 2
 72 |     else:
    |
+   = help: Remove unnecessary `else`
 
 RET506.py:80:5: RET506 Unnecessary `else` after `raise` statement
    |
@@ -70,5 +76,6 @@ RET506.py:80:5: RET506 Unnecessary `else` after `raise` statement
 81 |         try:
 82 |             raise Exception(False)
    |
+   = help: Remove unnecessary `else`
 
 

crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET507_RET507.py.snap

@@ -10,6 +10,7 @@ RET507.py:8:9: RET507 Unnecessary `elif` after `continue` statement
  9 |             continue
 10 |         else:
    |
+   = help: Remove unnecessary `elif`
 
 RET507.py:22:9: RET507 Unnecessary `elif` after `continue` statement
    |
@@ -20,6 +21,7 @@ RET507.py:22:9: RET507 Unnecessary `elif` after `continue` statement
 23 |             c = 2
 24 |         else:
    |
+   = help: Remove unnecessary `elif`
 
 RET507.py:36:9: RET507 Unnecessary `else` after `continue` statement
    |
@@ -29,6 +31,7 @@ RET507.py:36:9: RET507 Unnecessary `else` after `continue` statement
    |         ^^^^ RET507
 37 |             a = z
    |
+   = help: Remove unnecessary `else`
 
 RET507.py:47:13: RET507 Unnecessary `else` after `continue` statement
    |
@@ -39,6 +42,7 @@ RET507.py:47:13: RET507 Unnecessary `else` after `continue` statement
 48 |                 c = 3
 49 |                 continue
    |
+   = help: Remove unnecessary `else`
 
 RET507.py:63:9: RET507 Unnecessary `else` after `continue` statement
    |
@@ -49,6 +53,7 @@ RET507.py:63:9: RET507 Unnecessary `else` after `continue` statement
 64 |             c = 3
 65 |         continue
    |
+   = help: Remove unnecessary `else`
 
 RET507.py:74:13: RET507 Unnecessary `else` after `continue` statement
    |
@@ -59,6 +64,7 @@ RET507.py:74:13: RET507 Unnecessary `else` after `continue` statement
 75 |                 b = 2
 76 |         else:
    |
+   = help: Remove unnecessary `else`
 
 RET507.py:85:9: RET507 Unnecessary `else` after `continue` statement
    |
@@ -69,5 +75,6 @@ RET507.py:85:9: RET507 Unnecessary `else` after `continue` statement
 86 |             try:
 87 |                 return
    |
+   = help: Remove unnecessary `else`