Skip to content

Add check_self_items setting for needless_pass_by_ref_mut lint #12648

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.

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

Closed
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
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
3 changes: 3 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
blacklisted-names
cargo-ignore-publish
check-private-items
check-self-items
cognitive-complexity-threshold
cyclomatic-complexity-threshold
disallowed-macros
Expand Down Expand Up @@ -104,6 +105,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
blacklisted-names
cargo-ignore-publish
check-private-items
check-self-items
cognitive-complexity-threshold
cyclomatic-complexity-threshold
disallowed-macros
Expand Down Expand Up @@ -183,6 +185,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
blacklisted-names
cargo-ignore-publish
check-private-items
check-self-items
cognitive-complexity-threshold
cyclomatic-complexity-threshold
disallowed-macros
Expand Down