Make expr_use_ctxt always return Some unless the syntax context c…

…hanges.

clippy_utils/src/lib.rs

@@ -1,5 +1,6 @@
 #![feature(array_chunks)]
 #![feature(box_patterns)]
+#![feature(control_flow_enum)]
 #![feature(if_let_guard)]
 #![feature(let_chains)]
 #![feature(lint_reasons)]
@@ -2508,26 +2509,30 @@ pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool {
  && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests")
 }
 
-/// Walks the HIR tree from the given expression, up to the node where the value produced by the
-/// expression is consumed. Calls the function for every node encountered this way until it returns
-/// `Some`.
+/// Walks up the HIR tree from the given expression in an attempt to find where the value is
+/// consumed.
 ///
-/// This allows walking through `if`, `match`, `break`, block expressions to find where the value
-/// produced by the expression is consumed.
+/// Termination has three conditions:
+/// - The given function returns `Break`. This function will return the value.
+/// - The consuming node is found. This function will return `Continue(use_node, child_id)`.
+/// - No further parent nodes are found. This will trigger a debug assert or return `None`.
+///
+/// This allows walking through `if`, `match`, `break`, and block expressions to find where the
+/// value produced by the expression is consumed.
 pub fn walk_to_expr_usage<'tcx, T>(
  cx: &LateContext<'tcx>,
  e: &Expr<'tcx>,
