Skip to content

feat(unnecessary_mut_passed): support &raw references #15962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+180 −44
Open

feat(unnecessary_mut_passed): support &raw references #15962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 62 additions & 20 deletions clippy_lints/src/unnecessary_mut_passed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,30 @@ fn check_arguments<'tcx>(
let parameters = type_definition.fn_sig(cx.tcx).skip_binder().inputs();
for (argument, parameter) in iter::zip(arguments, parameters) {
if let ty::Ref(_, _, Mutability::Not) | ty::RawPtr(_, Mutability::Not) = parameter.kind()
&& let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, arg) = argument.kind
&& let ExprKind::AddrOf(borrow_kind, Mutability::Mut, arg) = argument.kind
{
let applicability = Applicability::MachineApplicable;
emit(cx, name, fn_kind, argument, borrow_kind, arg);
}
}
}
}

fn emit(cx: &LateContext<'_>, name: &str, fn_kind: &str, argument: &Expr<'_>, borrow_kind: BorrowKind, arg: &Expr<'_>) {
let applicability = Applicability::MachineApplicable;

let span_to_remove = {
let span_until_arg = argument.span.until(arg.span);
if let Some(Some(ref_pos)) = span_until_arg.with_source_text(cx, |src| {
span_lint_and_then(
cx,
UNNECESSARY_MUT_PASSED,
argument.span,
format!("the {fn_kind} `{name}` doesn't need a mutable reference"),
|diag| {
let span_until_arg = argument.span.until(arg.span);
match borrow_kind {
BorrowKind::Ref => {
let span_to_remove = if let Some(Some(ref_pos)) = span_until_arg.with_source_text(cx, |src| {
src
// we don't use `strip_prefix` here, because `argument` might be enclosed in parens, in
// which case `&` is no longer the prefix
// we don't use `strip_prefix` here, because `argument` might be enclosed in
// parens, in which case `&` is no longer the prefix
.find('&')
// just a sanity check, in case some proc-macro messes up the spans
.filter(|ref_pos| src[*ref_pos..].contains("mut"))
Expand All @@ -103,19 +117,47 @@ fn check_arguments<'tcx>(
span_until_arg.split_at(lo).1
} else {
return;
}
};
};
diag.span_suggestion_verbose(span_to_remove, "remove this `mut`", String::new(), applicability);
},
BorrowKind::Raw => {
let span_to_remove =
if let Some(Some(ref_pos)) = span_until_arg.with_source_text(cx, |src: &str| {
// we don't use `strip_prefix` here, because `argument` might be enclosed in
// parens, and there might be arbitrary whitespace between things
let src_after_addr_raw = src.split_once('&')?.1.split_once("raw")?.1.trim_start();

span_lint_and_then(
cx,
UNNECESSARY_MUT_PASSED,
argument.span,
format!("the {fn_kind} `{name}` doesn't need a mutable reference"),
|diag| {
diag.span_suggestion_verbose(span_to_remove, "remove this `mut`", String::new(), applicability);
},
);
Some(src_after_addr_raw)
// just a sanity check, in case some proc-macro messes up the spans
.filter(|trimmed| trimmed.contains("mut"))
.map(|trimmed| {
// SAFETY:
// - `trimmed` is derived from `src` by trimming characters from the latter's start
// - that also means that `trimmed` > `src`
unsafe { trimmed.as_ptr().offset_from_unsigned(src.as_ptr()) }
})
}) && let Ok(lo) = u32::try_from(ref_pos)
{
span_until_arg.split_at(lo).1
} else {
return;
};
diag.span_suggestion_verbose(
span_to_remove,
"make this a `const` ptr",
// the span points at `&raw mut x`
// ^^^^
// so we append a space to our suggestion
String::from("const "),
applicability,
);
},
BorrowKind::Pin => {
// it's fine to only "check" this after we've emitted the lint -- if the
// reference was an `&pin`, passing it into a function requiring a ptr wouldn't
// have type-checked in the first place
},
}
}
}
},
);
}
31 changes: 27 additions & 4 deletions tests/ui/unnecessary_mut_passed.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ struct MyStruct;

