From 580ae5a879073d535c60857b1fbc5204ad2c810e Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 19:47:51 +0200
Subject: [PATCH 01/29] Use `span_suggestion` in `FLOAT_CMP`

---
 clippy_lints/src/misc.rs        | 21 +++++++++-------
 tests/compile-fail/float_cmp.rs | 44 +++++++++++++++++++++++++--------
 2 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 495325764690..e57cd50899ef 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -164,15 +164,18 @@ impl LateLintPass for FloatCmp {
                         return;
                     }
                 }
-                span_lint(cx,
-                          FLOAT_CMP,
-                          expr.span,
-                          &format!("{}-comparison of f32 or f64 detected. Consider changing this to `({} - {}).abs() < \
-                                    epsilon` for some suitable value of epsilon. \
-                                    std::f32::EPSILON and std::f64::EPSILON are available.",
-                                   op.as_str(),
-                                   snippet(cx, left.span, ".."),
-                                   snippet(cx, right.span, "..")));
+                span_lint_and_then(cx,
+                                   FLOAT_CMP,
+                                   expr.span,
+                                   "strict comparison of f32 or f64",
+                                   |db| {
+                    db.span_suggestion(expr.span,
+                                       "consider comparing them within some error",
+                                       format!("({} - {}).abs() < error",
+                                               snippet(cx, left.span, ".."),
+                                               snippet(cx, right.span, "..")));
+                    db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
+                });
             }
         }
     }
