Skip to content

Commit

Permalink
Rollup merge of #94739 - estebank:suggest-let-else, r=oli-obk
Browse files Browse the repository at this point in the history
Suggest `if let`/`let_else` for refutable pat in `let`

r? `````@oli-obk`````
  • Loading branch information
matthiaskrgr authored Mar 9, 2022
2 parents d5c05fc + c3a998e commit ec09e70
Show file tree
Hide file tree
Showing 15 changed files with 162 additions and 58 deletions.
86 changes: 79 additions & 7 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use super::{PatCtxt, PatternError};
use rustc_arena::TypedArena;
use rustc_ast::Mutability;
use rustc_errors::{
error_code, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
error_code, pluralize, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder,
ErrorGuaranteed,
};
use rustc_hir as hir;
use rustc_hir::def::*;
Expand All @@ -20,7 +21,7 @@ use rustc_session::lint::builtin::{
};
use rustc_session::Session;
use rustc_span::source_map::Spanned;
use rustc_span::{DesugaringKind, ExpnKind, MultiSpan, Span};
use rustc_span::{BytePos, DesugaringKind, ExpnKind, MultiSpan, Span};

crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) {
let body_id = match def_id.as_local() {
Expand Down Expand Up @@ -241,6 +242,9 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
}

let joined_patterns = joined_uncovered_patterns(&cx, &witnesses);

let mut bindings = vec![];

let mut err = struct_span_err!(
self.tcx.sess,
pat.span,
Expand All @@ -257,6 +261,16 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
false
}
_ => {
pat.walk(&mut |pat: &hir::Pat<'_>| {
match pat.kind {
hir::PatKind::Binding(_, _, ident, _) => {
bindings.push(ident);
}
_ => {}
}
true
});

err.span_label(pat.span, pattern_not_covered_label(&witnesses, &joined_patterns));
true
}
Expand All @@ -267,13 +281,71 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
"`let` bindings require an \"irrefutable pattern\", like a `struct` or \
an `enum` with only one variant",
);
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
err.span_suggestion(
span,
"you might want to use `if let` to ignore the variant that isn't matched",
format!("if {} {{ /* */ }}", &snippet[..snippet.len() - 1]),
if self.tcx.sess.source_map().span_to_snippet(span).is_ok() {
let semi_span = span.shrink_to_hi().with_lo(span.hi() - BytePos(1));
let start_span = span.shrink_to_lo();
let end_span = semi_span.shrink_to_lo();
err.multipart_suggestion(
&format!(
"you might want to use `if let` to ignore the variant{} that {} matched",
pluralize!(witnesses.len()),
match witnesses.len() {
1 => "isn't",
_ => "aren't",
},
),
vec![
match &bindings[..] {
[] => (start_span, "if ".to_string()),
[binding] => (start_span, format!("let {} = if ", binding)),
bindings => (
start_span,
format!(
"let ({}) = if ",
bindings
.iter()
.map(|ident| ident.to_string())
.collect::<Vec<_>>()
.join(", ")
),
),
},
match &bindings[..] {
[] => (semi_span, " { todo!() }".to_string()),
[binding] => {
(end_span, format!(" {{ {} }} else {{ todo!() }}", binding))
}
bindings => (
end_span,
format!(
" {{ ({}) }} else {{ todo!() }}",
bindings
.iter()
.map(|ident| ident.to_string())
.collect::<Vec<_>>()
.join(", ")
),
),
},
],
Applicability::HasPlaceholders,
);
if !bindings.is_empty() && cx.tcx.sess.is_nightly_build() {
err.span_suggestion_verbose(
semi_span.shrink_to_lo(),
&format!(
"alternatively, on nightly, you might want to use \
`#![feature(let_else)]` to handle the variant{} that {} matched",
pluralize!(witnesses.len()),
match witnesses.len() {
1 => "isn't",
_ => "aren't",
},
),
" else { todo!() }".to_string(),
Applicability::HasPlaceholders,
);
}
}
err.note(
"for more information, visit \
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/consts/const-match-check.eval1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ LL | A = { let 0 = 0; 0 },
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | A = { if let 0 = 0 { /* */ } 0 },
| ~~~~~~~~~~~~~~~~~~~~~~
LL | A = { if let 0 = 0 { todo!() } 0 },
| ++ ~~~~~~~~~~~

error: aborting due to previous error

Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/consts/const-match-check.eval2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ LL | let x: [i32; { let 0 = 0; 0 }] = [];
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | let x: [i32; { if let 0 = 0 { /* */ } 0 }] = [];
| ~~~~~~~~~~~~~~~~~~~~~~
LL | let x: [i32; { if let 0 = 0 { todo!() } 0 }] = [];
| ++ ~~~~~~~~~~~

error: aborting due to previous error

Expand Down
24 changes: 12 additions & 12 deletions src/test/ui/consts/const-match-check.matchck.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ LL | const X: i32 = { let 0 = 0; 0 };
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | const X: i32 = { if let 0 = 0 { /* */ } 0 };
| ~~~~~~~~~~~~~~~~~~~~~~
LL | const X: i32 = { if let 0 = 0 { todo!() } 0 };
| ++ ~~~~~~~~~~~

error[E0005]: refutable pattern in local binding: `i32::MIN..=-1_i32` and `1_i32..=i32::MAX` not covered
--> $DIR/const-match-check.rs:8:23
Expand All @@ -21,10 +21,10 @@ LL | static Y: i32 = { let 0 = 0; 0 };
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | static Y: i32 = { if let 0 = 0 { /* */ } 0 };
| ~~~~~~~~~~~~~~~~~~~~~~
LL | static Y: i32 = { if let 0 = 0 { todo!() } 0 };
| ++ ~~~~~~~~~~~

error[E0005]: refutable pattern in local binding: `i32::MIN..=-1_i32` and `1_i32..=i32::MAX` not covered
--> $DIR/const-match-check.rs:13:26
Expand All @@ -35,10 +35,10 @@ LL | const X: i32 = { let 0 = 0; 0 };
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | const X: i32 = { if let 0 = 0 { /* */ } 0 };
| ~~~~~~~~~~~~~~~~~~~~~~
LL | const X: i32 = { if let 0 = 0 { todo!() } 0 };
| ++ ~~~~~~~~~~~

error[E0005]: refutable pattern in local binding: `i32::MIN..=-1_i32` and `1_i32..=i32::MAX` not covered
--> $DIR/const-match-check.rs:19:26
Expand All @@ -49,10 +49,10 @@ LL | const X: i32 = { let 0 = 0; 0 };
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | const X: i32 = { if let 0 = 0 { /* */ } 0 };
| ~~~~~~~~~~~~~~~~~~~~~~
LL | const X: i32 = { if let 0 = 0 { todo!() } 0 };
| ++ ~~~~~~~~~~~

error: aborting due to 4 previous errors

Expand Down
6 changes: 5 additions & 1 deletion src/test/ui/empty/empty-never-array.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@ LL | T(T, [!; 0]),
= note: the matched value is of type `Helper<T, U>`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let Helper::U(u) = Helper::T(t, []) { /* */ }
LL | let u = if let Helper::U(u) = Helper::T(t, []) { u } else { todo!() };
| ++++++++++ ++++++++++++++++++++++
help: alternatively, on nightly, you might want to use `#![feature(let_else)]` to handle the variant that isn't matched
|
LL | let Helper::U(u) = Helper::T(t, []) else { todo!() };
| ++++++++++++++++

error: aborting due to previous error

Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/error-codes/E0005.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ LL | | }
= note: the matched value is of type `Option<i32>`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let Some(y) = x { /* */ }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | let y = if let Some(y) = x { y } else { todo!() };
| ++++++++++ ++++++++++++++++++++++
help: alternatively, on nightly, you might want to use `#![feature(let_else)]` to handle the variant that isn't matched
|
LL | let Some(y) = x else { todo!() };
| ++++++++++++++++

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ LL | | }
= note: the matched value is of type `Result<u32, !>`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let Ok(_x) = foo() { /* */ }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | let _x = if let Ok(_x) = foo() { _x } else { todo!() };
| +++++++++++ +++++++++++++++++++++++
help: alternatively, on nightly, you might want to use `#![feature(let_else)]` to handle the variant that isn't matched
|
LL | let Ok(_x) = foo() else { todo!() };
| ++++++++++++++++

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ LL | let (0 | (1 | 2)) = 0;
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `i32`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | if let (0 | (1 | 2)) = 0 { /* */ }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | if let (0 | (1 | 2)) = 0 { todo!() }
| ++ ~~~~~~~~~~~

