Merge pull request #1355 from philipturnbull/deref-addrof

Lint usage of `*&` and `*&mut`

Author: Manishearth

CHANGELOG.md

@@ -263,6 +263,7 @@ All notable changes to this project will be documented in this file.
 [`crosspointer_transmute`]: https://github.com/Manishearth/rust-clippy/wiki#crosspointer_transmute
 [`cyclomatic_complexity`]: https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity
 [`deprecated_semver`]: https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver
+[`deref_addrof`]: https://github.com/Manishearth/rust-clippy/wiki#deref_addrof
 [`derive_hash_xor_eq`]: https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq
 [`diverging_sub_expression`]: https://github.com/Manishearth/rust-clippy/wiki#diverging_sub_expression
 [`doc_markdown`]: https://github.com/Manishearth/rust-clippy/wiki#doc_markdown

README.md

@@ -172,7 +172,7 @@ transparently:
 
 ## Lints
 
-There are 178 lints included in this crate:
+There are 179 lints included in this crate:
 
 name                                                                                                                   | default | triggers on
 -----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -204,6 +204,7 @@ name
 [crosspointer_transmute](https://github.com/Manishearth/rust-clippy/wiki#crosspointer_transmute)                       | warn    | transmutes that have to or from types that are a pointer to the other
 [cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity)                         | warn    | functions that should be split up into multiple functions
 [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver)                                 | warn    | use of `#[deprecated(since = "x")]` where x is not semver
+[deref_addrof](https://github.com/Manishearth/rust-clippy/wiki#deref_addrof)                                           | warn    | use of `*&` or `*&mut` in an expression
 [derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq)                               | warn    | deriving `Hash` but implementing `PartialEq` explicitly
 [diverging_sub_expression](https://github.com/Manishearth/rust-clippy/wiki#diverging_sub_expression)                   | warn    | whether an expression contains a diverging sub expression
 [doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown)                                           | warn    | presence of `_`, `::` or camel-case outside backticks in documentation

clippy_lints/src/lib.rs

@@ -116,6 +116,7 @@ pub mod precedence;
 pub mod print;
 pub mod ptr;
 pub mod ranges;
+pub mod reference;
 pub mod regex;
 pub mod returns;
 pub mod serde;
@@ -267,6 +268,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box ok_if_let::Pass);
     reg.register_late_lint_pass(box if_let_redundant_pattern_matching::Pass);
     reg.register_late_lint_pass(box partialeq_ne_impl::Pass);
+    reg.register_early_lint_pass(box reference::Pass);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -432,6 +434,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         ptr::PTR_ARG,
         ranges::RANGE_STEP_BY_ZERO,
         ranges::RANGE_ZIP_WITH_LEN,
+        reference::DEREF_ADDROF,
         regex::INVALID_REGEX,
         regex::REGEX_MACRO,
         regex::TRIVIAL_REGEX,

clippy_lints/src/reference.rs

@@ -0,0 +1,55 @@
+use syntax::ast::{Expr,ExprKind,UnOp};
+use rustc::lint::*;
+use utils::{span_lint_and_then, snippet};
+
+/// **What it does:** Checks for usage of `*&` and `*&mut` in expressions.
+///
+/// **Why is this bad?** Immediately dereferencing a reference is no-op and
+/// makes the code less clear.
+///
+/// **Known problems:** Multiple dereference/addrof pairs are not handled so
+/// the suggested fix for `x = **&&y` is `x = *&y`, which is still incorrect.
+///
+/// **Example:**
+/// ```rust
+/// let a = f(*&mut b);
+/// let c = *&d;
+/// ```
+declare_lint! {
+    pub DEREF_ADDROF,
+    Warn,
+    "use of `*&` or `*&mut` in an expression"
+}
+
+pub struct Pass;
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(DEREF_ADDROF)
+    }
+}
+
+fn without_parens(mut e: &Expr) -> &Expr {
+    while let ExprKind::Paren(ref child_e) = e.node {
+        e = child_e;
+    }
+    e
+}
+
+impl EarlyLintPass for Pass {
+    fn check_expr(&mut self, cx: &EarlyContext, e: &Expr) {
+        if let ExprKind::Unary(UnOp::Deref, ref deref_target) = e.node {
+            if let ExprKind::AddrOf(_, ref addrof_target) = without_parens(deref_target).node {
+                span_lint_and_then(
+                    cx,
+                    DEREF_ADDROF,
+                    e.span,
+                    "immediately dereferencing a reference",
+                    |db| {
+                        db.span_suggestion(e.span, "try this",
+                                             format!("{}", snippet(cx, addrof_target.span, "_")));
+                    });
+            }
+        }
+    }
+}

tests/compile-fail/formatting.rs

@@ -5,6 +5,7 @@
 #![allow(unused_variables)]
 #![allow(unused_assignments)]
 #![allow(if_same_then_else)]
+#![allow(deref_addrof)]
 
 fn foo() -> bool { true }
 

tests/compile-fail/no_effect.rs

@@ -4,6 +4,7 @@
 #![deny(no_effect, unnecessary_operation)]
 #![allow(dead_code)]
 #![allow(path_statements)]
+#![allow(deref_addrof)]
 #![feature(untagged_unions)]
 
 struct Unit;

tests/compile-fail/reference.rs

@@ -0,0 +1,88 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+fn get_number() -> usize {
+    10
+}
+
+fn get_reference(n : &usize) -> &usize {
+    n
+}
+
+#[allow(many_single_char_names)]
+#[allow(unused_variables)]
+#[deny(deref_addrof)]
+fn main() {
+    let a = 10;
+    let aref = &a;
+
+    let b = *&a;
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = a;
+
+    let b = *&get_number();
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = get_number();
+
+    let b = *get_reference(&a);
+
+    let bytes : Vec<usize> = vec![1, 2, 3, 4];
+    let b = *&bytes[1..2][0];
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = bytes[1..2][0];
+
+    //This produces a suggestion of 'let b = (a);' which
+    //will trigger the 'unused_parens' lint
+    let b = *&(a);
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = (a)
+
+    let b = *(&a);
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = a;
+
+    let b = *((&a));
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = a
+
+    let b = *&&a;
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = &a;
+
+    let b = **&aref;
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = *aref;
+
+    //This produces a suggestion of 'let b = *&a;' which
+    //will trigger the 'deref_addrof' lint again
+    let b = **&&a;
+    //~^ERROR immediately dereferencing a reference
+    //~|HELP try this
+    //~|SUGGESTION let b = *&a;
+
+    {
+        let mut x = 10;
+        let y = *&mut x;
+        //~^ERROR immediately dereferencing a reference
+        //~|HELP try this
+        //~|SUGGESTION let y = x;
+    }
+
+    {
+        //This produces a suggestion of 'let y = *&mut x' which
+        //will trigger the 'deref_addrof' lint again
+        let mut x = 10;
+        let y = **&mut &mut x;
+        //~^ERROR immediately dereferencing a reference
+        //~|HELP try this
+        //~|SUGGESTION let y = *&mut x;
+    }
+}