From cc18556ae550089bf79fb57013dc83ae5873336b Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 2 Jul 2016 17:24:24 +0200 Subject: [PATCH] Use `utils::sugg` in swap lints --- clippy_lints/src/swap.rs | 17 +++---- clippy_lints/src/utils/sugg.rs | 82 +++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 44 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); - - 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_opt(cx: &LateContext, expr: &hir::Expr) -> Option> { + 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), + } + }) } - pub fn ast(cx: &EarlyContext, expr: &'a ast::Expr, default: &'a str) -> Sugg<'a> { + 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: &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 ` suggestion. + pub fn mut_addr(self) -> Sugg<'static> { + make_unop("&mut ", self) + } + /// Convenience method to create the `..` or `...` suggestion. pub fn range(self, end: Self, limit: ast::RangeLimits) -> Sugg<'static> { match limit {