error[E0004]: non-exhaustive patterns: `i32::MIN..=-1_i32` and `3_i32..=i32::MAX` not covered
--> $DIR/issue-69875-should-have-been-expanded-earlier-non-exhaustive.rs:3:11
Expand Down
8 changes: 6 additions & 2 deletions src/test/ui/pattern/usefulness/issue-31561.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ LL | Bar,
LL | Baz
| ^^^ not covered
= note: the matched value is of type `Thing`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | if let Thing::Foo(y) = Thing::Foo(1) { /* */ }
LL | let y = if let Thing::Foo(y) = Thing::Foo(1) { y } else { todo!() };
| ++++++++++ ++++++++++++++++++++++
help: alternatively, on nightly, you might want to use `#![feature(let_else)]` to handle the variants that aren't matched
|
LL | let Thing::Foo(y) = Thing::Foo(1) else { todo!() };
| ++++++++++++++++

error: aborting due to previous error

Expand Down
26 changes: 15 additions & 11 deletions src/test/ui/pattern/usefulness/non-exhaustive-defined-here.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ LL | B,
LL | C
| ^ not covered
= note: the matched value is of type `E`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | if let E::A = e { /* */ }
| ~~~~~~~~~~~~~~~~~~~~~~~~~
LL | if let E::A = e { todo!() }
| ++ ~~~~~~~~~~~

