`double_ended_iterator_last`: note when drop order is changed

`iter.last()` will drop all elements of `iter` in order, while
`iter.next_back()` will drop the non-last elements of `iter` when
`iter` goes out of scope since `.next_back()` does not consume its
argument.

When the transformation proposed by `double_ended_iterator_last` would
concern an iterator whose element type has a significant drop, a note is
added to warn about the possible drop order change, and the suggestion
is switched from `MachineApplicable` to `MaybeIncorrect`.

6 files changed, 82 insertions, 2 deletions

diff --git a/clippy_lints/src/methods/double_ended_iterator_last.rs b/clippy_lints/src/methods/double_ended_iterator_last.rs
index 19797cc32f5..e82211bbf3e 100644
--- a/clippy_lints/src/methods/double_ended_iterator_last.rs
+++ b/clippy_lints/src/methods/double_ended_iterator_last.rs
@@ -49,15 +49,22 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Exp
             expr.span,
             "called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator",
             |diag| {
+                let expr_ty = cx.typeck_results().expr_ty(expr);
+                let droppable_elements = expr_ty.has_significant_drop(cx.tcx, cx.typing_env());
                 diag.multipart_suggestion(
                     "try",
                     sugg,
                     if dont_apply {
                         Applicability::Unspecified
+                    } else if droppable_elements {
+                        Applicability::MaybeIncorrect
                     } else {
                         Applicability::MachineApplicable
                     },
                 );
+                if droppable_elements {
+                    diag.note("this change will alter drop order which may be undesirable");
+                }
                 if dont_apply {
                     diag.span_note(self_expr.span, "this must be made mutable to use `.next_back()`");
                 }
diff --git a/tests/ui/double_ended_iterator_last.fixed b/tests/ui/double_ended_iterator_last.fixed
index 09eba48ea89..17d0d71a885 100644
--- a/tests/ui/double_ended_iterator_last.fixed
+++ b/tests/ui/double_ended_iterator_last.fixed
@@ -75,3 +75,18 @@ fn issue_14139() {
     let (mut subindex, _) = (index.by_ref().take(3), 42);
     let _ = subindex.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
 }
+
+fn drop_order() {
+    struct S(&'static str);
+    impl std::ops::Drop for S {
+        fn drop(&mut self) {
+            println!("Dropping {}", self.0);
+        }
+    }
+
+    let v = vec![S("one"), S("two"), S("three")];
+    let mut v = v.into_iter();
+    println!("Last element is {}", v.next_back().unwrap().0);
+    //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+    println!("Done");
+}
diff --git a/tests/ui/double_ended_iterator_last.rs b/tests/ui/double_ended_iterator_last.rs
index 2bde5e3ecb3..41bc669b171 100644
--- a/tests/ui/double_ended_iterator_last.rs
+++ b/tests/ui/double_ended_iterator_last.rs
@@ -75,3 +75,18 @@ fn issue_14139() {
     let (subindex, _) = (index.by_ref().take(3), 42);
     let _ = subindex.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
 }
+
+fn drop_order() {
+    struct S(&'static str);
+    impl std::ops::Drop for S {
+        fn drop(&mut self) {
+            println!("Dropping {}", self.0);
+        }
+    }
+
+    let v = vec![S("one"), S("two"), S("three")];
+    let v = v.into_iter();
+    println!("Last element is {}", v.last().unwrap().0);
+    //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+    println!("Done");
+}
diff --git a/tests/ui/double_ended_iterator_last.stderr b/tests/ui/double_ended_iterator_last.stderr
index 6c900b07826..1702a24d7a0 100644
--- a/tests/ui/double_ended_iterator_last.stderr
+++ b/tests/ui/double_ended_iterator_last.stderr
@@ -65,5 +65,18 @@ LL ~     let (mut subindex, _) = (index.by_ref().take(3), 42);
 LL ~     let _ = subindex.next_back();
    |
 
-error: aborting due to 7 previous errors
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:89:36
+   |
+LL |     println!("Last element is {}", v.last().unwrap().0);
+   |                                    ^^^^^^^^
+   |
+   = note: this change will alter drop order which may be undesirable
+help: try
+   |
+LL ~     let mut v = v.into_iter();
+LL ~     println!("Last element is {}", v.next_back().unwrap().0);
+   |
+
+error: aborting due to 8 previous errors
 
diff --git a/tests/ui/double_ended_iterator_last_unfixable.rs b/tests/ui/double_ended_iterator_last_unfixable.rs
index edc2a05649d..3f125c7f20c 100644
--- a/tests/ui/double_ended_iterator_last_unfixable.rs
+++ b/tests/ui/double_ended_iterator_last_unfixable.rs
@@ -6,3 +6,18 @@ fn main() {
     let subindex = (index.by_ref().take(3), 42);
     let _ = subindex.0.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
 }
+
+fn drop_order() {
+    struct S(&'static str);
+    impl std::ops::Drop for S {
+        fn drop(&mut self) {
+            println!("Dropping {}", self.0);
+        }
+    }
+
+    let v = vec![S("one"), S("two"), S("three")];
+    let v = (v.into_iter(), 42);
+    println!("Last element is {}", v.0.last().unwrap().0);
+    //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+    println!("Done");
+}
diff --git a/tests/ui/double_ended_iterator_last_unfixable.stderr b/tests/ui/double_ended_iterator_last_unfixable.stderr
index 9dd3f938648..f4be757d00d 100644
--- a/tests/ui/double_ended_iterator_last_unfixable.stderr
+++ b/tests/ui/double_ended_iterator_last_unfixable.stderr
@@ -14,5 +14,20 @@ LL |     let _ = subindex.0.last();
    = note: `-D clippy::double-ended-iterator-last` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::double_ended_iterator_last)]`
 
-error: aborting due to 1 previous error
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last_unfixable.rs:20:36
+   |
+LL |     println!("Last element is {}", v.0.last().unwrap().0);
+   |                                    ^^^^------
+   |                                        |
+   |                                        help: try: `next_back()`
+   |
+   = note: this change will alter drop order which may be undesirable
+note: this must be made mutable to use `.next_back()`
+  --> tests/ui/double_ended_iterator_last_unfixable.rs:20:36
+   |
+LL |     println!("Last element is {}", v.0.last().unwrap().0);
+   |                                    ^^^
+
+error: aborting due to 2 previous errors