move lint to loops, emit proper suggestion, more tests

10 files changed, 351 insertions, 150 deletions

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index ddc4b139670..ff418092e4b 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -258,6 +258,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::loops::WHILE_IMMUTABLE_CONDITION_INFO,
     crate::loops::WHILE_LET_LOOP_INFO,
     crate::loops::WHILE_LET_ON_ITERATOR_INFO,
+    crate::loops::WHILE_POP_UNWRAP_INFO,
     crate::macro_use::MACRO_USE_IMPORTS_INFO,
     crate::main_recursion::MAIN_RECURSION_INFO,
     crate::manual_assert::MANUAL_ASSERT_INFO,
@@ -645,7 +646,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::useless_conversion::USELESS_CONVERSION_INFO,
     crate::vec::USELESS_VEC_INFO,
     crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO,
-    crate::while_pop_unwrap::WHILE_POP_UNWRAP_INFO,
     crate::wildcard_imports::ENUM_GLOB_USE_INFO,
     crate::wildcard_imports::WILDCARD_IMPORTS_INFO,
     crate::write::PRINTLN_EMPTY_STRING_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 0d594612b79..48dbecc9f6a 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -324,7 +324,6 @@ mod use_self;
 mod useless_conversion;
 mod vec;
 mod vec_init_then_push;
-mod while_pop_unwrap;
 mod wildcard_imports;
 mod write;
 mod zero_div_zero;
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index 90012694062..02197e9c187 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -17,6 +17,7 @@ mod utils;
 mod while_immutable_condition;
 mod while_let_loop;
 mod while_let_on_iterator;
+mod while_pop_unwrap;
 
 use clippy_utils::higher;
 use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
@@ -25,8 +26,6 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor};
 