impl MyStruct {
fn takes_nothing(&self) {}
// XXX: enable the tests for these if `arbitrary_self_types` is extended to support `*const Self`
//
// fn takes_nothing_raw(self: *const Self) {}
// fn takes_nothing_raw_and_raw_const(self: *const Self, a: *const i32) {}
fn takes_ref(&self, a: &i32) {}
fn takes_refmut(&self, a: &mut i32) {}
fn takes_ref_ref(&self, a: &&i32) {}
Expand Down Expand Up @@ -146,16 +150,24 @@ fn main() {
my_struct.takes_raw_mut(a);
}

// not supported currently
fn raw_ptrs(my_struct: MyStruct) {
let mut n = 42;

takes_raw_const(&raw mut n);
takes_raw_const(&raw const n);
//~^ unnecessary_mut_passed

// bad formatting
#[rustfmt::skip]
#[allow(clippy::double_parens)]
takes_raw_const( & raw const n);
//~^ unnecessary_mut_passed

let as_ptr: fn(*const i32) = takes_raw_const;
as_ptr(&raw mut n);
as_ptr(&raw const n);
//~^ unnecessary_mut_passed

my_struct.takes_raw_const(&raw mut n);
my_struct.takes_raw_const(&raw const n);
//~^ unnecessary_mut_passed

// No error

Expand Down Expand Up @@ -187,4 +199,15 @@ fn issue15722(mut my_struct: MyStruct) {
#[expect(clippy::double_parens)]
my_struct.takes_ref((&42));
//~^ unnecessary_mut_passed

// XXX: enable these if `arbitrary_self_types` is extended to support `*const Self`
//
// let mut n = 42;
// (&raw mut my_struct).takes_nothing_raw();
// (&raw const my_struct).takes_nothing_raw();
// (&raw mut my_struct).takes_nothing_raw_and_raw_const(&raw mut n);
// #[expect(clippy::double_parens)]
// (&raw mut my_struct).takes_nothing_raw_and_raw_const((&raw mut n));
// #[expect(clippy::double_parens)]
// my_struct.takes_nothing_raw_and_raw_const((&raw mut n));
}
25 changes: 24 additions & 1 deletion tests/ui/unnecessary_mut_passed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ struct MyStruct;

impl MyStruct {
fn takes_nothing(&self) {}
// XXX: enable the tests for these if `arbitrary_self_types` is extended to support `*const Self`
//
// fn takes_nothing_raw(self: *const Self) {}
// fn takes_nothing_raw_and_raw_const(self: *const Self, a: *const i32) {}
fn takes_ref(&self, a: &i32) {}
fn takes_refmut(&self, a: &mut i32) {}
fn takes_ref_ref(&self, a: &&i32) {}
Expand Down Expand Up @@ -146,16 +150,24 @@ fn main() {
my_struct.takes_raw_mut(a);
}

// not supported currently
fn raw_ptrs(my_struct: MyStruct) {
let mut n = 42;

takes_raw_const(&raw mut n);
//~^ unnecessary_mut_passed

// bad formatting
#[rustfmt::skip]
#[allow(clippy::double_parens)]
takes_raw_const( & raw mut n);
//~^ unnecessary_mut_passed

let as_ptr: fn(*const i32) = takes_raw_const;
as_ptr(&raw mut n);
//~^ unnecessary_mut_passed

my_struct.takes_raw_const(&raw mut n);
//~^ unnecessary_mut_passed

// No error

Expand Down Expand Up @@ -187,4 +199,15 @@ fn issue15722(mut my_struct: MyStruct) {
#[expect(clippy::double_parens)]
my_struct.takes_ref((&mut 42));
//~^ unnecessary_mut_passed

// XXX: enable these if `arbitrary_self_types` is extended to support `*const Self`
//
// let mut n = 42;
// (&raw mut my_struct).takes_nothing_raw();
// (&raw const my_struct).takes_nothing_raw();
// (&raw mut my_struct).takes_nothing_raw_and_raw_const(&raw mut n);
// #[expect(clippy::double_parens)]
// (&raw mut my_struct).takes_nothing_raw_and_raw_const((&raw mut n));
// #[expect(clippy::double_parens)]
// my_struct.takes_nothing_raw_and_raw_const((&raw mut n));
}
Loading