diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 1d9e3984e74..ca5a097119d 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 8fddcb84922..94b36d28ed3 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 0931d5aca6b..b1627c31c8b 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -72,7 +72,7 @@ fn check_stmt(&mut self, cx: &LateContext, s: &Stmt) { 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 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { 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 90cb4cf00fd..d05629b1a76 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -5,6 +5,7 @@ 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 @@ pub fn ast(cx: &EarlyContext, expr: &'a ast::Expr, default: &'a str) -> 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 ` && ` suggestion. + pub fn and(self, rhs: Self) -> Sugg<'static> { + make_binop(ast::BinOpKind::And, &self, &rhs) } /// Convenience method to create the `&` suggestion. - pub fn addr(&self) -> Sugg<'static> { - make_unop("&", self) + pub fn addr(self) -> Sugg<'static> { + make_unop("&", &self) + } + + /// Convenience method to create the `..` or `...` 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::Sub<&'b Sugg<'b>> for &'a Sugg<'a> { +impl<'a, 'b> std::ops::Add> for Sugg<'a> { type Output = Sugg<'static>; - fn sub(self, rhs: &'b Sugg<'b>) -> Sugg<'static> { - make_binop(ast::BinOpKind::Sub, self, rhs) + fn add(self, rhs: Sugg<'b>) -> Sugg<'static> { + make_binop(ast::BinOpKind::Add, &self, &rhs) } } -impl<'a> std::ops::Not for &'a Sugg<'a> { +impl<'a, 'b> std::ops::Sub> for Sugg<'a> { + type Output = Sugg<'static>; + fn sub(self, rhs: Sugg<'b>) -> Sugg<'static> { + make_binop(ast::BinOpKind::Sub, &self, &rhs) + } +} + +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 @@ fn needs_paren(op: &AssocOp, other: &AssocOp, dir: Associativity) -> bool { 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)]