Merge #3400

3400: Fix a false-positive of needless_borrow r=phansch a=sinkuu



Co-authored-by: Shotaro Yamada <sinkuu@sinkuu.xyz>

clippy_lints/src/lib.rs

@@ -367,7 +367,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box zero_div_zero::Pass);
     reg.register_late_lint_pass(box mutex_atomic::MutexAtomic);
     reg.register_late_lint_pass(box needless_update::Pass);
-    reg.register_late_lint_pass(box needless_borrow::NeedlessBorrow);
+    reg.register_late_lint_pass(box needless_borrow::NeedlessBorrow::default());
     reg.register_late_lint_pass(box needless_borrowed_ref::NeedlessBorrowedRef);
     reg.register_late_lint_pass(box no_effect::Pass);
     reg.register_late_lint_pass(box temporary_assignment::Pass);

clippy_lints/src/needless_borrow.rs

@@ -12,14 +12,15 @@
 //!
 //! This lint is **warn** by default
 
+use crate::rustc::hir::{BindingAnnotation, Expr, ExprKind, Item, MutImmutable, Pat, PatKind};
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use crate::rustc::{declare_tool_lint, lint_array};
-use if_chain::if_chain;
-use crate::rustc::hir::{BindingAnnotation, Expr, ExprKind, MutImmutable, Pat, PatKind};
 use crate::rustc::ty;
 use crate::rustc::ty::adjustment::{Adjust, Adjustment};
-use crate::utils::{in_macro, snippet_opt, span_lint_and_then};
+use crate::rustc::{declare_tool_lint, lint_array};
 use crate::rustc_errors::Applicability;
+use crate::utils::{in_macro, snippet_opt, span_lint_and_then};
+use crate::syntax::ast::NodeId;
+use if_chain::if_chain;
 
 /// **What it does:** Checks for address of operations (`&`) that are going to
 /// be dereferenced immediately by the compiler.
@@ -32,26 +33,17 @@ use crate::rustc_errors::Applicability;
 /// let x: &i32 = &&&&&&5;
 /// ```
 ///
-/// **Known problems:** This will cause false positives in code generated by `derive`.
-/// For instance in the following snippet:
-/// ```rust
-/// #[derive(Debug)]
-/// pub enum Error {
-///     Type(
-///         &'static str,
-///     ),
-/// }
-/// ```
-/// A warning will be emitted that `&'static str` should be replaced with `&'static str`,
-/// however there is nothing that can or should be done to fix this.
+/// **Known problems:** None.
 declare_clippy_lint! {
     pub NEEDLESS_BORROW,
     nursery,
     "taking a reference that is going to be automatically dereferenced"
 }
 
-#[derive(Copy, Clone)]
-pub struct NeedlessBorrow;
+#[derive(Default)]
+pub struct NeedlessBorrow {
+    derived_item: Option<NodeId>,
+}
 
 impl LintPass for NeedlessBorrow {
     fn get_lints(&self) -> LintArray {
@@ -61,7 +53,7 @@ impl LintPass for NeedlessBorrow {
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrow {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
-        if in_macro(e.span) {
+        if in_macro(e.span) || self.derived_item.is_some() {
             return;
         }
         if let ExprKind::AddrOf(MutImmutable, ref inner) = e.node {
@@ -87,7 +79,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrow {
                             |db| {
                                 if let Some(snippet) = snippet_opt(cx, inner.span) {
                                     db.span_suggestion_with_applicability(
-                                        e.span, 
+                                        e.span,
                                         "change this to",
                                         snippet,
                                         Applicability::MachineApplicable,
@@ -101,7 +93,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrow {
         }
     }
     fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) {
-        if in_macro(pat.span) {
+        if in_macro(pat.span) || self.derived_item.is_some() {
             return;
         }
         if_chain! {
@@ -131,4 +123,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrow {
             }
         }
     }
+
+    fn check_item(&mut self, _: &LateContext<'a, 'tcx>, item: &'tcx Item) {
+        if item.attrs.iter().any(|a| a.check_name("automatically_derived")) {
+            debug_assert!(self.derived_item.is_none());
+            self.derived_item = Some(item.id);
+        }
+    }
+
+    fn check_item_post(&mut self, _: &LateContext<'a, 'tcx>, item: &'tcx Item) {
+        if let Some(id) = self.derived_item {
+            if item.id == id {
+                self.derived_item = None;
+            }
+        }
+    }
 }

tests/ui/needless_borrow.rs

@@ -63,3 +63,10 @@ fn issue_1432() {
 
     let _ = v.iter().filter(|&a| a.is_empty());
 }
+
+#[allow(dead_code)]
+#[warn(clippy::needless_borrow)]
+#[derive(Debug)]
+enum Foo<'a> {
+    Str(&'a str),
+}