Auto merge of rust-lang#16502 - davidsemakula:unnecessary-else-diagnostic, r=Veykril

feat: Add "unnecessary else" diagnostic and fix

Fixes rust-lang#9457

Author: bors

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

@@ -27,7 +27,7 @@ use crate::{
 
 pub(crate) use hir_def::{
     body::Body,
-    hir::{Expr, ExprId, MatchArm, Pat, PatId},
+    hir::{Expr, ExprId, MatchArm, Pat, PatId, Statement},
     LocalFieldId, VariantId,
 };
 
@@ -44,6 +44,9 @@ pub enum BodyValidationDiagnostic {
         match_expr: ExprId,
         uncovered_patterns: String,
     },
+    RemoveUnnecessaryElse {
+        if_expr: ExprId,
+    },
 }
 
 impl BodyValidationDiagnostic {
@@ -90,6 +93,9 @@ impl ExprValidator {
                 Expr::Call { .. } | Expr::MethodCall { .. } => {
                     self.validate_call(db, id, expr, &mut filter_map_next_checker);
                 }
+                Expr::If { .. } => {
+                    self.check_for_unnecessary_else(id, expr, &body);
+                }
                 _ => {}
             }
         }
@@ -237,6 +243,27 @@ impl ExprValidator {
         }
         pattern
     }
+
+    fn check_for_unnecessary_else(&mut self, id: ExprId, expr: &Expr, body: &Body) {
+        if let Expr::If { condition: _, then_branch, else_branch } = expr {
+            if else_branch.is_none() {
+                return;
+            }
+            if let Expr::Block { statements, tail, .. } = &body.exprs[*then_branch] {
+                let last_then_expr = tail.or_else(|| match statements.last()? {
+                    Statement::Expr { expr, .. } => Some(*expr),
+                    _ => None,
+                });
+                if let Some(last_then_expr) = last_then_expr {
+                    let last_then_expr_ty = &self.infer[last_then_expr];
+                    if last_then_expr_ty.is_never() {
+                        self.diagnostics
+                            .push(BodyValidationDiagnostic::RemoveUnnecessaryElse { if_expr: id })
+                    }
+                }
+            }
+        }
+    }
 }
 
 struct FilterMapNextChecker {

crates/hir/src/diagnostics.rs

@@ -68,6 +68,7 @@ diagnostics![
     PrivateAssocItem,
     PrivateField,
     ReplaceFilterMapNextWithFindMap,
+    RemoveUnnecessaryElse,
     TraitImplIncorrectSafety,
     TraitImplMissingAssocItems,
     TraitImplOrphan,
@@ -342,6 +343,11 @@ pub struct TraitImplRedundantAssocItems {
     pub assoc_item: (Name, AssocItem),
 }
 
+#[derive(Debug)]
+pub struct RemoveUnnecessaryElse {
+    pub if_expr: InFile<AstPtr<ast::IfExpr>>,
+}
+
 impl AnyDiagnostic {
     pub(crate) fn body_validation_diagnostic(
         db: &dyn HirDatabase,
@@ -444,6 +450,16 @@ impl AnyDiagnostic {
                     Err(SyntheticSyntax) => (),
                 }
             }
+            BodyValidationDiagnostic::RemoveUnnecessaryElse { if_expr } => {
+                if let Ok(source_ptr) = source_map.expr_syntax(if_expr) {
+                    if let Some(ptr) = source_ptr.value.cast::<ast::IfExpr>() {
+                        return Some(
+                            RemoveUnnecessaryElse { if_expr: InFile::new(source_ptr.file_id, ptr) }
+                                .into(),
+                        );
+                    }
+                }
+            }
         }
         None
     }

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

@@ -86,7 +86,7 @@ pub(super) fn token(parent: &SyntaxNode, kind: SyntaxKind) -> Option<SyntaxToken
 
 #[cfg(test)]
 mod tests {
-    use crate::tests::{check_diagnostics, check_fix};
+    use crate::tests::{check_diagnostics, check_diagnostics_with_disabled, check_fix};
 
     #[test]
     fn unused_mut_simple() {
@@ -428,7 +428,7 @@ fn main() {
 }
 "#,
         );
-        check_diagnostics(
+        check_diagnostics_with_disabled(
             r#"
 enum X {}
 fn g() -> X {
@@ -448,8 +448,9 @@ fn main(b: bool) {
     &mut x;
 }
 "#,
+            std::iter::once("remove-unnecessary-else".to_string()),
         );
-        check_diagnostics(
+        check_diagnostics_with_disabled(
             r#"
 fn main(b: bool) {
     if b {
@@ -462,6 +463,7 @@ fn main(b: bool) {
     &mut x;
 }
 "#,
+            std::iter::once("remove-unnecessary-else".to_string()),
         );
     }