-use crate::while_pop_unwrap;
-
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for for-loops that manually copy items between
@@ -577,6 +576,36 @@ declare_clippy_lint! {
     "manual implementation of `Iterator::find`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Looks for loops that check for emptiness of a `Vec` in the condition and pop an element
+    /// in the body as a separate operation.
+    ///
+    /// ### Why is this bad?
+    /// Such loops can be written in a more idiomatic way by using a while..let loop and directly
+    /// pattern matching on the return value of `Vec::pop()`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let mut numbers = vec![1, 2, 3, 4, 5];
+    /// while !numbers.is_empty() {
+    ///     let number = numbers.pop().unwrap();
+    ///     // use `number`
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let mut numbers = vec![1, 2, 3, 4, 5];
+    /// while let Some(number) = numbers.pop() {
+    ///     // use `number`
+    /// }
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub WHILE_POP_UNWRAP,
+    style,
+    "checking for emptiness of a `Vec` in the loop condition and popping an element in the body"
+}
+
 declare_lint_pass!(Loops => [
     MANUAL_MEMCPY,
     MANUAL_FLATTEN,
@@ -596,6 +625,7 @@ declare_lint_pass!(Loops => [
     SINGLE_ELEMENT_LOOP,
     MISSING_SPIN_LOOP,
     MANUAL_FIND,
+    WHILE_POP_UNWRAP
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -642,10 +672,10 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
 
         while_let_on_iterator::check(cx, expr);
 
-        if let Some(higher::While { condition, body }) = higher::While::hir(expr) {
+        if let Some(higher::While { condition, body, span }) = higher::While::hir(expr) {
             while_immutable_condition::check(cx, condition, body);
             missing_spin_loop::check(cx, condition, body);
-            while_pop_unwrap::check(cx, condition, body);
+            while_pop_unwrap::check(cx, condition, body, span);
         }
     }
 }
diff --git a/clippy_lints/src/loops/while_pop_unwrap.rs b/clippy_lints/src/loops/while_pop_unwrap.rs
new file mode 100644
index 00000000000..1e31184b34d
--- /dev/null
+++ b/clippy_lints/src/loops/while_pop_unwrap.rs
@@ -0,0 +1,109 @@
+use clippy_utils::{
+    diagnostics::{multispan_sugg_with_applicability, span_lint_and_then},
+    match_def_path, paths,
+    source::snippet,
+    SpanlessEq,
+};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, Pat, Stmt, StmtKind, UnOp};
+use rustc_lint::LateContext;
+use rustc_span::Span;
+use std::borrow::Cow;
+
+use super::WHILE_POP_UNWRAP;
+
+/// The kind of statement that the `pop()` call appeared in.
+///
+/// Depending on whether the value was assigned to a variable or not changes what pattern
+/// we use for the suggestion.
+enum PopStmt<'hir> {
+    /// `x.pop().unwrap()` was and assigned to a variable.
+    /// The pattern of this local variable will be used and the local statement
+    /// is deleted in the suggestion.
+    Local(&'hir Pat<'hir>),
+    /// `x.pop().unwrap()` appeared in an arbitrary expression and was not assigned to a variable.
+    /// The suggestion will use some placeholder identifier and the `x.pop().unwrap()` expression
+    /// is replaced with that identifier.
+    Anonymous,
+}
+
+fn report_lint(cx: &LateContext<'_>, pop_span: Span, pop_stmt_kind: PopStmt<'_>, loop_span: Span, receiver_span: Span) {
+    span_lint_and_then(
+        cx,
+        WHILE_POP_UNWRAP,
+        pop_span,
+        "you seem to be trying to pop elements from a `Vec` in a loop",
+        |diag| {
+            let (pat, pop_replacement) = match &pop_stmt_kind {
+                PopStmt::Local(pat) => (snippet(cx, pat.span, ".."), String::new()),
+                PopStmt::Anonymous => (Cow::Borrowed("element"), "element".into()),
+            };
+
+            let loop_replacement = format!("while let Some({}) = {}.pop()", pat, snippet(cx, receiver_span, ".."));
+            multispan_sugg_with_applicability(
+                diag,
+                "consider using a `while..let` loop",
+                Applicability::MachineApplicable,
+                [(loop_span, loop_replacement), (pop_span, pop_replacement)],
+            );
+        },
+    );
+}
+
+fn match_method_call(cx: &LateContext<'_>, expr: &Expr<'_>, method: &[&str]) -> bool {
+    if let ExprKind::MethodCall(..) = expr.kind
+        && let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
+    {
+        match_def_path(cx, id, method)
+    } else {
+        false
+    }
+}
+
+fn is_vec_pop_unwrap(cx: &LateContext<'_>, expr: &Expr<'_>, is_empty_recv: &Expr<'_>) -> bool {
+    if (match_method_call(cx, expr, &paths::OPTION_UNWRAP) || match_method_call(cx, expr, &paths::OPTION_EXPECT))
+        && let ExprKind::MethodCall(_, unwrap_recv, ..) = expr.kind
+        && match_method_call(cx, unwrap_recv, &paths::VEC_POP)
+        && let ExprKind::MethodCall(_, pop_recv, ..) = unwrap_recv.kind
+    {
+        // make sure they're the same `Vec`
+        SpanlessEq::new(cx).eq_expr(pop_recv, is_empty_recv)
+    } else {
+        false
+    }
+}
+
+fn check_local(cx: &LateContext<'_>, stmt: &Stmt<'_>, is_empty_recv: &Expr<'_>, loop_span: Span) {
+    if let StmtKind::Local(local) = stmt.kind
+        && let Some(init) = local.init
+        && is_vec_pop_unwrap(cx, init, is_empty_recv)
+    {
+        report_lint(cx, stmt.span, PopStmt::Local(&local.pat), loop_span, is_empty_recv.span);
+    }
+}
+
+fn check_call_arguments(cx: &LateContext<'_>, stmt: &Stmt<'_>, is_empty_recv: &Expr<'_>, loop_span: Span) {
+    if let StmtKind::Semi(expr) | StmtKind::Expr(expr) = stmt.kind {
+        if let ExprKind::MethodCall(.., args, _) | ExprKind::Call(_, args) = expr.kind {
+            let offending_arg = args
+                .iter()
+                .find_map(|arg| is_vec_pop_unwrap(cx, arg, is_empty_recv).then_some(arg.span));
+
+            if let Some(offending_arg) = offending_arg {
+                report_lint(cx, offending_arg, PopStmt::Anonymous, loop_span, is_empty_recv.span);
+            }
+        }
+    }
+}
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, full_cond: &'tcx Expr<'_>, body: &'tcx Expr<'_>, loop_span: Span) {
+    if let ExprKind::Unary(UnOp::Not, cond) = full_cond.kind
+        && let ExprKind::MethodCall(_, is_empty_recv, _, _) = cond.kind
+        && match_method_call(cx, cond, &paths::VEC_IS_EMPTY)
+        && let ExprKind::Block(body, _) = body.kind
+        && let Some(stmt) = body.stmts.first()
+    {
+        check_local(cx, stmt, is_empty_recv, loop_span);
+        check_call_arguments(cx, stmt, is_empty_recv, loop_span);
+    }
+}
diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs
index 1f632916c57..108077b9d15 100644
--- a/clippy_lints/src/utils/author.rs
+++ b/clippy_lints/src/utils/author.rs
@@ -333,7 +333,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
 
     #[allow(clippy::too_many_lines)]
     fn expr(&self, expr: &Binding<&hir::Expr<'_>>) {
-        if let Some(higher::While { condition, body }) = higher::While::hir(expr.value) {
+        if let Some(higher::While { condition, body, .. }) = higher::While::hir(expr.value) {
             bind!(self, condition, body);
             chain!(
                 self,
diff --git a/clippy_lints/src/while_pop_unwrap.rs b/clippy_lints/src/while_pop_unwrap.rs
deleted file mode 100644
index 0a3f08888a3..00000000000
--- a/clippy_lints/src/while_pop_unwrap.rs
+++ /dev/null
@@ -1,122 +0,0 @@
-use clippy_utils::{diagnostics::span_lint_and_then, match_def_path, paths, source::snippet};
-use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, PatKind, Stmt, StmtKind, UnOp};
-use rustc_lint::LateContext;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::{symbol::Ident, Span};
-
-declare_clippy_lint! {
-    /// ### What it does
-    /// Looks for loops that check for emptiness of a `Vec` in the condition and pop an element
-    /// in the body as a separate operation.
-    ///
-    /// ### Why is this bad?
-    /// Such loops can be written in a more idiomatic way by using a while..let loop and directly
-    /// pattern matching on the return value of `Vec::pop()`.
-    ///
-    /// ### Example
-    /// ```rust
-    /// let mut numbers = vec![1, 2, 3, 4, 5];
-    /// while !numbers.is_empty() {
-    ///     let number = numbers.pop().unwrap();
-    ///     // use `number`
-    /// }
-    /// ```
-    /// Use instead:
-    /// ```rust
-    /// let mut numbers = vec![1, 2, 3, 4, 5];
-    /// while let Some(number) = numbers.pop() {
-    ///     // use `number`
-    /// }
-    /// ```
-    #[clippy::version = "1.70.0"]
-    pub WHILE_POP_UNWRAP,
-    style,
-    "checking for emptiness of a `Vec` in the loop condition and popping an element in the body"
-}
-declare_lint_pass!(WhilePopUnwrap => [WHILE_POP_UNWRAP]);
-
-fn report_lint(cx: &LateContext<'_>, pop_span: Span, ident: Option<Ident>, loop_span: Span, receiver_span: Span) {
-    span_lint_and_then(
-        cx,
-        WHILE_POP_UNWRAP,
-        pop_span,
-        "you seem to be trying to pop elements from a `Vec` in a loop",
-        |diag| {
-            diag.span_suggestion(
-                loop_span,
-                "try",
-                format!(
-                    "while let Some({}) = {}.pop()",
-                    ident.as_ref().map_or("element", Ident::as_str),
-                    snippet(cx, receiver_span, "..")
-                ),
-                Applicability::MaybeIncorrect,
-            )
-            .note("this while loop can be written in a more idiomatic way");
-        },
-    );
-}
-
-fn match_method_call(cx: &LateContext<'_>, expr: &Expr<'_>, method: &[&str]) -> bool {
-    if let ExprKind::MethodCall(..) = expr.kind
-        && let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
-        && match_def_path(cx, id, method)
-    {
-        true
-    } else {
-        false
-    }
-}
-
-fn is_vec_pop(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    match_method_call(cx, expr, &paths::VEC_POP)
-}
-
-fn is_vec_pop_unwrap(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    if let ExprKind::MethodCall(_, inner, ..) = expr.kind
-        && (match_method_call(cx, expr, &paths::OPTION_UNWRAP) || match_method_call(cx, expr, &paths::OPTION_EXPECT))
-        && is_vec_pop(cx, inner)
-    {
-        true
-    } else {
-        false
-    }
-}
-
-fn check_local(cx: &LateContext<'_>, stmt: &Stmt<'_>, loop_span: Span, recv_span: Span) {
-    if let StmtKind::Local(local) = stmt.kind
-        && let PatKind::Binding(.., ident, _) = local.pat.kind
-        && let Some(init) = local.init
-        && let ExprKind::MethodCall(_, inner, ..) = init.kind
-        && is_vec_pop_unwrap(cx, init)
-    {
-        report_lint(cx, init.span.to(inner.span), Some(ident), loop_span, recv_span);
-    }
-}
-
-fn check_call_arguments(cx: &LateContext<'_>, stmt: &Stmt<'_>, loop_span: Span, recv_span: Span) {
-    if let StmtKind::Semi(expr) | StmtKind::Expr(expr) = stmt.kind {
-        if let ExprKind::MethodCall(_, _, args, _) | ExprKind::Call(_, args) = expr.kind {
-            let offending_arg = args
-                .iter()
-                .find_map(|arg| is_vec_pop_unwrap(cx, arg).then_some(arg.span));
-
-            if let Some(offending_arg) = offending_arg {
-                report_lint(cx, offending_arg, None, loop_span, recv_span);
-            }
-        }
-    }
-}
-
-pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, body: &'tcx Expr<'_>) {
-    if let ExprKind::Unary(UnOp::Not, cond) = cond.kind
-        && let ExprKind::MethodCall(_, Expr { span: recv_span, .. }, _, _) = cond.kind
-        && match_method_call(cx, cond, &paths::VEC_IS_EMPTY)
-        && let ExprKind::Block(body, _) = body.kind
-        && let Some(stmt) = body.stmts.first()
-    {
-        check_local(cx, stmt, cond.span, *recv_span);
-        check_call_arguments(cx, stmt, cond.span, *recv_span);
-    }
-}
diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs
index 50bef370930..a61e4c38088 100644
--- a/clippy_utils/src/higher.rs
+++ b/clippy_utils/src/higher.rs
@@ -311,6 +311,8 @@ pub struct While<'hir> {
     pub condition: &'hir Expr<'hir>,
     /// `while` loop body
     pub body: &'hir Expr<'hir>,
+    /// Span of the loop header
+    pub span: Span,
 }
 
 impl<'hir> While<'hir> {
@@ -336,10 +338,10 @@ impl<'hir> While<'hir> {
             },
             _,
             LoopSource::While,
-            _,
+            span,
         ) = expr.kind
         {
-            return Some(Self { condition, body });
+            return Some(Self { condition, body, span });
         }
         None
     }