diff --git a/tests/compile-fail/float_cmp.rs b/tests/compile-fail/float_cmp.rs
index 9f611dd3fd91..cf8cefb3af3e 100644
--- a/tests/compile-fail/float_cmp.rs
+++ b/tests/compile-fail/float_cmp.rs
@@ -40,23 +40,47 @@ fn main() {
     ZERO == 0.0; //no error, comparison with zero is ok
     ZERO + ZERO != 1.0; //no error, comparison with zero is ok
 
-    ONE == 1f32; //~ERROR ==-comparison of f32 or f64
-    ONE == (1.0 + 0.0); //~ERROR ==-comparison of f32 or f64
-
-    ONE + ONE == (ZERO + ONE + ONE); //~ERROR ==-comparison of f32 or f64
-
-    ONE != 2.0; //~ERROR !=-comparison of f32 or f64
+    ONE == 1f32;
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (ONE - 1f32).abs() < error
+    ONE == (1.0 + 0.0);
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (ONE - (1.0 + 0.0)).abs() < error
+
+    ONE + ONE == (ZERO + ONE + ONE);
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (ONE + ONE - (ZERO + ONE + ONE)).abs() < error
+
+    ONE != 2.0;
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (ONE - 2.0).abs() < error
     ONE != 0.0; // no error, comparison with zero is ok
-    twice(ONE) != ONE; //~ERROR !=-comparison of f32 or f64
-    ONE as f64 != 2.0; //~ERROR !=-comparison of f32 or f64
+    twice(ONE) != ONE;
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (twice(ONE) - ONE).abs() < error
+    ONE as f64 != 2.0;
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (ONE as f64 - 2.0).abs() < error
     ONE as f64 != 0.0; // no error, comparison with zero is ok
 
     let x : f64 = 1.0;
 
-    x == 1.0; //~ERROR ==-comparison of f32 or f64
+    x == 1.0;
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (x - 1.0).abs() < error
     x != 0f64; // no error, comparison with zero is ok
 
-    twice(x) != twice(ONE as f64); //~ERROR !=-comparison of f32 or f64
+    twice(x) != twice(ONE as f64);
+    //~^ ERROR strict comparison of f32 or f64
+    //~| HELP within some error
+    //~| SUGGESTION (twice(x) - twice(ONE as f64)).abs() < error
 
 
     x < 0.0; // no errors, lower or greater comparisons need no fuzzyness

From 9811dea2377cfe835dc22f8c129bf83a75669319 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 21:23:21 +0200
Subject: [PATCH 02/29] Add a module to pretty-print suggestions

---
 clippy_lints/src/lib.rs          |   1 +
 clippy_lints/src/utils/higher.rs |  28 ++++
 clippy_lints/src/utils/mod.rs    |   4 +-
 clippy_lints/src/utils/sugg.rs   | 258 +++++++++++++++++++++++++++++++
 4 files changed, 290 insertions(+), 1 deletion(-)
 create mode 100644 clippy_lints/src/utils/higher.rs
 create mode 100644 clippy_lints/src/utils/sugg.rs

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index e980f4d081df..b22531576aae 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -3,6 +3,7 @@
 #![feature(box_syntax)]
 #![feature(collections)]
 #![feature(custom_attribute)]
+#![feature(dotdot_in_tuple_patterns)]
 #![feature(iter_arith)]
 #![feature(question_mark)]
 #![feature(rustc_private)]
diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs
new file mode 100644
index 000000000000..5df91ee9ab40
--- /dev/null
+++ b/clippy_lints/src/utils/higher.rs
@@ -0,0 +1,28 @@
+//! This module contains functions for retrieve the original AST from lowered `hir`.
+
+use rustc::hir;
+use syntax::ast;
+
+/// Convert a hir binary operator to the corresponding `ast` type.
+pub fn binop(op: hir::BinOp_) -> ast::BinOpKind {
+    match op {
+        hir::BiEq => ast::BinOpKind::Eq,
+        hir::BiGe => ast::BinOpKind::Ge,
+        hir::BiGt => ast::BinOpKind::Gt,
+        hir::BiLe => ast::BinOpKind::Le,
+        hir::BiLt => ast::BinOpKind::Lt,
+        hir::BiNe => ast::BinOpKind::Ne,
+        hir::BiOr => ast::BinOpKind::Or,
+        hir::BiAdd => ast::BinOpKind::Add,
+        hir::BiAnd => ast::BinOpKind::And,
+        hir::BiBitAnd => ast::BinOpKind::BitAnd,
+        hir::BiBitOr => ast::BinOpKind::BitOr,
+        hir::BiBitXor => ast::BinOpKind::BitXor,
+        hir::BiDiv => ast::BinOpKind::Div,
+        hir::BiMul => ast::BinOpKind::Mul,
+        hir::BiRem => ast::BinOpKind::Rem,
+        hir::BiShl => ast::BinOpKind::Shl,
+        hir::BiShr => ast::BinOpKind::Shr,
+        hir::BiSub => ast::BinOpKind::Sub,
+    }
+}
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 9eb54c2a0139..6d41d5039f74 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -18,12 +18,14 @@ use syntax::codemap::{ExpnInfo, Span, ExpnFormat};
 use syntax::errors::DiagnosticBuilder;
 use syntax::ptr::P;
 
+pub mod cargo;
 pub mod comparisons;
 pub mod conf;
+pub mod higher;
 mod hir;
 pub mod paths;
+pub mod sugg;
 pub use self::hir::{SpanlessEq, SpanlessHash};
-pub mod cargo;
 
 pub type MethodArgs = HirVec<P<Expr>>;
 
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
new file mode 100644
index 000000000000..6667598b009a
--- /dev/null
+++ b/clippy_lints/src/utils/sugg.rs
@@ -0,0 +1,258 @@
+use rustc::hir;
+use rustc::lint::{EarlyContext, LateContext};
+use std::borrow::Cow;
+use std;
+use syntax::ast;
+use syntax::util::parser::AssocOp;
+use utils::{higher, snippet};
+
+/// A helper type to build suggestion correctly handling parenthesis.
+pub enum Sugg<'a> {
+    /// An expression that never needs parenthesis such as `1337` or `[0; 42]`.
+    NonParen(Cow<'a, str>),
+    /// An expression that does not fit in other variants.
+    MaybeParen(Cow<'a, str>),
+    /// A binary operator expression, including `as`-casts and explicit type coercion.
+    BinOp(AssocOp, Cow<'a, str>),
+}
+
+impl<'a> std::fmt::Display for Sugg<'a> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
+        match *self {
+            Sugg::NonParen(ref s) | Sugg::MaybeParen(ref s) | Sugg::BinOp(_, ref s) => {
+                s.fmt(f)
+            }
+        }
+    }
+}
+
+impl<'a> Sugg<'a> {
+    pub fn hir(cx: &LateContext, expr: &'a hir::Expr, default: &'a str) -> Sugg<'a> {
+        let snippet = snippet(cx, expr.span, default);
+
+        match expr.node {
+            hir::ExprAddrOf(..) |
+            hir::ExprBox(..) |
+            hir::ExprClosure(..) |
+            hir::ExprIf(..) |
+            hir::ExprUnary(..) |
+            hir::ExprMatch(..) => Sugg::MaybeParen(snippet),
+            hir::ExprAgain(..) |
+            hir::ExprBlock(..) |
+            hir::ExprBreak(..) |
+            hir::ExprCall(..) |
+            hir::ExprField(..) |
+            hir::ExprIndex(..) |
+            hir::ExprInlineAsm(..) |
+            hir::ExprLit(..) |
+            hir::ExprLoop(..) |
+            hir::ExprMethodCall(..) |
+            hir::ExprPath(..) |
+            hir::ExprRepeat(..) |
+            hir::ExprRet(..) |
+            hir::ExprStruct(..) |
+            hir::ExprTup(..) |
+            hir::ExprTupField(..) |
+            hir::ExprVec(..) |
+            hir::ExprWhile(..) => Sugg::NonParen(snippet),
+            hir::ExprAssign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
+            hir::ExprAssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet),
+            hir::ExprBinary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet),
+            hir::ExprCast(..) => Sugg::BinOp(AssocOp::As, snippet),
+            hir::ExprType(..) => Sugg::BinOp(AssocOp::Colon, snippet),
+        }
+    }
+
+    pub fn ast(cx: &EarlyContext, expr: &'a ast::Expr, default: &'a str) -> Sugg<'a> {
+        use syntax::ast::RangeLimits;
+
+        let snippet = snippet(cx, expr.span, default);
+
+        match expr.node {
+            ast::ExprKind::AddrOf(..) |
+            ast::ExprKind::Box(..) |
+            ast::ExprKind::Closure(..) |
+            ast::ExprKind::If(..) |
+            ast::ExprKind::IfLet(..) |
+            ast::ExprKind::InPlace(..) |
+            ast::ExprKind::Unary(..) |
+            ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet),
+            ast::ExprKind::Again(..) |
+            ast::ExprKind::Block(..) |
+            ast::ExprKind::Break(..) |
+            ast::ExprKind::Call(..) |
+            ast::ExprKind::Field(..) |
+            ast::ExprKind::ForLoop(..) |
+            ast::ExprKind::Index(..) |
+            ast::ExprKind::InlineAsm(..) |
+            ast::ExprKind::Lit(..) |
+            ast::ExprKind::Loop(..) |
+            ast::ExprKind::Mac(..) |
+            ast::ExprKind::MethodCall(..) |
+            ast::ExprKind::Paren(..) |
+            ast::ExprKind::Path(..) |
+            ast::ExprKind::Repeat(..) |
+            ast::ExprKind::Ret(..) |
+            ast::ExprKind::Struct(..) |
+            ast::ExprKind::Try(..) |
+            ast::ExprKind::Tup(..) |
+            ast::ExprKind::TupField(..) |
+            ast::ExprKind::Vec(..) |
+            ast::ExprKind::While(..) |
+            ast::ExprKind::WhileLet(..) => Sugg::NonParen(snippet),
+            ast::ExprKind::Range(.., RangeLimits::HalfOpen) => Sugg::BinOp(AssocOp::DotDot, snippet),
+            ast::ExprKind::Range(.., RangeLimits::Closed) => Sugg::BinOp(AssocOp::DotDotDot, snippet),
+            ast::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
+            ast::ExprKind::AssignOp(op, ..) => Sugg::BinOp(astbinop2assignop(op), snippet),
+            ast::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(op.node), snippet),
+            ast::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet),
+            ast::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet),
+        }
+    }
+
+    /// Convenience method to create the `lhs && rhs` suggestion.
+    pub fn and(&self, rhs: &Self) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::And, self, rhs)
+    }
+}
+
+impl<'a, 'b> std::ops::Sub<&'b Sugg<'b>> for &'a Sugg<'a> {
+    type Output = Sugg<'static>;
+    fn sub(self, rhs: &'b Sugg<'b>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Sub, self, rhs)
+    }
+}
+
+struct ParenHelper<T> {
+    paren: bool,
+    wrapped: T,
+}
+
+impl<T> ParenHelper<T> {
+    fn new(paren: bool, wrapped: T) -> Self {
+        ParenHelper {
+            paren: paren,
+            wrapped: wrapped,
+        }
+    }
+}
+
+impl<T: std::fmt::Display> std::fmt::Display for ParenHelper<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
+        if self.paren {
+            write!(f, "({})", self.wrapped)
+        } else {
+            self.wrapped.fmt(f)
+        }
+    }
+}
+
+/// Build the string for `<lhs> <op> <rhs>` adding parenthesis when necessary.
+///
+/// Precedence of shift operator relative to other arithmetic operation is often confusing so
+/// parenthesis will always be added for a mix of these.
+pub fn make_binop(op: ast::BinOpKind, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> {
+    fn is_shift(op: &AssocOp) -> bool {
+        matches!(*op, AssocOp::ShiftLeft | AssocOp::ShiftRight)
+    }
+
+    fn is_arith(op: &AssocOp) -> bool {
+        matches!(*op, AssocOp::Add | AssocOp::Subtract | AssocOp::Multiply | AssocOp::Divide | AssocOp::Modulus)
+    }
+
+    fn needs_paren(op: &AssocOp, other: &AssocOp, dir: Associativity) -> bool {
+        other.precedence() < op.precedence() ||
+            (other.precedence() == op.precedence() &&
+                ((op != other && associativity(op) != dir) ||
+                 (op == other && associativity(op) != Associativity::Both))) ||
+             is_shift(op) && is_arith(other) ||
+             is_shift(other) && is_arith(op)
+    }
+
+    let aop = AssocOp::from_ast_binop(op);
+
+    let lhs_paren = if let Sugg::BinOp(ref lop, _) = *lhs {
+        needs_paren(&aop, lop, Associativity::Left)
+    } else {
+        false
+    };
+
+    let rhs_paren = if let Sugg::BinOp(ref rop, _) = *rhs {
+        needs_paren(&aop, rop, Associativity::Right)
+    } else {
+        false
+    };
+
+    Sugg::BinOp(aop,
+                format!("{} {} {}",
+                        ParenHelper::new(lhs_paren, lhs),
+                        op.to_string(),
+                        ParenHelper::new(rhs_paren, rhs)).into())
+}
+
+#[derive(PartialEq, Eq)]
+enum Associativity {
+    Both,
+    Left,
+    None,
+    Right,
+}
+
+/// Return the associativity/fixity of an operator. The difference with `AssocOp::fixity` is that
+/// an operator can be both left and right associative (such as `+`:
+/// `a + b + c == (a + b) + c == a + (b + c)`.
+///
+/// Chained `as` and explicit `:` type coercion never need inner parenthesis so they are considered
+/// associative.
+fn associativity(op: &AssocOp) -> Associativity {
+    use syntax::util::parser::AssocOp::*;
+
+    match *op {
+        Inplace | Assign | AssignOp(_) => Associativity::Right,
+        Add | BitAnd | BitOr | BitXor | LAnd | LOr | Multiply |
+        As | Colon => Associativity::Both,
+    Divide | Equal | Greater | GreaterEqual | Less | LessEqual | Modulus | NotEqual | ShiftLeft |
+        ShiftRight | Subtract => Associativity::Left,
+        DotDot | DotDotDot => Associativity::None
+    }
+}
+
+/// Convert a `hir::BinOp` to the corresponding assigning binary operator.
+fn hirbinop2assignop(op: hir::BinOp) -> AssocOp {
+    use rustc::hir::BinOp_::*;
+    use syntax::parse::token::BinOpToken::*;
+
+    AssocOp::AssignOp(match op.node {
+        BiAdd => Plus,
+        BiBitAnd => And,
+        BiBitOr => Or,
+        BiBitXor => Caret,
+        BiDiv => Slash,
+        BiMul => Star,
+        BiRem => Percent,
+        BiShl => Shl,
+        BiShr => Shr,
+        BiSub => Minus,
+        BiAnd | BiEq | BiGe | BiGt | BiLe | BiLt | BiNe | BiOr => panic!("This operator does not exist"),
+    })
+}
+
+/// Convert an `ast::BinOp` to the corresponding assigning binary operator.
+fn astbinop2assignop(op: ast::BinOp) -> AssocOp {
+    use syntax::ast::BinOpKind::*;
+    use syntax::parse::token::BinOpToken;
+
+    AssocOp::AssignOp(match op.node {
+        Add => BinOpToken::Plus,
+        BitAnd => BinOpToken::And,
+        BitOr => BinOpToken::Or,
+        BitXor => BinOpToken::Caret,
+        Div => BinOpToken::Slash,
+        Mul => BinOpToken::Star,
+        Rem => BinOpToken::Percent,
+        Shl => BinOpToken::Shl,
+        Shr => BinOpToken::Shr,
+        Sub => BinOpToken::Minus,
+        And | Eq | Ge | Gt | Le | Lt | Ne | Or => panic!("This operator does not exist"),
+    })
+}

From 8d58a928e5956c8531fecc04cb92bf7d63979605 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 21:24:20 +0200
Subject: [PATCH 03/29] Use `utils::sugg` in `ASSIGN_OPS`

---
 README.md                        |  2 +-
 clippy_lints/src/assign_ops.rs   | 43 +++++++++++++-------------------
 tests/compile-fail/assign_ops.rs | 20 +++++++++++++++
 3 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/README.md b/README.md
index 2f54835d5387..20c6c6a38a8a 100644
--- a/README.md
+++ b/README.md
@@ -25,7 +25,7 @@ name
 [almost_swapped](https://github.com/Manishearth/rust-clippy/wiki#almost_swapped)                                     | warn    | `foo = bar; bar = foo` sequence
 [approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant)                                   | warn    | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
 [assign_op_pattern](https://github.com/Manishearth/rust-clippy/wiki#assign_op_pattern)                               | warn    | assigning the result of an operation on a variable to that same variable
-[assign_ops](https://github.com/Manishearth/rust-clippy/wiki#assign_ops)                                             | allow   | Any assignment operation
+[assign_ops](https://github.com/Manishearth/rust-clippy/wiki#assign_ops)                                             | allow   | any assignment operation
 [bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask)                                         | warn    | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
 [blacklisted_name](https://github.com/Manishearth/rust-clippy/wiki#blacklisted_name)                                 | warn    | usage of a blacklisted/placeholder name
 [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr)             | warn    | braces can be eliminated in conditions that are expressions, e.g `if { true } ...`
diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs
index 1a5ca16b9c57..6ae76f9c3eff 100644
--- a/clippy_lints/src/assign_ops.rs
+++ b/clippy_lints/src/assign_ops.rs
@@ -1,13 +1,14 @@
 use rustc::hir;
 use rustc::lint::*;
 use utils::{span_lint_and_then, span_lint, snippet_opt, SpanlessEq, get_trait_def_id, implements_trait};
+use utils::{higher, sugg};
 
-/// **What it does:** This lint checks for `+=` operations and similar
+/// **What it does:** This lint checks for `+=` operations and similar.
 ///
-/// **Why is this bad?** Projects with many developers from languages without those operations
-///                      may find them unreadable and not worth their weight
+/// **Why is this bad?** Projects with many developers from languages without those operations may
+/// find them unreadable and not worth their weight.
 ///
-/// **Known problems:** Types implementing `OpAssign` don't necessarily implement `Op`
+/// **Known problems:** Types implementing `OpAssign` don't necessarily implement `Op`.
 ///
 /// **Example:**
 /// ```
@@ -15,14 +16,14 @@ use utils::{span_lint_and_then, span_lint, snippet_opt, SpanlessEq, get_trait_de
 /// ```
 declare_restriction_lint! {
     pub ASSIGN_OPS,
-    "Any assignment operation"
+    "any assignment operation"
 }
 
-/// **What it does:** Check for `a = a op b` or `a = b commutative_op a` patterns
+/// **What it does:** Check for `a = a op b` or `a = b commutative_op a` patterns.
 ///
-/// **Why is this bad?** These can be written as the shorter `a op= b`
+/// **Why is this bad?** These can be written as the shorter `a op= b`.
 ///
-/// **Known problems:** While forbidden by the spec, `OpAssign` traits may have implementations that differ from the regular `Op` impl
+/// **Known problems:** While forbidden by the spec, `OpAssign` traits may have implementations that differ from the regular `Op` impl.
 ///
 /// **Example:**
 ///
@@ -50,24 +51,14 @@ impl LateLintPass for AssignOps {
     fn check_expr(&mut self, cx: &LateContext, expr: &hir::Expr) {
         match expr.node {
             hir::ExprAssignOp(op, ref lhs, ref rhs) => {
-                if let (Some(l), Some(r)) = (snippet_opt(cx, lhs.span), snippet_opt(cx, rhs.span)) {
-                    span_lint_and_then(cx, ASSIGN_OPS, expr.span, "assign operation detected", |db| {
-                        match rhs.node {
-                            hir::ExprBinary(op2, _, _) if op2 != op => {
-                                db.span_suggestion(expr.span,
-                                                   "replace it with",
-                                                   format!("{} = {} {} ({})", l, l, op.node.as_str(), r));
-                            }
-                            _ => {
-                                db.span_suggestion(expr.span,
-                                                   "replace it with",
-                                                   format!("{} = {} {} {}", l, l, op.node.as_str(), r));
-                            }
-                        }
-                    });
-                } else {
-                    span_lint(cx, ASSIGN_OPS, expr.span, "assign operation detected");
-                }
+                span_lint_and_then(cx, ASSIGN_OPS, expr.span, "assign operation detected", |db| {
+                    let lhs = &sugg::Sugg::hir(cx, lhs, "..");
+                    let rhs = &sugg::Sugg::hir(cx, rhs, "..");
+
+                    db.span_suggestion(expr.span,
+                                       "replace it with",
+                                       format!("{} = {}", lhs, sugg::make_binop(higher::binop(op.node), lhs, rhs)));
+                });
             }
             hir::ExprAssign(ref assignee, ref e) => {
                 if let hir::ExprBinary(op, ref l, ref r) = e.node {
diff --git a/tests/compile-fail/assign_ops.rs b/tests/compile-fail/assign_ops.rs
index 84d868ecfcc4..2b69e110f43d 100644
--- a/tests/compile-fail/assign_ops.rs
+++ b/tests/compile-fail/assign_ops.rs
@@ -8,15 +8,31 @@ fn main() {
     i += 2; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i + 2
+    i += 2 + 17; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i + 2 + 17
     i -= 6; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i - 6
+    i -= 2 - 1;
+    //~^ ERROR assign operation detected
+    //~| HELP replace it with
+    //~| SUGGESTION i = i - (2 - 1)
     i *= 5; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i * 5
+    i *= 1+5; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i * (1+5)
     i /= 32; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i / 32
+    i /= 32 | 5; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i / (32 | 5)
+    i /= 32 / 5; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i / (32 / 5)
     i %= 42; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i % 42
@@ -26,6 +42,10 @@ fn main() {
     i <<= 9 + 6 - 7; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i << (9 + 6 - 7)
+    i += 1 << 5;
+    //~^ ERROR assign operation detected
+    //~| HELP replace it with
+    //~| SUGGESTION i = i + (1 << 5)
 }
 
 #[allow(dead_code, unused_assignments)]

From 2e8edde6e9b42b3a8c7e8aefb2ad6c86be404915 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 21:25:23 +0200
Subject: [PATCH 04/29] Use `utils::sugg` in `FLOAT_CMP`

---
 clippy_lints/src/misc.rs        | 8 +++++---
 tests/compile-fail/float_cmp.rs | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index e57cd50899ef..9c04ad6f3e45 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -13,6 +13,7 @@ use utils::{
     get_item_name, get_parent_expr, implements_trait, in_macro, is_integer_literal, match_path,
     snippet, span_lint, span_lint_and_then, walk_ptrs_ty
 };
+use utils::sugg::Sugg;
 
 /// **What it does:** This lint checks for function arguments and let bindings denoted as `ref`.
 ///
@@ -169,11 +170,12 @@ impl LateLintPass for FloatCmp {
                                    expr.span,
                                    "strict comparison of f32 or f64",
                                    |db| {
+                    let lhs = &Sugg::hir(cx, left, "..");
+                    let rhs = &Sugg::hir(cx, right, "..");
+
                     db.span_suggestion(expr.span,
                                        "consider comparing them within some error",
-                                       format!("({} - {}).abs() < error",
-                                               snippet(cx, left.span, ".."),
-                                               snippet(cx, right.span, "..")));
+                                       format!("({}).abs() < error", lhs - rhs));
                     db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
                 });
             }
diff --git a/tests/compile-fail/float_cmp.rs b/tests/compile-fail/float_cmp.rs
index cf8cefb3af3e..314cc7214255 100644
--- a/tests/compile-fail/float_cmp.rs
+++ b/tests/compile-fail/float_cmp.rs
@@ -44,12 +44,12 @@ fn main() {
     //~^ ERROR strict comparison of f32 or f64
     //~| HELP within some error
     //~| SUGGESTION (ONE - 1f32).abs() < error
-    ONE == (1.0 + 0.0);
+    ONE == 1.0 + 0.0;
     //~^ ERROR strict comparison of f32 or f64
     //~| HELP within some error
     //~| SUGGESTION (ONE - (1.0 + 0.0)).abs() < error
 
-    ONE + ONE == (ZERO + ONE + ONE);
+    ONE + ONE == ZERO + ONE + ONE;
     //~^ ERROR strict comparison of f32 or f64
     //~| HELP within some error
     //~| SUGGESTION (ONE + ONE - (ZERO + ONE + ONE)).abs() < error

From 66808c1e776ca9303cc04abce4b3b95ee5b669fc Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 21:26:26 +0200
Subject: [PATCH 05/29] Use `utils::sugg` in `COLLAPSIBLE_IF`

---
 clippy_lints/src/collapsible_if.rs   | 27 +++++----------------
 tests/compile-fail/collapsible_if.rs | 36 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs
index 2921bc2769c2..350a3cc3ef21 100644
--- a/clippy_lints/src/collapsible_if.rs
+++ b/clippy_lints/src/collapsible_if.rs
@@ -13,11 +13,10 @@
 //! This lint is **warn** by default
 
 use rustc::lint::*;
-use std::borrow::Cow;
-use syntax::codemap::Spanned;
 use syntax::ast;
 
-use utils::{in_macro, snippet, snippet_block, span_lint_and_then};
+use utils::{in_macro, snippet_block, span_lint_and_then};
+use utils::sugg::Sugg;
 
 /// **What it does:** This lint checks for nested `if`-statements which can be collapsed by
 /// `&&`-combining their conditions and for `else { if .. }` expressions that can be collapsed to
@@ -103,31 +102,17 @@ fn check_collapsible_no_if_let(
             return;
         }
         span_lint_and_then(cx, COLLAPSIBLE_IF, expr.span, "this if statement can be collapsed", |db| {
+            let lhs = Sugg::ast(cx, check, "..");
+            let rhs = Sugg::ast(cx, check_inner, "..");
             db.span_suggestion(expr.span,
                                "try",
-                               format!("if {} && {} {}",
-                                       check_to_string(cx, check),
-                                       check_to_string(cx, check_inner),
+                               format!("if {} {}",
+                                       lhs.and(&rhs),
                                        snippet_block(cx, content.span, "..")));
         });
     }}
 }
 
-fn requires_brackets(e: &ast::Expr) -> bool {
-    match e.node {
-        ast::ExprKind::Binary(Spanned { node: n, .. }, _, _) if n == ast::BinOpKind::Eq => false,
-        _ => true,
-    }
-}
-
-fn check_to_string(cx: &EarlyContext, e: &ast::Expr) -> Cow<'static, str> {
-    if requires_brackets(e) {
-        format!("({})", snippet(cx, e.span, "..")).into()
-    } else {
-        snippet(cx, e.span, "..")
-    }
-}
-
 fn single_stmt_of_block(block: &ast::Block) -> Option<&ast::Expr> {
     if block.stmts.len() == 1 && block.expr.is_none() {
         if let ast::StmtKind::Expr(ref expr, _) = block.stmts[0].node {
diff --git a/tests/compile-fail/collapsible_if.rs b/tests/compile-fail/collapsible_if.rs
index ea2ef284f38f..f7e6f15b1212 100644
--- a/tests/compile-fail/collapsible_if.rs
+++ b/tests/compile-fail/collapsible_if.rs
@@ -23,6 +23,42 @@ fn main() {
         }
     }
 
+    if x == "hello" && x == "world" {
+    //~^ ERROR this if statement can be collapsed
+    //~| HELP try
+    //~| SUGGESTION if x == "hello" && x == "world" && (y == "world" || y == "hello") {
+        if y == "world" || y == "hello" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" || x == "world" {
+    //~^ ERROR this if statement can be collapsed
+    //~| HELP try
+    //~| SUGGESTION if (x == "hello" || x == "world") && y == "world" && y == "hello" {
+        if y == "world" && y == "hello" {
+            println!("Hello world!");
+        }
+    }
+
+    if x == "hello" && x == "world" {
+    //~^ ERROR this if statement can be collapsed
+    //~| HELP try
+    //~| SUGGESTION if x == "hello" && x == "world" && y == "world" && y == "hello" {
+        if y == "world" && y == "hello" {
+            println!("Hello world!");
+        }
+    }
+
+    if 42 == 1337 {
+    //~^ ERROR this if statement can be collapsed
+    //~| HELP try
+    //~| SUGGESTION if 42 == 1337 && 'a' != 'A' {
+        if 'a' != 'A' {
+            println!("world!")
+        }
+    }
+
     // Collaspe `else { if .. }` to `else if ..`
     if x == "hello" {
         print!("Hello ");

From 7a1fc9fce5e9d05864567fd3c36502198588a623 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 21:50:21 +0200
Subject: [PATCH 06/29] Use `utils::sugg` in `MATCH_BOOL`

---
 clippy_lints/src/matches.rs    |  6 ++++--
 clippy_lints/src/utils/sugg.rs | 16 ++++++++++++++++
 tests/compile-fail/matches.rs  | 13 +++++++++++--
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index e578fbf6d68c..8fddcb84922b 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -10,6 +10,7 @@ use syntax::ast::LitKind;
 use syntax::codemap::Span;
 use utils::paths;
 use utils::{match_type, snippet, span_note_and_lint, span_lint_and_then, in_external_macro, expr_block};
+use utils::sugg::Sugg;
 
 /// **What it does:** This lint checks for matches with a single arm where an `if let` will usually suffice.
 ///
@@ -262,8 +263,9 @@ fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
                         Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
                     }
                     (true, false) => {
-                        Some(format!("try\nif !{} {}",
-                                     snippet(cx, ex.span, "b"),
+                        let test = &Sugg::hir(cx, ex, "..");
+                        Some(format!("if {} {}",
+                                     !test,
                                      expr_block(cx, false_expr, None, "..")))
                     }
                     (true, true) => None,
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 6667598b009a..f4b359b35de1 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -123,6 +123,13 @@ impl<'a, 'b> std::ops::Sub<&'b Sugg<'b>> for &'a Sugg<'a> {
     }
 }
 
+impl<'a> std::ops::Not for &'a Sugg<'a> {
+    type Output = Sugg<'static>;
+    fn not(self) -> Sugg<'static> {
+        make_unop("!", self)
+    }
+}
+
 struct ParenHelper<T> {
     paren: bool,
     wrapped: T,
@@ -147,6 +154,15 @@ impl<T: std::fmt::Display> std::fmt::Display for ParenHelper<T> {
     }
 }
 
+/// Build the string for `<op> <expr>` adding parenthesis when necessary.
+///
+/// For convenience, the operator is taken as a string because all unary operators have the same
+/// precedence.
+pub fn make_unop(op: &str, expr: &Sugg) -> Sugg<'static> {
+    let needs_paren = !matches!(*expr, Sugg::NonParen(..));
+    Sugg::MaybeParen(format!("{}{}", op, ParenHelper::new(needs_paren, expr)).into())
+}
+
 /// Build the string for `<lhs> <op> <rhs>` adding parenthesis when necessary.
 ///
 /// Precedence of shift operator relative to other arithmetic operation is often confusing so
diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs
index 989106a5a16e..e49aeaa6ec86 100644
--- a/tests/compile-fail/matches.rs
+++ b/tests/compile-fail/matches.rs
@@ -130,7 +130,7 @@ fn match_bool() {
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
     //~| HELP try
-    //~^^ SUGGESTION if !test { println!("Noooo!"); };
+    //~| SUGGESTION if !test { println!("Noooo!"); };
         true => (),
         false => { println!("Noooo!"); }
     };
@@ -138,7 +138,16 @@ fn match_bool() {
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
     //~| HELP try
-    //~^^ SUGGESTION if !test { println!("Noooo!"); };
+    //~| SUGGESTION if !test { println!("Noooo!"); };
+        false => { println!("Noooo!"); }
+        _ => (),
+    };
+
+    match test && test {
+    //~^ ERROR you seem to be trying to match on a boolean expression
+    //~| HELP try
+    //~| SUGGESTION if !(test && test) { println!("Noooo!"); };
+    //~| ERROR equal expressions as operands
         false => { println!("Noooo!"); }
         _ => (),
     };

From a3c505551fc1d5ed4270357e6a616b01b45e9a7d Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 22:35:58 +0200
Subject: [PATCH 07/29] Cleanup

---
 clippy_lints/src/methods.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs
index a959dbe1d6ad..8a1ebd162af8 100644
--- a/clippy_lints/src/methods.rs
+++ b/clippy_lints/src/methods.rs
@@ -672,20 +672,20 @@ fn lint_cstring_as_ptr(cx: &LateContext, expr: &hir::Expr, new: &hir::Expr, unwr
 #[allow(ptr_arg)]
 // Type of MethodArgs is potentially a Vec
 fn lint_iter_nth(cx: &LateContext, expr: &hir::Expr, iter_args: &MethodArgs, is_mut: bool){
-    let caller_type;
     let mut_str = if is_mut { "_mut" } else {""};
-    if let Some(_) = derefs_to_slice(cx, &iter_args[0], &cx.tcx.expr_ty(&iter_args[0])) {
-        caller_type = "slice";
+    let caller_type = if let Some(_) = derefs_to_slice(cx, &iter_args[0], &cx.tcx.expr_ty(&iter_args[0])) {
+        "slice"
     }
     else if match_type(cx, cx.tcx.expr_ty(&iter_args[0]), &paths::VEC) {
-        caller_type = "Vec";
+        "Vec"
     }
     else if match_type(cx, cx.tcx.expr_ty(&iter_args[0]), &paths::VEC_DEQUE) {
-        caller_type = "VecDeque";
+        "VecDeque"
     }
     else {
         return; // caller is not a type that we want to lint
-    }
+    };
+
     span_lint(
         cx,
         ITER_NTH,

From 702398802097e5081b15b1a1250e88f6510c3eef Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 22:37:10 +0200
Subject: [PATCH 08/29] Use `utils::sugg` in `TOPLEVEL_REF_ARG`

---
 clippy_lints/src/misc.rs               | 5 +++--
 clippy_lints/src/utils/sugg.rs         | 5 +++++
 tests/compile-fail/toplevel_ref_arg.rs | 7 ++++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 9c04ad6f3e45..e5bfc3c34b1c 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -72,12 +72,13 @@ impl LateLintPass for TopLevelRefPass {
                 l.pat.span,
                 "`ref` on an entire `let` pattern is discouraged, take a reference with & instead",
                 |db| {
+                    let init = &Sugg::hir(cx, init, "..");
                     db.span_suggestion(s.span,
                                        "try",
-                                       format!("let {}{} = &{};",
+                                       format!("let {}{} = {};",
                                                snippet(cx, i.span, "_"),
                                                tyopt,
-                                               snippet(cx, init.span, "_")));
+                                               init.addr()));
                 }
             );
         }}
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index f4b359b35de1..a28c00efcae5 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -114,6 +114,11 @@ impl<'a> Sugg<'a> {
     pub fn and(&self, rhs: &Self) -> Sugg<'static> {
         make_binop(ast::BinOpKind::And, self, rhs)
     }
+
+    /// Convenience method to create the `&<expr>` suggestion.
+    pub fn addr(&self) -> Sugg<'static> {
+        make_unop("&", self)
+    }
 }
 
 impl<'a, 'b> std::ops::Sub<&'b Sugg<'b>> for &'a Sugg<'a> {
diff --git a/tests/compile-fail/toplevel_ref_arg.rs b/tests/compile-fail/toplevel_ref_arg.rs
index de1556ed0e39..b2240cffb1a6 100644
--- a/tests/compile-fail/toplevel_ref_arg.rs
+++ b/tests/compile-fail/toplevel_ref_arg.rs
@@ -20,11 +20,16 @@ fn main() {
   //~| HELP try
   //~| SUGGESTION let x = &1;
 
-  let ref y : (&_, u8) = (&1, 2);
+  let ref y: (&_, u8) = (&1, 2);
   //~^ ERROR `ref` on an entire `let` pattern is discouraged
   //~| HELP try
   //~| SUGGESTION let y: (&_, u8) = &(&1, 2);
 
+  let ref z = 1 + 2;
+  //~^ ERROR `ref` on an entire `let` pattern is discouraged
+  //~| HELP try
+  //~| SUGGESTION let z = &(1 + 2);
+
   let (ref x, _) = (1,2); // okay, not top level
   println!("The answer is {}.", x);
 }

From 169b63a84abf1598a83038fcdedceba40255288a Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 22:45:58 +0200
Subject: [PATCH 09/29] Improve `TOPLEVEL_REF_ARG` message

---
 clippy_lints/src/misc.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index e5bfc3c34b1c..0931d5aca6b8 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -70,7 +70,7 @@ impl LateLintPass for TopLevelRefPass {
             span_lint_and_then(cx,
                 TOPLEVEL_REF_ARG,
                 l.pat.span,
-                "`ref` on an entire `let` pattern is discouraged, take a reference with & instead",
+                "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
                 |db| {
                     let init = &Sugg::hir(cx, init, "..");
                     db.span_suggestion(s.span,

From ebf72cb67f3dcdd05ce812b020346dc318624b8a Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 23:02:15 +0200
Subject: [PATCH 10/29] Use `util::sugg` in `TRANSMUTE_PTR_TO_REF`

---
 clippy_lints/src/transmute.rs   | 32 +++++++++++++-------------------
 tests/compile-fail/transmute.rs |  1 +
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs
index bf6af4411b89..12d2184693a4 100644
--- a/clippy_lints/src/transmute.rs
+++ b/clippy_lints/src/transmute.rs
@@ -3,6 +3,7 @@ use rustc::ty::TypeVariants::{TyRawPtr, TyRef};
 use rustc::ty;
 use rustc::hir::*;
 use utils::{match_def_path, paths, snippet_opt, span_lint, span_lint_and_then};
+use utils::sugg;
 
 /// **What it does:** This lint checks for transmutes that can't ever be correct on any architecture
 ///
@@ -148,28 +149,21 @@ impl LateLintPass for Transmute {
                                     from_ty,
                                     to_ty),
                             |db| {
-                                if let Some(arg) = snippet_opt(cx, args[0].span) {
-                                    let (deref, cast) = if to_rty.mutbl == Mutability::MutMutable {
-                                        ("&mut *", "*mut")
-                                    } else {
-                                        ("&*", "*const")
-                                    };
+                                let arg = &sugg::Sugg::hir(cx, &args[0], "..");
+                                let (deref, cast) = if to_rty.mutbl == Mutability::MutMutable {
+                                    ("&mut *", "*mut")
+                                } else {
+                                    ("&*", "*const")
+                                };
 
 
-                                    let sugg = if from_pty.ty == to_rty.ty {
-                                        // Put things in parentheses if they are more complex
-                                        match args[0].node {
-                                            ExprPath(..) | ExprCall(..) | ExprMethodCall(..) | ExprBlock(..) => {
-                                                format!("{}{}", deref, arg)
-                                            }
-                                            _ => format!("{}({})", deref, arg)
-                                        }
-                                    } else {
-                                        format!("{}({} as {} {})", deref, arg, cast, to_rty.ty)
-                                    };
+                                let sugg = if from_pty.ty == to_rty.ty {
+                                    sugg::make_unop(deref, arg).to_string()
+                                } else {
+                                    format!("{}({} as {} {})", deref, arg, cast, to_rty.ty)
+                                };
 
-                                    db.span_suggestion(e.span, "try", sugg);
-                                }
+                                db.span_suggestion(e.span, "try", sugg);
                             },
                         ),
                         _ => return,
diff --git a/tests/compile-fail/transmute.rs b/tests/compile-fail/transmute.rs
index 4a120a6eedde..0dbd58b13089 100644
--- a/tests/compile-fail/transmute.rs
+++ b/tests/compile-fail/transmute.rs
@@ -62,6 +62,7 @@ unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
     //~^ ERROR transmute from a pointer type (`*mut T`) to a reference type (`&mut T`)
     //~| HELP try
     //~| SUGGESTION = &mut *(p as *mut T);
+    let _ = &mut *(p as *mut T);
 
     let _: &T = std::mem::transmute(o);
     //~^ ERROR transmute from a pointer type (`*const U`) to a reference type (`&T`)

From 92b04129fe6b1931f14199b01a5bfc78edb66a72 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 29 Jun 2016 23:16:44 +0200
Subject: [PATCH 11/29] Move `unsugar_range` to `utils::higher`

---
 clippy_lints/src/array_indexing.rs |  6 +-
 clippy_lints/src/loops.rs          |  7 +--
 clippy_lints/src/ranges.rs         |  5 +-
 clippy_lints/src/utils/higher.rs   | 89 ++++++++++++++++++++++++++++++
 clippy_lints/src/utils/mod.rs      | 89 +-----------------------------
 5 files changed, 99 insertions(+), 97 deletions(-)

diff --git a/clippy_lints/src/array_indexing.rs b/clippy_lints/src/array_indexing.rs
index f3b7297b2969..2c84c7cc1321 100644
--- a/clippy_lints/src/array_indexing.rs
+++ b/clippy_lints/src/array_indexing.rs
@@ -6,7 +6,7 @@ use rustc_const_eval::eval_const_expr_partial;
 use rustc_const_math::ConstInt;
 use rustc::hir::*;
 use syntax::ast::RangeLimits;
-use utils;
+use utils::{self, higher};
 
 /// **What it does:** Check for out of bounds array indexing with a constant index.
 ///
@@ -77,7 +77,7 @@ impl LateLintPass for ArrayIndexing {
                 }
 
                 // Index is a constant range
-                if let Some(range) = utils::unsugar_range(index) {
+                if let Some(range) = higher::range(index) {
                     let start = range.start
                         .map(|start| eval_const_expr_partial(cx.tcx, start, ExprTypeChecked, None))
                         .map(|v| v.ok());
@@ -94,7 +94,7 @@ impl LateLintPass for ArrayIndexing {
                 }
             }
 
-            if let Some(range) = utils::unsugar_range(index) {
+            if let Some(range) = higher::range(index) {
                 // Full ranges are always valid
                 if range.start.is_none() && range.end.is_none() {
                     return;
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 1816d88bddb5..e945afbf659e 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -14,10 +14,9 @@ use std::collections::HashMap;
 use syntax::ast;
 
 use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro,
-            span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, unsugar_range,
+            span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
             walk_ptrs_ty, recover_for_loop};
 use utils::paths;
-use utils::UnsugaredRange;
 
 /// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index.
 ///
@@ -333,7 +332,7 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E
 /// Check for looping over a range and then indexing a sequence with it.
 /// The iteratee must be a range literal.
 fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) {
-    if let Some(UnsugaredRange { start: Some(ref start), ref end, .. }) = unsugar_range(arg) {
+    if let Some(higher::Range { start: Some(ref start), ref end, .. }) = higher::range(arg) {
         // the var must be a single name
         if let PatKind::Binding(_, ref ident, _) = pat.node {
             let mut visitor = VarVisitor {
@@ -427,7 +426,7 @@ fn is_len_call(expr: &Expr, var: &Name) -> bool {
 
 fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) {
     // if this for loop is iterating over a two-sided range...
-    if let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), limits }) = unsugar_range(arg) {
+    if let Some(higher::Range { start: Some(ref start), end: Some(ref end), limits }) = higher::range(arg) {
         // ...and both sides are compile-time constant integers...
         if let Ok(start_idx) = eval_const_expr_partial(cx.tcx, start, ExprTypeChecked, None) {
             if let Ok(end_idx) = eval_const_expr_partial(cx.tcx, end, ExprTypeChecked, None) {
diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs
index 8eacbadf8df6..a042c966d942 100644
--- a/clippy_lints/src/ranges.rs
+++ b/clippy_lints/src/ranges.rs
@@ -1,7 +1,8 @@
 use rustc::lint::*;
 use rustc::hir::*;
 use syntax::codemap::Spanned;
-use utils::{is_integer_literal, match_type, paths, snippet, span_lint, unsugar_range, UnsugaredRange};
+use utils::{is_integer_literal, match_type, paths, snippet, span_lint};
+use utils::higher;
 
 /// **What it does:** This lint checks for iterating over ranges with a `.step_by(0)`, which never terminates.
 ///
@@ -54,7 +55,7 @@ impl LateLintPass for StepByZero {
                     let ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = *iter,
                     iter_name.as_str() == "iter",
                     // range expression in .zip() call: 0..x.len()
-                    let Some(UnsugaredRange { start: Some(ref start), end: Some(ref end), .. }) = unsugar_range(zip_arg),
+                    let Some(higher::Range { start: Some(ref start), end: Some(ref end), .. }) = higher::range(zip_arg),
                     is_integer_literal(start, 0),
                     // .len() call
                     let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = end.node,
diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs
index 5df91ee9ab40..979c044cdbf9 100644
--- a/clippy_lints/src/utils/higher.rs
+++ b/clippy_lints/src/utils/higher.rs
@@ -2,6 +2,7 @@
 
 use rustc::hir;
 use syntax::ast;
+use utils::{match_path, paths};
 
 /// Convert a hir binary operator to the corresponding `ast` type.
 pub fn binop(op: hir::BinOp_) -> ast::BinOpKind {
@@ -26,3 +27,91 @@ pub fn binop(op: hir::BinOp_) -> ast::BinOpKind {
         hir::BiSub => ast::BinOpKind::Sub,
     }
 }
+
+/// Represent a range akin to `ast::ExprKind::Range`.
+#[derive(Debug, Copy, Clone)]
+pub struct Range<'a> {
+    pub start: Option<&'a hir::Expr>,
+    pub end: Option<&'a hir::Expr>,
+    pub limits: ast::RangeLimits,
+}
+
+/// Higher a `hir` range to something similar to `ast::ExprKind::Range`.
+pub fn range(expr: &hir::Expr) -> Option<Range> {
+    // To be removed when ranges get stable.
+    fn unwrap_unstable(expr: &hir::Expr) -> &hir::Expr {
+        if let hir::ExprBlock(ref block) = expr.node {
+            if block.rules == hir::BlockCheckMode::PushUnstableBlock || block.rules == hir::BlockCheckMode::PopUnstableBlock {
+                if let Some(ref expr) = block.expr {
+                    return expr;
+                }
+            }
+        }
+
+        expr
+    }
+
+    fn get_field<'a>(name: &str, fields: &'a [hir::Field]) -> Option<&'a hir::Expr> {
+        let expr = &fields.iter()
+                          .find(|field| field.name.node.as_str() == name)
+                          .unwrap_or_else(|| panic!("missing {} field for range", name))
+                          .expr;
+
+        Some(unwrap_unstable(expr))
+    }
+
+    // The range syntax is expanded to literal paths starting with `core` or `std` depending on
+    // `#[no_std]`. Testing both instead of resolving the paths.
+
+    match unwrap_unstable(expr).node {
+        hir::ExprPath(None, ref path) => {
+            if match_path(path, &paths::RANGE_FULL_STD) || match_path(path, &paths::RANGE_FULL) {
+                Some(Range {
+                    start: None,
+                    end: None,
+                    limits: ast::RangeLimits::HalfOpen,
+                })
+            } else {
+                None
+            }
+        }
+        hir::ExprStruct(ref path, ref fields, None) => {
+            if match_path(path, &paths::RANGE_FROM_STD) || match_path(path, &paths::RANGE_FROM) {
+                Some(Range {
+                    start: get_field("start", fields),
+                    end: None,
+                    limits: ast::RangeLimits::HalfOpen,
+                })
+            } else if match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY_STD) ||
+               match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY) {
+                Some(Range {
+                    start: get_field("start", fields),
+                    end: get_field("end", fields),
+                    limits: ast::RangeLimits::Closed,
+                })
+            } else if match_path(path, &paths::RANGE_STD) || match_path(path, &paths::RANGE) {
+                Some(Range {
+                    start: get_field("start", fields),
+                    end: get_field("end", fields),
+                    limits: ast::RangeLimits::HalfOpen,
+                })
+            } else if match_path(path, &paths::RANGE_TO_INCLUSIVE_STD) || match_path(path, &paths::RANGE_TO_INCLUSIVE) {
+                Some(Range {
+                    start: None,
+                    end: get_field("end", fields),
+                    limits: ast::RangeLimits::Closed,
+                })
+            } else if match_path(path, &paths::RANGE_TO_STD) || match_path(path, &paths::RANGE_TO) {
+                Some(Range {
+                    start: None,
+                    end: get_field("end", fields),
+                    limits: ast::RangeLimits::HalfOpen,
+                })
+            } else {
+                None
+            }
+        }
+        _ => None,
+    }
+}
+
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 6d41d5039f74..3ceae788a4d0 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -13,7 +13,7 @@ use std::borrow::Cow;
 use std::env;
 use std::mem;
 use std::str::FromStr;
-use syntax::ast::{self, LitKind, RangeLimits};
+use syntax::ast::{self, LitKind};
 use syntax::codemap::{ExpnInfo, Span, ExpnFormat};
 use syntax::errors::DiagnosticBuilder;
 use syntax::ptr::P;
@@ -683,93 +683,6 @@ pub fn camel_case_from(s: &str) -> usize {
     last_i
 }
 
-/// Represent a range akin to `ast::ExprKind::Range`.
-#[derive(Debug, Copy, Clone)]
-pub struct UnsugaredRange<'a> {
-    pub start: Option<&'a Expr>,
-    pub end: Option<&'a Expr>,
-    pub limits: RangeLimits,
-}
-
-/// Unsugar a `hir` range.
-pub fn unsugar_range(expr: &Expr) -> Option<UnsugaredRange> {
-    // To be removed when ranges get stable.
-    fn unwrap_unstable(expr: &Expr) -> &Expr {
-        if let ExprBlock(ref block) = expr.node {
-            if block.rules == BlockCheckMode::PushUnstableBlock || block.rules == BlockCheckMode::PopUnstableBlock {
-                if let Some(ref expr) = block.expr {
-                    return expr;
-                }
-            }
-        }
-
-        expr
-    }
-
-    fn get_field<'a>(name: &str, fields: &'a [Field]) -> Option<&'a Expr> {
-        let expr = &fields.iter()
-                          .find(|field| field.name.node.as_str() == name)
-                          .unwrap_or_else(|| panic!("missing {} field for range", name))
-                          .expr;
-
-        Some(unwrap_unstable(expr))
-    }
-
-    // The range syntax is expanded to literal paths starting with `core` or `std` depending on
-    // `#[no_std]`. Testing both instead of resolving the paths.
-
-    match unwrap_unstable(expr).node {
-        ExprPath(None, ref path) => {
-            if match_path(path, &paths::RANGE_FULL_STD) || match_path(path, &paths::RANGE_FULL) {
-                Some(UnsugaredRange {
-                    start: None,
-                    end: None,
-                    limits: RangeLimits::HalfOpen,
-                })
-            } else {
-                None
-            }
-        }
-        ExprStruct(ref path, ref fields, None) => {
-            if match_path(path, &paths::RANGE_FROM_STD) || match_path(path, &paths::RANGE_FROM) {
-                Some(UnsugaredRange {
-                    start: get_field("start", fields),
-                    end: None,
-                    limits: RangeLimits::HalfOpen,
-                })
-            } else if match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY_STD) ||
-               match_path(path, &paths::RANGE_INCLUSIVE_NON_EMPTY) {
-                Some(UnsugaredRange {
-                    start: get_field("start", fields),
-                    end: get_field("end", fields),
-                    limits: RangeLimits::Closed,
-                })
-            } else if match_path(path, &paths::RANGE_STD) || match_path(path, &paths::RANGE) {
-                Some(UnsugaredRange {
-                    start: get_field("start", fields),
-                    end: get_field("end", fields),
-                    limits: RangeLimits::HalfOpen,
-                })
-            } else if match_path(path, &paths::RANGE_TO_INCLUSIVE_STD) || match_path(path, &paths::RANGE_TO_INCLUSIVE) {
-                Some(UnsugaredRange {
-                    start: None,
-                    end: get_field("end", fields),
-                    limits: RangeLimits::Closed,
-                })
-            } else if match_path(path, &paths::RANGE_TO_STD) || match_path(path, &paths::RANGE_TO) {
-                Some(UnsugaredRange {
-                    start: None,
-                    end: get_field("end", fields),
-                    limits: RangeLimits::HalfOpen,
-                })
-            } else {
-                None
-            }
-        }
-        _ => None,
-    }
-}
-
 /// Convenience function to get the return type of a function or `None` if the function diverges.
 pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: NodeId) -> Option<ty::Ty<'tcx>> {
     let parameter_env = ty::ParameterEnvironment::for_item(cx.tcx, fn_item);

From 4dff4df57711b1da61dcbc361b02c6becdefe035 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Thu, 30 Jun 2016 00:08:43 +0200
Subject: [PATCH 12/29] Move more functions to `utils::higher`

---
 clippy_lints/src/formatting.rs   |  2 +-
 clippy_lints/src/loops.rs        |  4 ++--
 clippy_lints/src/mut_mut.rs      |  4 ++--
 clippy_lints/src/shadow.rs       |  4 ++--
 clippy_lints/src/types.rs        |  4 ++--
 clippy_lints/src/utils/higher.rs | 33 +++++++++++++++++++++++++++
 clippy_lints/src/utils/mod.rs    | 38 ++------------------------------
 clippy_lints/src/vec.rs          |  4 ++--
 8 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs
index aa6dd46cf0b9..cee18337ba10 100644
--- a/clippy_lints/src/formatting.rs
+++ b/clippy_lints/src/formatting.rs
@@ -156,7 +156,7 @@ fn check_consecutive_ifs(cx: &EarlyContext, first: &ast::Expr, second: &ast::Exp
     }
 }
 
-/// Match `if` or `else if` expressions and return the `then` and `else` block.
+/// Match `if` or `if let` expressions and return the `then` and `else` block.
 fn unsugar_if(expr: &ast::Expr) -> Option<(&P<ast::Block>, &Option<P<ast::Expr>>)> {
     match expr.node {
         ast::ExprKind::If(_, ref then, ref else_) |
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index e945afbf659e..1a6ac75eb377 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -15,7 +15,7 @@ use syntax::ast;
 
 use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro,
             span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
-            walk_ptrs_ty, recover_for_loop};
+            walk_ptrs_ty};
 use utils::paths;
 
 /// **What it does:** This lint checks for looping over the range of `0..len` of some collection just to get the values by index.
@@ -223,7 +223,7 @@ impl LintPass for Pass {
 
 impl LateLintPass for Pass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
-        if let Some((pat, arg, body)) = recover_for_loop(expr) {
+        if let Some((pat, arg, body)) = higher::for_loop(expr) {
             check_for_loop(cx, pat, arg, body, expr);
         }
         // check for `loop { if let {} else break }` that could be `while let`
diff --git a/clippy_lints/src/mut_mut.rs b/clippy_lints/src/mut_mut.rs
index 8a5439fb11e3..d5d9fe1a0bc8 100644
--- a/clippy_lints/src/mut_mut.rs
+++ b/clippy_lints/src/mut_mut.rs
@@ -2,7 +2,7 @@ use rustc::hir;
 use rustc::hir::intravisit;
 use rustc::lint::*;
 use rustc::ty::{TypeAndMut, TyRef};
-use utils::{in_external_macro, recover_for_loop, span_lint};
+use utils::{higher, in_external_macro, span_lint};
 
 /// **What it does:** This lint checks for instances of `mut mut` references.
 ///
@@ -49,7 +49,7 @@ impl<'a, 'tcx, 'v> intravisit::Visitor<'v> for MutVisitor<'a, 'tcx> {
             return;
         }
 
-        if let Some((_, arg, body)) = recover_for_loop(expr) {
+        if let Some((_, arg, body)) = higher::for_loop(expr) {
             // A `for` loop lowers to:
             // ```rust
             // match ::std::iter::Iterator::next(&mut iter) {
diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs
index b12e68185c2d..7bda94ed6397 100644
--- a/clippy_lints/src/shadow.rs
+++ b/clippy_lints/src/shadow.rs
@@ -5,7 +5,7 @@ use rustc::hir::*;
 use rustc::hir::intravisit::{Visitor, FnKind};
 use std::ops::Deref;
 use syntax::codemap::Span;
-use utils::{is_from_for_desugar, in_external_macro, snippet, span_lint_and_then};
+use utils::{higher, in_external_macro, snippet, span_lint_and_then};
 
 /// **What it does:** This lint checks for bindings that shadow other bindings already in scope, while just changing reference level or mutability.
 ///
@@ -91,7 +91,7 @@ fn check_decl(cx: &LateContext, decl: &Decl, bindings: &mut Vec<(Name, Span)>) {
     if in_external_macro(cx, decl.span) {
         return;
     }
-    if is_from_for_desugar(decl) {
+    if higher::is_from_for_desugar(decl) {
         return;
     }
     if let DeclLocal(ref local) = decl.node {
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index 83e2c1b549c1..629834a3c0f7 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -6,7 +6,7 @@ use rustc::ty;
 use std::cmp::Ordering;
 use syntax::ast::{IntTy, UintTy, FloatTy};
 use syntax::codemap::Span;
-use utils::{comparisons, in_external_macro, in_macro, is_from_for_desugar, match_def_path, snippet,
+use utils::{comparisons, higher, in_external_macro, in_macro, match_def_path, snippet,
             span_help_and_lint, span_lint};
 use utils::paths;
 
@@ -106,7 +106,7 @@ fn check_let_unit(cx: &LateContext, decl: &Decl) {
             if in_external_macro(cx, decl.span) || in_macro(cx, local.pat.span) {
                 return;
             }
-            if is_from_for_desugar(decl) {
+            if higher::is_from_for_desugar(decl) {
                 return;
             }
             span_lint(cx,
diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs
index 979c044cdbf9..16ab44881a55 100644
--- a/clippy_lints/src/utils/higher.rs
+++ b/clippy_lints/src/utils/higher.rs
@@ -115,3 +115,36 @@ pub fn range(expr: &hir::Expr) -> Option<Range> {
     }
 }
 
+/// Checks if a `let` decl is from a `for` loop desugaring.
+pub fn is_from_for_desugar(decl: &hir::Decl) -> bool {
+    if_let_chain! {[
+        let hir::DeclLocal(ref loc) = decl.node,
+        let Some(ref expr) = loc.init,
+        let hir::ExprMatch(_, _, hir::MatchSource::ForLoopDesugar) = expr.node,
+    ], {
+        return true;
+    }}
+    false
+}
+
+/// Recover the essential nodes of a desugared for loop:
+/// `for pat in arg { body }` becomes `(pat, arg, body)`.
+pub fn for_loop(expr: &hir::Expr) -> Option<(&hir::Pat, &hir::Expr, &hir::Expr)> {
+    if_let_chain! {[
+        let hir::ExprMatch(ref iterexpr, ref arms, _) = expr.node,
+        let hir::ExprCall(_, ref iterargs) = iterexpr.node,
+        iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(),
+        let hir::ExprLoop(ref block, _) = arms[0].body.node,
+        block.stmts.is_empty(),
+        let Some(ref loopexpr) = block.expr,
+        let hir::ExprMatch(_, ref innerarms, hir::MatchSource::ForLoopDesugar) = loopexpr.node,
+        innerarms.len() == 2 && innerarms[0].pats.len() == 1,
+        let hir::PatKind::TupleStruct(_, ref somepats, _) = innerarms[0].pats[0].node,
+        somepats.len() == 1
+    ], {
+        return Some((&somepats[0],
+                     &iterargs[0],
+                     &innerarms[0].body));
+    }}
+    None
+}
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 3ceae788a4d0..df9d09ed0106 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -21,7 +21,6 @@ use syntax::ptr::P;
 pub mod cargo;
 pub mod comparisons;
 pub mod conf;
-pub mod higher;
 mod hir;
 pub mod paths;
 pub mod sugg;
@@ -82,6 +81,8 @@ macro_rules! if_let_chain {
     };
 }
 
+pub mod higher;
+
 /// Returns true if the two spans come from differing expansions (i.e. one is from a macro and one
 /// isn't).
 pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool {
@@ -319,19 +320,6 @@ pub fn get_item_name(cx: &LateContext, expr: &Expr) -> Option<Name> {
     }
 }
 
-/// Checks if a `let` decl is from a `for` loop desugaring.
-pub fn is_from_for_desugar(decl: &Decl) -> bool {
-    if_let_chain! {[
-        let DeclLocal(ref loc) = decl.node,
-        let Some(ref expr) = loc.init,
-        let ExprMatch(_, _, MatchSource::ForLoopDesugar) = expr.node
-    ], {
-        return true;
-    }}
-    false
-}
-
-
 /// Convert a span to a code snippet if available, otherwise use default.
 ///
 /// # Example
@@ -706,25 +694,3 @@ pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: ty::Ty<'tcx>, b: ty::Ty
         infcx.can_equate(&new_a, &new_b).is_ok()
     })
 }
-
-/// Recover the essential nodes of a desugared for loop:
-/// `for pat in arg { body }` becomes `(pat, arg, body)`.
-pub fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> {
-    if_let_chain! {[
-        let ExprMatch(ref iterexpr, ref arms, _) = expr.node,
-        let ExprCall(_, ref iterargs) = iterexpr.node,
-        iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(),
-        let ExprLoop(ref block, _) = arms[0].body.node,
-        block.stmts.is_empty(),
-        let Some(ref loopexpr) = block.expr,
-        let ExprMatch(_, ref innerarms, MatchSource::ForLoopDesugar) = loopexpr.node,
-        innerarms.len() == 2 && innerarms[0].pats.len() == 1,
-        let PatKind::TupleStruct(_, ref somepats, _) = innerarms[0].pats[0].node,
-        somepats.len() == 1
-    ], {
-        return Some((&somepats[0],
-                     &iterargs[0],
-                     &innerarms[0].body));
-    }}
-    None
-}
diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs
index 73f2fb7caa81..97a45da4536f 100644
--- a/clippy_lints/src/vec.rs
+++ b/clippy_lints/src/vec.rs
@@ -3,7 +3,7 @@ use rustc::ty::TypeVariants;
 use rustc::hir::*;
 use syntax::codemap::Span;
 use syntax::ptr::P;
-use utils::{is_expn_of, match_path, paths, recover_for_loop, snippet, span_lint_and_then};
+use utils::{higher, is_expn_of, match_path, paths, snippet, span_lint_and_then};
 
 /// **What it does:** This lint warns about using `&vec![..]` when using `&[..]` would be possible.
 ///
@@ -42,7 +42,7 @@ impl LateLintPass for Pass {
         }}
 
         // search for `for _ in vec![…]`
-        if let Some((_, arg, _)) = recover_for_loop(expr) {
+        if let Some((_, arg, _)) = higher::for_loop(expr) {
             // report the error around the `vec!` not inside `<std macros>:`
             let span = cx.sess().codemap().source_callsite(arg.span);
             check_vec_macro(cx, arg, span);

From 98f18f047482c08f173e757c0521820ba2deea58 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Thu, 30 Jun 2016 19:49:34 +0200
Subject: [PATCH 13/29] Move `vec!` unexpanding function to `utils::higher`

---
 clippy_lints/src/utils/higher.rs | 42 ++++++++++++++++++++++++++++-
 clippy_lints/src/vec.rs          | 46 +++-----------------------------
 2 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs
index 16ab44881a55..ab8b7d8b2b8f 100644
--- a/clippy_lints/src/utils/higher.rs
+++ b/clippy_lints/src/utils/higher.rs
@@ -1,8 +1,10 @@
 //! This module contains functions for retrieve the original AST from lowered `hir`.
 
 use rustc::hir;
+use rustc::lint::LateContext;
 use syntax::ast;
-use utils::{match_path, paths};
+use syntax::ptr::P;
+use utils::{is_expn_of, match_path, paths};
 
 /// Convert a hir binary operator to the corresponding `ast` type.
 pub fn binop(op: hir::BinOp_) -> ast::BinOpKind {
@@ -148,3 +150,41 @@ pub fn for_loop(expr: &hir::Expr) -> Option<(&hir::Pat, &hir::Expr, &hir::Expr)>
     }}
     None
 }
+
+/// Represent the pre-expansion arguments of a `vec!` invocation.
+pub enum VecArgs<'a> {
+    /// `vec![elem; len]`
+    Repeat(&'a P<hir::Expr>, &'a P<hir::Expr>),
+    /// `vec![a, b, c]`
+    Vec(&'a [P<hir::Expr>]),
+}
+
+/// Returns the arguments of the `vec!` macro if this expression was expanded from `vec!`.
+pub fn vec_macro<'e>(cx: &LateContext, expr: &'e hir::Expr) -> Option<VecArgs<'e>> {
+    if_let_chain!{[
+        let hir::ExprCall(ref fun, ref args) = expr.node,
+        let hir::ExprPath(_, ref path) = fun.node,
+        is_expn_of(cx, fun.span, "vec").is_some()
+    ], {
+        return if match_path(path, &paths::VEC_FROM_ELEM) && args.len() == 2 {
+            // `vec![elem; size]` case
+            Some(VecArgs::Repeat(&args[0], &args[1]))
+        }
+        else if match_path(path, &["into_vec"]) && args.len() == 1 {
+            // `vec![a, b, c]` case
+            if_let_chain!{[
+                let hir::ExprBox(ref boxed) = args[0].node,
+                let hir::ExprVec(ref args) = boxed.node
+            ], {
+                return Some(VecArgs::Vec(&*args));
+            }}
+
+            None
+        }
+        else {
+            None
+        };
+    }}
+
+    None
+}
diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs
index 97a45da4536f..8e09f9341c2d 100644
--- a/clippy_lints/src/vec.rs
+++ b/clippy_lints/src/vec.rs
@@ -2,8 +2,7 @@ use rustc::lint::*;
 use rustc::ty::TypeVariants;
 use rustc::hir::*;
 use syntax::codemap::Span;
-use syntax::ptr::P;
-use utils::{higher, is_expn_of, match_path, paths, snippet, span_lint_and_then};
+use utils::{higher, snippet, span_lint_and_then};
 
 /// **What it does:** This lint warns about using `&vec![..]` when using `&[..]` would be possible.
 ///
@@ -51,12 +50,12 @@ impl LateLintPass for Pass {
 }
 
 fn check_vec_macro(cx: &LateContext, vec: &Expr, span: Span) {
-    if let Some(vec_args) = unexpand(cx, vec) {
+    if let Some(vec_args) = higher::vec_macro(cx, vec) {
         let snippet = match vec_args {
-            Args::Repeat(elem, len) => {
+            higher::VecArgs::Repeat(elem, len) => {
                 format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into()
             }
-            Args::Vec(args) => {
+            higher::VecArgs::Vec(args) => {
                 if let Some(last) = args.iter().last() {
                     let span = Span {
                         lo: args[0].span.lo,
@@ -77,40 +76,3 @@ fn check_vec_macro(cx: &LateContext, vec: &Expr, span: Span) {
     }
 }
 
-/// Represent the pre-expansion arguments of a `vec!` invocation.
-pub enum Args<'a> {
-    /// `vec![elem; len]`
-    Repeat(&'a P<Expr>, &'a P<Expr>),
-    /// `vec![a, b, c]`
-    Vec(&'a [P<Expr>]),
-}
-
-/// Returns the arguments of the `vec!` macro if this expression was expanded from `vec!`.
-pub fn unexpand<'e>(cx: &LateContext, expr: &'e Expr) -> Option<Args<'e>> {
-    if_let_chain!{[
-        let ExprCall(ref fun, ref args) = expr.node,
-        let ExprPath(_, ref path) = fun.node,
-        is_expn_of(cx, fun.span, "vec").is_some()
-    ], {
-        return if match_path(path, &paths::VEC_FROM_ELEM) && args.len() == 2 {
-            // `vec![elem; size]` case
-            Some(Args::Repeat(&args[0], &args[1]))
-        }
-        else if match_path(path, &["into_vec"]) && args.len() == 1 {
-            // `vec![a, b, c]` case
-            if_let_chain!{[
-                let ExprBox(ref boxed) = args[0].node,
-                let ExprVec(ref args) = boxed.node
-            ], {
-                return Some(Args::Vec(&*args));
-            }}
-
-            None
-        }
-        else {
-            None
-        };
-    }}
-
-    None
-}

From 28bd591f05770fd35cc6f2f82f0a13e71d13dcea Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Thu, 30 Jun 2016 19:50:03 +0200
Subject: [PATCH 14/29] Only build suggestion if necessary in `USELESS_VEC`

---
 clippy_lints/src/vec.rs | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs
index 8e09f9341c2d..6f4c5a1e0e26 100644
--- a/clippy_lints/src/vec.rs
+++ b/clippy_lints/src/vec.rs
@@ -51,26 +51,26 @@ impl LateLintPass for Pass {
 
 fn check_vec_macro(cx: &LateContext, vec: &Expr, span: Span) {
     if let Some(vec_args) = higher::vec_macro(cx, vec) {
-        let snippet = match vec_args {
-            higher::VecArgs::Repeat(elem, len) => {
-                format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into()
-            }
-            higher::VecArgs::Vec(args) => {
-                if let Some(last) = args.iter().last() {
-                    let span = Span {
-                        lo: args[0].span.lo,
-                        hi: last.span.hi,
-                        expn_id: args[0].span.expn_id,
-                    };
+        span_lint_and_then(cx, USELESS_VEC, span, "useless use of `vec!`", |db| {
+            let snippet = match vec_args {
+                higher::VecArgs::Repeat(elem, len) => {
+                    format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into()
+                }
+                higher::VecArgs::Vec(args) => {
+                    if let Some(last) = args.iter().last() {
+                        let span = Span {
+                            lo: args[0].span.lo,
+                            hi: last.span.hi,
+                            expn_id: args[0].span.expn_id,
+                        };
 
-                    format!("&[{}]", snippet(cx, span, "..")).into()
-                } else {
-                    "&[]".into()
+                        format!("&[{}]", snippet(cx, span, "..")).into()
+                    } else {
+                        "&[]".into()
+                    }
                 }
-            }
-        };
+            };
 
-        span_lint_and_then(cx, USELESS_VEC, span, "useless use of `vec!`", |db| {
             db.span_suggestion(span, "you can use a slice directly", snippet);
         });
     }

From 97f65b02962af06e351bf33783803caace767f4e Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 17:49:05 +0200
Subject: [PATCH 15/29] Rustup to ea0dc9297283daff6486807f43e190b4eb561412 III

---
 clippy_lints/src/utils/sugg.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index a28c00efcae5..90cb4cf00fd7 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -77,10 +77,10 @@ impl<'a> Sugg<'a> {
             ast::ExprKind::InPlace(..) |
             ast::ExprKind::Unary(..) |
             ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet),
-            ast::ExprKind::Again(..) |
             ast::ExprKind::Block(..) |
             ast::ExprKind::Break(..) |
             ast::ExprKind::Call(..) |
+            ast::ExprKind::Continue(..) |
             ast::ExprKind::Field(..) |
             ast::ExprKind::ForLoop(..) |
             ast::ExprKind::Index(..) |

From e613c8b492495aeb5f75f94a0cdcf252573a345f Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 18:38:50 +0200
Subject: [PATCH 16/29] Introduce `multispan_sugg`

---
 clippy_lints/src/lib.rs       |  1 +
 clippy_lints/src/utils/mod.rs | 22 +++++++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 41a61feab539..6bdfbe98b5bc 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -38,6 +38,7 @@ extern crate quine_mc_cluskey;
 
 extern crate rustc_serialize;
 
+extern crate rustc_errors;
 extern crate rustc_plugin;
 extern crate rustc_const_eval;
 extern crate rustc_const_math;
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index df9d09ed0106..3e0f0db6ac17 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -9,12 +9,13 @@ use rustc::traits::ProjectionMode;
 use rustc::traits;
 use rustc::ty::subst::Subst;
 use rustc::ty;
+use rustc_errors;
 use std::borrow::Cow;
 use std::env;
 use std::mem;
 use std::str::FromStr;
 use syntax::ast::{self, LitKind};
-use syntax::codemap::{ExpnInfo, Span, ExpnFormat};
+use syntax::codemap::{ExpnFormat, ExpnInfo, MultiSpan, Span};
 use syntax::errors::DiagnosticBuilder;
 use syntax::ptr::P;
 
@@ -490,6 +491,25 @@ pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint,
     }
 }
 
+/// Create a suggestion made from several `span → replacement`.
+///
+/// Note: in the JSON format (used by `compiletest_rs`), the help message will appear once per
+/// replacement. In human-readable format though, it only appears once before the whole suggestion.
+pub fn multispan_sugg(db: &mut DiagnosticBuilder, help_msg: String, sugg: &[(Span, &str)]) {
+    let sugg = rustc_errors::RenderSpan::Suggestion(rustc_errors::CodeSuggestion {
+        msp: MultiSpan::from_spans(sugg.iter().map(|&(span, _)| span).collect()),
+        substitutes: sugg.iter().map(|&(_, subs)| subs.to_owned()).collect(),
+    });
+
+    let sub = rustc_errors::SubDiagnostic {
+        level: rustc_errors::Level::Help,
+        message: help_msg,
+        span: MultiSpan::new(),
+        render_span: Some(sugg),
+    };
+    db.children.push(sub);
+}
+
 /// Return the base type for references and raw pointers.
 pub fn walk_ptrs_ty(ty: ty::Ty) -> ty::Ty {
     match ty.sty {

From 9bd7fa05e0fd3049e74a900e39c17cd9d6ebfbbc Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 18:44:59 +0200
Subject: [PATCH 17/29] Improve `NEEDLESS_RANGE_LOOP` error reporting

---
 clippy_lints/src/loops.rs      | 41 +++++++++++-----------
 tests/compile-fail/for_loop.rs | 62 ++++++++++++++++++++++++++++------
 2 files changed, 73 insertions(+), 30 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 1a6ac75eb377..0f9d030c2f28 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -13,7 +13,7 @@ use std::borrow::Cow;
 use std::collections::HashMap;
 use syntax::ast;
 
-use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, in_external_macro,
+use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg, in_external_macro,
             span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
             walk_ptrs_ty};
 use utils::paths;
@@ -377,17 +377,16 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
                 };
 
                 if visitor.nonindex {
-                    span_lint(cx,
-                              NEEDLESS_RANGE_LOOP,
-                              expr.span,
-                              &format!("the loop variable `{}` is used to index `{}`. Consider using `for ({}, \
-                                        item) in {}.iter().enumerate(){}{}` or similar iterators",
-                                       ident.node,
-                                       indexed,
-                                       ident.node,
-                                       indexed,
-                                       take,
-                                       skip));
+                    span_lint_and_then(cx,
+                                       NEEDLESS_RANGE_LOOP,
+                                       expr.span,
+                                       &format!("the loop variable `{}` is used to index `{}`", ident.node, indexed),
+                                       |db| {
+                        multispan_sugg(db, "consider using an iterator".to_string(), &[
+                            (pat.span, &format!("({}, <item>)", ident.node)),
+                            (arg.span, &format!("{}.iter().enumerate(){}{}", indexed, take, skip)),
+                        ]);
+                    });
                 } else {
                     let repl = if starts_at_zero && take.is_empty() {
                         format!("&{}", indexed)
@@ -395,14 +394,16 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
                         format!("{}.iter(){}{}", indexed, take, skip)
                     };
 
-                    span_lint(cx,
-                              NEEDLESS_RANGE_LOOP,
-                              expr.span,
-                              &format!("the loop variable `{}` is only used to index `{}`. \
-                                        Consider using `for item in {}` or similar iterators",
-                                       ident.node,
-                                       indexed,
-                                       repl));
+                    span_lint_and_then(cx,
+                                       NEEDLESS_RANGE_LOOP,
+                                       expr.span,
+                                       &format!("the loop variable `{}` is only used to index `{}`.", ident.node, indexed),
+                                       |db| {
+                        multispan_sugg(db, "consider using an iterator".to_string(), &[
+                            (pat.span, "<item>"),
+                            (arg.span, &repl),
+                        ]);
+                    });
                 }
             }
         }
diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs
index 411a4b11c17a..6028c5c0f3a8 100644
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -96,23 +96,41 @@ fn main() {
     let mut vec = vec![1, 2, 3, 4];
     let vec2 = vec![1, 2, 3, 4];
     for i in 0..vec.len() {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in &vec`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in &vec {
         println!("{}", vec[i]);
     }
 
+    for i in 0..vec.len() { let _ = vec[i]; }
+    //~^ ERROR `i` is only used to index `vec`
+    //~| HELP consider
+    //~| HELP consider
+    //~| SUGGESTION for <item> in &vec { let _ = vec[i]; }
+
     // ICE #746
     for j in 0..4 {
         //~^ ERROR `j` is only used to index `STATIC`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in STATIC.iter().take(4) {
         println!("{:?}", STATIC[j]);
     }
 
     for j in 0..4 {
         //~^ ERROR `j` is only used to index `CONST`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in CONST.iter().take(4) {
         println!("{:?}", CONST[j]);
     }
 
     for i in 0..vec.len() {
-        //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate()`
+        //~^ ERROR `i` is used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for (i, <item>) in vec.iter().enumerate() {
         println!("{} {}", vec[i], i);
     }
     for i in 0..vec.len() {      // not an error, indexing more than one variable
@@ -120,42 +138,66 @@ fn main() {
     }
 
     for i in 0..vec.len() {
-        //~^ ERROR `i` is only used to index `vec2`. Consider using `for item in vec2.iter().take(vec.len())`
+        //~^ ERROR `i` is only used to index `vec2`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec2.iter().take(vec.len()) {
         println!("{}", vec2[i]);
     }
 
     for i in 5..vec.len() {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().skip(5)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().skip(5) {
         println!("{}", vec[i]);
     }
 
     for i in 0..MAX_LEN {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
         println!("{}", vec[i]);
     }
 
     for i in 0...MAX_LEN {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(MAX_LEN)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
         println!("{}", vec[i]);
     }
 
     for i in 5..10 {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
         println!("{}", vec[i]);
     }
 
     for i in 5...10 {
-        //~^ ERROR `i` is only used to index `vec`. Consider using `for item in vec.iter().take(10).skip(5)`
+        //~^ ERROR `i` is only used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
         println!("{}", vec[i]);
     }
 
     for i in 5..vec.len() {
-        //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().skip(5)`
+        //~^ ERROR `i` is used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for (i, <item>) in vec.iter().enumerate().skip(5) {
         println!("{} {}", vec[i], i);
     }
 
     for i in 5..10 {
-        //~^ ERROR `i` is used to index `vec`. Consider using `for (i, item) in vec.iter().enumerate().take(10).skip(5)`
+        //~^ ERROR `i` is used to index `vec`
+        //~| HELP consider
+        //~| HELP consider
+        //~| SUGGESTION for (i, <item>) in vec.iter().enumerate().take(10).skip(5) {
         println!("{} {}", vec[i], i);
     }
 

From dbf6dc66d8ad9ed6af686aa9fe9ecb893a6cf23e Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 19:30:38 +0200
Subject: [PATCH 18/29] Add more sugggestion-building functions

---
 clippy_lints/src/collapsible_if.rs |  2 +-
 clippy_lints/src/matches.rs        |  2 +-
 clippy_lints/src/misc.rs           |  6 +-
 clippy_lints/src/utils/sugg.rs     | 88 +++++++++++++++++++++++-------
 4 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs
index 1d9e3984e743..ca5a097119db 100644
--- a/clippy_lints/src/collapsible_if.rs
+++ b/clippy_lints/src/collapsible_if.rs
@@ -107,7 +107,7 @@ fn check_collapsible_no_if_let(
             db.span_suggestion(expr.span,
                                "try",
                                format!("if {} {}",
-                                       lhs.and(&rhs),
+                                       lhs.and(rhs),
                                        snippet_block(cx, content.span, "..")));
         });
     }}
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 8fddcb84922b..94b36d28ed35 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -263,7 +263,7 @@ fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
                         Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
                     }
                     (true, false) => {
-                        let test = &Sugg::hir(cx, ex, "..");
+                        let test = Sugg::hir(cx, ex, "..");
                         Some(format!("if {} {}",
                                      !test,
                                      expr_block(cx, false_expr, None, "..")))
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 0931d5aca6b8..b1627c31c8b8 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -72,7 +72,7 @@ impl LateLintPass for TopLevelRefPass {
                 l.pat.span,
                 "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead",
                 |db| {
-                    let init = &Sugg::hir(cx, init, "..");
+                    let init = Sugg::hir(cx, init, "..");
                     db.span_suggestion(s.span,
                                        "try",
                                        format!("let {}{} = {};",
@@ -171,8 +171,8 @@ impl LateLintPass for FloatCmp {
                                    expr.span,
                                    "strict comparison of f32 or f64",
                                    |db| {
-                    let lhs = &Sugg::hir(cx, left, "..");
-                    let rhs = &Sugg::hir(cx, right, "..");
+                    let lhs = Sugg::hir(cx, left, "..");
+                    let rhs = Sugg::hir(cx, right, "..");
 
                     db.span_suggestion(expr.span,
                                        "consider comparing them within some error",
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 90cb4cf00fd7..d05629b1a76e 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -5,6 +5,7 @@ use std;
 use syntax::ast;
 use syntax::util::parser::AssocOp;
 use utils::{higher, snippet};
+use syntax::print::pprust::binop_to_string;
 
 /// A helper type to build suggestion correctly handling parenthesis.
 pub enum Sugg<'a> {
@@ -16,6 +17,9 @@ pub enum Sugg<'a> {
     BinOp(AssocOp, Cow<'a, str>),
 }
 
+/// Literal constant `1`, for convenience.
+pub const ONE: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("1"));
+
 impl<'a> std::fmt::Display for Sugg<'a> {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
         match *self {
@@ -110,28 +114,43 @@ impl<'a> Sugg<'a> {
         }
     }
 
-    /// Convenience method to create the `lhs && rhs` suggestion.
-    pub fn and(&self, rhs: &Self) -> Sugg<'static> {
-        make_binop(ast::BinOpKind::And, self, rhs)
+    /// Convenience method to create the `<lhs> && <rhs>` suggestion.
+    pub fn and(self, rhs: Self) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::And, &self, &rhs)
     }
 
     /// Convenience method to create the `&<expr>` suggestion.
-    pub fn addr(&self) -> Sugg<'static> {
-        make_unop("&", self)
+    pub fn addr(self) -> Sugg<'static> {
+        make_unop("&", &self)
+    }
+
+    /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>` suggestion.
+    pub fn range(self, end: Self, limit: ast::RangeLimits) -> Sugg<'static> {
+        match limit {
+            ast::RangeLimits::HalfOpen => make_assoc(AssocOp::DotDot, &self, &end),
+            ast::RangeLimits::Closed => make_assoc(AssocOp::DotDotDot, &self, &end),
+        }
+    }
+}
+
+impl<'a, 'b> std::ops::Add<Sugg<'b>> for Sugg<'a> {
+    type Output = Sugg<'static>;
+    fn add(self, rhs: Sugg<'b>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Add, &self, &rhs)
     }
 }
 
-impl<'a, 'b> std::ops::Sub<&'b Sugg<'b>> for &'a Sugg<'a> {
+impl<'a, 'b> std::ops::Sub<Sugg<'b>> for Sugg<'a> {
     type Output = Sugg<'static>;
-    fn sub(self, rhs: &'b Sugg<'b>) -> Sugg<'static> {
-        make_binop(ast::BinOpKind::Sub, self, rhs)
+    fn sub(self, rhs: Sugg<'b>) -> Sugg<'static> {
+        make_binop(ast::BinOpKind::Sub, &self, &rhs)
     }
 }
 
-impl<'a> std::ops::Not for &'a Sugg<'a> {
+impl<'a> std::ops::Not for Sugg<'a> {
     type Output = Sugg<'static>;
     fn not(self) -> Sugg<'static> {
-        make_unop("!", self)
+        make_unop("!", &self)
     }
 }
 
@@ -172,7 +191,7 @@ pub fn make_unop(op: &str, expr: &Sugg) -> Sugg<'static> {
 ///
 /// Precedence of shift operator relative to other arithmetic operation is often confusing so
 /// parenthesis will always be added for a mix of these.
-pub fn make_binop(op: ast::BinOpKind, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> {
+pub fn make_assoc(op: AssocOp, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> {
     fn is_shift(op: &AssocOp) -> bool {
         matches!(*op, AssocOp::ShiftLeft | AssocOp::ShiftRight)
     }
@@ -190,25 +209,54 @@ pub fn make_binop(op: ast::BinOpKind, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> {
              is_shift(other) && is_arith(op)
     }
 
-    let aop = AssocOp::from_ast_binop(op);
-
     let lhs_paren = if let Sugg::BinOp(ref lop, _) = *lhs {
-        needs_paren(&aop, lop, Associativity::Left)
+        needs_paren(&op, lop, Associativity::Left)
     } else {
         false
     };
 
     let rhs_paren = if let Sugg::BinOp(ref rop, _) = *rhs {
-        needs_paren(&aop, rop, Associativity::Right)
+        needs_paren(&op, rop, Associativity::Right)
     } else {
         false
     };
 
-    Sugg::BinOp(aop,
-                format!("{} {} {}",
-                        ParenHelper::new(lhs_paren, lhs),
-                        op.to_string(),
-                        ParenHelper::new(rhs_paren, rhs)).into())
+    let lhs = ParenHelper::new(lhs_paren, lhs);
+    let rhs = ParenHelper::new(rhs_paren, rhs);
+    let sugg = match op {
+        AssocOp::Add |
+        AssocOp::BitAnd |
+        AssocOp::BitOr |
+        AssocOp::BitXor |
+        AssocOp::Divide |
+        AssocOp::Equal |
+        AssocOp::Greater |
+        AssocOp::GreaterEqual |
+        AssocOp::LAnd |
+        AssocOp::LOr |
+        AssocOp::Less |
+        AssocOp::LessEqual |
+        AssocOp::Modulus |
+        AssocOp::Multiply |
+        AssocOp::NotEqual |
+        AssocOp::ShiftLeft |
+        AssocOp::ShiftRight |
+        AssocOp::Subtract => format!("{} {} {}", lhs, op.to_ast_binop().expect("Those are AST ops").to_string(), rhs),
+        AssocOp::Inplace => format!("in ({}) {}", lhs, rhs),
+        AssocOp::Assign => format!("{} = {}", lhs, rhs),
+        AssocOp::AssignOp(op) => format!("{} {}= {}", lhs, binop_to_string(op), rhs),
+        AssocOp::As => format!("{} as {}", lhs, rhs),
+        AssocOp::DotDot => format!("{}..{}", lhs, rhs),
+        AssocOp::DotDotDot => format!("{}...{}", lhs, rhs),
+        AssocOp::Colon => format!("{}: {}", lhs, rhs),
+    };
+
+    Sugg::BinOp(op, sugg.into())
+}
+
+/// Convinience wrapper arround `make_assoc` and `AssocOp::from_ast_binop`.
+pub fn make_binop(op: ast::BinOpKind, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> {
+    make_assoc(AssocOp::from_ast_binop(op), lhs, rhs)
 }
 
 #[derive(PartialEq, Eq)]

From f6c9490e6527d204bd4a22273e01f76632460506 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 19:31:14 +0200
Subject: [PATCH 19/29] Fix wrong suggestion with `...` and for loops

---
 clippy_lints/src/loops.rs      | 26 +++++++++++++++++---------
 tests/compile-fail/for_loop.rs |  4 ++--
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 0f9d030c2f28..2aa0332a8241 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -9,9 +9,9 @@ use rustc::middle::region::CodeExtent;
 use rustc::ty;
 use rustc_const_eval::EvalHint::ExprTypeChecked;
 use rustc_const_eval::eval_const_expr_partial;
-use std::borrow::Cow;
 use std::collections::HashMap;
 use syntax::ast;
+use utils::sugg;
 
 use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg, in_external_macro,
             span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
@@ -332,7 +332,7 @@ fn check_for_loop(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &E
 /// Check for looping over a range and then indexing a sequence with it.
 /// The iteratee must be a range literal.
 fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) {
-    if let Some(higher::Range { start: Some(ref start), ref end, .. }) = higher::range(arg) {
+    if let Some(higher::Range { start: Some(ref start), ref end, limits }) = higher::range(arg) {
         // the var must be a single name
         if let PatKind::Binding(_, ref ident, _) = pat.node {
             let mut visitor = VarVisitor {
@@ -360,20 +360,28 @@ fn check_for_loop_range(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, ex
 
                 let starts_at_zero = is_integer_literal(start, 0);
 
-                let skip: Cow<_> = if starts_at_zero {
-                    "".into()
+                let skip = if starts_at_zero {
+                    "".to_owned()
                 } else {
-                    format!(".skip({})", snippet(cx, start.span, "..")).into()
+                    format!(".skip({})", snippet(cx, start.span, ".."))
                 };
 
-                let take: Cow<_> = if let Some(ref end) = *end {
+                let take = if let Some(ref end) = *end {
                     if is_len_call(end, &indexed) {
-                        "".into()
+                        "".to_owned()
                     } else {
-                        format!(".take({})", snippet(cx, end.span, "..")).into()
+                        match limits {
+                            ast::RangeLimits::Closed => {
+                                let end = sugg::Sugg::hir(cx, end, "<count>");
+                                format!(".take({})", end + sugg::ONE)
+                            }
+                            ast::RangeLimits::HalfOpen => {
+                                format!(".take({})", snippet(cx, end.span, ".."))
+                            }
+                        }
                     }
                 } else {
-                    "".into()
+                    "".to_owned()
                 };
 
                 if visitor.nonindex {
diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs
index 6028c5c0f3a8..1ef8eacd1d94 100644
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -165,7 +165,7 @@ fn main() {
         //~^ ERROR `i` is only used to index `vec`
         //~| HELP consider
         //~| HELP consider
-        //~| SUGGESTION for <item> in vec.iter().take(MAX_LEN) {
+        //~| SUGGESTION for <item> in vec.iter().take(MAX_LEN + 1) {
         println!("{}", vec[i]);
     }
 
@@ -181,7 +181,7 @@ fn main() {
         //~^ ERROR `i` is only used to index `vec`
         //~| HELP consider
         //~| HELP consider
-        //~| SUGGESTION for <item> in vec.iter().take(10).skip(5) {
+        //~| SUGGESTION for <item> in vec.iter().take(10 + 1).skip(5) {
         println!("{}", vec[i]);
     }
 

From 2a45a2ab6b5fd9075b3488c32825a6686f532559 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 20:55:45 +0200
Subject: [PATCH 20/29] Use `utils::sugg` in `FOR_KV_MAP`

---
 clippy_lints/src/loops.rs      | 27 ++++++++++++++-------------
 clippy_lints/src/transmute.rs  |  2 +-
 clippy_lints/src/utils/sugg.rs | 20 ++++++++++++++------
 tests/compile-fail/for_loop.rs | 18 ++++++++++++++++--
 4 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 2aa0332a8241..a4e338053e27 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -596,18 +596,20 @@ fn check_for_loop_explicit_counter(cx: &LateContext, arg: &Expr, body: &Expr, ex
 
 /// Check for the `FOR_KV_MAP` lint.
 fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) {
+    let pat_span = pat.span;
+
     if let PatKind::Tuple(ref pat, _) = pat.node {
         if pat.len() == 2 {
-            let (pat_span, kind) = match (&pat[0].node, &pat[1].node) {
-                (key, _) if pat_is_wild(key, body) => (&pat[1].span, "values"),
-                (_, value) if pat_is_wild(value, body) => (&pat[0].span, "keys"),
+            let (new_pat_span, kind) = match (&pat[0].node, &pat[1].node) {
+                (key, _) if pat_is_wild(key, body) => (pat[1].span, "value"),
+                (_, value) if pat_is_wild(value, body) => (pat[0].span, "key"),
                 _ => return,
             };
 
-            let arg_span = match arg.node {
-                ExprAddrOf(MutImmutable, ref expr) => expr.span,
+            let (arg_span, arg) = match arg.node {
+                ExprAddrOf(MutImmutable, ref expr) => (arg.span, &**expr),
                 ExprAddrOf(MutMutable, _) => return, // for _ in &mut _, there is no {values,keys}_mut method
-                _ => arg.span,
+                _ => (arg.span, arg),
             };
 
             let ty = walk_ptrs_ty(cx.tcx.expr_ty(arg));
@@ -615,14 +617,13 @@ fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Ex
                 span_lint_and_then(cx,
                                    FOR_KV_MAP,
                                    expr.span,
-                                   &format!("you seem to want to iterate on a map's {}", kind),
+                                   &format!("you seem to want to iterate on a map's {}s", kind),
                                    |db| {
-                    db.span_suggestion(expr.span,
-                                       "use the corresponding method",
-                                       format!("for {} in {}.{}() {{ .. }}",
-                                               snippet(cx, *pat_span, ".."),
-                                               snippet(cx, arg_span, ".."),
-                                               kind));
+                    let map = sugg::Sugg::hir(cx, arg, "map");
+                    multispan_sugg(db, "use the corresponding method".into(), &[
+                        (pat_span, &snippet(cx, new_pat_span, kind)),
+                        (arg_span, &format!("{}.{}s()", map.maybe_par(), kind)),
+                    ]);
                 });
             }
         }
diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs
index 12d2184693a4..d95fad8fc0f0 100644
--- a/clippy_lints/src/transmute.rs
+++ b/clippy_lints/src/transmute.rs
@@ -149,7 +149,7 @@ impl LateLintPass for Transmute {
                                     from_ty,
                                     to_ty),
                             |db| {
-                                let arg = &sugg::Sugg::hir(cx, &args[0], "..");
+                                let arg = sugg::Sugg::hir(cx, &args[0], "..");
                                 let (deref, cast) = if to_rty.mutbl == Mutability::MutMutable {
                                     ("&mut *", "*mut")
                                 } else {
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index d05629b1a76e..c2fd2c1a6b7b 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -121,7 +121,7 @@ impl<'a> Sugg<'a> {
 
     /// Convenience method to create the `&<expr>` suggestion.
     pub fn addr(self) -> Sugg<'static> {
-        make_unop("&", &self)
+        make_unop("&", self)
     }
 
     /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>` suggestion.
@@ -131,6 +131,15 @@ impl<'a> Sugg<'a> {
             ast::RangeLimits::Closed => make_assoc(AssocOp::DotDotDot, &self, &end),
         }
     }
+
+    /// Add parenthesis to any expression that might need them. Suitable to the `self` argument of
+    /// a method call (eg. to build `bar.foo()` or `(1 + 2).foo()`).
+    pub fn maybe_par(self) -> Self {
+        match self {
+            Sugg::NonParen(..) => self,
+            Sugg::MaybeParen(sugg) | Sugg::BinOp(_, sugg) => Sugg::NonParen(format!("({})", sugg).into()),
+        }
+    }
 }
 
 impl<'a, 'b> std::ops::Add<Sugg<'b>> for Sugg<'a> {
@@ -150,7 +159,7 @@ impl<'a, 'b> std::ops::Sub<Sugg<'b>> for Sugg<'a> {
 impl<'a> std::ops::Not for Sugg<'a> {
     type Output = Sugg<'static>;
     fn not(self) -> Sugg<'static> {
-        make_unop("!", &self)
+        make_unop("!", self)
     }
 }
 
@@ -178,13 +187,12 @@ impl<T: std::fmt::Display> std::fmt::Display for ParenHelper<T> {
     }
 }
 
-/// Build the string for `<op> <expr>` adding parenthesis when necessary.
+/// Build the string for `<op><expr>` adding parenthesis when necessary.
 ///
 /// For convenience, the operator is taken as a string because all unary operators have the same
 /// precedence.
-pub fn make_unop(op: &str, expr: &Sugg) -> Sugg<'static> {
-    let needs_paren = !matches!(*expr, Sugg::NonParen(..));
-    Sugg::MaybeParen(format!("{}{}", op, ParenHelper::new(needs_paren, expr)).into())
+pub fn make_unop(op: &str, expr: Sugg) -> Sugg<'static> {
+    Sugg::MaybeParen(format!("{}{}", op, expr.maybe_par()).into())
 }
 
 /// Build the string for `<lhs> <op> <rhs>` adding parenthesis when necessary.
diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs
index 1ef8eacd1d94..bcb20be4ff46 100644
--- a/tests/compile-fail/for_loop.rs
+++ b/tests/compile-fail/for_loop.rs
@@ -2,6 +2,7 @@
 #![plugin(clippy)]
 
 use std::collections::*;
+use std::rc::Rc;
 
 static STATIC: [usize; 4] = [ 0,  1,  8, 16 ];
 const CONST: [usize; 4] = [ 0,  1,  8, 16 ];
@@ -388,8 +389,20 @@ fn main() {
     for (_, v) in &m {
         //~^ you seem to want to iterate on a map's values
         //~| HELP use the corresponding method
-        //~| SUGGESTION for v in m.values()
+        //~| HELP use the corresponding method
+        //~| SUGGESTION for v in m.values() {
+        let _v = v;
+    }
+
+    let m : Rc<HashMap<u64, u64>> = Rc::new(HashMap::new());
+    for (_, v) in &*m {
+        //~^ you seem to want to iterate on a map's values
+        //~| HELP use the corresponding method
+        //~| HELP use the corresponding method
+        //~| SUGGESTION for v in (*m).values() {
         let _v = v;
+        // Here the `*` is not actually necesarry, but the test tests that we don't suggest
+        // `in *m.values()` as we used to
     }
 
     let mut m : HashMap<u64, u64> = HashMap::new();
@@ -403,7 +416,8 @@ fn main() {
     for (k, _value) in rm {
         //~^ you seem to want to iterate on a map's keys
         //~| HELP use the corresponding method
-        //~| SUGGESTION for k in rm.keys()
+        //~| HELP use the corresponding method
+        //~| SUGGESTION for k in rm.keys() {
         let _k = k;
     }
 

From 139b977d9d64d67cddabbfee8536b0a7d7ed6de4 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 1 Jul 2016 21:01:56 +0200
Subject: [PATCH 21/29] Cleanup

---
 clippy_lints/src/misc_early.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs
index 45964cf3ce70..c0f1c611f714 100644
--- a/clippy_lints/src/misc_early.rs
+++ b/clippy_lints/src/misc_early.rs
@@ -149,7 +149,7 @@ impl EarlyLintPass for MiscEarly {
                                            "Try not to call a closure in the expression where it is declared.",
                                            |db| {
                                                if decl.inputs.is_empty() {
-                                                   let hint = format!("{}", snippet(cx, block.span, ".."));
+                                                   let hint = snippet(cx, block.span, "..").into_owned();
                                                    db.span_suggestion(expr.span, "Try doing something like: ", hint);
                                                }
                                            });

From cc18556ae550089bf79fb57013dc83ae5873336b Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Sat, 2 Jul 2016 17:24:24 +0200
Subject: [PATCH 22/29] Use `utils::sugg` in swap lints

---
 clippy_lints/src/swap.rs       | 17 ++++----
 clippy_lints/src/utils/sugg.rs | 80 +++++++++++++++++++---------------
 2 files changed, 54 insertions(+), 43 deletions(-)

diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs
index 5a3adfee409d..667c450e6697 100644
--- a/clippy_lints/src/swap.rs
+++ b/clippy_lints/src/swap.rs
@@ -2,7 +2,8 @@ use rustc::hir::*;
 use rustc::lint::*;
 use rustc::ty;
 use syntax::codemap::mk_sp;
-use utils::{differing_macro_contexts, match_type, paths, snippet, snippet_opt, span_lint_and_then, walk_ptrs_ty, SpanlessEq};
+use utils::{differing_macro_contexts, match_type, paths, snippet, span_lint_and_then, walk_ptrs_ty, SpanlessEq};
+use utils::sugg::Sugg;
 
 /// **What it does:** This lints manual swapping.
 ///
@@ -100,17 +101,17 @@ fn check_manual_swap(cx: &LateContext, block: &Block) {
             }
 
             let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) {
-                if let Some(slice) = snippet_opt(cx, slice.span) {
+                if let Some(slice) = Sugg::hir_opt(cx, slice) {
                     (false,
                      format!(" elements of `{}`", slice),
-                     format!("{}.swap({}, {})",slice,  snippet(cx, idx1.span, ".."), snippet(cx, idx2.span, "..")))
+                     format!("{}.swap({}, {})", slice.maybe_par(), snippet(cx, idx1.span, ".."), snippet(cx, idx2.span, "..")))
                 } else {
                     (false, "".to_owned(), "".to_owned())
                 }
             } else {
-                 if let (Some(first), Some(second)) = (snippet_opt(cx, lhs1.span), snippet_opt(cx, rhs1.span)) {
+                 if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) {
                     (true, format!(" `{}` and `{}`", first, second),
-                     format!("std::mem::swap(&mut {}, &mut {})", first, second))
+                     format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()))
                 } else {
                     (true, "".to_owned(), "".to_owned())
                 }
@@ -147,8 +148,8 @@ fn check_suspicious_swap(cx: &LateContext, block: &Block) {
             SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1),
             SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0)
         ], {
-            let (what, lhs, rhs) = if let (Some(first), Some(second)) = (snippet_opt(cx, lhs0.span), snippet_opt(cx, rhs0.span)) {
-                (format!(" `{}` and `{}`", first, second), first, second)
+            let (what, lhs, rhs) = if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs0), Sugg::hir_opt(cx, rhs0)) {
+                (format!(" `{}` and `{}`", first, second), first.mut_addr().to_string(), second.mut_addr().to_string())
             } else {
                 ("".to_owned(), "".to_owned(), "".to_owned())
             };
@@ -162,7 +163,7 @@ fn check_suspicious_swap(cx: &LateContext, block: &Block) {
                                |db| {
                                    if !what.is_empty() {
                                        db.span_suggestion(span, "try",
-                                                          format!("std::mem::swap(&mut {}, &mut {})", lhs, rhs));
+                                                          format!("std::mem::swap({}, {})", lhs, rhs));
                                        db.note("or maybe you should use `std::mem::replace`?");
                                    }
                                });
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index c2fd2c1a6b7b..bbfa8648c531 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -4,7 +4,7 @@ use std::borrow::Cow;
 use std;
 use syntax::ast;
 use syntax::util::parser::AssocOp;
-use utils::{higher, snippet};
+use utils::{higher, snippet, snippet_opt};
 use syntax::print::pprust::binop_to_string;
 
 /// A helper type to build suggestion correctly handling parenthesis.
@@ -31,43 +31,48 @@ impl<'a> std::fmt::Display for Sugg<'a> {
 }
 
 impl<'a> Sugg<'a> {
-    pub fn hir(cx: &LateContext, expr: &'a hir::Expr, default: &'a str) -> Sugg<'a> {
-        let snippet = snippet(cx, expr.span, default);
+    pub fn hir_opt(cx: &LateContext, expr: &hir::Expr) -> Option<Sugg<'a>> {
+        snippet_opt(cx, expr.span).map(|snippet| {
+            let snippet = Cow::Owned(snippet);
+            match expr.node {
+                hir::ExprAddrOf(..) |
+                hir::ExprBox(..) |
+                hir::ExprClosure(..) |
+                hir::ExprIf(..) |
+                hir::ExprUnary(..) |
+                hir::ExprMatch(..) => Sugg::MaybeParen(snippet),
+                hir::ExprAgain(..) |
+                hir::ExprBlock(..) |
+                hir::ExprBreak(..) |
+                hir::ExprCall(..) |
+                hir::ExprField(..) |
+                hir::ExprIndex(..) |
+                hir::ExprInlineAsm(..) |
+                hir::ExprLit(..) |
+                hir::ExprLoop(..) |
+                hir::ExprMethodCall(..) |
+                hir::ExprPath(..) |
+                hir::ExprRepeat(..) |
+                hir::ExprRet(..) |
+                hir::ExprStruct(..) |
+                hir::ExprTup(..) |
+                hir::ExprTupField(..) |
+                hir::ExprVec(..) |
+                hir::ExprWhile(..) => Sugg::NonParen(snippet),
+                hir::ExprAssign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
+                hir::ExprAssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet),
+                hir::ExprBinary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet),
+                hir::ExprCast(..) => Sugg::BinOp(AssocOp::As, snippet),
+                hir::ExprType(..) => Sugg::BinOp(AssocOp::Colon, snippet),
+            }
+        })
+    }
 
-        match expr.node {
-            hir::ExprAddrOf(..) |
-            hir::ExprBox(..) |
-            hir::ExprClosure(..) |
-            hir::ExprIf(..) |
-            hir::ExprUnary(..) |
-            hir::ExprMatch(..) => Sugg::MaybeParen(snippet),
-            hir::ExprAgain(..) |
-            hir::ExprBlock(..) |
-            hir::ExprBreak(..) |
-            hir::ExprCall(..) |
-            hir::ExprField(..) |
-            hir::ExprIndex(..) |
-            hir::ExprInlineAsm(..) |
-            hir::ExprLit(..) |
-            hir::ExprLoop(..) |
-            hir::ExprMethodCall(..) |
-            hir::ExprPath(..) |
-            hir::ExprRepeat(..) |
-            hir::ExprRet(..) |
-            hir::ExprStruct(..) |
-            hir::ExprTup(..) |
-            hir::ExprTupField(..) |
-            hir::ExprVec(..) |
-            hir::ExprWhile(..) => Sugg::NonParen(snippet),
-            hir::ExprAssign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
-            hir::ExprAssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet),
-            hir::ExprBinary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet),
-            hir::ExprCast(..) => Sugg::BinOp(AssocOp::As, snippet),
-            hir::ExprType(..) => Sugg::BinOp(AssocOp::Colon, snippet),
-        }
+    pub fn hir(cx: &LateContext, expr: &hir::Expr, default: &'a str) -> Sugg<'a> {
+        Self::hir_opt(cx, expr).unwrap_or_else(|| Sugg::NonParen(Cow::Borrowed(default)))
     }
 
-    pub fn ast(cx: &EarlyContext, expr: &'a ast::Expr, default: &'a str) -> Sugg<'a> {
+    pub fn ast(cx: &EarlyContext, expr: &ast::Expr, default: &'a str) -> Sugg<'a> {
         use syntax::ast::RangeLimits;
 
         let snippet = snippet(cx, expr.span, default);
@@ -124,6 +129,11 @@ impl<'a> Sugg<'a> {
         make_unop("&", self)
     }
 
+    /// Convenience method to create the `&mut <expr>` suggestion.
+    pub fn mut_addr(self) -> Sugg<'static> {
+        make_unop("&mut ", self)
+    }
+
     /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>` suggestion.
     pub fn range(self, end: Self, limit: ast::RangeLimits) -> Sugg<'static> {
         match limit {

From ffa840d4f2bbe31721b40d56093259c4a5c0b50b Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Sun, 3 Jul 2016 19:13:01 +0200
Subject: [PATCH 23/29] Use `utils::sugg` in `match` related lints

Also don't build suggestion when unnecessary.
---
 clippy_lints/src/matches.rs    | 114 ++++++++++++++++-----------------
 clippy_lints/src/utils/sugg.rs |   5 ++
 tests/compile-fail/matches.rs  |  12 ++--
 3 files changed, 67 insertions(+), 64 deletions(-)

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 94b36d28ed35..de21cabc6fb4 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -235,57 +235,54 @@ fn check_single_match_opt_like(cx: &LateContext, ex: &Expr, arms: &[Arm], expr:
 fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
     // type of expression == bool
     if cx.tcx.expr_ty(ex).sty == ty::TyBool {
-        let sugg = if arms.len() == 2 && arms[0].pats.len() == 1 {
-            // no guards
-            let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
-                if let ExprLit(ref lit) = arm_bool.node {
-                    match lit.node {
-                        LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
-                        LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
-                        _ => None,
+        span_lint_and_then(cx,
+                           MATCH_BOOL,
+                           expr.span,
+                           "you seem to be trying to match on a boolean expression",
+                           move |db| {
+            if arms.len() == 2 && arms[0].pats.len() == 1 {
+                // no guards
+                let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
+                    if let ExprLit(ref lit) = arm_bool.node {
+                        match lit.node {
+                            LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
+                            LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
+                            _ => None,
+                        }
+                    } else {
+                        None
                     }
                 } else {
                     None
-                }
-            } else {
-                None
-            };
-
-            if let Some((ref true_expr, ref false_expr)) = exprs {
-                match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
-                    (false, false) => {
-                        Some(format!("if {} {} else {}",
-                                     snippet(cx, ex.span, "b"),
-                                     expr_block(cx, true_expr, None, ".."),
-                                     expr_block(cx, false_expr, None, "..")))
+                };
+
+                if let Some((ref true_expr, ref false_expr)) = exprs {
+                    let sugg = match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
+                        (false, false) => {
+                            Some(format!("if {} {} else {}",
+                                         snippet(cx, ex.span, "b"),
+                                         expr_block(cx, true_expr, None, ".."),
+                                         expr_block(cx, false_expr, None, "..")))
+                        }
+                        (false, true) => {
+                            Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
+                        }
+                        (true, false) => {
+                            let test = Sugg::hir(cx, ex, "..");
+                            Some(format!("if {} {}",
+                                         !test,
+                                         expr_block(cx, false_expr, None, "..")))
+                        }
+                        (true, true) => None,
+                    };
+
+                    if let Some(sugg) = sugg {
+                        db.span_suggestion(expr.span, "consider using an if/else expression", sugg);
                     }
-                    (false, true) => {
-                        Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
-                    }
-                    (true, false) => {
-                        let test = Sugg::hir(cx, ex, "..");
-                        Some(format!("if {} {}",
-                                     !test,
-                                     expr_block(cx, false_expr, None, "..")))
-                    }
-                    (true, true) => None,
                 }
-            } else {
-                None
             }
-        } else {
-            None
-        };
 
-        span_lint_and_then(cx,
-                           MATCH_BOOL,
-                           expr.span,
-                           "you seem to be trying to match on a boolean expression. Consider using an if..else block:",
-                           move |db| {
-                               if let Some(sugg) = sugg {
-                                   db.span_suggestion(expr.span, "try this", sugg);
-                               }
-                           });
+       });
     }
 }
 
@@ -309,26 +306,28 @@ fn check_overlapping_arms(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
 fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: MatchSource, expr: &Expr) {
     if has_only_ref_pats(arms) {
         if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
-            let template = match_template(cx, expr.span, source, "", inner);
             span_lint_and_then(cx,
                                MATCH_REF_PATS,
                                expr.span,
                                "you don't need to add `&` to both the expression and the patterns",
                                |db| {
-                                   db.span_suggestion(expr.span, "try", template);
-                               });
+                let inner = Sugg::hir(cx, inner, "..");
+                let template = match_template(expr.span, source, inner);
+                db.span_suggestion(expr.span, "try", template);
+            });
         } else {
-            let template = match_template(cx, expr.span, source, "*", ex);
             span_lint_and_then(cx,
                                MATCH_REF_PATS,
                                expr.span,
                                "you don't need to add `&` to all patterns",
                                |db| {
-                                   db.span_suggestion(expr.span,
-                                                      "instead of prefixing all patterns with `&`, you can \
-                                                       dereference the expression",
-                                                      template);
-                               });
+                let ex = Sugg::hir(cx, ex, "..");
+                let template = match_template(expr.span, source, ex.deref());
+                db.span_suggestion(expr.span,
+                                   "instead of prefixing all patterns with `&`, you can \
+                                   dereference the expression",
+                                   template);
+            });
         }
     }
 }
@@ -411,12 +410,11 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
     mapped.map_or(false, |v| v.iter().any(|el| *el))
 }
 
-fn match_template(cx: &LateContext, span: Span, source: MatchSource, op: &str, expr: &Expr) -> String {
-    let expr_snippet = snippet(cx, expr.span, "..");
+fn match_template(span: Span, source: MatchSource, expr: Sugg) -> String {
     match source {
-        MatchSource::Normal => format!("match {}{} {{ .. }}", op, expr_snippet),
-        MatchSource::IfLetDesugar { .. } => format!("if let .. = {}{} {{ .. }}", op, expr_snippet),
-        MatchSource::WhileLetDesugar => format!("while let .. = {}{} {{ .. }}", op, expr_snippet),
+        MatchSource::Normal => format!("match {} {{ .. }}", expr),
+        MatchSource::IfLetDesugar { .. } => format!("if let .. = {} {{ .. }}", expr),
+        MatchSource::WhileLetDesugar => format!("while let .. = {} {{ .. }}", expr),
         MatchSource::ForLoopDesugar => span_bug!(span, "for loop desugared to match with &-patterns!"),
         MatchSource::TryDesugar => span_bug!(span, "`?` operator desugared to match with &-patterns!"),
     }
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index bbfa8648c531..f857c821e7ba 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -134,6 +134,11 @@ impl<'a> Sugg<'a> {
         make_unop("&mut ", self)
     }
 
+    /// Convenience method to create the `*<expr>` suggestion.
+    pub fn deref(self) -> Sugg<'static> {
+        make_unop("*", self)
+    }
+
     /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>` suggestion.
     pub fn range(self, end: Self, limit: ast::RangeLimits) -> Sugg<'static> {
         match limit {
diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs
index e49aeaa6ec86..f64cceb5c347 100644
--- a/tests/compile-fail/matches.rs
+++ b/tests/compile-fail/matches.rs
@@ -112,7 +112,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if test { 0 } else { 42 };
         true => 0,
         false => 42,
@@ -121,7 +121,7 @@ fn match_bool() {
     let option = 1;
     match option == 1 {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if option == 1 { 1 } else { 0 };
         true => 1,
         false => 0,
@@ -129,7 +129,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if !test { println!("Noooo!"); };
         true => (),
         false => { println!("Noooo!"); }
@@ -137,7 +137,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if !test { println!("Noooo!"); };
         false => { println!("Noooo!"); }
         _ => (),
@@ -145,7 +145,7 @@ fn match_bool() {
 
     match test && test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if !(test && test) { println!("Noooo!"); };
     //~| ERROR equal expressions as operands
         false => { println!("Noooo!"); }
@@ -154,7 +154,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if test { println!("Yes!"); } else { println!("Noooo!"); };
         false => { println!("Noooo!"); }
         true => { println!("Yes!"); }

From 2f259b8cd345988090c401f551001dfc63ccfd2d Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Sun, 3 Jul 2016 19:24:44 +0200
Subject: [PATCH 24/29] Use `span_suggestion` in entry lints

---
 clippy_lints/src/entry.rs   |  8 ++++----
 tests/compile-fail/entry.rs | 34 ++++++++++++++++++++--------------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs
index bc209fd4846e..c7afc2d5cd9e 100644
--- a/clippy_lints/src/entry.rs
+++ b/clippy_lints/src/entry.rs
@@ -121,21 +121,21 @@ impl<'a, 'tcx, 'v, 'b> Visitor<'v> for InsertVisitor<'a, 'tcx, 'b> {
             SpanlessEq::new(self.cx).eq_expr(self.key, &params[1])
         ], {
             span_lint_and_then(self.cx, MAP_ENTRY, self.span,
-                               &format!("usage of `contains_key` followed by `insert` on `{}`", self.ty), |db| {
+                               &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| {
                 if self.sole_expr {
                     let help = format!("{}.entry({}).or_insert({})",
                                        snippet(self.cx, self.map.span, "map"),
                                        snippet(self.cx, params[1].span, ".."),
                                        snippet(self.cx, params[2].span, ".."));
 
-                    db.span_suggestion(self.span, "Consider using", help);
+                    db.span_suggestion(self.span, "consider using", help);
                 }
                 else {
-                    let help = format!("Consider using `{}.entry({})`",
+                    let help = format!("{}.entry({})",
                                        snippet(self.cx, self.map.span, "map"),
                                        snippet(self.cx, params[1].span, ".."));
 
-                    db.span_note(self.span, &help);
+                    db.span_suggestion(self.span, "consider using", help);
                 }
             });
         }}
diff --git a/tests/compile-fail/entry.rs b/tests/compile-fail/entry.rs
index 7dc4054ec5bf..ec3b75abb371 100755
--- a/tests/compile-fail/entry.rs
+++ b/tests/compile-fail/entry.rs
@@ -11,45 +11,51 @@ fn foo() {}
 
 fn insert_if_absent0<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
     if !m.contains_key(&k) { m.insert(k, v); }
-    //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
-    //~| HELP Consider
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `HashMap`
+    //~| HELP consider
     //~| SUGGESTION m.entry(k).or_insert(v)
 }
 
 fn insert_if_absent1<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
     if !m.contains_key(&k) { foo(); m.insert(k, v); }
-    //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
-    //~| NOTE Consider using `m.entry(k)`
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `HashMap`
+    //~| HELP consider
+    //~| SUGGESTION m.entry(k)
 }
 
 fn insert_if_absent2<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
     if !m.contains_key(&k) { m.insert(k, v) } else { None };
-    //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
-    //~| NOTE Consider using `m.entry(k)`
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `HashMap`
+    //~| HELP consider
+    //~| SUGGESTION m.entry(k)
 }
 
 fn insert_if_present2<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
     if m.contains_key(&k) { None } else { m.insert(k, v) };
-    //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
-    //~| NOTE Consider using `m.entry(k)`
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `HashMap`
+    //~| HELP consider
+    //~| SUGGESTION m.entry(k)
 }
 
 fn insert_if_absent3<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
     if !m.contains_key(&k) { foo(); m.insert(k, v) } else { None };
-    //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
-    //~| NOTE Consider using `m.entry(k)`
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `HashMap`
+    //~| HELP consider
+    //~| SUGGESTION m.entry(k)
 }
 
 fn insert_if_present3<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, v: V) {
     if m.contains_key(&k) { None } else { foo(); m.insert(k, v) };
-    //~^ ERROR usage of `contains_key` followed by `insert` on `HashMap`
-    //~| NOTE Consider using `m.entry(k)`
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `HashMap`
+    //~| HELP consider
+    //~| SUGGESTION m.entry(k)
 }
 
 fn insert_in_btreemap<K: Ord, V>(m: &mut BTreeMap<K, V>, k: K, v: V) {
     if !m.contains_key(&k) { foo(); m.insert(k, v) } else { None };
-    //~^ ERROR usage of `contains_key` followed by `insert` on `BTreeMap`
-    //~| NOTE Consider using `m.entry(k)`
+    //~^ ERROR usage of `contains_key` followed by `insert` on a `BTreeMap`
+    //~| HELP consider
+    //~| SUGGESTION m.entry(k)
 }
 
 fn insert_other_if_absent<K: Eq + Hash, V>(m: &mut HashMap<K, V>, k: K, o: K, v: V) {

From 9b79b1022c68b79dbcdf2e51d46843c6e801d1f7 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Mon, 4 Jul 2016 01:17:31 +0200
Subject: [PATCH 25/29] Fix suggestions for `needless_bool`

---
 clippy_lints/src/needless_bool.rs   | 36 ++++++++++++++++++-----------
 tests/compile-fail/needless_bool.rs | 31 +++++++++++++++++++++----
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index 5f912366770c..fa2a350c36f0 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -6,7 +6,8 @@ use rustc::lint::*;
 use rustc::hir::*;
 use syntax::ast::LitKind;
 use syntax::codemap::Spanned;
-use utils::{span_lint, span_lint_and_then, snippet, snippet_opt};
+use utils::{span_lint, span_lint_and_then, snippet};
+use utils::sugg::Sugg;
 
 /// **What it does:** This lint checks for expressions of the form `if c { true } else { false }` (or vice versa) and suggest using the condition directly.
 ///
@@ -49,11 +50,20 @@ impl LateLintPass for NeedlessBool {
     fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
         use self::Expression::*;
         if let ExprIf(ref pred, ref then_block, Some(ref else_expr)) = e.node {
-            let reduce = |hint: &str, not| {
-                let hint = match snippet_opt(cx, pred.span) {
-                    Some(pred_snip) => format!("`{}{}`", not, pred_snip),
-                    None => hint.into(),
+            let reduce = |ret, not| {
+                let snip = Sugg::hir(cx, pred, "<predicate>");
+                let snip = if not {
+                    !snip
+                } else {
+                    snip
+                };
+
+                let hint = if ret {
+                    format!("return {};", snip)
+                } else {
+                    snip.to_string()
                 };
+
                 span_lint_and_then(cx,
                                    NEEDLESS_BOOL,
                                    e.span,
@@ -77,10 +87,10 @@ impl LateLintPass for NeedlessBool {
                               e.span,
                               "this if-then-else expression will always return false");
                 }
-                (RetBool(true), RetBool(false)) => reduce("its predicate", "return "),
-                (Bool(true), Bool(false)) => reduce("its predicate", ""),
-                (RetBool(false), RetBool(true)) => reduce("`!` and its predicate", "return !"),
-                (Bool(false), Bool(true)) => reduce("`!` and its predicate", "!"),
+                (RetBool(true), RetBool(false)) => reduce(true, false),
+                (Bool(true), Bool(false)) => reduce(false, false),
+                (RetBool(false), RetBool(true)) => reduce(true, true),
+                (Bool(false), Bool(true)) => reduce(false, true),
                 _ => (),
             }
         }
@@ -122,23 +132,23 @@ impl LateLintPass for BoolComparison {
                                        });
                 }
                 (Bool(false), Other) => {
-                    let hint = format!("!{}", snippet(cx, right_side.span, ".."));
+                    let hint = Sugg::hir(cx, right_side, "..");
                     span_lint_and_then(cx,
                                        BOOL_COMPARISON,
                                        e.span,
                                        "equality checks against false can be replaced by a negation",
                                        |db| {
-                                           db.span_suggestion(e.span, "try simplifying it as shown:", hint);
+                                           db.span_suggestion(e.span, "try simplifying it as shown:", (!hint).to_string());
                                        });
                 }
                 (Other, Bool(false)) => {
-                    let hint = format!("!{}", snippet(cx, left_side.span, ".."));
+                    let hint = Sugg::hir(cx, left_side, "..");
                     span_lint_and_then(cx,
                                        BOOL_COMPARISON,
                                        e.span,
                                        "equality checks against false can be replaced by a negation",
                                        |db| {
-                                           db.span_suggestion(e.span, "try simplifying it as shown:", hint);
+                                           db.span_suggestion(e.span, "try simplifying it as shown:", (!hint).to_string());
                                        });
                 }
                 _ => (),
diff --git a/tests/compile-fail/needless_bool.rs b/tests/compile-fail/needless_bool.rs
index 480c16f16660..fb81d44308a0 100644
--- a/tests/compile-fail/needless_bool.rs
+++ b/tests/compile-fail/needless_bool.rs
@@ -5,21 +5,28 @@
 #[allow(if_same_then_else)]
 fn main() {
     let x = true;
+    let y = false;
     if x { true } else { true }; //~ERROR this if-then-else expression will always return true
     if x { false } else { false }; //~ERROR this if-then-else expression will always return false
     if x { true } else { false };
     //~^ ERROR this if-then-else expression returns a bool literal
     //~| HELP you can reduce it to
-    //~| SUGGESTION `x`
+    //~| SUGGESTION x
     if x { false } else { true };
     //~^ ERROR this if-then-else expression returns a bool literal
     //~| HELP you can reduce it to
-    //~| SUGGESTION `!x`
+    //~| SUGGESTION !x
+    if x && y { false } else { true };
+    //~^ ERROR this if-then-else expression returns a bool literal
+    //~| HELP you can reduce it to
+    //~| SUGGESTION !(x && y)
     if x { x } else { false }; // would also be questionable, but we don't catch this yet
     bool_ret(x);
     bool_ret2(x);
     bool_ret3(x);
+    bool_ret5(x, x);
     bool_ret4(x);
+    bool_ret6(x, x);
 }
 
 #[allow(if_same_then_else, needless_return)]
@@ -39,7 +46,15 @@ fn bool_ret3(x: bool) -> bool {
     if x { return true } else { return false };
     //~^ ERROR this if-then-else expression returns a bool literal
     //~| HELP you can reduce it to
-    //~| SUGGESTION `return x`
+    //~| SUGGESTION return x
+}
+
+#[allow(needless_return)]
+fn bool_ret5(x: bool, y: bool) -> bool {
+    if x && y { return true } else { return false };
+    //~^ ERROR this if-then-else expression returns a bool literal
+    //~| HELP you can reduce it to
+    //~| SUGGESTION return x && y
 }
 
 #[allow(needless_return)]
@@ -47,5 +62,13 @@ fn bool_ret4(x: bool) -> bool {
     if x { return false } else { return true };
     //~^ ERROR this if-then-else expression returns a bool literal
     //~| HELP you can reduce it to
-    //~| SUGGESTION `return !x`
+    //~| SUGGESTION return !x
+}
+
+#[allow(needless_return)]
+fn bool_ret6(x: bool, y: bool) -> bool {
+    if x && y { return false } else { return true };
+    //~^ ERROR this if-then-else expression returns a bool literal
+    //~| HELP you can reduce it to
+    //~| SUGGESTION return !(x && y)
 }

From c5e91e70d03ace2c9647e509540a5b6b88aaf801 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Mon, 4 Jul 2016 02:22:57 +0200
Subject: [PATCH 26/29] Use `sugg::Sugg` in transmute links

---
 clippy_lints/src/transmute.rs   | 22 +++++++++++-----------
 clippy_lints/src/utils/sugg.rs  |  6 ++++++
 tests/compile-fail/transmute.rs |  7 +++++++
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs
index d95fad8fc0f0..88ed11d26e58 100644
--- a/clippy_lints/src/transmute.rs
+++ b/clippy_lints/src/transmute.rs
@@ -2,7 +2,7 @@ use rustc::lint::*;
 use rustc::ty::TypeVariants::{TyRawPtr, TyRef};
 use rustc::ty;
 use rustc::hir::*;
-use utils::{match_def_path, paths, snippet_opt, span_lint, span_lint_and_then};
+use utils::{match_def_path, paths, span_lint, span_lint_and_then};
 use utils::sugg;
 
 /// **What it does:** This lint checks for transmutes that can't ever be correct on any architecture
@@ -93,14 +93,14 @@ impl LateLintPass for Transmute {
                             e.span,
                             "transmute from a reference to a pointer",
                             |db| {
-                                if let Some(arg) = snippet_opt(cx, args[0].span) {
+                                if let Some(arg) = sugg::Sugg::hir_opt(cx, &*args[0]) {
                                     let sugg = if ptr_ty == rty {
-                                        format!("{} as {}", arg, to_ty)
+                                        arg.as_ty(&to_ty.to_string())
                                     } else {
-                                        format!("{} as {} as {}", arg, cx.tcx.mk_ptr(rty), to_ty)
+                                        arg.as_ty(&format!("{} as {}", cx.tcx.mk_ptr(rty), to_ty))
                                     };
 
-                                    db.span_suggestion(e.span, "try", sugg);
+                                    db.span_suggestion(e.span, "try", sugg.to_string());
                                 }
                             },
                         ),
@@ -111,8 +111,8 @@ impl LateLintPass for Transmute {
                             e.span,
                             "transmute from an integer to a pointer",
                             |db| {
-                                if let Some(arg) = snippet_opt(cx, args[0].span) {
-                                    db.span_suggestion(e.span, "try", format!("{} as {}", arg, to_ty));
+                                if let Some(arg) = sugg::Sugg::hir_opt(cx, &*args[0]) {
+                                    db.span_suggestion(e.span, "try", arg.as_ty(&to_ty.to_string()).to_string());
                                 }
                             },
                         ),
@@ -157,13 +157,13 @@ impl LateLintPass for Transmute {
                                 };
 
 
-                                let sugg = if from_pty.ty == to_rty.ty {
-                                    sugg::make_unop(deref, arg).to_string()
+                                let arg = if from_pty.ty == to_rty.ty {
+                                    arg
                                 } else {
-                                    format!("{}({} as {} {})", deref, arg, cast, to_rty.ty)
+                                    arg.as_ty(&format!("{} {}", cast, to_rty.ty))
                                 };
 
-                                db.span_suggestion(e.span, "try", sugg);
+                                db.span_suggestion(e.span, "try", sugg::make_unop(deref, arg).to_string());
                             },
                         ),
                         _ => return,
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index f857c821e7ba..4a9543d47c1c 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -30,6 +30,7 @@ impl<'a> std::fmt::Display for Sugg<'a> {
     }
 }
 
+#[allow(wrong_self_convention)] // ok, because of the function `as_ty` method
 impl<'a> Sugg<'a> {
     pub fn hir_opt(cx: &LateContext, expr: &hir::Expr) -> Option<Sugg<'a>> {
         snippet_opt(cx, expr.span).map(|snippet| {
@@ -124,6 +125,11 @@ impl<'a> Sugg<'a> {
         make_binop(ast::BinOpKind::And, &self, &rhs)
     }
 
+    /// Convenience method to create the `<lhs> as <rhs>` suggestion.
+    pub fn as_ty(self, rhs: &str) -> Sugg<'static> {
+        make_assoc(AssocOp::As, &self, &Sugg::NonParen(rhs.into()))
+    }
+
     /// Convenience method to create the `&<expr>` suggestion.
     pub fn addr(self) -> Sugg<'static> {
         make_unop("&", self)
diff --git a/tests/compile-fail/transmute.rs b/tests/compile-fail/transmute.rs
index 0dbd58b13089..1344858996e9 100644
--- a/tests/compile-fail/transmute.rs
+++ b/tests/compile-fail/transmute.rs
@@ -111,6 +111,13 @@ fn useless() {
         //~^ ERROR transmute from an integer to a pointer
         //~| HELP try
         //~| SUGGESTION 5_isize as *const usize
+        let _  = 5_isize as *const usize;
+
+        let _: *const usize = std::mem::transmute(1+1usize);
+        //~^ ERROR transmute from an integer to a pointer
+        //~| HELP try
+        //~| SUGGESTION (1+1usize) as *const usize
+        let _  = (1+1_usize) as *const usize;
     }
 }
 

From 8aaaf198e31678d5036eae5acd32e71a208df828 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Tue, 5 Jul 2016 23:26:47 +0200
Subject: [PATCH 27/29] Use `utils::sugg` in methods lints

---
 clippy_lints/src/methods.rs | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs
index 9d5c436c2cbc..f70ec4eac9f8 100644
--- a/clippy_lints/src/methods.rs
+++ b/clippy_lints/src/methods.rs
@@ -11,10 +11,11 @@ use std::fmt;
 use syntax::codemap::Span;
 use syntax::ptr::P;
 use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path, match_trait_method,
-            match_type, method_chain_args, return_ty, same_tys, snippet, snippet_opt, span_lint,
+            match_type, method_chain_args, return_ty, same_tys, snippet, span_lint,
             span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
 use utils::MethodArgs;
 use utils::paths;
+use utils::sugg;
 
 #[derive(Clone)]
 pub struct Pass;
@@ -628,8 +629,8 @@ fn lint_clone_double_ref(cx: &LateContext, expr: &hir::Expr, arg: &hir::Expr, ty
                                expr.span,
                                "using `clone` on a double-reference; \
                                 this will copy the reference instead of cloning the inner type",
-                               |db| if let Some(snip) = snippet_opt(cx, arg.span) {
-                                   db.span_suggestion(expr.span, "try dereferencing it", format!("(*{}).clone()", snip));
+                               |db| if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) {
+                                   db.span_suggestion(expr.span, "try dereferencing it", format!("({}).clone()", snip.deref()));
                                });
         }
     }
@@ -641,14 +642,13 @@ fn lint_extend(cx: &LateContext, expr: &hir::Expr, args: &MethodArgs) {
         return;
     }
     let arg_ty = cx.tcx.expr_ty(&args[1]);
-    if let Some((span, r)) = derefs_to_slice(cx, &args[1], &arg_ty) {
+    if let Some(slice) = derefs_to_slice(cx, &args[1], &arg_ty) {
         span_lint_and_then(cx, EXTEND_FROM_SLICE, expr.span, "use of `extend` to extend a Vec by a slice", |db| {
             db.span_suggestion(expr.span,
                                "try this",
-                               format!("{}.extend_from_slice({}{})",
+                               format!("{}.extend_from_slice({})",
                                        snippet(cx, args[0].span, "_"),
-                                       r,
-                                       snippet(cx, span, "_")));
+                                       slice));
         });
     }
 }
@@ -695,7 +695,7 @@ fn lint_iter_nth(cx: &LateContext, expr: &hir::Expr, iter_args: &MethodArgs, is_
     );
 }
 
-fn derefs_to_slice(cx: &LateContext, expr: &hir::Expr, ty: &ty::Ty) -> Option<(Span, &'static str)> {
+fn derefs_to_slice(cx: &LateContext, expr: &hir::Expr, ty: &ty::Ty) -> Option<sugg::Sugg<'static>> {
     fn may_slice(cx: &LateContext, ty: &ty::Ty) -> bool {
         match ty.sty {
             ty::TySlice(_) => true,
@@ -706,19 +706,22 @@ fn derefs_to_slice(cx: &LateContext, expr: &hir::Expr, ty: &ty::Ty) -> Option<(S
             _ => false,
         }
     }
+
     if let hir::ExprMethodCall(name, _, ref args) = expr.node {
         if &name.node.as_str() == &"iter" && may_slice(cx, &cx.tcx.expr_ty(&args[0])) {
-            Some((args[0].span, "&"))
+            sugg::Sugg::hir_opt(cx, &*args[0]).map(|sugg| {
+                sugg.addr()
+            })
         } else {
             None
         }
     } else {
         match ty.sty {
-            ty::TySlice(_) => Some((expr.span, "")),
+            ty::TySlice(_) => sugg::Sugg::hir_opt(cx, expr),
             ty::TyRef(_, ty::TypeAndMut { ty: ref inner, .. }) |
             ty::TyBox(ref inner) => {
                 if may_slice(cx, inner) {
-                    Some((expr.span, ""))
+                    sugg::Sugg::hir_opt(cx, expr)
                 } else {
                     None
                 }

From ded5b30f2379fd564f07a4d08d2c1a8c970cd1cd Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 6 Jul 2016 14:38:48 +0200
Subject: [PATCH 28/29] Mention the major sugg. refactoring in CHANGELOG

---
 CHANGELOG.md | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ae316a90b05a..e0ea031c73f2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,10 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
-## 0.0.78 - 2016-07-02
+## 0.0.79 — ?
+* Major suggestions refactoring
+
+## 0.0.78 — 2016-07-02
 * Rustup to *rustc 1.11.0-nightly (01411937f 2016-07-01)*
 * New lints: [`wrong_transmute`, `double_neg`]
 * For compatibility, `cargo clippy` does not defines the `clippy` feature

From bf513229b1be77aca80199d7220257548dff32c8 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 6 Jul 2016 15:36:42 +0200
Subject: [PATCH 29/29] Address PR's comments

---
 clippy_lints/src/needless_bool.rs | 2 +-
 clippy_lints/src/transmute.rs     | 4 ++--
 clippy_lints/src/utils/sugg.rs    | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index fa2a350c36f0..c3e2a0e61679 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -59,7 +59,7 @@ impl LateLintPass for NeedlessBool {
                 };
 
                 let hint = if ret {
-                    format!("return {};", snip)
+                    format!("return {}", snip)
                 } else {
                     snip.to_string()
                 };
diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs
index 88ed11d26e58..ba4da0e8f654 100644
--- a/clippy_lints/src/transmute.rs
+++ b/clippy_lints/src/transmute.rs
@@ -95,9 +95,9 @@ impl LateLintPass for Transmute {
                             |db| {
                                 if let Some(arg) = sugg::Sugg::hir_opt(cx, &*args[0]) {
                                     let sugg = if ptr_ty == rty {
-                                        arg.as_ty(&to_ty.to_string())
+                                        arg.as_ty(to_ty)
                                     } else {
-                                        arg.as_ty(&format!("{} as {}", cx.tcx.mk_ptr(rty), to_ty))
+                                        arg.as_ty(cx.tcx.mk_ptr(rty)).as_ty(to_ty)
                                     };
 
                                     db.span_suggestion(e.span, "try", sugg.to_string());
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 4a9543d47c1c..624c030cdd44 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -126,8 +126,8 @@ impl<'a> Sugg<'a> {
     }
 
     /// Convenience method to create the `<lhs> as <rhs>` suggestion.
-    pub fn as_ty(self, rhs: &str) -> Sugg<'static> {
-        make_assoc(AssocOp::As, &self, &Sugg::NonParen(rhs.into()))
+    pub fn as_ty<R: std::fmt::Display>(self, rhs: R) -> Sugg<'static> {
+        make_assoc(AssocOp::As, &self, &Sugg::NonParen(rhs.to_string().into()))
     }
 
     /// Convenience method to create the `&<expr>` suggestion.