Auto merge of #12822 - Alexendoo:for-each-expr, r=dswij

Make `for_each_expr` visit closures by default, rename the old version `for_each_expr_without_closures`

A lot of the time `for_each_expr` is picked when closures should be visited so I think it makes sense for this to be the default with the alternative available for when you don't need to visit them.

The first commit renames `for_each_expr` to `for_each_expr_without_closures` and `for_each_expr_with_closures` to `for_each_expr`

The second commit switches a few uses that I caught over to include closures to fix a few bugs

changelog: none

Author: bors

clippy_lints/src/blocks_in_conditions.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
 use clippy_utils::source::snippet_block_with_applicability;
 use clippy_utils::ty::implements_trait;
-use clippy_utils::visitors::{for_each_expr, Descend};
+use clippy_utils::visitors::{for_each_expr_without_closures, Descend};
 use clippy_utils::{get_parent_expr, higher, is_from_proc_macro};
 use core::ops::ControlFlow;
 use rustc_errors::Applicability;
@@ -125,7 +125,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInConditions {
                 }
             }
         } else {
-            let _: Option<!> = for_each_expr(cond, |e| {
+            let _: Option<!> = for_each_expr_without_closures(cond, |e| {
                 if let ExprKind::Closure(closure) = e.kind {
                     // do not lint if the closure is called using an iterator (see #1141)
                     if let Some(parent) = get_parent_expr(cx, e)

clippy_lints/src/casts/cast_sign_loss.rs

@@ -3,7 +3,7 @@ use std::ops::ControlFlow;
 
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::visitors::{for_each_expr, Descend};
+use clippy_utils::visitors::{for_each_expr_without_closures, Descend};
 use clippy_utils::{method_chain_args, sext};
 use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::LateContext;
@@ -266,7 +266,7 @@ fn expr_add_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign {
 fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> {
     let mut res = vec![];
 
-    for_each_expr(expr, |sub_expr| -> ControlFlow<Infallible, Descend> {
+    for_each_expr_without_closures(expr, |sub_expr| -> ControlFlow<Infallible, Descend> {
         // We don't check for mul/div/rem methods here, but we could.
         if let ExprKind::Binary(op, lhs, _rhs) = sub_expr.kind {
             if matches!(op.node, BinOpKind::Mul | BinOpKind::Div) {
@@ -315,7 +315,7 @@ fn exprs_with_muldiv_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> {
 fn exprs_with_add_binop_peeled<'e>(expr: &'e Expr<'_>) -> Vec<&'e Expr<'e>> {
     let mut res = vec![];
 
-    for_each_expr(expr, |sub_expr| -> ControlFlow<Infallible, Descend> {
+    for_each_expr_without_closures(expr, |sub_expr| -> ControlFlow<Infallible, Descend> {
         // We don't check for add methods here, but we could.
         if let ExprKind::Binary(op, _lhs, _rhs) = sub_expr.kind {
             if matches!(op.node, BinOpKind::Add) {

clippy_lints/src/casts/unnecessary_cast.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::numeric_literal::NumericLiteral;
 use clippy_utils::source::snippet_opt;
-use clippy_utils::visitors::{for_each_expr, Visitable};
+use clippy_utils::visitors::{for_each_expr_without_closures, Visitable};
 use clippy_utils::{get_parent_expr, is_hir_ty_cfg_dependant, is_ty_alias, path_to_local};
 use rustc_ast::{LitFloatType, LitIntType, LitKind};
 use rustc_errors::Applicability;
@@ -245,7 +245,7 @@ fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 {
 /// TODO: Maybe we should move this to `clippy_utils` so others won't need to go down this dark,
 /// dark path reimplementing this (or something similar).
 fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx>, cast_from: Ty<'tcx>) -> bool {
-    for_each_expr(expr, |expr| {
+    for_each_expr_without_closures(expr, |expr| {
         // Calls are a `Path`, and usage of locals are a `Path`. So, this checks
         // - call() as i32
         // - local as i32

clippy_lints/src/cognitive_complexity.rs

@@ -3,7 +3,7 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::visitors::for_each_expr;
+use clippy_utils::visitors::for_each_expr_without_closures;
 use clippy_utils::{get_async_fn_body, is_async_fn, LimitStack};
 use core::ops::ControlFlow;
 use rustc_ast::ast::Attribute;
@@ -65,7 +65,7 @@ impl CognitiveComplexity {
 
         let mut cc = 1u64;
         let mut returns = 0u64;
-        let _: Option<!> = for_each_expr(expr, |e| {
+        let _: Option<!> = for_each_expr_without_closures(expr, |e| {
             match e.kind {
                 ExprKind::If(_, _, _) => {
                     cc += 1;

clippy_lints/src/collection_is_never_read.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
-use clippy_utils::visitors::{for_each_expr_with_closures, Visitable};
+use clippy_utils::visitors::{for_each_expr, Visitable};
 use clippy_utils::{get_enclosing_block, path_to_local_id};
 use core::ops::ControlFlow;
 use rustc_hir::{Body, ExprKind, HirId, LangItem, LetStmt, Node, PatKind};
@@ -82,7 +82,7 @@ fn has_no_read_access<'tcx, T: Visitable<'tcx>>(cx: &LateContext<'tcx>, id: HirI
     let mut has_read_access = false;
 
     // Inspect all expressions and sub-expressions in the block.
-    for_each_expr_with_closures(cx, block, |expr| {
+    for_each_expr(cx, block, |expr| {
         // Ignore expressions that are not simply `id`.
         if !path_to_local_id(expr, id) {
             return ControlFlow::Continue(());

clippy_lints/src/copies.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
 use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
 use clippy_utils::ty::{needs_ordered_drop, InteriorMut};
-use clippy_utils::visitors::for_each_expr;
+use clippy_utils::visitors::for_each_expr_without_closures;
 use clippy_utils::{
     capture_local_usage, eq_expr_value, find_binding_init, get_enclosing_block, hash_expr, hash_stmt, if_sequence,
     is_else_clause, is_lint_allowed, path_to_local, search_same, ContainsName, HirEqInterExpr, SpanlessEq,
@@ -362,7 +362,7 @@ fn eq_binding_names(s: &Stmt<'_>, names: &[(HirId, Symbol)]) -> bool {
 
 /// Checks if the statement modifies or moves any of the given locals.
 fn modifies_any_local<'tcx>(cx: &LateContext<'tcx>, s: &'tcx Stmt<'_>, locals: &HirIdSet) -> bool {
-    for_each_expr(s, |e| {
+    for_each_expr_without_closures(s, |e| {
         if let Some(id) = path_to_local(e)
             && locals.contains(&id)
             && !capture_local_usage(cx, e).is_imm_ref()
@@ -413,7 +413,7 @@ fn scan_block_for_eq<'tcx>(
 
     let mut cond_locals = HirIdSet::default();
     for &cond in conds {
-        let _: Option<!> = for_each_expr(cond, |e| {
+        let _: Option<!> = for_each_expr_without_closures(cond, |e| {
             if let Some(id) = path_to_local(e) {
                 cond_locals.insert(id);
             }

clippy_lints/src/functions/must_use.rs

@@ -14,7 +14,7 @@ use clippy_utils::attrs::is_proc_macro;
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::is_must_use_ty;
-use clippy_utils::visitors::for_each_expr;
+use clippy_utils::visitors::for_each_expr_without_closures;
 use clippy_utils::{return_ty, trait_ref_of_method};
 
 use core::ops::ControlFlow;
@@ -226,7 +226,7 @@ fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
 }
 
 fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
-    for_each_expr(body.value, |e| {
+    for_each_expr_without_closures(body.value, |e| {
         use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall};
 
         match e.kind {

clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs

@@ -5,7 +5,7 @@ use rustc_span::def_id::LocalDefId;
 
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::ty::type_is_unsafe_function;
-use clippy_utils::visitors::for_each_expr_with_closures;
+use clippy_utils::visitors::for_each_expr;
 use clippy_utils::{iter_input_pats, path_to_local};
 
 use core::ops::ControlFlow;
@@ -49,7 +49,7 @@ fn check_raw_ptr<'tcx>(
 
         if !raw_ptrs.is_empty() {
             let typeck = cx.tcx.typeck_body(body.id());
-            let _: Option<!> = for_each_expr_with_closures(cx, body.value, |e| {
+            let _: Option<!> = for_each_expr(cx, body.value, |e| {
                 match e.kind {
                     hir::ExprKind::Call(f, args) if type_is_unsafe_function(cx, typeck.expr_ty(f)) => {
                         for arg in args {

clippy_lints/src/implicit_return.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::source::{snippet_with_applicability, snippet_with_context, walk_span_to_context};
-use clippy_utils::visitors::for_each_expr;
+use clippy_utils::visitors::for_each_expr_without_closures;
 use clippy_utils::{get_async_fn_body, is_async_fn};
 use core::ops::ControlFlow;
 use rustc_errors::Applicability;
@@ -153,7 +153,7 @@ fn lint_implicit_returns(
 
         ExprKind::Loop(block, ..) => {
             let mut add_return = false;
-            let _: Option<!> = for_each_expr(block, |e| {
+            let _: Option<!> = for_each_expr_without_closures(block, |e| {
                 if let ExprKind::Break(dest, sub_expr) = e.kind {
                     if dest.target_id.ok() == Some(expr.hir_id) {
                         if call_site_span.is_none() && e.span.ctxt() == ctxt {

clippy_lints/src/matches/redundant_guards.rs

@@ -2,7 +2,7 @@ use clippy_config::msrvs::Msrv;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::macros::matching_root_macro_call;
 use clippy_utils::source::snippet;
-use clippy_utils::visitors::{for_each_expr, is_local_used};
+use clippy_utils::visitors::{for_each_expr_without_closures, is_local_used};
 use clippy_utils::{in_constant, path_to_local};
 use rustc_ast::{BorrowKind, LitKind};
 use rustc_errors::Applicability;
@@ -249,7 +249,7 @@ fn emit_redundant_guards<'tcx>(
 /// an error in the future, and rustc already actively warns against this (see rust#41620),
 /// so we don't consider those as usable within patterns for linting purposes.
 fn expr_can_be_pat(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    for_each_expr(expr, |expr| {
+    for_each_expr_without_closures(expr, |expr| {
         if match expr.kind {
             ExprKind::ConstBlock(..) => cx.tcx.features().inline_const_pat,
             ExprKind::Call(c, ..) if let ExprKind::Path(qpath) = c.kind => {