diff --git a/tests/ui/while_pop_unwrap.fixed b/tests/ui/while_pop_unwrap.fixed
new file mode 100644
index 00000000000..a635e054e6e
--- /dev/null
+++ b/tests/ui/while_pop_unwrap.fixed
@@ -0,0 +1,93 @@
+// run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::while_pop_unwrap)]
+
+struct VecInStruct {
+    numbers: Vec<i32>,
+    unrelated: String,
+}
+
+struct Foo {
+    a: i32,
+    b: i32,
+}
+
+fn accept_i32(_: i32) {}
+fn accept_optional_i32(_: Option<i32>) {}
+fn accept_i32_tuple(_: (i32, i32)) {}
+
+fn main() {
+    let mut numbers = vec![1, 2, 3, 4, 5];
+    while let Some(number) = numbers.pop() {
+        
+    }
+
+    let mut val = VecInStruct {
+        numbers: vec![1, 2, 3, 4, 5],
+        unrelated: String::new(),
+    };
+    while let Some(number) = val.numbers.pop() {
+        
+    }
+
+    while let Some(element) = numbers.pop() {
+        accept_i32(element);
+    }
+
+    while let Some(element) = numbers.pop() {
+        accept_i32(element);
+    }
+
+    // This should not warn. It "conditionally" pops elements.
+    while !numbers.is_empty() {
+        if true {
+            accept_i32(numbers.pop().unwrap());
+        }
+    }
+
+    // This should also not warn. It conditionally pops elements.
+    while !numbers.is_empty() {
+        if false {
+            continue;
+        }
+        accept_i32(numbers.pop().unwrap());
+    }
+
+    // This should not warn. It pops elements, but does not unwrap it.
+    // Might handle the Option in some other arbitrary way.
+    while !numbers.is_empty() {
+        accept_optional_i32(numbers.pop());
+    }
+
+    let unrelated_vec: Vec<String> = Vec::new();
+    // This should not warn. It pops elements from a different vector.
+    while !unrelated_vec.is_empty() {
+        accept_i32(numbers.pop().unwrap());
+    }
+
+    macro_rules! generate_loop {
+        () => {
+            while !numbers.is_empty() {
+                accept_i32(numbers.pop().unwrap());
+            }
+        };
+    }
+    // Do not warn if the loop is in a macro.
+    generate_loop!();
+
+    // Try other kinds of patterns
+    let mut numbers = vec![(0, 0), (1, 1), (2, 2)];
+    while let Some((a, b)) = numbers.pop() {
+        
+    }
+
+    while let Some(element) = numbers.pop() {
+        accept_i32_tuple(element);
+    }
+
+    let mut results = vec![Foo { a: 1, b: 2 }, Foo { a: 3, b: 4 }];
+    while let Some(Foo { a, b }) = results.pop() {
+        
+    }
+}
diff --git a/tests/ui/while_pop_unwrap.rs b/tests/ui/while_pop_unwrap.rs
index 1b6de8f1f89..820ff211086 100644
--- a/tests/ui/while_pop_unwrap.rs
+++ b/tests/ui/while_pop_unwrap.rs
@@ -1,3 +1,5 @@
+// run-rustfix
+
 #![allow(unused)]
 #![warn(clippy::while_pop_unwrap)]
 
