From 2da0edbdf10799f3245b08a2c7b41f3bf7890f6d Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Mon, 9 Oct 2023 11:48:13 +0200 Subject: [PATCH] Honor avoid-breaking-exported-api in needless_pass_by_ref_mut 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. --- clippy_config/src/conf.rs | 2 +- clippy_lints/src/needless_pass_by_ref_mut.rs | 10 ++-- .../needless_pass_by_ref_mut/clippy.toml | 1 + .../needless_pass_by_ref_mut.fixed | 10 ++++ .../needless_pass_by_ref_mut.rs | 10 ++++ .../needless_pass_by_ref_mut.stderr | 12 +++++ tests/ui/needless_pass_by_ref_mut.rs | 18 ++++--- tests/ui/needless_pass_by_ref_mut.stderr | 52 ++++++++----------- tests/ui/needless_pass_by_ref_mut2.fixed | 4 +- tests/ui/needless_pass_by_ref_mut2.rs | 4 +- tests/ui/needless_pass_by_ref_mut2.stderr | 15 +++--- 11 files changed, 85 insertions(+), 53 deletions(-) create mode 100644 tests/ui-toml/needless_pass_by_ref_mut/clippy.toml create mode 100644 tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.fixed create mode 100644 tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.rs create mode 100644 tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.stderr diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 9cc119c68cb..7f53aad6793 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -263,7 +263,7 @@ pub fn get_configuration_metadata() -> Vec { /// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"] /// ``` (arithmetic_side_effects_allowed_unary: FxHashSet = <_>::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), diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs index 57ba0da5331..13774987b8e 100644 --- a/clippy_lints/src/needless_pass_by_ref_mut.rs +++ b/clippy_lints/src/needless_pass_by_ref_mut.rs @@ -27,7 +27,7 @@ /// 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 @@ -262,8 +262,10 @@ fn check_crate_post(&mut self, cx: &LateContext<'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 { @@ -284,7 +286,7 @@ fn check_crate_post(&mut self, cx: &LateContext<'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 { diff --git a/tests/ui-toml/needless_pass_by_ref_mut/clippy.toml b/tests/ui-toml/needless_pass_by_ref_mut/clippy.toml new file mode 100644 index 00000000000..cda8d17eed4 --- /dev/null +++ b/tests/ui-toml/needless_pass_by_ref_mut/clippy.toml @@ -0,0 +1 @@ +avoid-breaking-exported-api = false diff --git a/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.fixed b/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.fixed new file mode 100644 index 00000000000..40556ca5410 --- /dev/null +++ b/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.fixed @@ -0,0 +1,10 @@ +#![warn(clippy::needless_pass_by_ref_mut)] +#![allow(clippy::ptr_arg)] + +// Should warn +pub fn pub_foo(s: &Vec, b: &u32, x: &mut u32) { + //~^ ERROR: this argument is a mutable reference, but not used mutably + *x += *b + s.len() as u32; +} + +fn main() {} diff --git a/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.rs b/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.rs new file mode 100644 index 00000000000..bbc63ceb15a --- /dev/null +++ b/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.rs @@ -0,0 +1,10 @@ +#![warn(clippy::needless_pass_by_ref_mut)] +#![allow(clippy::ptr_arg)] + +// Should warn +pub fn pub_foo(s: &mut Vec, b: &u32, x: &mut u32) { + //~^ ERROR: this argument is a mutable reference, but not used mutably + *x += *b + s.len() as u32; +} + +fn main() {} diff --git a/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.stderr b/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.stderr new file mode 100644 index 00000000000..c10607bf4ba --- /dev/null +++ b/tests/ui-toml/needless_pass_by_ref_mut/needless_pass_by_ref_mut.stderr @@ -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, b: &u32, x: &mut u32) { + | ^^^^^^^^^^^^^ help: consider changing to: `&Vec` + | + = 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 + diff --git a/tests/ui/needless_pass_by_ref_mut.rs b/tests/ui/needless_pass_by_ref_mut.rs index eee62122fdf..162ec82aede 100644 --- a/tests/ui/needless_pass_by_ref_mut.rs +++ b/tests/ui/needless_pass_by_ref_mut.rs @@ -232,43 +232,48 @@ async fn async_vec2(b: &mut Vec) { } 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, b: &u32, x: &mut u32) { + *x += *b + s.len() as u32; +} + // Should not warn. async fn _f(v: &mut Vec<()>) { let x = || v.pop(); @@ -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); } diff --git a/tests/ui/needless_pass_by_ref_mut.stderr b/tests/ui/needless_pass_by_ref_mut.stderr index 51e3ba37ded..f462fa9099e 100644 --- a/tests/ui/needless_pass_by_ref_mut.stderr +++ b/tests/ui/needless_pass_by_ref_mut.stderr @@ -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 + --> tests/ui/needless_pass_by_ref_mut.rs:254: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 - --> tests/ui/needless_pass_by_ref_mut.rs:265:26 + --> tests/ui/needless_pass_by_ref_mut.rs:265:22 | -LL | pub async fn closure4(n: &mut usize) { - | ^^^^^^^^^^ help: consider changing to: `&usize` - | - = 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)` diff --git a/tests/ui/needless_pass_by_ref_mut2.fixed b/tests/ui/needless_pass_by_ref_mut2.fixed index 3c2576213cd..f26b39ea6a1 100644 --- a/tests/ui/needless_pass_by_ref_mut2.fixed +++ b/tests/ui/needless_pass_by_ref_mut2.fixed @@ -5,7 +5,7 @@ #![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; @@ -13,7 +13,7 @@ pub async fn inner_async3(x: &i32, y: &mut u32) { .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; diff --git a/tests/ui/needless_pass_by_ref_mut2.rs b/tests/ui/needless_pass_by_ref_mut2.rs index 34b0b564deb..4220215b1fe 100644 --- a/tests/ui/needless_pass_by_ref_mut2.rs +++ b/tests/ui/needless_pass_by_ref_mut2.rs @@ -5,7 +5,7 @@ #![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; @@ -13,7 +13,7 @@ pub async fn inner_async3(x: &mut i32, y: &mut u32) { .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; diff --git a/tests/ui/needless_pass_by_ref_mut2.stderr b/tests/ui/needless_pass_by_ref_mut2.stderr index c8753603225..1c0136cf5d5 100644 --- a/tests/ui/needless_pass_by_ref_mut2.stderr +++ b/tests/ui/needless_pass_by_ref_mut2.stderr @@ -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