From d4370f8b07b8aac41a94187f4d334e0903a68711 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Fri, 2 Nov 2018 15:56:47 +0900 Subject: [PATCH] Fix a false-positive of needless_borrow --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/needless_borrow.rs | 49 ++++++++++++++++------------- tests/ui/needless_borrow.rs | 7 +++++ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 207dc40fa1d..c4913862642 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -365,7 +365,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); diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index 639358a7ce7..7892467b7f8 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/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 @@ /// 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, +} impl LintPass for NeedlessBorrow { fn get_lints(&self) -> LintArray { @@ -61,7 +53,7 @@ fn get_lints(&self) -> LintArray { 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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { |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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { } } 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 @@ fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) { } } } + + 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; + } + } + } } diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index f8a170f38d4..29e6ccca94d 100644 --- a/tests/ui/needless_borrow.rs +++ b/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), +} -- 2.44.0