feat: Add a diagnostic with a fix to remove return at the end of functions

crates/hir/src/diagnostics.rs

@@ -49,6 +49,7 @@ diagnostics![
     UnresolvedMacroCall,
     UnresolvedModule,
     UnresolvedProcMacro,
+    RemoveTrailingReturn,
 ];
 
 #[derive(Debug)]
@@ -174,3 +175,9 @@ pub struct AddReferenceHere {
 }
 
 pub use hir_ty::diagnostics::IncorrectCase;
+
+#[derive(Debug)]
+pub struct RemoveTrailingReturn {
+    pub file: HirFileId,
+    pub return_expr: AstPtr<ast::Expr>,
+}

crates/hir/src/lib.rs

@@ -35,6 +35,7 @@ use std::{iter, ops::ControlFlow, sync::Arc};
 
 use arrayvec::ArrayVec;
 use base_db::{CrateDisplayName, CrateId, CrateOrigin, Edition, FileId};
+use diagnostics::RemoveTrailingReturn;
 use either::Either;
 use hir_def::{
     adt::{ReprKind, VariantData},
@@ -1232,6 +1233,16 @@ impl DefWithBody {
                         );
                     }
                 }
+
+                BodyValidationDiagnostic::RemoveTrailingReturn { return_expr } => {
+                    if let Ok(expr) = source_map.expr_syntax(return_expr) {
+                        acc.push(
+                            RemoveTrailingReturn { file: expr.file_id, return_expr: expr.value }
+                                .into(),
+                        );
+                    }
+                }
+
                 BodyValidationDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
                     match source_map.expr_syntax(call_expr) {
                         Ok(source_ptr) => acc.push(

crates/hir_ty/src/diagnostics/expr.rs

@@ -55,6 +55,9 @@ pub enum BodyValidationDiagnostic {
         arg_expr: ExprId,
         mutability: Mutability,
     },
+    RemoveTrailingReturn {
+        return_expr: ExprId,
+    },
 }
 
 impl BodyValidationDiagnostic {
@@ -80,6 +83,7 @@ impl ExprValidator {
 
     fn validate_body(&mut self, db: &dyn HirDatabase) {
         self.check_for_filter_map_next(db);
+        self.check_for_trailing_return(db);
 
         let body = db.body(self.owner);
 
@@ -141,6 +145,30 @@ impl ExprValidator {
             });
     }
 
+    fn check_for_trailing_return(&mut self, db: &dyn HirDatabase) {
+        // todo: handle inner functions and lambdas.
+        if !matches!(self.owner, DefWithBodyId::FunctionId(_)) {
+            return;
+        }
+
+        let body = db.body(self.owner);
+
+        let return_expr = match &body.exprs[body.body_expr] {
+            Expr::Block { statements, tail, .. } => tail
+                .or_else(|| match statements.last()? {
+                    Statement::Expr { expr, .. } => Some(*expr),
+                    _ => None,
+                })
+                .filter(|expr| matches!(body.exprs[*expr], Expr::Return { .. })),
+            Expr::Return { .. } => Some(body.body_expr),
+            _ => return,
+        };
+
+        if let Some(return_expr) = return_expr {
+            self.diagnostics.push(BodyValidationDiagnostic::RemoveTrailingReturn { return_expr })
+        }
+    }
+
     fn check_for_filter_map_next(&mut self, db: &dyn HirDatabase) {
         // Find the FunctionIds for Iterator::filter_map and Iterator::next
         let iterator_path = path![core::iter::Iterator];

crates/ide_diagnostics/src/handlers/remove_trailing_return.rs

@@ -0,0 +1,158 @@
+use hir::db::AstDatabase;
+use hir::diagnostics::RemoveTrailingReturn;
+use hir::InFile;
+use ide_db::assists::Assist;
+use ide_db::source_change::SourceChange;
+use syntax::{ast, AstNode, NodeOrToken, TextRange, T};
+use text_edit::TextEdit;
+
+use crate::{fix, Diagnostic, DiagnosticsContext, Severity};
+
+// Diagnostic: remove-trailing-return
+//
+// This diagnostic is triggered when there is a redundant `return` at the end of a function.
+pub(crate) fn remove_trailing_return(
+    ctx: &DiagnosticsContext<'_>,
+    d: &RemoveTrailingReturn,
+) -> Diagnostic {
+    Diagnostic::new(
+        "remove-trailing-return",
+        "replace return <expr>; with <expr>",
+        ctx.sema.diagnostics_display_range(InFile::new(d.file, d.return_expr.clone().into())).range,
+    )
+    .severity(Severity::WeakWarning)
+    .with_fixes(fixes(ctx, d))
+}
+
+fn fixes(ctx: &DiagnosticsContext<'_>, d: &RemoveTrailingReturn) -> Option<Vec<Assist>> {
+    let root = ctx.sema.db.parse_or_expand(d.file)?;
+
+    let return_expr = d.return_expr.to_node(&root);
+
+    let return_expr = ast::ReturnExpr::cast(return_expr.syntax().clone())?;
+
+    let semi = match return_expr.syntax().next_sibling_or_token() {
+        Some(NodeOrToken::Token(token)) if token.kind() == T![;] => Some(token),
+        _ => None,
+    };
+
+    let range_to_replace = match semi {
+        Some(semi) => {
+            TextRange::new(return_expr.syntax().text_range().start(), semi.text_range().end())
+        }
+        None => return_expr.syntax().text_range(),
+    };
+
+    let replacement =
+        return_expr.expr().map_or_else(String::new, |expr| format!("{}", expr.syntax().text()));
+
+    // this *seems* like a reasonable range to trigger in?
+    let trigger_range = range_to_replace;
+
+    let edit = TextEdit::replace(range_to_replace, replacement);
+
+    let source_change = SourceChange::from_text_edit(d.file.original_file(ctx.sema.db), edit);
+
+    Some(vec![fix(
+        "replace_with_inner",
+        "Replace return <expr>; with <expr>",
+        source_change,
+        trigger_range,
+    )])
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_diagnostics, check_fix};
+
+    #[test]
+    fn remove_trailing_return() {
+        // fixme: this should include the semi.
+        check_diagnostics(
+            r#"
+fn foo() -> i8 {
+    return 2;
+} //^^^^^^^^ 💡 weak: replace return <expr>; with <expr>
+"#,
+        );
+    }
+
+    // fixme: implement this for lambdas and inner functions.
+    #[test]
+    fn remove_trailing_return_no_lambda() {
+        // fixme: this should include the semi.
+        check_diagnostics(
+            r#"
+fn foo() -> i8 {
+    let bar = || return 2;
+    bar()
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn remove_trailing_return_unit() {
+        check_diagnostics(
+            r#"
+fn foo() -> i8 {
+    return
+} //^^^^^^ 💡 weak: replace return <expr>; with <expr>
+"#,
+        );
+    }
+
+    #[test]
+    fn remove_trailing_return_no_diagnostic_if_no_return_keyword() {
+        check_diagnostics(
+            r#"
+fn foo() -> i8 {
+    3
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn remove_trailing_return_no_diagnostic_if_not_at_and() {
+        check_diagnostics(
+            r#"
+fn foo() -> i8 {
+    if true { return 2; }
+    3
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn replace_with_expr() {
+        check_fix(
+            r#"
+fn foo() -> i8 {
+    return$0 2;
+}
+"#,
+            r#"
+fn foo() -> i8 {
+    2
+}
+"#,
+        );
+    }
+    #[test]
+    fn replace_with_unit() {
+        check_fix(
+            r#"
+fn foo() {
+    return$0/*ensure tidy is happy*/
+}
+"#,
+            r#"
+fn foo() {
+    /*ensure tidy is happy*/
+}
+"#,
+        );
+    }
+}

crates/ide_diagnostics/src/lib.rs

@@ -38,6 +38,7 @@ mod handlers {
     pub(crate) mod missing_unsafe;
     pub(crate) mod no_such_field;
     pub(crate) mod remove_this_semicolon;
+    pub(crate) mod remove_trailing_return;
     pub(crate) mod replace_filter_map_next_with_find_map;
     pub(crate) mod unimplemented_builtin_macro;
     pub(crate) mod unresolved_extern_crate;
@@ -204,6 +205,8 @@ pub fn diagnostics(
                 Some(it) => it,
                 None => continue,
             }
+
+            AnyDiagnostic::RemoveTrailingReturn(d) => handlers::remove_trailing_return::remove_trailing_return(&ctx, &d),
         };
         res.push(d)
     }