@@ -6,7 +8,14 @@ struct VecInStruct {
     unrelated: String,
 }
 
+struct Foo {
+    a: i32,
+    b: i32,
+}
+
 fn accept_i32(_: i32) {}
+fn accept_optional_i32(_: Option<i32>) {}
+fn accept_i32_tuple(_: (i32, i32)) {}
 
 fn main() {
     let mut numbers = vec![1, 2, 3, 4, 5];
@@ -44,4 +53,41 @@ fn main() {
         }
         accept_i32(numbers.pop().unwrap());
     }
+
+    // This should not warn. It pops elements, but does not unwrap it.
+    // Might handle the Option in some other arbitrary way.
+    while !numbers.is_empty() {
+        accept_optional_i32(numbers.pop());
+    }
+
+    let unrelated_vec: Vec<String> = Vec::new();
+    // This should not warn. It pops elements from a different vector.
+    while !unrelated_vec.is_empty() {
+        accept_i32(numbers.pop().unwrap());
+    }
+
+    macro_rules! generate_loop {
+        () => {
+            while !numbers.is_empty() {
+                accept_i32(numbers.pop().unwrap());
+            }
+        };
+    }
+    // Do not warn if the loop is in a macro.
+    generate_loop!();
+
+    // Try other kinds of patterns
+    let mut numbers = vec![(0, 0), (1, 1), (2, 2)];
+    while !numbers.is_empty() {
+        let (a, b) = numbers.pop().unwrap();
+    }
+
+    while !numbers.is_empty() {
+        accept_i32_tuple(numbers.pop().unwrap());
+    }
+
+    let mut results = vec![Foo { a: 1, b: 2 }, Foo { a: 3, b: 4 }];
+    while !results.is_empty() {
+        let Foo { a, b } = results.pop().unwrap();
+    }
 }
