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.

Author: flip1995

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
@@ -241,8 +241,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 {
@@ -263,7 +265,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 {

clippy_lints/src/utils/conf.rs

@@ -264,7 +264,7 @@ define_Conf! {
     /// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
     /// ```
     (arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::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),

tests/ui-toml/needless_pass_by_ref_mut/clippy.toml

@@ -0,0 +1 @@
+avoid-breaking-exported-api = false

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() {}

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() {}

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
+

tests/ui/needless_pass_by_ref_mut.rs

@@ -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();
@@ -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);
 }

tests/ui/needless_pass_by_ref_mut.stderr

@@ -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