Skip to content
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

Honor avoid-breaking-exported-api in needless_pass_by_ref_mut #11647

Merged
merged 3 commits into from
Jul 3, 2024
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
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
* [`enum_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names)
* [`large_types_passed_by_value`](https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value)
* [`linkedlist`](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
* [`needless_pass_by_ref_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut)
* [`option_option`](https://rust-lang.github.io/rust-clippy/master/index.html#option_option)
* [`rc_buffer`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer)
* [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,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
11 changes: 6 additions & 5 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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 @@ -138,6 +138,10 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
return;
}

if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(fn_def_id) {
return;
}

let hir_id = cx.tcx.local_def_id_to_hir_id(fn_def_id);
let is_async = match kind {
FnKind::ItemFn(.., header) => {
Expand Down Expand Up @@ -262,9 +266,6 @@ 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 mut is_cfged = None;
for input in unused {
// If the argument is never used mutably, we emit the warning.
Expand All @@ -284,7 +285,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 cx.effective_visibilities.is_exported(*fn_def_id) {
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,10 @@
#![warn(clippy::needless_pass_by_ref_mut)]
#![allow(clippy::ptr_arg)]

// 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() {}
10 changes: 10 additions & 0 deletions tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![warn(clippy::needless_pass_by_ref_mut)]
#![allow(clippy::ptr_arg)]

// 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
--> tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.rs:5: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 1 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 @@ -232,43 +232,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 @@ -365,4 +370,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);
}
52 changes: 23 additions & 29 deletions tests/ui/needless_pass_by_ref_mut.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -108,109 +108,103 @@ 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
--> tests/ui/needless_pass_by_ref_mut.rs:235:34
--> tests/ui/needless_pass_by_ref_mut.rs:235: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
--> tests/ui/needless_pass_by_ref_mut.rs:254:20
|
LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
| ^^^^^^^^^^ help: consider changing to: `&usize`
--> tests/ui/needless_pass_by_ref_mut.rs:254:16
|
= 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
--> tests/ui/needless_pass_by_ref_mut.rs:265:26
|
LL | pub async fn closure4(n: &mut usize) {
| ^^^^^^^^^^ help: consider changing to: `&usize`
--> tests/ui/needless_pass_by_ref_mut.rs:265:22
|
= warning: changing this function will impact semver compatibility
LL | async fn closure4(n: &mut usize) {
| ^^^^^^^^^^ help: consider changing to: `&usize`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:314:12
--> tests/ui/needless_pass_by_ref_mut.rs:319:12
|
LL | fn bar(&mut self) {}
| ^^^^^^^^^ help: consider changing to: `&self`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:316:18
--> tests/ui/needless_pass_by_ref_mut.rs:321:18
|
LL | async fn foo(&mut self, u: &mut i32, v: &mut u32) {
| ^^^^^^^^^ help: consider changing to: `&self`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:316:45
--> tests/ui/needless_pass_by_ref_mut.rs:321:45
|
LL | async fn foo(&mut self, u: &mut i32, v: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:324:46
--> tests/ui/needless_pass_by_ref_mut.rs:329:46
|
LL | async fn foo2(&mut self, u: &mut i32, v: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:340:18
--> tests/ui/needless_pass_by_ref_mut.rs:345:18
|
LL | fn _empty_tup(x: &mut (())) {}
| ^^^^^^^^^ help: consider changing to: `&()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:341:19
--> tests/ui/needless_pass_by_ref_mut.rs:346:19
|
LL | fn _single_tup(x: &mut ((i32,))) {}
| ^^^^^^^^^^^^^ help: consider changing to: `&(i32,)`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:342:18
--> tests/ui/needless_pass_by_ref_mut.rs:347:18
|
LL | fn _multi_tup(x: &mut ((i32, u32))) {}
| ^^^^^^^^^^^^^^^^^ help: consider changing to: `&(i32, u32)`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:343:11
--> tests/ui/needless_pass_by_ref_mut.rs:348:11
|
LL | fn _fn(x: &mut (fn())) {}
| ^^^^^^^^^^^ help: consider changing to: `&fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:345:23
--> tests/ui/needless_pass_by_ref_mut.rs:350:23
|
LL | fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "Rust" fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:346:20
--> tests/ui/needless_pass_by_ref_mut.rs:351:20
|
LL | fn _extern_c_fn(x: &mut extern "C" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "C" fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:347:18
--> tests/ui/needless_pass_by_ref_mut.rs:352:18
|
LL | fn _unsafe_fn(x: &mut unsafe fn()) {}
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:348:25
--> tests/ui/needless_pass_by_ref_mut.rs:353:25
|
LL | fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn()`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:349:20
--> tests/ui/needless_pass_by_ref_mut.rs:354:20
|
LL | fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn(i32)`

error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut.rs:350:20
--> tests/ui/needless_pass_by_ref_mut.rs:355:20
|
LL | fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn() -> (i32)`
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/needless_pass_by_ref_mut2.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
#![allow(clippy::redundant_closure_call)]
#![warn(clippy::needless_pass_by_ref_mut)]

pub async fn inner_async3(x: &i32, y: &mut u32) {
async fn inner_async3(x: &i32, y: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
*y += 1;
}
.await;
}

pub async fn inner_async4(u: &mut i32, v: &u32) {
async fn inner_async4(u: &mut i32, v: &u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
*u += 1;
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/needless_pass_by_ref_mut2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
#![allow(clippy::redundant_closure_call)]
#![warn(clippy::needless_pass_by_ref_mut)]

pub async fn inner_async3(x: &mut i32, y: &mut u32) {
async fn inner_async3(x: &mut i32, y: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
*y += 1;
}
.await;
}

pub async fn inner_async4(u: &mut i32, v: &mut u32) {
async fn inner_async4(u: &mut i32, v: &mut u32) {
//~^ ERROR: this argument is a mutable reference, but not used mutably
async {
*u += 1;
Expand Down
15 changes: 6 additions & 9 deletions tests/ui/needless_pass_by_ref_mut2.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
error: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut2.rs:8:30
--> tests/ui/needless_pass_by_ref_mut2.rs:8:26
|
LL | pub async fn inner_async3(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&i32`
LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&i32`
|
= 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: this argument is a mutable reference, but not used mutably
--> tests/ui/needless_pass_by_ref_mut2.rs:16:43
--> tests/ui/needless_pass_by_ref_mut2.rs:16:39
|
LL | pub async fn inner_async4(u: &mut i32, v: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`
|
= warning: changing this function will impact semver compatibility
LL | async fn inner_async4(u: &mut i32, v: &mut u32) {
| ^^^^^^^^ help: consider changing to: `&u32`

error: aborting due to 2 previous errors

Loading