- mut f: impl FnMut(Node<'tcx>, HirId) -> Option<T>,
-) -> Option<T> {
+ mut f: impl FnMut(HirId, Node<'tcx>, HirId) -> ControlFlow<T>,
+) -> Option<ControlFlow<T, (Node<'tcx>, HirId)>> {
  let map = cx.tcx.hir();
  let mut iter = map.parent_iter(e.hir_id);
  let mut child_id = e.hir_id;
 
  while let Some((parent_id, parent)) = iter.next() {
- if let Some(x) = f(parent, child_id) {
- return Some(x);
+ if let ControlFlow::Break(x) = f(parent_id, parent, child_id) {
+ return Some(ControlFlow::Break(x));
  }
- let parent = match parent {
+ let parent_expr = match parent {
  Node::Expr(e) => e,
  Node::Block(Block { expr: Some(body), .. }) | Node::Arm(Arm { body, .. }) if body.hir_id == child_id => {
  child_id = parent_id;
@@ -2537,18 +2542,19 @@ pub fn walk_to_expr_usage<'tcx, T>(
  child_id = parent_id;
  continue;
  },
- _ => return None,
+ _ => return Some(ControlFlow::Continue((parent, child_id))),
  };
- match parent.kind {
+ match parent_expr.kind {
  ExprKind::If(child, ..) | ExprKind::Match(child, ..) if child.hir_id != child_id => child_id = parent_id,
  ExprKind::Break(Destination { target_id: Ok(id), .. }, _) => {
  child_id = id;
  iter = map.parent_iter(id);
  },
- ExprKind::Block(..) => child_id = parent_id,
- _ => return None,
+ ExprKind::Block(..) | ExprKind::DropTemps(_) => child_id = parent_id,
+ _ => return Some(ControlFlow::Continue((parent, child_id))),
  }
  }
+ debug_assert!(false, "no parent node found for `{child_id:?}`");
  None
 }
 
@@ -2592,6 +2598,8 @@ pub enum ExprUseNode<'tcx> {
  Callee,
  /// Access of a field.
  FieldAccess(Ident),
+ Expr,
+ Other,
 }
 impl<'tcx> ExprUseNode<'tcx> {
  /// Checks if the value is returned from the function.
@@ -2668,144 +2676,104 @@ impl<'tcx> ExprUseNode<'tcx> {
  let sig = cx.tcx.fn_sig(id).skip_binder();
  Some(DefinedTy::Mir(cx.tcx.param_env(id).and(sig.input(i))))
  },
- Self::Local(_) | Self::FieldAccess(..) | Self::Callee => None,
+ Self::Local(_) | Self::FieldAccess(..) | Self::Callee | Self::Expr | Self::Other => None,
  }
  }
 }
 
 /// Gets the context an expression's value is used in.
-#[expect(clippy::too_many_lines)]
 pub fn expr_use_ctxt<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<ExprUseCtxt<'tcx>> {
  let mut adjustments = [].as_slice();
  let mut is_ty_unified = false;
  let mut moved_before_use = false;
  let ctxt = e.span.ctxt();
- walk_to_expr_usage(cx, e, &mut |parent, child_id| {
- // LocalTableInContext returns the wrong lifetime, so go use `expr_adjustments` instead.
+ walk_to_expr_usage(cx, e, &mut |parent_id, parent, child_id| {
  if adjustments.is_empty()
  && let Node::Expr(e) = cx.tcx.hir().get(child_id)
  {
  adjustments = cx.typeck_results().expr_adjustments(e);
  }
- match parent {
- Node::Local(l) if l.span.ctxt() == ctxt => Some(ExprUseCtxt {
- node: ExprUseNode::Local(l),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
+ if !cx.tcx.hir().opt_span(parent_id).is_some_and(|x| x.ctxt() == ctxt) {
+ return ControlFlow::Break(());
+ }
+ if let Node::Expr(e) = parent {
+ match e.kind {
+ ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
+ is_ty_unified = true;
+ moved_before_use = true;
+ },
+ ExprKind::Block(_, Some(_)) | ExprKind::Break(..) => {
+ is_ty_unified = true;
+ moved_before_use = true;
+ },
+ ExprKind::Block(..) => moved_before_use = true,
+ _ => {},
+ }
+ }
+ ControlFlow::Continue(())
+ })?
+ .continue_value()
+ .map(|(use_node, child_id)| {
+ let node = match use_node {
+ Node::Local(l) => ExprUseNode::Local(l),
+ Node::ExprField(field) => ExprUseNode::Field(field),
+
  Node::Item(&Item {
  kind: ItemKind::Static(..) | ItemKind::Const(..),
  owner_id,
- span,
  ..
  })
  | Node::TraitItem(&TraitItem {
  kind: TraitItemKind::Const(..),
  owner_id,
- span,
  ..
  })
  | Node::ImplItem(&ImplItem {
  kind: ImplItemKind::Const(..),
  owner_id,
- span,
  ..
- }) if span.ctxt() == ctxt => Some(ExprUseCtxt {
- node: ExprUseNode::ConstStatic(owner_id),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
+ }) => ExprUseNode::ConstStatic(owner_id),
 
  Node::Item(&Item {
  kind: ItemKind::Fn(..),
  owner_id,
- span,
  ..
  })
  | Node::TraitItem(&TraitItem {
  kind: TraitItemKind::Fn(..),
  owner_id,
- span,
  ..
  })
  | Node::ImplItem(&ImplItem {
  kind: ImplItemKind::Fn(..),
  owner_id,
- span,
  ..
- }) if span.ctxt() == ctxt => Some(ExprUseCtxt {
- node: ExprUseNode::Return(owner_id),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
-
- Node::ExprField(field) if field.span.ctxt() == ctxt => Some(ExprUseCtxt {
- node: ExprUseNode::Field(field),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
+ }) => ExprUseNode::Return(owner_id),
 
- Node::Expr(parent) if parent.span.ctxt() == ctxt => match parent.kind {
- ExprKind::Ret(_) => Some(ExprUseCtxt {
- node: ExprUseNode::Return(OwnerId {
- def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
- }),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
- ExprKind::Closure(closure) => Some(ExprUseCtxt {
- node: ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
- ExprKind::Call(func, args) => Some(ExprUseCtxt {
- node: match args.iter().position(|arg| arg.hir_id == child_id) {
- Some(i) => ExprUseNode::FnArg(func, i),
- None => ExprUseNode::Callee,
- },
- adjustments,
- is_ty_unified,
- moved_before_use,
+ Node::Expr(use_expr) => match use_expr.kind {
+ ExprKind::Ret(_) => ExprUseNode::Return(OwnerId {
+ def_id: cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()),
  }),
- ExprKind::MethodCall(name, _, args, _) => Some(ExprUseCtxt {
- node: ExprUseNode::MethodArg(
- parent.hir_id,
- name.args,
- args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
- ),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
- ExprKind::Field(child, name) if child.hir_id == e.hir_id => Some(ExprUseCtxt {
- node: ExprUseNode::FieldAccess(name),
- adjustments,
- is_ty_unified,
- moved_before_use,
- }),
- ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id != child_id => {
- is_ty_unified = true;
- moved_before_use = true;
- None
- },
- ExprKind::Block(_, Some(_)) | ExprKind::Break(..) => {
- is_ty_unified = true;
- moved_before_use = true;
- None
+ ExprKind::Closure(closure) => ExprUseNode::Return(OwnerId { def_id: closure.def_id }),
+ ExprKind::Call(func, args) => match args.iter().position(|arg| arg.hir_id == child_id) {
+ Some(i) => ExprUseNode::FnArg(func, i),
+ None => ExprUseNode::Callee,
  },
- ExprKind::Block(..) => {
- moved_before_use = true;
- None
- },
- _ => None,
+ ExprKind::MethodCall(name, _, args, _) => ExprUseNode::MethodArg(
+ use_expr.hir_id,
+ name.args,
+ args.iter().position(|arg| arg.hir_id == child_id).map_or(0, |i| i + 1),
+ ),
+ ExprKind::Field(child, name) if child.hir_id == e.hir_id => ExprUseNode::FieldAccess(name),
+ _ => ExprUseNode::Expr,
  },
- _ => None,
+ _ => ExprUseNode::Other,
+ };
+ ExprUseCtxt {
+ node,
+ adjustments,
+ is_ty_unified,
+ moved_before_use,
  }
  })
 }

tests/ui/ignored_unit_patterns.fixed

@@ -1,6 +1,11 @@
 //@aux-build:proc_macro_derive.rs
 #![warn(clippy::ignored_unit_patterns)]
-#![allow(clippy::let_unit_value, clippy::redundant_pattern_matching, clippy::single_match)]
+#![allow(
+ clippy::let_unit_value,
+ clippy::redundant_pattern_matching,
+ clippy::single_match,
+ clippy::needless_borrow
+)]
 
 fn foo() -> Result<(), ()> {
  unimplemented!()

tests/ui/ignored_unit_patterns.rs

@@ -1,6 +1,11 @@
 //@aux-build:proc_macro_derive.rs
 #![warn(clippy::ignored_unit_patterns)]
-#![allow(clippy::let_unit_value, clippy::redundant_pattern_matching, clippy::single_match)]
+#![allow(
+ clippy::let_unit_value,
+ clippy::redundant_pattern_matching,
+ clippy::single_match,
+ clippy::needless_borrow
+)]
 
 fn foo() -> Result<(), ()> {
  unimplemented!()

tests/ui/ignored_unit_patterns.stderr

@@ -1,5 +1,5 @@
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:11:12
+ --> $DIR/ignored_unit_patterns.rs:16:12
  |
 LL | Ok(_) => {},
  | ^ help: use `()` instead of `_`: `()`
@@ -8,49 +8,49 @@ LL | Ok(_) => {},
  = help: to override `-D warnings` add `#[allow(clippy::ignored_unit_patterns)]`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:12:13
+ --> $DIR/ignored_unit_patterns.rs:17:13
  |
 LL | Err(_) => {},
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:14:15
+ --> $DIR/ignored_unit_patterns.rs:19:15
  |
 LL | if let Ok(_) = foo() {}
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:16:28
+ --> $DIR/ignored_unit_patterns.rs:21:28
  |
 LL | let _ = foo().map_err(|_| todo!());
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:22:16
+ --> $DIR/ignored_unit_patterns.rs:27:16
  |
 LL | Ok(_) => {},
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:24:17
+ --> $DIR/ignored_unit_patterns.rs:29:17
  |
 LL | Err(_) => {},
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:36:9
+ --> $DIR/ignored_unit_patterns.rs:41:9
  |
 LL | let _ = foo().unwrap();
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:45:13
+ --> $DIR/ignored_unit_patterns.rs:50:13
  |
 LL | (1, _) => unimplemented!(),
  | ^ help: use `()` instead of `_`: `()`
 
 error: matching over `()` is more explicit
- --> $DIR/ignored_unit_patterns.rs:52:13
+ --> $DIR/ignored_unit_patterns.rs:57:13
  |
 LL | for (x, _) in v {
  | ^ help: use `()` instead of `_`: `()`

tests/ui/needless_borrow.fixed

@@ -131,6 +131,9 @@ fn main() {
  0
  }
  }
+
+ // issue #11786
+ let x: (&str,) = ("",);
 }
 
 #[allow(clippy::needless_borrowed_reference)]

tests/ui/needless_borrow.rs

@@ -131,6 +131,9 @@ fn main() {
  0
  }
  }
+
+ // issue #11786
+ let x: (&str,) = (&"",);
 }
 
 #[allow(clippy::needless_borrowed_reference)]