diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ff3526d841..3fb3c686948 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,6 +149,7 @@ All notable changes to this project will be documented in this file. [`mutex_atomic`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic [`mutex_integer`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_integer [`needless_bool`]: https://github.com/Manishearth/rust-clippy/wiki#needless_bool +[`needless_borrow`]: https://github.com/Manishearth/rust-clippy/wiki#needless_borrow [`needless_lifetimes`]: https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes [`needless_range_loop`]: https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop [`needless_return`]: https://github.com/Manishearth/rust-clippy/wiki#needless_return diff --git a/README.md b/README.md index 17c0ce5235e..8f3519c95c4 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 146 lints included in this crate: +There are 147 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -97,6 +97,7 @@ name [mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic) | warn | using a Mutex where an atomic value could be used instead [mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer) | allow | using a Mutex for an integer type [needless_bool](https://github.com/Manishearth/rust-clippy/wiki#needless_bool) | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }` +[needless_borrow](https://github.com/Manishearth/rust-clippy/wiki#needless_borrow) | warn | taking a reference that is going to be automatically dereferenced [needless_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes) | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them [needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop) | warn | for-looping over a range of indices where an iterator over items would do [needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return) | warn | using a return statement like `return expr;` where an expression would suffice diff --git a/src/lib.rs b/src/lib.rs index 435e6998c16..dc0efe815b2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -96,6 +96,7 @@ pub mod mut_mut; pub mod mut_reference; pub mod mutex_atomic; pub mod needless_bool; +pub mod needless_borrow; pub mod needless_update; pub mod neg_multiply; pub mod new_without_default; @@ -210,6 +211,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box zero_div_zero::ZeroDivZeroPass); reg.register_late_lint_pass(box mutex_atomic::MutexAtomic); reg.register_late_lint_pass(box needless_update::NeedlessUpdatePass); + reg.register_late_lint_pass(box needless_borrow::NeedlessBorrow); reg.register_late_lint_pass(box no_effect::NoEffectPass); reg.register_late_lint_pass(box map_clone::MapClonePass); reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass); @@ -364,6 +366,7 @@ pub fn plugin_registrar(reg: &mut Registry) { mutex_atomic::MUTEX_ATOMIC, needless_bool::BOOL_COMPARISON, needless_bool::NEEDLESS_BOOL, + needless_borrow::NEEDLESS_BORROW, needless_update::NEEDLESS_UPDATE, neg_multiply::NEG_MULTIPLY, new_without_default::NEW_WITHOUT_DEFAULT, diff --git a/src/needless_borrow.rs b/src/needless_borrow.rs new file mode 100644 index 00000000000..294a12dad99 --- /dev/null +++ b/src/needless_borrow.rs @@ -0,0 +1,50 @@ +//! Checks for needless address of operations (`&`) +//! +//! This lint is **warn** by default + +use rustc::lint::*; +use rustc::hir::*; +use rustc::ty::TyRef; +use utils::{span_lint, in_macro}; + +/// **What it does:** This lint checks for address of operations (`&`) that are going to be dereferenced immediately by the compiler +/// +/// **Why is this bad?** Suggests that the receiver of the expression borrows the expression +/// +/// **Known problems:** +/// +/// **Example:** `let x: &i32 = &&&&&&5;` +declare_lint! { + pub NEEDLESS_BORROW, + Warn, + "taking a reference that is going to be automatically dereferenced" +} + +#[derive(Copy,Clone)] +pub struct NeedlessBorrow; + +impl LintPass for NeedlessBorrow { + fn get_lints(&self) -> LintArray { + lint_array!(NEEDLESS_BORROW) + } +} + +impl LateLintPass for NeedlessBorrow { + fn check_expr(&mut self, cx: &LateContext, e: &Expr) { + if in_macro(cx, e.span) { + return; + } + if let ExprAddrOf(MutImmutable, ref inner) = e.node { + if let TyRef(..) = cx.tcx.expr_ty(inner).sty { + let ty = cx.tcx.expr_ty(e); + let adj_ty = cx.tcx.expr_ty_adjusted(e); + if ty != adj_ty { + span_lint(cx, + NEEDLESS_BORROW, + e.span, + "this expression borrows a reference that is immediately dereferenced by the compiler"); + } + } + } + } +} diff --git a/tests/compile-fail/eta.rs b/tests/compile-fail/eta.rs index a744489fa9c..c932f8f9a0f 100644 --- a/tests/compile-fail/eta.rs +++ b/tests/compile-fail/eta.rs @@ -18,6 +18,7 @@ fn main() { //~| SUGGESTION let c = Some(1u8).map({1+2; foo}); let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted + //~^ WARN needless_borrow unsafe { Some(1u8).map(|a| unsafe_fn(a)); // unsafe fn } diff --git a/tests/compile-fail/needless_borrow.rs b/tests/compile-fail/needless_borrow.rs new file mode 100644 index 00000000000..242691aa268 --- /dev/null +++ b/tests/compile-fail/needless_borrow.rs @@ -0,0 +1,27 @@ +#![feature(plugin)] +#![plugin(clippy)] + +fn x(y: &i32) -> i32 { + *y +} + +#[deny(clippy)] +#[allow(unused_variables)] +fn main() { + let a = 5; + let b = x(&a); + let c = x(&&a); //~ ERROR: needless_borrow + let s = &String::from("hi"); + let s_ident = f(&s); // should not error, because `&String` implements Copy, but `String` does not + let g_val = g(&Vec::new()); // should not error, because `&Vec` derefs to `&[T]` + let vec = Vec::new(); + let vec_val = g(&vec); // should not error, because `&Vec` derefs to `&[T]` +} + +fn f(y: &T) -> T { + *y +} + +fn g(y: &[u8]) -> u8 { + y[0] +}