Skip to content

Commit ff8fe52

Browse files
borsbors
committed
Auto merge of #16303 - rosefromthedead:non-exhaustive-let, r=Veykril
feat: add non-exhaustive-let diagnostic I want this to have a quickfix to add an else branch but I couldn't figure out how to do that, so here's the diagnostic on its own. It pretends a `let` is a match with one arm, and asks the match checking whether that match would be exhaustive. Previously the pattern was checked based on its own type, but that was causing a panic in `match_check` (while processing e.g. `crates/hir/src/lib.rs`) so I changed it to use the initialiser's type instead, to align with the checking of actual match expressions. I think the panic can still happen, but I hear that `match_check` is going to be updated to a new version from rustc, so I'm posting this now in the hopes that the panic will magically go away when that happens.
2 parents d8c8ccc + 5390e4c commit ff8fe52

5 files changed

Lines changed: 137 additions & 17 deletions

File tree

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

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use hir_expand::name;
1212
use itertools::Itertools;
1313
use rustc_hash::FxHashSet;
1414
use rustc_pattern_analysis::usefulness::{compute_match_usefulness,ValidityConstraint};
15+
use tracing::debug;
1516
use triomphe::Arc;
1617
use typed_arena::Arena;
1718

@@ -44,6 +45,10 @@ pub enum BodyValidationDiagnostic {
4445
match_expr:ExprId,
4546
uncovered_patterns:String,
4647
},
48+
NonExhaustiveLet{
49+
pat:PatId,
50+
uncovered_patterns:String,
51+
},
4752
RemoveTrailingReturn{
4853
return_expr:ExprId,
4954
},
@@ -57,26 +62,25 @@ impl BodyValidationDiagnostic {
5762
let _p =
5863
tracing::span!(tracing::Level::INFO,"BodyValidationDiagnostic::collect").entered();
5964
let infer = db.infer(owner);
60-
letmut validator = ExprValidator::new(owner, infer);
65+
let body = db.body(owner);
66+
letmut validator = ExprValidator{ owner, body, infer,diagnostics:Vec::new()};
6167
validator.validate_body(db);
6268
validator.diagnostics
6369
}
6470
}
6571

6672
structExprValidator{
6773
owner:DefWithBodyId,
74+
body:Arc<Body>,
6875
infer:Arc<InferenceResult>,
69-
pub(super)diagnostics:Vec<BodyValidationDiagnostic>,
76+
diagnostics:Vec<BodyValidationDiagnostic>,
7077
}
7178

