Skip to content

Commit

Permalink
Add check_self_items setting for needless_pass_by_ref_mut lint
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeGomez committed Apr 8, 2024
1 parent 2202493 commit b115ce2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
4 changes: 4 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,10 @@ define_Conf! {
/// 2. Paths with any segment that containing the word 'prelude'
/// are already allowed by default.
(allowed_wildcard_imports: FxHashSet<String> = FxHashSet::default()),
/// Lint: NEEDLESS_PASS_BY_REF_MUT.
///
/// Lint on `self`.
(check_self_items: bool = false),
}

/// Search for the configuration file.
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {

blacklisted_names: _,
cyclomatic_complexity_threshold: _,
check_self_items,
} = *conf;
let msrv = || msrv.clone();

Expand Down Expand Up @@ -1070,6 +1071,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| {
Box::new(needless_pass_by_ref_mut::NeedlessPassByRefMut::new(
avoid_breaking_exported_api,
check_self_items,
))
});
store.register_late_pass(|_| Box::new(non_canonical_impls::NonCanonicalImpls));
Expand Down
14 changes: 11 additions & 3 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ declare_clippy_lint! {
/// Be careful if the function is publicly reexported as it would break compatibility with
/// users of this function.
///
/// By default, `&mut self` is ignored. If you want it to be taken into account, set the
/// `check_self_items` clippy setting to `true`.
///
/// ### Why is this bad?
/// Less `mut` means less fights with the borrow checker. It can also lead to more
/// opportunities for parallelization.
Expand Down Expand Up @@ -57,14 +60,16 @@ pub struct NeedlessPassByRefMut<'tcx> {
avoid_breaking_exported_api: bool,
used_fn_def_ids: FxHashSet<LocalDefId>,
fn_def_ids_to_maybe_unused_mut: FxIndexMap<LocalDefId, Vec<rustc_hir::Ty<'tcx>>>,
check_self_items: bool,
}

impl NeedlessPassByRefMut<'_> {
pub fn new(avoid_breaking_exported_api: bool) -> Self {
pub fn new(avoid_breaking_exported_api: bool, check_self_items: bool) -> Self {
Self {
avoid_breaking_exported_api,
used_fn_def_ids: FxHashSet::default(),
fn_def_ids_to_maybe_unused_mut: FxIndexMap::default(),
check_self_items,
}
}
}
Expand All @@ -76,13 +81,14 @@ fn should_skip<'tcx>(
input: rustc_hir::Ty<'tcx>,
ty: Ty<'_>,
arg: &rustc_hir::Param<'_>,
check_self_items: bool,
) -> bool {
// We check if this a `&mut`. `ref_mutability` returns `None` if it's not a reference.
if !matches!(ty.ref_mutability(), Some(Mutability::Mut)) {
return true;
}

if is_self(arg) {
if !check_self_items && is_self(arg) {
return true;
}

Expand Down Expand Up @@ -172,13 +178,15 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
let fn_sig = cx.tcx.fn_sig(fn_def_id).instantiate_identity();
let fn_sig = cx.tcx.liberate_late_bound_regions(fn_def_id.to_def_id(), fn_sig);

let check_self_items = self.check_self_items;

// If there are no `&mut` argument, no need to go any further.
let mut it = decl
.inputs
.iter()
.zip(fn_sig.inputs())
.zip(body.params)
.filter(|((&input, &ty), arg)| !should_skip(cx, input, ty, arg))
.filter(|((&input, &ty), arg)| !should_skip(cx, input, ty, arg, check_self_items))
.peekable();
if it.peek().is_none() {
return;
Expand Down

0 comments on commit b115ce2

Please sign in to comment.