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.
rust-lang · Dec 2, 2023 · b26c6a3 · b26c6a3
1 parent ee83760
commit b26c6a3
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 48 deletions.
diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs
@@ -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),

diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs
@@ -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
@@ -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 {
@@ -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 {

diff --git a/tests/ui-toml/needless_pass_by_ref_mut/clippy.toml 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
@@ -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() {}
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
@@ -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() {}
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
@@ -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
+
diff --git a/tests/ui/needless_pass_by_ref_mut.rs b/tests/ui/needless_pass_by_ref_mut.rs
@@ -238,43 +238,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();
@@ -323,4 +328,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
@@ -1,5 +1,5 @@
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:12:11
+  --> $DIR/needless_pass_by_ref_mut.rs:7:11
    |
 LL | fn foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
    |           ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`
@@ -8,136 +8,128 @@ LL | fn foo(s: &mut Vec<u32>, b: &u32, x: &mut u32) {
    = help: to override `-D warnings` add `#[allow(clippy::needless_pass_by_ref_mut)]`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:37:12
+  --> $DIR/needless_pass_by_ref_mut.rs:32:12
    |
 LL | fn foo6(s: &mut Vec<u32>) {
    |            ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:50:29
+  --> $DIR/needless_pass_by_ref_mut.rs:45:29
    |
 LL |     fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
    |                             ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:55:31
+  --> $DIR/needless_pass_by_ref_mut.rs:50:31
    |
 LL |     fn badger(&mut self, vec: &mut Vec<i32>) -> usize {
    |                               ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:132:16
+  --> $DIR/needless_pass_by_ref_mut.rs:127:16
    |
 LL | async fn a1(x: &mut i32) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:136:16
+  --> $DIR/needless_pass_by_ref_mut.rs:131:16
    |
 LL | async fn a2(x: &mut i32, y: String) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:140:16
+  --> $DIR/needless_pass_by_ref_mut.rs:135:16
    |
 LL | async fn a3(x: &mut i32, y: String, z: String) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:144:16
+  --> $DIR/needless_pass_by_ref_mut.rs:139:16
    |
 LL | async fn a4(x: &mut i32, y: i32) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:148:24
+  --> $DIR/needless_pass_by_ref_mut.rs:143:24
    |
 LL | async fn a5(x: i32, y: &mut i32) {
    |                        ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:152:24
+  --> $DIR/needless_pass_by_ref_mut.rs:147:24
    |
 LL | async fn a6(x: i32, y: &mut i32) {
    |                        ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:156:32
+  --> $DIR/needless_pass_by_ref_mut.rs:151:32
    |
 LL | async fn a7(x: i32, y: i32, z: &mut i32) {
    |                                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:160:24
+  --> $DIR/needless_pass_by_ref_mut.rs:155:24
    |
 LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
    |                        ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:160:45
+  --> $DIR/needless_pass_by_ref_mut.rs:155:45
    |
 LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
    |                                             ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:194:16
+  --> $DIR/needless_pass_by_ref_mut.rs:189:16
    |
 LL | fn cfg_warn(s: &mut u32) {}
    |                ^^^^^^^^ help: consider changing to: `&u32`
    |
    = note: this is cfg-gated and may require further changes
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:200:20
+  --> $DIR/needless_pass_by_ref_mut.rs:195:20
    |
 LL |     fn cfg_warn(s: &mut u32) {}
    |                    ^^^^^^^^ help: consider changing to: `&u32`
    |
    = note: this is cfg-gated and may require further changes
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:214:39
+  --> $DIR/needless_pass_by_ref_mut.rs:209:39
    |
 LL | async fn inner_async2(x: &mut i32, y: &mut u32) {
    |                                       ^^^^^^^^ help: consider changing to: `&u32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> $DIR/needless_pass_by_ref_mut.rs:222:26
+  --> $DIR/needless_pass_by_ref_mut.rs:217:26
    |
 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:241: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:253: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:260: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:271: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