7279
implExprValidator{
73-
fnnew(owner:DefWithBodyId,infer:Arc<InferenceResult>) -> ExprValidator{
74-
ExprValidator{ owner, infer,diagnostics:Vec::new()}
75-
}
76-
7780
fnvalidate_body(&mutself,db:&dynHirDatabase){
78-
let body = db.body(self.owner);
7981
letmut filter_map_next_checker = None;
82+
// we'll pass &mut self while iterating over body.exprs, so they need to be disjoint
83+
let body = Arc::clone(&self.body);
8084

8185
ifmatches!(self.owner,DefWithBodyId::FunctionId(_)){
8286
self.check_for_trailing_return(body.body_expr,&body);
@@ -106,6 +110,9 @@ impl ExprValidator {
106110
Expr::If{ .. } => {
107111
self.check_for_unnecessary_else(id, expr,&body);
108112
}
113+
Expr::Block{ .. } => {
114+
self.validate_block(db, expr);
115+
}
109116
_ => {}
110117
}
111118
}
@@ -162,8 +169,6 @@ impl ExprValidator {
162169
arms:&[MatchArm],
163170
db:&dynHirDatabase,
164171
){
165-
let body = db.body(self.owner);
166-
167172
let scrut_ty = &self.infer[scrutinee_expr];
168173
if scrut_ty.is_unknown(){
169174
return;
@@ -191,12 +196,12 @@ impl ExprValidator {
191196
.as_reference()
192197
.map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
193198
.unwrap_or(false))
194-
&& types_of_subpatterns_do_match(arm.pat,&body,&self.infer)
199+
&& types_of_subpatterns_do_match(arm.pat,&self.body,&self.infer)
195200
{
196201
// If we had a NotUsefulMatchArm diagnostic, we could
197202
// check the usefulness of each pattern as we added it
198203
// to the matrix here.
199-
let pat = self.lower_pattern(&cx, arm.pat, db,&body,&mut has_lowering_errors);
204+
let pat = self.lower_pattern(&cx, arm.pat, db,&mut has_lowering_errors);
200205
let m_arm = pat_analysis::MatchArm{
201206
pat: pattern_arena.alloc(pat),
202207
has_guard: arm.guard.is_some(),
@@ -234,20 +239,63 @@ impl ExprValidator {
234239
if !witnesses.is_empty(){
235240
self.diagnostics.push(BodyValidationDiagnostic::MissingMatchArms{
236241
match_expr,
237-
uncovered_patterns:missing_match_arms(&cx, scrut_ty, witnesses,arms),
242+
uncovered_patterns:missing_match_arms(&cx, scrut_ty, witnesses,m_arms.is_empty()),
238243
});
239244
}
240245
}
241246

247+
fnvalidate_block(&mutself,db:&dynHirDatabase,expr:&Expr){
248+
letExpr::Block{ statements, .. } = expr else{return};
249+
let pattern_arena = Arena::new();
250+
let cx = MatchCheckCtx::new(self.owner.module(db.upcast()),self.owner, db);
251+
for stmt in&**statements {
252+
let&Statement::Let{ pat, initializer,else_branch:None, .. } = stmt else{
253+
continue;
254+
};
255+
letSome(initializer) = initializer else{continue};
256+
let ty = &self.infer[initializer];
257+
258+
letmut have_errors = false;
259+
let deconstructed_pat = self.lower_pattern(&cx, pat, db,&mut have_errors);
260+
let match_arm = rustc_pattern_analysis::MatchArm{
261+
pat: pattern_arena.alloc(deconstructed_pat),
262+
has_guard:false,
263+
arm_data:(),
264+
};
265+
if have_errors {
266+
continue;
267+
}
268+
269+
let report = matchcompute_match_usefulness(
270+
&cx,
271+
&[match_arm],
272+
ty.clone(),
273+
ValidityConstraint::ValidOnly,
274+
){
275+
Ok(v) => v,
276+
Err(e) => {
277+
debug!(?e,"match usefulness error");
278+
continue;
279+
}
280+
};
281+
let witnesses = report.non_exhaustiveness_witnesses;
282+
if !witnesses.is_empty(){
283+
self.diagnostics.push(BodyValidationDiagnostic::NonExhaustiveLet{
284+
pat,
285+
uncovered_patterns:missing_match_arms(&cx, ty, witnesses,false),
286+
});
287+
}
288+
}
289+
}
290+
242291
fnlower_pattern<'p>(
243292
&self,
244293
cx:&MatchCheckCtx<'p>,
245294
pat:PatId,
246295
db:&dynHirDatabase,
247-
body:&Body,
248296
have_errors:&mutbool,
249297
) -> DeconstructedPat<'p>{
250-
letmut patcx = match_check::PatCtxt::new(db,&self.infer, body);
298+
letmut patcx = match_check::PatCtxt::new(db,&self.infer,&self.body);
251299
let pattern = patcx.lower_pattern(pat);
252300
let pattern = cx.lower_pat(&pattern);
253301
if !patcx.errors.is_empty(){
@@ -448,7 +496,7 @@ fn missing_match_arms<'p>(
448496
cx:&MatchCheckCtx<'p>,
449497
scrut_ty:&Ty,
450498
witnesses:Vec<WitnessPat<'p>>,
451-
arms:&[MatchArm],
499+
arms_is_empty:bool,
452500
) -> String{
453501
structDisplayWitness<'a,'p>(&'aWitnessPat<'p>,&'aMatchCheckCtx<'p>);
454502
impl fmt::DisplayforDisplayWitness<'_,'_>{
@@ -463,7 +511,7 @@ fn missing_match_arms<'p>(
463511
Some((AdtId::EnumId(e), _)) => !cx.db.enum_data(e).variants.is_empty(),
464512
_ => false,
465513
};
466-
ifarms.is_empty() && !non_empty_enum {
514+
ifarms_is_empty && !non_empty_enum {
467515
format!("type `{}` is non-empty", scrut_ty.display(cx.db))
468516
}else{
469517
let pat_display = |witness| DisplayWitness(witness, cx);

‎crates/hir/src/diagnostics.rs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ diagnostics![
6464
MissingUnsafe,
6565
MovedOutOfRef,
6666
NeedMut,
67+
NonExhaustiveLet,
6768
NoSuchField,
6869
PrivateAssocItem,
6970
PrivateField,
@@ -280,6 +281,12 @@ pub struct MissingMatchArms {
280281
pubuncovered_patterns:String,
281282
}
282283

284+
#[derive(Debug)]
285+
pubstructNonExhaustiveLet{
286+
pubpat:InFile<AstPtr<ast::Pat>>,
287+
pubuncovered_patterns:String,
288+
}
289+
283290
#[derive(Debug)]
284291
pubstructTypeMismatch{
285292
pubexpr_or_pat:InFile<AstPtr<Either<ast::Expr, ast::Pat>>>,
@@ -456,6 +463,22 @@ impl AnyDiagnostic {
456463
Err(SyntheticSyntax) => (),
457464
}
458465
}
466+
BodyValidationDiagnostic::NonExhaustiveLet{ pat, uncovered_patterns } => {
467+
match source_map.pat_syntax(pat){
468+
Ok(source_ptr) => {
469+
ifletSome(ast_pat) = source_ptr.value.cast::<ast::Pat>(){
470+
returnSome(
471+
NonExhaustiveLet{
472+
pat:InFile::new(source_ptr.file_id, ast_pat),
473+
uncovered_patterns,
474+
}
475+
.into(),
476+
);
477+
}
478+
}
479+
Err(SyntheticSyntax) => {}
480+
}
481+
}
459482
BodyValidationDiagnostic::RemoveTrailingReturn{ return_expr } => {
460483
ifletOk(source_ptr) = source_map.expr_syntax(return_expr){
461484
// Filters out desugared return expressions (e.g. desugared try operators).

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ fn f() {
817817
//- minicore: option
818818
fn f(_: i32) {}
819819
fn main() {
820-
let ((Some(mut x), None) | (_, Some(mut x))) = (None, Some(7));
820+
let ((Some(mut x), None) | (_, Some(mut x))) = (None, Some(7)) else { return };
821821
//^^^^^ 💡 warn: variable does not need to be mutable
822822
f(x);
823823
}

‎crates/ide-diagnostics/src/handlers/non_exhaustive_let.rs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
usecrate::{Diagnostic,DiagnosticCode,DiagnosticsContext};
2+
3+
// Diagnostic: non-exhaustive-let
4+
//
5+
// This diagnostic is triggered if a `let` statement without an `else` branch has a non-exhaustive
6+
// pattern.
7+
pub(crate)fnnon_exhaustive_let(
8+
ctx:&DiagnosticsContext<'_>,
9+
d:&hir::NonExhaustiveLet,
10+
) -> Diagnostic{
11+
Diagnostic::new_with_syntax_node_ptr(
12+
ctx,
13+
DiagnosticCode::RustcHardError("E0005"),
14+
format!("non-exhaustive pattern: {}", d.uncovered_patterns),
15+
d.pat.map(Into::into),
16+
)
17+
}
18+
19+
#[cfg(test)]
20+
mod tests {
21+
usecrate::tests::check_diagnostics;
22+
23+
#[test]
24+
fnoption_nonexhaustive(){
25+
check_diagnostics(
26+
r#"
27+
//- minicore: option
28+
fn main() {
29+
let None = Some(5);
30+
//^^^^ error: non-exhaustive pattern: `Some(_)` not covered
31+
}
32+
"#,
33+
);
34+
}
35+
36+
#[test]
37+
fnoption_exhaustive(){
38+
check_diagnostics(
39+
r#"
40+
//- minicore: option
41+
fn main() {
42+
let Some(_) | None = Some(5);
43+
}
44+
"#,
45+
);
46+
}
47+
}

‎crates/ide-diagnostics/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ mod handlers {
4141
pub(crate)mod moved_out_of_ref;
4242
pub(crate)mod mutability_errors;
4343
pub(crate)mod no_such_field;
44+
pub(crate)mod non_exhaustive_let;
4445
pub(crate)mod private_assoc_item;
4546
pub(crate)mod private_field;
4647
pub(crate)mod remove_trailing_return;
@@ -359,6 +360,7 @@ pub fn diagnostics(
359360
AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx,&d),
360361
AnyDiagnostic::MovedOutOfRef(d) => handlers::moved_out_of_ref::moved_out_of_ref(&ctx,&d),
361362
AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx,&d),
363+
AnyDiagnostic::NonExhaustiveLet(d) => handlers::non_exhaustive_let::non_exhaustive_let(&ctx,&d),
362364
AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx,&d),
363365
AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx,&d),
364366
AnyDiagnostic::PrivateField(d) => handlers::private_field::private_field(&ctx,&d),

0 commit comments

Comments
 (0)