fix "needless return" for trailing item declarations

Author: davidsemakula

crates/hir-def/src/body/lower.rs

@@ -271,7 +271,7 @@ impl ExprCollector<'_> {
             ast::Expr::BlockExpr(e) => match e.modifier() {
                 Some(ast::BlockModifier::Try(_)) => self.desugar_try_block(e),
                 Some(ast::BlockModifier::Unsafe(_)) => {
-                    self.collect_block_(e, |id, statements, tail| Expr::Unsafe {
+                    self.collect_block_(e, |id, statements, tail, _| Expr::Unsafe {
                         id,
                         statements,
                         tail,
@@ -280,17 +280,20 @@ impl ExprCollector<'_> {
                 Some(ast::BlockModifier::Label(label)) => {
                     let label = self.collect_label(label);
                     self.with_labeled_rib(label, |this| {
-                        this.collect_block_(e, |id, statements, tail| Expr::Block {
-                            id,
-                            statements,
-                            tail,
-                            label: Some(label),
+                        this.collect_block_(e, |id, statements, tail, has_trailing_item_decl| {
+                            Expr::Block {
+                                id,
+                                statements,
+                                tail,
+                                label: Some(label),
+                                has_trailing_item_decl,
+                            }
                         })
                     })
                 }
                 Some(ast::BlockModifier::Async(_)) => {
                     self.with_label_rib(RibKind::Closure, |this| {
-                        this.collect_block_(e, |id, statements, tail| Expr::Async {
+                        this.collect_block_(e, |id, statements, tail, _| Expr::Async {
                             id,
                             statements,
                             tail,
@@ -710,9 +713,15 @@ impl ExprCollector<'_> {
 
         let (btail, expr_id) = self.with_labeled_rib(label, |this| {
             let mut btail = None;
-            let block = this.collect_block_(e, |id, statements, tail| {
+            let block = this.collect_block_(e, |id, statements, tail, _| {
                 btail = tail;
-                Expr::Block { id, statements, tail, label: Some(label) }
+                Expr::Block {
+                    id,
+                    statements,
+                    tail,
+                    label: Some(label),
+                    has_trailing_item_decl: false,
+                }
             });
             (btail, block)
         });
@@ -1118,18 +1127,19 @@ impl ExprCollector<'_> {
     }
 
     fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId {
-        self.collect_block_(block, |id, statements, tail| Expr::Block {
+        self.collect_block_(block, |id, statements, tail, has_trailing_item_decl| Expr::Block {
             id,
             statements,
             tail,
             label: None,
+            has_trailing_item_decl,
         })
     }
 
     fn collect_block_(
         &mut self,
         block: ast::BlockExpr,
-        mk_block: impl FnOnce(Option<BlockId>, Box<[Statement]>, Option<ExprId>) -> Expr,
+        mk_block: impl FnOnce(Option<BlockId>, Box<[Statement]>, Option<ExprId>, bool) -> Expr,
     ) -> ExprId {
         let block_has_items = {
             let statement_has_item = block.statements().any(|stmt| match stmt {
@@ -1178,9 +1188,15 @@ impl ExprCollector<'_> {
             None
         });
 
+        let has_trailing_item_decl = block
+            .statements()
+            .last()
+            .map_or(false, |last_stmt| matches!(last_stmt, ast::Stmt::Item(_)));
         let syntax_node_ptr = AstPtr::new(&block.into());
-        let expr_id = self
-            .alloc_expr(mk_block(block_id, statements.into_boxed_slice(), tail), syntax_node_ptr);
+        let expr_id = self.alloc_expr(
+            mk_block(block_id, statements.into_boxed_slice(), tail, has_trailing_item_decl),
+            syntax_node_ptr,
+        );
 
         self.def_map = prev_def_map;
         self.expander.module = prev_local_module;

crates/hir-def/src/body/pretty.rs

@@ -445,7 +445,7 @@ impl Printer<'_> {
                 w!(self, "]");
             }
             Expr::Literal(lit) => self.print_literal(lit),
-            Expr::Block { id: _, statements, tail, label } => {
+            Expr::Block { id: _, statements, tail, label, .. } => {
                 let label =
                     label.map(|lbl| format!("{}: ", self.body[lbl].name.display(self.db.upcast())));
                 self.print_block(label.as_deref(), statements, tail);

crates/hir-def/src/body/scope.rs

@@ -210,7 +210,7 @@ fn compute_expr_scopes(expr: ExprId, body: &Body, scopes: &mut ExprScopes, scope
 
     scopes.set_scope(expr, *scope);
     match &body[expr] {
-        Expr::Block { statements, tail, id, label } => {
+        Expr::Block { statements, tail, id, label, .. } => {
             let mut scope = scopes.new_block_scope(*scope, *id, make_label(label));
             // Overwrite the old scope for the block expr, so that every block scope can be found
             // via the block itself (important for blocks that only contain items, no expressions).

crates/hir-def/src/hir.rs

@@ -175,6 +175,7 @@ pub enum Expr {
         statements: Box<[Statement]>,
         tail: Option<ExprId>,
         label: Option<LabelId>,
+        has_trailing_item_decl: bool,
     },
     Async {
         id: Option<BlockId>,

crates/hir-ty/src/diagnostics/expr.rs

@@ -258,7 +258,10 @@ impl ExprValidator {
 
     fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) {
         match &body.exprs[body_expr] {
-            Expr::Block { statements, tail, .. } => {
+            Expr::Block { statements, tail, has_trailing_item_decl, .. } => {
+                if *has_trailing_item_decl {
+                    return;
+                }
                 let last_stmt = tail.or_else(|| match statements.last()? {
                     Statement::Expr { expr, .. } => Some(*expr),
                     _ => None,

crates/hir-ty/src/infer/expr.rs

@@ -165,7 +165,7 @@ impl InferenceContext<'_> {
                 self.infer_top_pat(pat, &input_ty);
                 self.result.standard_types.bool_.clone()
             }
-            Expr::Block { statements, tail, label, id } => {
+            Expr::Block { statements, tail, label, id, .. } => {
                 self.infer_block(tgt_expr, *id, statements, *tail, *label, expected)
             }
             Expr::Unsafe { id, statements, tail } => {

crates/hir-ty/src/infer/mutability.rs

@@ -49,7 +49,7 @@ impl InferenceContext<'_> {
                 self.infer_mut_expr(loc.root, Mutability::Not);
             }
             Expr::Let { pat, expr } => self.infer_mut_expr(*expr, self.pat_bound_mutability(*pat)),
-            Expr::Block { id: _, statements, tail, label: _ }
+            Expr::Block { id: _, statements, tail, .. }
             | Expr::Async { id: _, statements, tail }
             | Expr::Unsafe { id: _, statements, tail } => {
                 for st in statements.iter() {

crates/hir-ty/src/mir/lower.rs

@@ -558,7 +558,7 @@ impl<'ctx> MirLowerCtx<'ctx> {
             Expr::Unsafe { id: _, statements, tail } => {
                 self.lower_block_to_place(statements, current, *tail, place, expr_id.into())
             }
-            Expr::Block { id: _, statements, tail, label } => {
+            Expr::Block { id: _, statements, tail, label, .. } => {
                 if let Some(label) = label {
                     self.lower_loop(current, place, Some(*label), expr_id.into(), |this, begin| {
                         if let Some(current) = this.lower_block_to_place(

crates/ide-diagnostics/src/handlers/remove_trailing_return.rs

@@ -182,6 +182,18 @@ fn foo() -> u8 {
         );
     }
 
+    #[test]
+    fn no_diagnostic_if_not_last_statement2() {
+        check_diagnostics(
+            r#"
+fn foo() -> u8 {
+    return 2;
+    fn bar() {}
+}
+"#,
+        );
+    }
+
     #[test]
     fn replace_with_expr() {
         check_fix(