diff --git a/clippy_lints/src/mut_mut.rs b/clippy_lints/src/mut_mut.rs index 588afd85afb0..a4750a271ff7 100644 --- a/clippy_lints/src/mut_mut.rs +++ b/clippy_lints/src/mut_mut.rs @@ -1,13 +1,11 @@ -use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; -use clippy_utils::higher; -use clippy_utils::source::snippet_with_applicability; -use clippy_utils::sugg::Sugg; -use rustc_data_structures::fx::FxHashSet; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::walk_span_to_context; use rustc_errors::Applicability; -use rustc_hir::{self as hir, AmbigArg, BorrowKind, Expr, ExprKind, HirId, Mutability, TyKind, intravisit}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_hir::{self as hir, AmbigArg, BorrowKind, Expr, ExprKind, HirId, Mutability, TyKind}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::impl_lint_pass; +use rustc_span::ExpnKind; declare_clippy_lint! { /// ### What it does @@ -44,136 +42,99 @@ impl_lint_pass!(MutMut => [MUT_MUT]); #[derive(Default)] pub(crate) struct MutMut { - seen_tys: FxHashSet, + skip_id: Option, } impl<'tcx> LateLintPass<'tcx> for MutMut { - fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) { - intravisit::walk_block(&mut MutVisitor { cx }, block); - } - - fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx hir::Ty<'_, AmbigArg>) { - if let TyKind::Ref(_, mty) = ty.kind - && mty.mutbl == Mutability::Mut - && let TyKind::Ref(_, mty2) = mty.ty.kind - && mty2.mutbl == Mutability::Mut - && !ty.span.in_external_macro(cx.sess().source_map()) + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, base) = e.kind + && let ctxt = e.span.ctxt() + && ctxt == base.span.ctxt() { - if self.seen_tys.contains(&ty.hir_id) { - // we have 2+ `&mut`s, e.g., `&mut &mut &mut x` - // and we have already flagged on the outermost `&mut &mut (&mut x)`, - // so don't flag the inner `&mut &mut (x)` + if self.skip_id.replace(base.hir_id) == Some(e.hir_id) { return; } - // if there is an even longer chain, like `&mut &mut &mut x`, suggest peeling off - // all extra ones at once - let (mut t, mut t2) = (mty.ty, mty2.ty); - let mut many_muts = false; - loop { - // this should allow us to remember all the nested types, so that the `contains` - // above fails faster - self.seen_tys.insert(t.hir_id); - if let TyKind::Ref(_, next) = t2.kind - && next.mutbl == Mutability::Mut + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, mut base2) = base.kind { + while let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, next) = base2.kind + && ctxt == base2.span.ctxt() { - (t, t2) = (t2, next.ty); - many_muts = true; - } else { - break; + base2 = next; } + if !ctxt.in_external_macro(cx.tcx.sess.source_map()) + && let Some(sp) = walk_span_to_context(base2.span, ctxt) + { + span_lint_and_then( + cx, + MUT_MUT, + e.span.until(sp), + "multiple successive mutable borrows", + |diag| { + diag.span_suggestion_verbose( + base.span.until(sp), + "make only a single borrow", + "", + Applicability::MaybeIncorrect, + ); + }, + ); + } + } else if let ty::Ref(_, ty, Mutability::Mut) = *cx.typeck_results().expr_ty(base).kind() + && ty.peel_refs().is_sized(cx.tcx, cx.typing_env()) + && !ctxt.in_external_macro(cx.tcx.sess.source_map()) + // Don't lint on the explicit borrow in for-loop desugarings. + && !matches!(ctxt.outer_expn_data().kind, ExpnKind::Desugaring(_)) + && let Some(sp) = walk_span_to_context(base.span, ctxt) + { + span_lint_and_then(cx, MUT_MUT, e.span.until(sp), "borrow of a mutable reference", |diag| { + diag.span_suggestion_verbose( + sp.shrink_to_lo(), + "reborrow instead", + "*", + Applicability::MaybeIncorrect, + ); + }); } - - let mut applicability = Applicability::MaybeIncorrect; - let sugg = snippet_with_applicability(cx.sess(), t.span, "..", &mut applicability); - let suffix = if many_muts { "s" } else { "" }; - span_lint_and_sugg( - cx, - MUT_MUT, - ty.span, - "a type of form `&mut &mut _`", - format!("remove the extra `&mut`{suffix}"), - sugg.to_string(), - applicability, - ); } } -} -pub struct MutVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, -} - -impl<'tcx> intravisit::Visitor<'tcx> for MutVisitor<'_, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if expr.span.in_external_macro(self.cx.sess().source_map()) { - return; - } + fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx hir::Ty<'_, AmbigArg>) { + if let TyKind::Ref(_, base) = ty.kind + && base.mutbl.is_mut() + && let ctxt = ty.span.ctxt() + && ctxt == base.ty.span.ctxt() + { + if self.skip_id.replace(base.ty.hir_id) == Some(ty.hir_id) { + return; + } - if let Some(higher::ForLoop { arg, body, .. }) = higher::ForLoop::hir(expr) { - // A `for` loop lowers to: - // ```rust - // match ::std::iter::Iterator::next(&mut iter) { - // // ^^^^ - // ``` - // Let's ignore the generated code. - intravisit::walk_expr(self, arg); - intravisit::walk_expr(self, body); - } else if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, e) = expr.kind { - if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, e2) = e.kind { - if !expr.span.eq_ctxt(e.span) { - return; + if let TyKind::Ref(_, mut base2) = base.ty.kind + && base2.mutbl.is_mut() + { + while let TyKind::Ref(_, next) = base2.ty.kind + && next.mutbl.is_mut() + && ctxt == base2.ty.span.ctxt() + { + base2 = next; } - - // if there is an even longer chain, like `&mut &mut &mut x`, suggest peeling off - // all extra ones at once - let (mut e, mut e2) = (e, e2); - let mut many_muts = false; - loop { - if !e.span.eq_ctxt(e2.span) { - return; - } - if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, next) = e2.kind { - (e, e2) = (e2, next); - many_muts = true; - } else { - break; - } + if !ctxt.in_external_macro(cx.tcx.sess.source_map()) + && let Some(sp) = walk_span_to_context(base2.ty.span, ctxt) + { + span_lint_and_then( + cx, + MUT_MUT, + ty.span.until(sp), + "multiple successive mutable references", + |diag| { + diag.span_suggestion_verbose( + base.ty.span.until(sp), + "use only a single mutable reference", + "", + Applicability::MaybeIncorrect, + ); + }, + ); } - - let mut applicability = Applicability::MaybeIncorrect; - let sugg = Sugg::hir_with_applicability(self.cx, e, "..", &mut applicability); - let suffix = if many_muts { "s" } else { "" }; - span_lint_hir_and_then( - self.cx, - MUT_MUT, - expr.hir_id, - expr.span, - "an expression of form `&mut &mut _`", - |diag| { - diag.span_suggestion( - expr.span, - format!("remove the extra `&mut`{suffix}"), - sugg, - applicability, - ); - }, - ); - } else if let ty::Ref(_, ty, Mutability::Mut) = self.cx.typeck_results().expr_ty(e).kind() - && ty.peel_refs().is_sized(self.cx.tcx, self.cx.typing_env()) - { - let mut applicability = Applicability::MaybeIncorrect; - let sugg = Sugg::hir_with_applicability(self.cx, e, "..", &mut applicability).mut_addr_deref(); - span_lint_hir_and_then( - self.cx, - MUT_MUT, - expr.hir_id, - expr.span, - "this expression mutably borrows a mutable reference", - |diag| { - diag.span_suggestion(expr.span, "reborrow instead", sugg, applicability); - }, - ); } } } diff --git a/tests/ui/mut_mut.stderr b/tests/ui/mut_mut.stderr index 0a7a923e0f48..a7b721539f80 100644 --- a/tests/ui/mut_mut.stderr +++ b/tests/ui/mut_mut.stderr @@ -1,47 +1,87 @@ -error: a type of form `&mut &mut _` +error: multiple successive mutable references --> tests/ui/mut_mut.rs:8:11 | LL | fn fun(x: &mut &mut u32) { - | ^^^^^^^^^^^^^ help: remove the extra `&mut`: `&mut u32` + | ^^^^^^^^^^ | = note: `-D clippy::mut-mut` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mut_mut)]` +help: use only a single mutable reference + | +LL - fn fun(x: &mut &mut u32) { +LL + fn fun(x: &mut u32) { + | -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut.rs:24:17 | LL | let mut x = &mut &mut 1u32; - | ^^^^^^^^^^^^^^ help: remove the extra `&mut`: `&mut 1u32` + | ^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let mut x = &mut &mut 1u32; +LL + let mut x = &mut 1u32; + | -error: this expression mutably borrows a mutable reference +error: borrow of a mutable reference --> tests/ui/mut_mut.rs:27:21 | LL | let mut y = &mut x; - | ^^^^^^ help: reborrow instead: `&mut *x` + | ^^^^^ + | +help: reborrow instead + | +LL | let mut y = &mut *x; + | + -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut.rs:32:32 | LL | let y: &mut &mut u32 = &mut &mut 2; - | ^^^^^^^^^^^ help: remove the extra `&mut`: `&mut 2` + | ^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let y: &mut &mut u32 = &mut &mut 2; +LL + let y: &mut &mut u32 = &mut 2; + | -error: a type of form `&mut &mut _` +error: multiple successive mutable references --> tests/ui/mut_mut.rs:32:16 | LL | let y: &mut &mut u32 = &mut &mut 2; - | ^^^^^^^^^^^^^ help: remove the extra `&mut`: `&mut u32` + | ^^^^^^^^^^ + | +help: use only a single mutable reference + | +LL - let y: &mut &mut u32 = &mut &mut 2; +LL + let y: &mut u32 = &mut &mut 2; + | -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut.rs:38:37 | LL | let y: &mut &mut &mut u32 = &mut &mut &mut 2; - | ^^^^^^^^^^^^^^^^ help: remove the extra `&mut`s: `&mut 2` + | ^^^^^^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let y: &mut &mut &mut u32 = &mut &mut &mut 2; +LL + let y: &mut &mut &mut u32 = &mut 2; + | -error: a type of form `&mut &mut _` +error: multiple successive mutable references --> tests/ui/mut_mut.rs:38:16 | LL | let y: &mut &mut &mut u32 = &mut &mut &mut 2; - | ^^^^^^^^^^^^^^^^^^ help: remove the extra `&mut`s: `&mut u32` + | ^^^^^^^^^^^^^^^ + | +help: use only a single mutable reference + | +LL - let y: &mut &mut &mut u32 = &mut &mut &mut 2; +LL + let y: &mut u32 = &mut &mut &mut 2; + | error: aborting due to 7 previous errors diff --git a/tests/ui/mut_mut_unfixable.stderr b/tests/ui/mut_mut_unfixable.stderr index 7e7fb801ce1e..07cc965a8a17 100644 --- a/tests/ui/mut_mut_unfixable.stderr +++ b/tests/ui/mut_mut_unfixable.stderr @@ -1,41 +1,75 @@ -error: a type of form `&mut &mut _` +error: multiple successive mutable references --> tests/ui/mut_mut_unfixable.rs:8:11 | LL | fn fun(x: &mut &mut u32) -> bool { - | ^^^^^^^^^^^^^ help: remove the extra `&mut`: `&mut u32` + | ^^^^^^^^^^ | = note: `-D clippy::mut-mut` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mut_mut)]` +help: use only a single mutable reference + | +LL - fn fun(x: &mut &mut u32) -> bool { +LL + fn fun(x: &mut u32) -> bool { + | -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut_unfixable.rs:14:17 | LL | let mut x = &mut &mut 1u32; - | ^^^^^^^^^^^^^^ help: remove the extra `&mut`: `&mut 1u32` + | ^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let mut x = &mut &mut 1u32; +LL + let mut x = &mut 1u32; + | -error: this expression mutably borrows a mutable reference +error: borrow of a mutable reference --> tests/ui/mut_mut_unfixable.rs:17:21 | LL | let mut y = &mut x; - | ^^^^^^ help: reborrow instead: `&mut *x` + | ^^^^^ + | +help: reborrow instead + | +LL | let mut y = &mut *x; + | + -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut_unfixable.rs:23:17 | LL | let y = &mut &mut 2; - | ^^^^^^^^^^^ help: remove the extra `&mut`: `&mut 2` + | ^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let y = &mut &mut 2; +LL + let y = &mut 2; + | -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut_unfixable.rs:29:17 | LL | let y = &mut &mut &mut 2; - | ^^^^^^^^^^^^^^^^ help: remove the extra `&mut`s: `&mut 2` + | ^^^^^^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let y = &mut &mut &mut 2; +LL + let y = &mut 2; + | -error: an expression of form `&mut &mut _` +error: multiple successive mutable borrows --> tests/ui/mut_mut_unfixable.rs:38:17 | LL | let y = &mut &mut x; - | ^^^^^^^^^^^ help: remove the extra `&mut`: `&mut x` + | ^^^^^^^^^^ + | +help: make only a single borrow + | +LL - let y = &mut &mut x; +LL + let y = &mut x; + | error: aborting due to 6 previous errors