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

This commit is contained in:
Jason Newcomb 2024-08-21 23:29:51 -04:00
parent 982c6f8721
commit 4a2e08af22
3 changed files with 57 additions and 8 deletions

View File

@ -79,6 +79,8 @@ enum LetSource {
IfLetGuard, IfLetGuard,
LetElse, LetElse,
WhileLet, WhileLet,
Else,
ElseIfLet,
} }
struct MatchVisitor<'p, 'tcx> { struct MatchVisitor<'p, 'tcx> {
@ -129,15 +131,20 @@ fn visit_expr(&mut self, ex: &'p Expr<'tcx>) {
// Give a specific `let_source` for the condition. // Give a specific `let_source` for the condition.
let let_source = match ex.span.desugaring_kind() { let let_source = match ex.span.desugaring_kind() {
Some(DesugaringKind::WhileLoop) => LetSource::WhileLet, Some(DesugaringKind::WhileLoop) => LetSource::WhileLet,
_ => match self.let_source {
LetSource::Else => LetSource::ElseIfLet,
_ => LetSource::IfLet, _ => LetSource::IfLet,
},
}; };
self.with_let_source(let_source, |this| this.visit_expr(&self.thir[cond])); self.with_let_source(let_source, |this| this.visit_expr(&self.thir[cond]));
self.with_let_source(LetSource::None, |this| { self.with_let_source(LetSource::None, |this| {
this.visit_expr(&this.thir[then]); 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; return;
} }
ExprKind::Match { scrutinee, scrutinee_hir_id: _, box ref arms, match_source } => { ExprKind::Match { scrutinee, scrutinee_hir_id: _, box ref arms, match_source } => {
@ -569,9 +576,13 @@ fn check_let_chain(
// and we shouldn't lint. // and we shouldn't lint.
// For let guards inside a match, prefixes might use bindings of the match pattern, // For let guards inside a match, prefixes might use bindings of the match pattern,
// so can't always be moved out. // 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, // FIXME: Add checking whether the bindings are actually used in the prefix,
// and lint if they are not. // 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 // Emit the lint
let prefix = &chain_refutabilities[..until]; let prefix = &chain_refutabilities[..until];
let span_start = prefix[0].unwrap().0; let span_start = prefix[0].unwrap().0;
@ -902,8 +913,8 @@ macro_rules! emit_diag {
} }
match source { match source {
LetSource::None | LetSource::PlainLet => bug!(), LetSource::None | LetSource::PlainLet | LetSource::Else => bug!(),
LetSource::IfLet => emit_diag!(IrrefutableLetPatternsIfLet), LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard), LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
LetSource::LetElse => emit_diag!(IrrefutableLetPatternsLetElse), LetSource::LetElse => emit_diag!(IrrefutableLetPatternsLetElse),
LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet), 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 = note: these patterns will always match
= help: consider moving them into the body = 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 Range { start: local_start, end: _ } = first
&& let None = local_start { && 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)
{}
}
} }