Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 82 additions & 121 deletions clippy_lints/src/mut_mut.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -44,136 +42,99 @@ impl_lint_pass!(MutMut => [MUT_MUT]);

#[derive(Default)]
pub(crate) struct MutMut {
seen_tys: FxHashSet<HirId>,
skip_id: Option<HirId>,
}

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);
},
);
}
}
}
Expand Down
68 changes: 54 additions & 14 deletions tests/ui/mut_mut.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading