Add lint ref_binding_to_reference

rust-lang · Apr 21, 2021 · d6ab6f5 · d6ab6f5
1 parent 65cee85
commit d6ab6f5
Show file tree

Hide file tree

Showing 8 changed files with 212 additions and 72 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2434,6 +2434,7 @@ Released 2018-09-13
 [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
 [`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
+[`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -846,6 +846,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         needless_bool::BOOL_COMPARISON,
         needless_bool::NEEDLESS_BOOL,
         needless_borrow::NEEDLESS_BORROW,
+        needless_borrow::REF_BINDING_TO_REFERENCE,
         needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
         needless_continue::NEEDLESS_CONTINUE,
         needless_for_each::NEEDLESS_FOR_EACH,
@@ -1629,6 +1630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
         LintId::of(needless_bool::BOOL_COMPARISON),
         LintId::of(needless_bool::NEEDLESS_BOOL),
+        LintId::of(needless_borrow::REF_BINDING_TO_REFERENCE),
         LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
         LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
         LintId::of(needless_update::NEEDLESS_UPDATE),
@@ -1813,6 +1815,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(misc_early::REDUNDANT_PATTERN),
         LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
         LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
+        LintId::of(needless_borrow::REF_BINDING_TO_REFERENCE),
         LintId::of(neg_multiply::NEG_MULTIPLY),
         LintId::of(new_without_default::NEW_WITHOUT_DEFAULT),
         LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),

diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs
@@ -37,7 +37,33 @@ declare_clippy_lint! {
     "taking a reference that is going to be automatically dereferenced"
 }
 
-declare_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW]);
+declare_clippy_lint! {
+    /// **What it does:** Checks for `ref` bindings which create a reference to a reference.
+    ///
+    /// **Why is this bad?** The address-of operator at the use site is clearer about the need for a reference.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// // Bad
+    /// let x = Some("");
+    /// if let Some(ref x) = x {
+    ///     // use `x` here
+    /// }
+    ///
+    /// // Good
+    /// let x = Some("");
+    /// if let Some(x) = x {
+    ///     // use `&x` here
+    /// }
+    /// ```
+    pub REF_BINDING_TO_REFERENCE,
+    style,
+    "`ref` binding to a reference"
+}
+
+declare_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW, REF_BINDING_TO_REFERENCE]);
 
 impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
@@ -102,6 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
 
                 let mut app = Applicability::MachineApplicable;
                 let mut can_fix = true;
+                let mut lint = NEEDLESS_BORROW;
                 let mut changes = vec![
                     (pat.span, snippet_with_context(cx, name.span, ctxt, "..", &mut app).0.into_owned()),
                 ];
@@ -121,6 +148,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
                             }
                             _ if e.span.ctxt() == ctxt => {
                                 // Double reference might be needed at this point.
+                                lint = REF_BINDING_TO_REFERENCE;
                                 let snip = snippet_with_applicability(cx, e.span, "..", &mut app);
                                 changes.push((e.span, format!("&{}", snip)));
                             }
@@ -135,7 +163,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
 
                 span_lint_and_then(
                     cx,
-                    NEEDLESS_BORROW,
+                    lint,
                     pat.span,
                     "this pattern creates a reference to a reference",
                     |diag| {

diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs
@@ -70,7 +70,7 @@ pub fn range<'a>(expr: &'a hir::Expr<'_>) -> Option<Range<'a>> {
                 limits: ast::RangeLimits::Closed,
             })
         },
