Auto merge of rust-lang#5877 - ebroto:5872_loops_ice, r=Manishearth

Fix ICE in `loops` module

changelog: Fix ICE related to `needless_collect` when a call to `iter()` was not present.

I went for restoring the old suggestion of `next().is_some()` over `get(0).is_some()` given that `iter()` is not necessarily present (could be e.g. `into_iter()` or `iter_mut()`)  and that the old suggestion could change semantics, e.g. a call to `filter()` could be present between `iter()` and the collect part.

Fixes rust-lang#5872

Author: bors

clippy_lints/src/loops.rs

@@ -2374,7 +2374,7 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
                 match_type(cx, ty, &paths::BTREEMAP) ||
                 is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) {
                 if method.ident.name == sym!(len) {
-                    let span = shorten_span(expr, sym!(collect));
+                    let span = shorten_needless_collect_span(expr);
                     span_lint_and_sugg(
                         cx,
                         NEEDLESS_COLLECT,
@@ -2386,20 +2386,20 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
                     );
                 }
                 if method.ident.name == sym!(is_empty) {
-                    let span = shorten_span(expr, sym!(iter));
+                    let span = shorten_needless_collect_span(expr);
                     span_lint_and_sugg(
                         cx,
                         NEEDLESS_COLLECT,
                         span,
                         NEEDLESS_COLLECT_MSG,
                         "replace with",
-                        "get(0).is_none()".to_string(),
+                        "next().is_none()".to_string(),
                         Applicability::MachineApplicable,
                     );
                 }
                 if method.ident.name == sym!(contains) {
                     let contains_arg = snippet(cx, args[1].span, "??");
-                    let span = shorten_span(expr, sym!(collect));
+                    let span = shorten_needless_collect_span(expr);
                     span_lint_and_then(
                         cx,
                         NEEDLESS_COLLECT,
@@ -2579,13 +2579,13 @@ fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, identifier: Ident)
     }
 }
 
-fn shorten_span(expr: &Expr<'_>, target_fn_name: Symbol) -> Span {
-    let mut current_expr = expr;
-    while let ExprKind::MethodCall(ref path, ref span, ref args, _) = current_expr.kind {
-        if path.ident.name == target_fn_name {
+fn shorten_needless_collect_span(expr: &Expr<'_>) -> Span {
+    if_chain! {
+        if let ExprKind::MethodCall(.., args, _) = &expr.kind;
+        if let ExprKind::MethodCall(_, span, ..) = &args[0].kind;
+        then {
             return expr.span.with_lo(span.lo());
         }
-        current_expr = &args[0];
     }
-    unreachable!()
+    unreachable!();
 }

tests/ui/crashes/ice-5872.rs

@@ -0,0 +1,5 @@
+#![warn(clippy::needless_collect)]
+
+fn main() {
+    let _ = vec![1, 2, 3].into_iter().collect::<Vec<_>>().is_empty();
+}

tests/ui/crashes/ice-5872.stderr

@@ -0,0 +1,10 @@
+error: avoid using `collect()` when not needed
+  --> $DIR/ice-5872.rs:4:39
+   |
+LL |     let _ = vec![1, 2, 3].into_iter().collect::<Vec<_>>().is_empty();
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
+   |
+   = note: `-D clippy::needless-collect` implied by `-D warnings`
+
+error: aborting due to previous error
+

tests/ui/needless_collect.fixed

@@ -5,11 +5,11 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 
 #[warn(clippy::needless_collect)]
-#[allow(unused_variables, clippy::iter_cloned_collect)]
+#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
 fn main() {
     let sample = [1; 5];
     let len = sample.iter().count();
-    if sample.get(0).is_none() {
+    if sample.iter().next().is_none() {
         // Empty
     }
     sample.iter().cloned().any(|x| x == 1);

tests/ui/needless_collect.rs

@@ -5,7 +5,7 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 
 #[warn(clippy::needless_collect)]
-#[allow(unused_variables, clippy::iter_cloned_collect)]
+#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
 fn main() {
     let sample = [1; 5];
     let len = sample.iter().collect::<Vec<_>>().len();

tests/ui/needless_collect.stderr

@@ -7,10 +7,10 @@ LL |     let len = sample.iter().collect::<Vec<_>>().len();
    = note: `-D clippy::needless-collect` implied by `-D warnings`
 
 error: avoid using `collect()` when not needed
-  --> $DIR/needless_collect.rs:12:15
+  --> $DIR/needless_collect.rs:12:22
    |
 LL |     if sample.iter().collect::<Vec<_>>().is_empty() {
-   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get(0).is_none()`
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
 
 error: avoid using `collect()` when not needed
   --> $DIR/needless_collect.rs:15:28