Check for let expr ancestors instead of tail expr

Author: ShoyuVanilla

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

@@ -12,6 +12,7 @@ use hir_expand::name;
 use itertools::Itertools;
 use rustc_hash::FxHashSet;
 use rustc_pattern_analysis::usefulness::{compute_match_usefulness,ValidityConstraint};
+use syntax::{ast,AstNode};
 use tracing::debug;
 use triomphe::Arc;
 use typed_arena::Arena;
@@ -108,7 +109,7 @@ impl ExprValidator {
 self.check_for_trailing_return(*body_expr,&body);
 }
 Expr::If{ .. } => {
-self.check_for_unnecessary_else(id, expr,&body);
+self.check_for_unnecessary_else(id, expr,db);
 }
 Expr::Block{ .. } => {
 self.validate_block(db, expr);
@@ -336,31 +337,34 @@ impl ExprValidator {
 }
 }
 
-fncheck_for_unnecessary_else(&mutself,id:ExprId,expr:&Expr,body:&Body){
+fncheck_for_unnecessary_else(&mutself,id:ExprId,expr:&Expr,db:&dynHirDatabase){
 ifletExpr::If{condition: _, then_branch, else_branch } = expr {
-ifletSome(else_branch) = else_branch {
-// If else branch has a tail, it is an "expression" that produces a value,
-// e.g. `let a = if { ... } else { ... };` and this `else` is not unnecessary
-letmut branch = *else_branch;
-loop{
-match body.exprs[branch]{
-Expr::Block{tail:Some(_), .. } => return,
-Expr::If{ then_branch, else_branch, .. } => {
-ifletExpr::Block{tail:Some(_), .. } = body.exprs[then_branch]{
-return;
-}
-ifletSome(else_branch) = else_branch {
-// Continue checking for branches like `if { ... } else if { ... } else...`
- branch = else_branch;
-continue;
-}
-}
- _ => break,
-}
-break;
-}
-}else{
+if else_branch.is_none(){
+return;
+}
+let(body, source_map) = db.body_with_source_map(self.owner);
+letOk(source_ptr) = source_map.expr_syntax(id)else{
 return;
+};
+let root = source_ptr.file_syntax(db.upcast());
+let ast::Expr::IfExpr(if_expr) = source_ptr.value.to_node(&root)else{
+return;
+};
+letmut top_if_expr = if_expr;
+loop{
+let parent = top_if_expr.syntax().parent();
+let has_parent_let_stmt =
+ parent.as_ref().map_or(false, |node| ast::LetStmt::can_cast(node.kind()));
+if has_parent_let_stmt {
+// Bail if parent or direct ancestor is a let stmt.
+return;
+}
+letSome(parent_if_expr) = parent.and_then(ast::IfExpr::cast)else{
+// Parent is neither an if expr nor a let stmt.
+break;
+};
+// Check parent if expr.
+ top_if_expr = parent_if_expr;
 }
 ifletExpr::Block{ statements, tail, .. } = &body.exprs[*then_branch]{
 let last_then_expr = tail.or_else(|| match statements.last()? {

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

@@ -400,15 +400,7 @@ fn test1(a: bool) {
  };
 }
 
-fn test2(a: bool) -> i32 {
- if a {
- return 1;
- } else {
- 0
- }
-}
-
-fn test3(a: bool, b: bool, c: bool) {
+fn test2(a: bool, b: bool, c: bool) {
  let _x = if a {
  return;
  } else if b {