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`.

Author: samueltardieu

clippy_lints/src/methods/double_ended_iterator_last.rs

@@ -47,7 +47,20 @@ 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| {
-                diag.multipart_suggestion("try", sugg, Applicability::MachineApplicable);
+                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 droppable_elements {
+                        Applicability::MaybeIncorrect
+                    } else {
+                        Applicability::MachineApplicable
+                    },
+                );
+                if droppable_elements {
+                    diag.note("this might cause unretrieved elements to be dropped after the retrieved one");
+                }
             },
         );
     }

tests/ui/double_ended_iterator_last.fixed

@@ -79,3 +79,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");
+}

tests/ui/double_ended_iterator_last.rs

@@ -79,3 +79,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");
+}

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:93:36
+   |
+LL |     println!("Last element is {}", v.last().unwrap().0);
+   |                                    ^^^^^^^^
+   |
+   = note: this might cause unretrieved elements to be dropped after the retrieved one
+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