Skip to content

Commit

Permalink
Fix a false-positive of needless_borrow
Browse files Browse the repository at this point in the history
  • Loading branch information
sinkuu committed Nov 2, 2018
1 parent 73458ae commit d4370f8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 22 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
49 changes: 28 additions & 21 deletions clippy_lints/src/needless_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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! {
Expand Down Expand Up @@ -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;
}
}
}
}
7 changes: 7 additions & 0 deletions tests/ui/needless_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

0 comments on commit d4370f8

Please sign in to comment.