feat: introduce path_to_local_with_projections (#15396)

As suggested in
#15268 (comment)

WIP because:
- [x] what should be done with the now-error-pattern-less
`tests/ui/double_ended_iterator_last_unfixable.rs`?
- [x] is the change in behaviour of `double_ended_iterator_last` okay in
general?
- cc @samueltardieu because this changes the code you added in
#14140

changelog: none

r? @y21

Author: y21

clippy_lints/src/methods/double_ended_iterator_last.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::ty::{has_non_owning_mutable_access, implements_trait};
-use clippy_utils::{is_mutable, is_trait_method, path_to_local, sym};
+use clippy_utils::{is_mutable, is_trait_method, path_to_local_with_projections, sym};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, Node, PatKind};
 use rustc_lint::LateContext;
@@ -37,7 +37,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Exp
         // TODO: Change this to lint only when the referred iterator is not used later. If it is used later,
         // changing to `next_back()` may change its behavior.
         if !(is_mutable(cx, self_expr) || self_type.is_ref()) {
-            if let Some(hir_id) = path_to_local(self_expr)
+            if let Some(hir_id) = path_to_local_with_projections(self_expr)
                 && let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
                 && let PatKind::Binding(_, _, ident, _) = pat.kind
             {

clippy_lints/src/methods/filter_next.rs

@@ -1,4 +1,5 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
+use clippy_utils::path_to_local_with_projections;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::implements_trait;
 use rustc_ast::{BindingMode, Mutability};
@@ -9,21 +10,6 @@ use rustc_span::sym;
 
 use super::FILTER_NEXT;
 
-fn path_to_local(expr: &hir::Expr<'_>) -> Option<hir::HirId> {
-    match expr.kind {
-        hir::ExprKind::Field(f, _) => path_to_local(f),
-        hir::ExprKind::Index(recv, _, _) => path_to_local(recv),
-        hir::ExprKind::Path(hir::QPath::Resolved(
-            _,
-            hir::Path {
-                res: rustc_hir::def::Res::Local(local),
-                ..
-            },
-        )) => Some(*local),
-        _ => None,
-    }
-}
-
 /// lint use of `filter().next()` for `Iterators`
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
@@ -44,7 +30,7 @@ pub(super) fn check<'tcx>(
             let iter_snippet = snippet(cx, recv.span, "..");
             // add note if not multi-line
             span_lint_and_then(cx, FILTER_NEXT, expr.span, msg, |diag| {
-                let (applicability, pat) = if let Some(id) = path_to_local(recv)
+                let (applicability, pat) = if let Some(id) = path_to_local_with_projections(recv)
                     && let hir::Node::Pat(pat) = cx.tcx.hir_node(id)
                     && let hir::PatKind::Binding(BindingMode(_, Mutability::Not), _, ident, _) = pat.kind
                 {

clippy_lints/src/transmute/eager_transmute.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::{eq_expr_value, path_to_local, sym};
+use clippy_utils::{eq_expr_value, path_to_local_with_projections, sym};
 use rustc_abi::WrappingRange;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, Node};
@@ -63,11 +63,7 @@ fn binops_with_local(cx: &LateContext<'_>, local_expr: &Expr<'_>, expr: &Expr<'_
 /// Checks if an expression is a path to a local variable (with optional projections), e.g.
 /// `x.field[0].field2` would return true.
 fn is_local_with_projections(expr: &Expr<'_>) -> bool {
-    match expr.kind {
-        ExprKind::Path(_) => path_to_local(expr).is_some(),
-        ExprKind::Field(expr, _) | ExprKind::Index(expr, ..) => is_local_with_projections(expr),
-        _ => false,
-    }
+    path_to_local_with_projections(expr).is_some()
 }
 
 pub(super) fn check<'tcx>(

clippy_utils/src/lib.rs

@@ -460,6 +460,23 @@ pub fn path_to_local_id(expr: &Expr<'_>, id: HirId) -> bool {
     path_to_local(expr) == Some(id)
 }
 
+/// If the expression is a path to a local (with optional projections),
+/// returns the canonical `HirId` of the local.
+///
+/// For example, `x.field[0].field2` would return the `HirId` of `x`.
+pub fn path_to_local_with_projections(expr: &Expr<'_>) -> Option<HirId> {
+    match expr.kind {
+        ExprKind::Field(recv, _) | ExprKind::Index(recv, _, _) => path_to_local_with_projections(recv),
+        ExprKind::Path(QPath::Resolved(
+            _,
+            Path {
+                res: Res::Local(local), ..
+            },
+        )) => Some(*local),
+        _ => None,
+    }
+}
+
 pub trait MaybePath<'hir> {
     fn hir_id(&self) -> HirId;
     fn qpath_opt(&self) -> Option<&QPath<'hir>>;

tests/ui/double_ended_iterator_last.fixed

@@ -81,6 +81,11 @@ fn issue_14139() {
     let (subindex, _) = (index.by_ref().take(3), 42);
     let _ = subindex.last();
     let _ = index.next();
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let subindex = (index.by_ref().take(3), 42);
+    let _ = subindex.0.last();
+    let _ = index.next();
 }
 
 fn drop_order() {
@@ -108,6 +113,12 @@ fn drop_order() {
     let mut v = DropDeIterator(v.into_iter());
     println!("Last element is {}", v.next_back().unwrap().0);
     //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
+    let v = vec![S("four"), S("five"), S("six")];
+    let mut v = (DropDeIterator(v.into_iter()), 42);
+    println!("Last element is {}", v.0.next_back().unwrap().0);
+    //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
     println!("Done");
 }
 

tests/ui/double_ended_iterator_last.rs

@@ -81,6 +81,11 @@ fn issue_14139() {
     let (subindex, _) = (index.by_ref().take(3), 42);
     let _ = subindex.last();
     let _ = index.next();
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let subindex = (index.by_ref().take(3), 42);
+    let _ = subindex.0.last();
+    let _ = index.next();
 }
 
 fn drop_order() {
@@ -108,6 +113,12 @@ fn drop_order() {
     let v = DropDeIterator(v.into_iter());
     println!("Last element is {}", v.last().unwrap().0);
     //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
+    let v = vec![S("four"), S("five"), S("six")];
+    let v = (DropDeIterator(v.into_iter()), 42);
+    println!("Last element is {}", v.0.last().unwrap().0);
+    //~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
     println!("Done");
 }
 

tests/ui/double_ended_iterator_last.stderr

@@ -18,7 +18,7 @@ LL |     let _ = DeIterator.last();
    |                        help: try: `next_back()`
 
 error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
-  --> tests/ui/double_ended_iterator_last.rs:109:36
+  --> tests/ui/double_ended_iterator_last.rs:114:36
    |
 LL |     println!("Last element is {}", v.last().unwrap().0);
    |                                    ^^^^^^^^
@@ -30,5 +30,18 @@ LL ~     let mut v = DropDeIterator(v.into_iter());
 LL ~     println!("Last element is {}", v.next_back().unwrap().0);
    |
 
-error: aborting due to 3 previous errors
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:119:36
+   |
+LL |     println!("Last element is {}", v.0.last().unwrap().0);
+   |                                    ^^^^^^^^^^
+   |
+   = note: this change will alter drop order which may be undesirable
+help: try
+   |
+LL ~     let mut v = (DropDeIterator(v.into_iter()), 42);
+LL ~     println!("Last element is {}", v.0.next_back().unwrap().0);
+   |
+
+error: aborting due to 4 previous errors