Skip to content

Commit

Permalink
Honor avoid-breaking-exported-api in needless_pass_by_ref_mut
Browse files Browse the repository at this point in the history
Until now, the lint only emitted a warning, when breaking public API. Now it
doesn't lint at all when the config value is not set to `false`, bringing it in
line with the other lints using this config value.

Also ensures that this config value is documented in the lint.
  • Loading branch information
flip1995 committed Nov 15, 2023
1 parent 7ad3373 commit c5bf27a
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 31 deletions.
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ define_Conf! {
/// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
/// ```
(arithmetic_side_effects_allowed_unary: FxHashSet<String> = <_>::default()),
/// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS, SINGLE_CALL_FN.
/// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS, SINGLE_CALL_FN, NEEDLESS_PASS_BY_REF_MUT.
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
Expand Down
10 changes: 6 additions & 4 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ declare_clippy_lint! {
/// Check if a `&mut` function argument is actually used mutably.
///
/// Be careful if the function is publicly reexported as it would break compatibility with
/// users of this function.
/// users of this function, when the users pass this function as an argument.
///
/// ### Why is this bad?
/// Less `mut` means less fights with the borrow checker. It can also lead to more
Expand Down Expand Up @@ -244,8 +244,10 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
.iter()
.filter(|(def_id, _)| !self.used_fn_def_ids.contains(def_id))
{
let show_semver_warning =
self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(*fn_def_id);
let is_exported = cx.effective_visibilities.is_exported(*fn_def_id);
if self.avoid_breaking_exported_api && is_exported {
continue;
}

let mut is_cfged = None;
for input in unused {
Expand All @@ -266,7 +268,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
format!("&{}", snippet(cx, cx.tcx.hir().span(inner_ty.ty.hir_id), "_"),),
Applicability::Unspecified,
);
if show_semver_warning {
if is_exported {
diag.warn("changing this function will impact semver compatibility");
}
if *is_cfged {
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/needless_pass_by_ref_mut/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
avoid-breaking-exported-api = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![warn(clippy::needless_pass_by_ref_mut)]

// Should warn
pub fn pub_foo(s: &Vec<u32>, b: &u32, x: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
*x += *b + s.len() as u32;
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![warn(clippy::needless_pass_by_ref_mut)]

// Should warn
pub fn pub_foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
*x += *b + s.len() as u32;
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:4:19
|
LL | pub fn pub_foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
| ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`
|
= warning: changing this function will impact semver compatibility
= note: `-D clippy::needless-pass-by-ref-mut` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::needless_pass_by_ref_mut)]`

error: aborting due to previous error

18 changes: 12 additions & 6 deletions tests/ui/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,43 +233,48 @@ async fn async_vec2(b: &mut Vec<bool>) {
}
fn non_mut(n: &str) {}
//Should warn
pub async fn call_in_closure1(n: &mut str) {
async fn call_in_closure1(n: &mut str) {
(|| non_mut(n))()
}
fn str_mut(str: &mut String) -> bool {
str.pop().is_some()
}
//Should not warn
pub async fn call_in_closure2(str: &mut String) {
async fn call_in_closure2(str: &mut String) {
(|| str_mut(str))();
}

// Should not warn.
pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
async fn closure(n: &mut usize) -> impl '_ + FnMut() {
|| {
*n += 1;
}
}

// Should warn.
pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
//~^ ERROR: this argument is a mutable reference, but not used mutably
|| *n + 1
}

// Should not warn.
pub async fn closure3(n: &mut usize) {
async fn closure3(n: &mut usize) {
(|| *n += 1)();
}

// Should warn.
pub async fn closure4(n: &mut usize) {
async fn closure4(n: &mut usize) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
(|| {
let _x = *n + 1;
})();
}

// Should not warn: pub
pub fn pub_foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
*x += *b + s.len() as u32;
}

// Should not warn.
async fn _f(v: &mut Vec<()>) {
let x = || v.pop();
Expand Down Expand Up @@ -318,4 +323,5 @@ fn main() {
used_as_path;
let _: fn(&mut u32) = passed_as_local;
let _ = if v[0] == 0 { ty_unify_1 } else { ty_unify_2 };
pub_foo(&mut v, &0, &mut u);
}
32 changes: 12 additions & 20 deletions tests/ui/needless_pass_by_ref_mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -108,36 +108,28 @@ LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&i32`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:236:34
--> $DIR/needless_pass_by_ref_mut.rs:236:30
|
LL | pub async fn call_in_closure1(n: &mut str) {
| ^^^^^^^^ help: consider changing to: `&str`
|
= warning: changing this function will impact semver compatibility
LL | async fn call_in_closure1(n: &mut str) {
| ^^^^^^^^ help: consider changing to: `&str`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:248:25
|
LL | pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
| ^^^^^^^^^^ help: consider changing to: `&usize`
--> $DIR/needless_pass_by_ref_mut.rs:248:21
|
= warning: changing this function will impact semver compatibility
LL | async fn closure(n: &mut usize) -> impl '_ + FnMut() {
| ^^^^^^^^^^ help: consider changing to: `&usize`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:255:20
--> $DIR/needless_pass_by_ref_mut.rs:255:16
|
LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
| ^^^^^^^^^^ help: consider changing to: `&usize`
|
= warning: changing this function will impact semver compatibility
LL | fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
| ^^^^^^^^^^ help: consider changing to: `&usize`

error: this argument is a mutable reference, but not used mutably
--> $DIR/needless_pass_by_ref_mut.rs:266:26
|
LL | pub async fn closure4(n: &mut usize) {
| ^^^^^^^^^^ help: consider changing to: `&usize`
--> $DIR/needless_pass_by_ref_mut.rs:266:22
|
= warning: changing this function will impact semver compatibility
LL | async fn closure4(n: &mut usize) {
| ^^^^^^^^^^ help: consider changing to: `&usize`

error: aborting due to 21 previous errors

0 comments on commit c5bf27a

Please sign in to comment.