error[E0004]: non-exhaustive patterns: `&B` and `&C` not covered
--> $DIR/non-exhaustive-defined-here.rs:52:11
Expand Down Expand Up @@ -91,10 +91,10 @@ LL | B,
LL | C
| ^ not covered
= note: the matched value is of type `&E`
help: you might want to use `if let` to ignore the variant that isn't matched
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | if let E::A = e { /* */ }
| ~~~~~~~~~~~~~~~~~~~~~~~~~
LL | if let E::A = e { todo!() }
| ++ ~~~~~~~~~~~

error[E0004]: non-exhaustive patterns: `&&mut &B` and `&&mut &C` not covered
--> $DIR/non-exhaustive-defined-here.rs:66:11
Expand Down Expand Up @@ -140,10 +140,10 @@ LL | B,
LL | C
| ^ not covered
= note: the matched value is of type `&&mut &E`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let E::A = e { /* */ }
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | if let E::A = e { todo!() }
| ++ ~~~~~~~~~~~

error[E0004]: non-exhaustive patterns: `None` not covered
--> $DIR/non-exhaustive-defined-here.rs:92:11
Expand Down Expand Up @@ -185,8 +185,12 @@ LL | None,
= note: the matched value is of type `Opt`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let Opt::Some(ref _x) = e { /* */ }
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | let _x = if let Opt::Some(ref _x) = e { _x } else { todo!() };
| +++++++++++ +++++++++++++++++++++++
help: alternatively, on nightly, you might want to use `#![feature(let_else)]` to handle the variant that isn't matched
|
LL | let Opt::Some(ref _x) = e else { todo!() };
| ++++++++++++++++

error: aborting due to 8 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ LL | let (1, (Some(1), 2..=3)) = (1, (None, 2));
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `(i32, (Option<i32>, i32))`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let (1, (Some(1), 2..=3)) = (1, (None, 2)) { /* */ }
help: you might want to use `if let` to ignore the variants that aren't matched
|
LL | if let (1, (Some(1), 2..=3)) = (1, (None, 2)) { todo!() }
| ++ ~~~~~~~~~~~

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ LL | | }
= note: the matched value is of type `Result<u32, &R>`
help: you might want to use `if let` to ignore the variant that isn't matched
|
LL | if let Ok(x) = res { /* */ }
LL | let x = if let Ok(x) = res { x } else { todo!() };
| ++++++++++ ++++++++++++++++++++++
help: alternatively, on nightly, you might want to use `#![feature(let_else)]` to handle the variant that isn't matched
|
LL | let Ok(x) = res else { todo!() };
| ++++++++++++++++

error: aborting due to previous error

Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/uninhabited/uninhabited-irrefutable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ enum Foo {
A(foo::SecretlyEmpty),
B(foo::NotSoSecretlyEmpty),
C(NotSoSecretlyEmpty),
D(u32),
D(u32, u32),
}

fn main() {
let x: Foo = Foo::D(123);
let Foo::D(_y) = x; //~ ERROR refutable pattern in local binding: `A(_)` not covered
let x: Foo = Foo::D(123, 456);
let Foo::D(_y, _z) = x; //~ ERROR refutable pattern in local binding: `A(_)` not covered
}
Loading

0 comments on commit ec09e70

Please sign in to comment.