Rollup merge of #129394 - Jarcho:irrefutable_let_patterns, r=Nadrieril

Don't lint `irrefutable_let_patterns` on leading patterns if `else if` let-chains

fixes #128661

Is there any preference where the test goes? There looks to be several places it could fit.
This commit is contained in:
Matthias Krüger 2024-10-30 06:40:34 +01:00 committed by GitHub
commit 87d348b333
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 57 additions and 8 deletions

View File

@ -79,6 +79,8 @@ enum LetSource {
IfLetGuard,
LetElse,
WhileLet,
Else,
ElseIfLet,
}
struct MatchVisitor<'p, 'tcx> {
@ -129,15 +131,20 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
// Give a specific `let_source` for the condition.
let let_source = match ex.span.desugaring_kind() {
Some(DesugaringKind::WhileLoop) => LetSource::WhileLet,
_ => match self.let_source {
LetSource::Else => LetSource::ElseIfLet,
_ => LetSource::IfLet,
},
};
self.with_let_source(let_source, |this| this.visit_expr(&self.thir[cond]));
self.with_let_source(LetSource::None, |this| {
this.visit_expr(&this.thir[then]);
if let Some(else_) = else_opt {
this.visit_expr(&this.thir[else_]);
}
});
if let Some(else_) = else_opt {
self.with_let_source(LetSource::Else, |this| {
this.visit_expr(&this.thir[else_])
});
}
return;
}
ExprKind::Match { scrutinee, scrutinee_hir_id: _, box ref arms, match_source } => {
@ -573,9 +580,13 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
// and we shouldn't lint.
// For let guards inside a match, prefixes might use bindings of the match pattern,
// so can't always be moved out.
// For `else if let`, an extra indentation level would be required to move the bindings.
// FIXME: Add checking whether the bindings are actually used in the prefix,
// and lint if they are not.
if !matches!(self.let_source, LetSource::WhileLet | LetSource::IfLetGuard) {
if !matches!(
self.let_source,
LetSource::WhileLet | LetSource::IfLetGuard | LetSource::ElseIfLet
) {
// Emit the lint
let prefix = &chain_refutabilities[..until];
let span_start = prefix[0].unwrap().0;
@ -906,8 +917,8 @@ fn report_irrefutable_let_patterns(
}
match source {
LetSource::None | LetSource::PlainLet => bug!(),
LetSource::IfLet => emit_diag!(IrrefutableLetPatternsIfLet),
LetSource::None | LetSource::PlainLet | LetSource::Else => bug!(),
LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
LetSource::LetElse => emit_diag!(IrrefutableLetPatternsLetElse),
LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet),

View File

@ -111,5 +111,23 @@ LL | while let Some(ref first) = opt && let second = first && let _third = s
= note: these patterns will always match
= help: consider moving them into the body
error: aborting due to 12 previous errors
error: trailing irrefutable pattern in let chain
--> $DIR/irrefutable-lets.rs:87:12
|
LL | && let x = &opt
| ^^^^^^^^^^^^
|
= note: this pattern will always match
= help: consider moving it into the body
error: leading irrefutable pattern in let chain
--> $DIR/irrefutable-lets.rs:93:12
|
LL | if let x = opt.clone().map(|_| 1)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this pattern will always match
= help: consider moving it outside of the construct
error: aborting due to 14 previous errors

View File

@ -75,4 +75,24 @@ fn main() {
&& let Range { start: local_start, end: _ } = first
&& let None = local_start {
}
// No error. An extra nesting level would be required for the `else if`.
if opt == Some(None..None) {
} else if let x = opt.clone().map(|_| 1)
&& x == Some(1)
{}
if opt == Some(None..None) {
} else if opt.is_some()
&& let x = &opt
//[disallowed]~^ ERROR trailing irrefutable pattern in let chain
{}
if opt == Some(None..None) {
} else {
if let x = opt.clone().map(|_| 1)
//[disallowed]~^ ERROR leading irrefutable pattern in let chain
&& x == Some(1)
{}
}
}