-        hir::ExprKind::Struct(ref path, ref fields, None) => match path {
+        hir::ExprKind::Struct(path, ref fields, None) => match path {
             hir::QPath::LangItem(hir::LangItem::RangeFull, _) => Some(Range {
                 start: None,
                 end: None,

diff --git a/tests/ui/needless_borrow_pat.rs b/tests/ui/needless_borrow_pat.rs
@@ -10,11 +10,6 @@ macro_rules! m1 {
         f1($e);
     };
 }
-macro_rules! m2 {
-    ($e:expr) => {
-        f1(*$e);
-    };
-}
 macro_rules! m3 {
     ($i:ident) => {
         Some(ref $i)
@@ -56,13 +51,7 @@ fn main() {
     };
 
     // Err, reference to a &String
-    let _: &&String = match Some(&x) {
-        Some(ref x) => x,
-        None => return,
-    };
-
-    // Err, reference to a &String
-    let _: &&String = match Some(&x) {
+    let _: &String = match Some(&x) {
         Some(ref x) => {
             f1(x);
             f1(*x);
@@ -77,20 +66,10 @@ fn main() {
         None => return,
     };
 
-    // Err, reference to a &String
-    match Some(&x) {
-        Some(ref x) => m2!(x),
-        None => return,
-    }
-
     // Err, reference to a &String
     let _ = |&ref x: &&String| {
         let _: &String = x;
     };
-    // Err, reference to a &String
-    let _ = |&ref x: &&String| {
-        let _: &&String = x;
-    };
 
     // Err, reference to a &String
     let (ref y,) = (&x,);
@@ -103,7 +82,7 @@ fn main() {
 
 // Err, reference to a &String
 fn f2<'a>(&ref x: &&'a String) -> &'a String {
-    let _: &&String = x;
+    let _: &String = x;
     *x
 }
 

diff --git a/tests/ui/needless_borrow_pat.stderr b/tests/ui/needless_borrow_pat.stderr
@@ -1,13 +1,13 @@
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:48:14
+  --> $DIR/needless_borrow_pat.rs:43:14
    |
 LL |         Some(ref x) => x,
    |              ^^^^^ help: try this: `x`
    |
    = note: `-D clippy::needless-borrow` implied by `-D warnings`
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:54:14
+  --> $DIR/needless_borrow_pat.rs:49:14
    |
 LL |         Some(ref x) => *x,
    |              ^^^^^
@@ -18,18 +18,7 @@ LL |         Some(x) => x,
    |              ^     ^
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:60:14
-   |
-LL |         Some(ref x) => x,
-   |              ^^^^^
-   |
-help: try this
-   |
-LL |         Some(x) => &x,
-   |              ^     ^^
-
-error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:66:14
+  --> $DIR/needless_borrow_pat.rs:55:14
    |
 LL |         Some(ref x) => {
    |              ^^^^^
@@ -39,46 +28,22 @@ help: try this
 LL |         Some(x) => {
 LL |             f1(x);
 LL |             f1(x);
-LL |             &x
    |
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:76:14
+  --> $DIR/needless_borrow_pat.rs:65:14
    |
 LL |         Some(ref x) => m1!(x),
    |              ^^^^^ help: try this: `x`
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:82:14
-   |
-LL |         Some(ref x) => m2!(x),
-   |              ^^^^^
-   |
-help: try this
-   |
-LL |         Some(x) => m2!(&x),
-   |              ^         ^^
-
-error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:87:15
+  --> $DIR/needless_borrow_pat.rs:70:15
    |
 LL |     let _ = |&ref x: &&String| {
    |               ^^^^^ help: try this: `x`
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:91:15
-   |
-LL |     let _ = |&ref x: &&String| {
-   |               ^^^^^
-   |
-help: try this
-   |
-LL |     let _ = |&x: &&String| {
-LL |         let _: &&String = &x;
-   |
-
-error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:96:10
+  --> $DIR/needless_borrow_pat.rs:75:10
    |
 LL |     let (ref y,) = (&x,);
    |          ^^^^^
@@ -90,26 +55,26 @@ LL |     let _: &String = y;
    |
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:105:12
+  --> $DIR/needless_borrow_pat.rs:84:12
    |
 LL | fn f2<'a>(&ref x: &&'a String) -> &'a String {
    |            ^^^^^
    |
 help: try this
    |
 LL | fn f2<'a>(&x: &&'a String) -> &'a String {
-LL |     let _: &&String = &x;
+LL |     let _: &String = x;
 LL |     x
    |
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:112:11
+  --> $DIR/needless_borrow_pat.rs:91:11
    |
 LL |     fn f(&ref x: &&String) {
    |           ^^^^^ help: try this: `x`
 
 error: this pattern creates a reference to a reference
-  --> $DIR/needless_borrow_pat.rs:120:11
+  --> $DIR/needless_borrow_pat.rs:99:11
    |
 LL |     fn f(&ref x: &&String) {
    |           ^^^^^
@@ -120,5 +85,5 @@ LL |     fn f(&x: &&String) {
 LL |         let _: &String = x;
    |
 
-error: aborting due to 12 previous errors
+error: aborting due to 9 previous errors
 
diff --git a/tests/ui/ref_binding_to_reference.rs b/tests/ui/ref_binding_to_reference.rs
@@ -0,0 +1,76 @@
+// edition:2018
+// FIXME: run-rustfix waiting on multi-span suggestions
+
+#![warn(clippy::ref_binding_to_reference)]
+#![allow(clippy::needless_borrowed_reference)]
+
+fn f1(_: &str) {}
+macro_rules! m2 {
+    ($e:expr) => {
+        f1(*$e);
+    };
+}
+macro_rules! m3 {
+    ($i:ident) => {
+        Some(ref $i)
+    };
+}
+
+#[allow(dead_code)]
+fn main() {
+    let x = String::new();
+
+    // Ok, the pattern is in a different context than the match arm
+    let _: &&String = match Some(&x) {
+        m3!(x) => x,
+        None => return,
+    };
+
+    // Err, reference to a &String
+    let _: &&String = match Some(&x) {
+        Some(ref x) => x,
+        None => return,
+    };
+
+    // Err, reference to a &String
+    let _: &&String = match Some(&x) {
+        Some(ref x) => {
+            f1(x);
+            f1(*x);
+            x
+        },
+        None => return,
+    };
+
+    // Err, reference to a &String
+    match Some(&x) {
+        Some(ref x) => m2!(x),
+        None => return,
+    }
+
+    // Err, reference to a &String
+    let _ = |&ref x: &&String| {
+        let _: &&String = x;
+    };
+}
+
+// Err, reference to a &String
+fn f2<'a>(&ref x: &&'a String) -> &'a String {
+    let _: &&String = x;
+    *x
+}
+
+trait T1 {
+    // Err, reference to a &String
+    fn f(&ref x: &&String) {
+        let _: &&String = x;
+    }
+}
+
+struct S;
+impl T1 for S {
+    // Err, reference to a &String
+    fn f(&ref x: &&String) {
+        let _: &&String = x;
+    }
+}