Unnamed repository; edit this file 'description' to name the repository.
Diffstat (limited to 'crates/hir-ty/src/diagnostics/expr.rs')
| -rw-r--r-- | crates/hir-ty/src/diagnostics/expr.rs | 117 |
1 files changed, 98 insertions, 19 deletions
diff --git a/crates/hir-ty/src/diagnostics/expr.rs b/crates/hir-ty/src/diagnostics/expr.rs index c4329a7b82..6c8a187516 100644 --- a/crates/hir-ty/src/diagnostics/expr.rs +++ b/crates/hir-ty/src/diagnostics/expr.rs @@ -12,6 +12,8 @@ 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; @@ -44,6 +46,10 @@ pub enum BodyValidationDiagnostic { match_expr: ExprId, uncovered_patterns: String, }, + NonExhaustiveLet { + pat: PatId, + uncovered_patterns: String, + }, RemoveTrailingReturn { return_expr: ExprId, }, @@ -57,7 +63,8 @@ impl BodyValidationDiagnostic { let _p = tracing::span!(tracing::Level::INFO, "BodyValidationDiagnostic::collect").entered(); let infer = db.infer(owner); - let mut validator = ExprValidator::new(owner, infer); + let body = db.body(owner); + let mut validator = ExprValidator { owner, body, infer, diagnostics: Vec::new() }; validator.validate_body(db); validator.diagnostics } @@ -65,18 +72,16 @@ impl BodyValidationDiagnostic { struct ExprValidator { owner: DefWithBodyId, + body: Arc<Body>, infer: Arc<InferenceResult>, - pub(super) diagnostics: Vec<BodyValidationDiagnostic>, + diagnostics: Vec<BodyValidationDiagnostic>, } impl ExprValidator { - fn new(owner: DefWithBodyId, infer: Arc<InferenceResult>) -> ExprValidator { - ExprValidator { owner, infer, diagnostics: Vec::new() } - } - fn validate_body(&mut self, db: &dyn HirDatabase) { - let body = db.body(self.owner); let mut filter_map_next_checker = None; + // we'll pass &mut self while iterating over body.exprs, so they need to be disjoint + let body = Arc::clone(&self.body); if matches!(self.owner, DefWithBodyId::FunctionId(_)) { self.check_for_trailing_return(body.body_expr, &body); @@ -104,7 +109,10 @@ 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); } _ => {} } @@ -162,8 +170,6 @@ impl ExprValidator { arms: &[MatchArm], db: &dyn HirDatabase, ) { - let body = db.body(self.owner); - let scrut_ty = &self.infer[scrutinee_expr]; if scrut_ty.is_unknown() { return; @@ -191,12 +197,12 @@ impl ExprValidator { .as_reference() .map(|(match_expr_ty, ..)| match_expr_ty == pat_ty) .unwrap_or(false)) - && types_of_subpatterns_do_match(arm.pat, &body, &self.infer) + && types_of_subpatterns_do_match(arm.pat, &self.body, &self.infer) { // If we had a NotUsefulMatchArm diagnostic, we could // check the usefulness of each pattern as we added it // to the matrix here. - let pat = self.lower_pattern(&cx, arm.pat, db, &body, &mut has_lowering_errors); + let pat = self.lower_pattern(&cx, arm.pat, db, &mut has_lowering_errors); let m_arm = pat_analysis::MatchArm { pat: pattern_arena.alloc(pat), has_guard: arm.guard.is_some(), @@ -234,20 +240,63 @@ impl ExprValidator { if !witnesses.is_empty() { self.diagnostics.push(BodyValidationDiagnostic::MissingMatchArms { match_expr, - uncovered_patterns: missing_match_arms(&cx, scrut_ty, witnesses, arms), + uncovered_patterns: missing_match_arms(&cx, scrut_ty, witnesses, m_arms.is_empty()), }); } } + fn validate_block(&mut self, db: &dyn HirDatabase, expr: &Expr) { + let Expr::Block { statements, .. } = expr else { return }; + let pattern_arena = Arena::new(); + let cx = MatchCheckCtx::new(self.owner.module(db.upcast()), self.owner, db); + for stmt in &**statements { + let &Statement::Let { pat, initializer, else_branch: None, .. } = stmt else { + continue; + }; + let Some(initializer) = initializer else { continue }; + let ty = &self.infer[initializer]; + + let mut have_errors = false; + let deconstructed_pat = self.lower_pattern(&cx, pat, db, &mut have_errors); + let match_arm = rustc_pattern_analysis::MatchArm { + pat: pattern_arena.alloc(deconstructed_pat), + has_guard: false, + arm_data: (), + }; + if have_errors { + continue; + } + + let report = match compute_match_usefulness( + &cx, + &[match_arm], + ty.clone(), + ValidityConstraint::ValidOnly, + ) { + Ok(v) => v, + Err(e) => { + debug!(?e, "match usefulness error"); + continue; + } + }; + let witnesses = report.non_exhaustiveness_witnesses; + if !witnesses.is_empty() { + self.diagnostics.push(BodyValidationDiagnostic::NonExhaustiveLet { + pat, + uncovered_patterns: missing_match_arms(&cx, ty, witnesses, false), + }); + } + } + } + fn lower_pattern<'p>( &self, cx: &MatchCheckCtx<'p>, pat: PatId, db: &dyn HirDatabase, - body: &Body, have_errors: &mut bool, ) -> DeconstructedPat<'p> { - let mut patcx = match_check::PatCtxt::new(db, &self.infer, body); + let mut patcx = match_check::PatCtxt::new(db, &self.infer, &self.body); let pattern = patcx.lower_pattern(pat); let pattern = cx.lower_pat(&pattern); if !patcx.errors.is_empty() { @@ -288,12 +337,12 @@ impl ExprValidator { } } - fn check_for_unnecessary_else(&mut self, id: ExprId, expr: &Expr, body: &Body) { + fn check_for_unnecessary_else(&mut self, id: ExprId, expr: &Expr, db: &dyn HirDatabase) { 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] { + if let Expr::Block { statements, tail, .. } = &self.body.exprs[*then_branch] { let last_then_expr = tail.or_else(|| match statements.last()? { Statement::Expr { expr, .. } => Some(*expr), _ => None, @@ -301,6 +350,36 @@ impl ExprValidator { 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() { + // Only look at sources if the then branch diverges and we have an else branch. + let (_, source_map) = db.body_with_source_map(self.owner); + let Ok(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; + }; + let mut top_if_expr = if_expr; + loop { + let parent = top_if_expr.syntax().parent(); + let has_parent_expr_stmt_or_stmt_list = + parent.as_ref().map_or(false, |node| { + ast::ExprStmt::can_cast(node.kind()) + | ast::StmtList::can_cast(node.kind()) + }); + if has_parent_expr_stmt_or_stmt_list { + // Only emit diagnostic if parent or direct ancestor is either + // an expr stmt or a stmt list. + break; + } + let Some(parent_if_expr) = parent.and_then(ast::IfExpr::cast) else { + // Bail if parent is neither an if expr, an expr stmt nor a stmt list. + return; + }; + // Check parent if expr. + top_if_expr = parent_if_expr; + } + self.diagnostics .push(BodyValidationDiagnostic::RemoveUnnecessaryElse { if_expr: id }) } @@ -448,7 +527,7 @@ fn missing_match_arms<'p>( cx: &MatchCheckCtx<'p>, scrut_ty: &Ty, witnesses: Vec<WitnessPat<'p>>, - arms: &[MatchArm], + arms_is_empty: bool, ) -> String { struct DisplayWitness<'a, 'p>(&'a WitnessPat<'p>, &'a MatchCheckCtx<'p>); impl fmt::Display for DisplayWitness<'_, '_> { @@ -463,7 +542,7 @@ fn missing_match_arms<'p>( Some((AdtId::EnumId(e), _)) => !cx.db.enum_data(e).variants.is_empty(), _ => false, }; - if arms.is_empty() && !non_empty_enum { + if arms_is_empty && !non_empty_enum { format!("type `{}` is non-empty", scrut_ty.display(cx.db)) } else { let pat_display = |witness| DisplayWitness(witness, cx); |