diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs index 7b00eabf97b4..36ca61c0b44d 100644 --- a/clippy_lints/src/needless_pass_by_ref_mut.rs +++ b/clippy_lints/src/needless_pass_by_ref_mut.rs @@ -1,6 +1,7 @@ use super::needless_pass_by_value::requires_exact_signature; use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::source::snippet; +use clippy_utils::visitors::for_each_expr_with_closures; use clippy_utils::{get_parent_node, inherits_cfg, is_from_proc_macro, is_self}; use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; use rustc_errors::Applicability; @@ -9,7 +10,7 @@ use rustc_hir::{ Body, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath, }; use rustc_hir_typeck::expr_use_visitor as euv; -use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::map::associated_body; use rustc_middle::hir::nested_filter::OnlyBodies; @@ -21,6 +22,8 @@ use rustc_span::symbol::kw; use rustc_span::Span; use rustc_target::spec::abi::Abi; +use core::ops::ControlFlow; + declare_clippy_lint! { /// ### What it does /// Check if a `&mut` function argument is actually used mutably. @@ -95,6 +98,30 @@ fn should_skip<'tcx>( is_from_proc_macro(cx, &input) } +fn check_closures<'tcx>( + ctx: &mut MutablyUsedVariablesCtxt<'tcx>, + cx: &LateContext<'tcx>, + infcx: &InferCtxt<'tcx>, + checked_closures: &mut FxHashSet, + closures: FxHashSet, +) { + let hir = cx.tcx.hir(); + for closure in closures { + if !checked_closures.insert(closure) { + continue; + } + ctx.prev_bind = None; + ctx.prev_move_to_closure.clear(); + if let Some(body) = hir + .find_by_def_id(closure) + .and_then(associated_body) + .map(|(_, body_id)| hir.body(body_id)) + { + euv::ExprUseVisitor::new(ctx, infcx, closure, cx.param_env, cx.typeck_results()).consume_body(body); + } + } +} + impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> { fn check_fn( &mut self, @@ -161,25 +188,22 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> { euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.typeck_results()).consume_body(body); if is_async { let mut checked_closures = FxHashSet::default(); - while !ctx.async_closures.is_empty() { - let closures = ctx.async_closures.clone(); - ctx.async_closures.clear(); - let hir = cx.tcx.hir(); - for closure in closures { - if !checked_closures.insert(closure) { - continue; - } - ctx.prev_bind = None; - ctx.prev_move_to_closure.clear(); - if let Some(body) = hir - .find_by_def_id(closure) - .and_then(associated_body) - .map(|(_, body_id)| hir.body(body_id)) - { - euv::ExprUseVisitor::new(&mut ctx, &infcx, closure, cx.param_env, cx.typeck_results()) - .consume_body(body); - } + + // We retrieve all the closures declared in the async function because they will + // not be found by `euv::Delegate`. + let mut closures: FxHashSet = FxHashSet::default(); + for_each_expr_with_closures(cx, body, |expr| { + if let ExprKind::Closure(closure) = expr.kind { + closures.insert(closure.def_id); } + ControlFlow::<()>::Continue(()) + }); + check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures); + + while !ctx.async_closures.is_empty() { + let async_closures = ctx.async_closures.clone(); + ctx.async_closures.clear(); + check_closures(&mut ctx, cx, &infcx, &mut checked_closures, async_closures); } } ctx @@ -244,6 +268,10 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> { struct MutablyUsedVariablesCtxt<'tcx> { mutably_used_vars: HirIdSet, prev_bind: Option, + /// In async functions, the inner AST is composed of multiple layers until we reach the code + /// defined by the user. Because of that, some variables are marked as mutably borrowed even + /// though they're not. This field lists the `HirId` that should not be considered as mutable + /// use of a variable. prev_move_to_closure: HirIdSet, aliases: HirIdMap, async_closures: FxHashSet, diff --git a/tests/ui/needless_pass_by_ref_mut.rs b/tests/ui/needless_pass_by_ref_mut.rs index e1e5e8fd220b..da2a72caacb4 100644 --- a/tests/ui/needless_pass_by_ref_mut.rs +++ b/tests/ui/needless_pass_by_ref_mut.rs @@ -1,4 +1,4 @@ -#![allow(clippy::if_same_then_else, clippy::no_effect)] +#![allow(clippy::if_same_then_else, clippy::no_effect, clippy::redundant_closure_call)] #![feature(lint_reasons)] //@no-rustfix use std::ptr::NonNull; @@ -231,6 +231,32 @@ async fn async_vec2(b: &mut Vec) { b.push(true); } +// Should not warn. +pub async fn closure(n: &mut usize) -> impl '_ + FnMut() { + || { + *n += 1; + } +} + +// Should warn. +pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize { + //~^ ERROR: this argument is a mutable reference, but not used mutably + || *n + 1 +} + +// Should not warn. +pub async fn closure3(n: &mut usize) { + (|| *n += 1)(); +} + +// Should warn. +pub async fn closure4(n: &mut usize) { + //~^ ERROR: this argument is a mutable reference, but not used mutably + (|| { + let _x = *n + 1; + })(); +} + fn main() { let mut u = 0; let mut v = vec![0]; diff --git a/tests/ui/needless_pass_by_ref_mut.stderr b/tests/ui/needless_pass_by_ref_mut.stderr index df3df045776b..0fb9458d7948 100644 --- a/tests/ui/needless_pass_by_ref_mut.stderr +++ b/tests/ui/needless_pass_by_ref_mut.stderr @@ -107,5 +107,29 @@ error: this argument is a mutable reference, but not used mutably LL | async fn inner_async3(x: &mut i32, y: &mut u32) { | ^^^^^^^^ help: consider changing to: `&i32` -error: aborting due to 17 previous errors +error: this argument is a mutable reference, but not used mutably + --> $DIR/needless_pass_by_ref_mut.rs:235:25 + | +LL | pub async fn closure(n: &mut usize) -> impl '_ + FnMut() { + | ^^^^^^^^^^ help: consider changing to: `&usize` + | + = warning: changing this function will impact semver compatibility + +error: this argument is a mutable reference, but not used mutably + --> $DIR/needless_pass_by_ref_mut.rs:242:20 + | +LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize { + | ^^^^^^^^^^ help: consider changing to: `&usize` + | + = warning: changing this function will impact semver compatibility + +error: this argument is a mutable reference, but not used mutably + --> $DIR/needless_pass_by_ref_mut.rs:253:26 + | +LL | pub async fn closure4(n: &mut usize) { + | ^^^^^^^^^^ help: consider changing to: `&usize` + | + = warning: changing this function will impact semver compatibility + +error: aborting due to 20 previous errors