Use utils::sugg in swap lints

Author: mcarton

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`?");
                                    }
                                });

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 {