diff --git a/tests/ui/while_pop_unwrap.stderr b/tests/ui/while_pop_unwrap.stderr
index 8dc77db8b5a..f8a6cfcda0e 100644
--- a/tests/ui/while_pop_unwrap.stderr
+++ b/tests/ui/while_pop_unwrap.stderr
@@ -1,43 +1,87 @@
 error: you seem to be trying to pop elements from a `Vec` in a loop
-  --> $DIR/while_pop_unwrap.rs:14:22
+  --> $DIR/while_pop_unwrap.rs:23:9
    |
-LL |     while !numbers.is_empty() {
-   |            ------------------ help: try: `while let Some(number) = numbers.pop()`
 LL |         let number = numbers.pop().unwrap();
-   |                      ^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: this while loop can be written in a more idiomatic way
    = note: `-D clippy::while-pop-unwrap` implied by `-D warnings`
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some(number) = numbers.pop() {
+LL ~         
+   |
 
 error: you seem to be trying to pop elements from a `Vec` in a loop
-  --> $DIR/while_pop_unwrap.rs:22:22
+  --> $DIR/while_pop_unwrap.rs:31:9
    |
-LL |     while !val.numbers.is_empty() {
-   |            ---------------------- help: try: `while let Some(number) = val.numbers.pop()`
 LL |         let number = val.numbers.pop().unwrap();
-   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some(number) = val.numbers.pop() {
+LL ~         
    |
-   = note: this while loop can be written in a more idiomatic way
 
 error: you seem to be trying to pop elements from a `Vec` in a loop
-  --> $DIR/while_pop_unwrap.rs:26:20
+  --> $DIR/while_pop_unwrap.rs:35:20
    |
-LL |     while !numbers.is_empty() {
-   |            ------------------ help: try: `while let Some(element) = numbers.pop()`
 LL |         accept_i32(numbers.pop().unwrap());
    |                    ^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: this while loop can be written in a more idiomatic way
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some(element) = numbers.pop() {
+LL ~         accept_i32(element);
+   |
 
 error: you seem to be trying to pop elements from a `Vec` in a loop
-  --> $DIR/while_pop_unwrap.rs:30:20
+  --> $DIR/while_pop_unwrap.rs:39:20
    |
-LL |     while !numbers.is_empty() {
-   |            ------------------ help: try: `while let Some(element) = numbers.pop()`
 LL |         accept_i32(numbers.pop().expect(""));
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: this while loop can be written in a more idiomatic way
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some(element) = numbers.pop() {
+LL ~         accept_i32(element);
+   |
+
+error: you seem to be trying to pop elements from a `Vec` in a loop
+  --> $DIR/while_pop_unwrap.rs:82:9
+   |
+LL |         let (a, b) = numbers.pop().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some((a, b)) = numbers.pop() {
+LL ~         
+   |
+
+error: you seem to be trying to pop elements from a `Vec` in a loop
+  --> $DIR/while_pop_unwrap.rs:86:26
+   |
+LL |         accept_i32_tuple(numbers.pop().unwrap());
+   |                          ^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some(element) = numbers.pop() {
+LL ~         accept_i32_tuple(element);
+   |
+
+error: you seem to be trying to pop elements from a `Vec` in a loop
+  --> $DIR/while_pop_unwrap.rs:91:9
+   |
+LL |         let Foo { a, b } = results.pop().unwrap();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: consider using a `while..let` loop
+   |
+LL ~     while let Some(Foo { a, b }) = results.pop() {
+LL ~         
+   |
 
-error: aborting due to 4 previous errors
+error: aborting due to 7 previous errors