refactor ```Sugg::BinOp```

4 files changed, 166 insertions, 138 deletions

diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs
index aa382f8a974..c62fa5e998b 100644
--- a/clippy_lints/src/loops/manual_memcpy.rs
+++ b/clippy_lints/src/loops/manual_memcpy.rs
@@ -12,6 +12,7 @@ use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Pat, PatKind, StmtKind}
 use rustc_lint::LateContext;
 use rustc_middle::ty::{self, Ty};
 use rustc_span::symbol::sym;
+use std::fmt::Display;
 use std::iter::Iterator;
 
 /// Checks for for loops that sequentially copy items from one slice-like
@@ -108,7 +109,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
     src: &IndexExpr<'_>,
 ) -> String {
     fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
-        if offset.as_str() == "0" {
+        if offset.to_string() == "0" {
             sugg::EMPTY.into()
         } else {
             offset
@@ -123,7 +124,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
             if let Some(arg) = len_args.get(0);
             if path_to_local(arg) == path_to_local(base);
             then {
-                if sugg.as_str() == end_str {
+                if sugg.to_string() == end_str {
                     sugg::EMPTY.into()
                 } else {
                     sugg
@@ -147,7 +148,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
             print_offset(apply_offset(&start_str, &idx_expr.idx_offset)).into_sugg(),
             print_limit(
                 end,
-                end_str.as_str(),
+                end_str.to_string().as_str(),
                 idx_expr.base,
                 apply_offset(&end_str, &idx_expr.idx_offset),
             )
@@ -159,7 +160,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
                 print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)).into_sugg(),
                 print_limit(
                     end,
-                    end_str.as_str(),
+                    end_str.to_string().as_str(),
                     idx_expr.base,
                     apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str,
                 )
@@ -202,12 +203,13 @@ fn build_manual_memcpy_suggestion<'tcx>(
 #[derive(Clone)]
 struct MinifyingSugg<'a>(Sugg<'a>);
 
-impl<'a> MinifyingSugg<'a> {
-    fn as_str(&self) -> &str {
-        let (Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s)) = &self.0;
-        s.as_ref()
+impl Display for MinifyingSugg<'a> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.fmt(f)
     }
+}
 
+impl<'a> MinifyingSugg<'a> {
     fn into_sugg(self) -> Sugg<'a> {
         self.0
     }
@@ -222,7 +224,7 @@ impl<'a> From<Sugg<'a>> for MinifyingSugg<'a> {
 impl std::ops::Add for &MinifyingSugg<'static> {
     type Output = MinifyingSugg<'static>;
     fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
-        match (self.as_str(), rhs.as_str()) {
+        match (self.to_string().as_str(), rhs.to_string().as_str()) {
             ("0", _) => rhs.clone(),
             (_, "0") => self.clone(),
             (_, _) => (&self.0 + &rhs.0).into(),
@@ -233,7 +235,7 @@ impl std::ops::Add for &MinifyingSugg<'static> {
 impl std::ops::Sub for &MinifyingSugg<'static> {
     type Output = MinifyingSugg<'static>;
     fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
-        match (self.as_str(), rhs.as_str()) {
+        match (self.to_string().as_str(), rhs.to_string().as_str()) {
             (_, "0") => self.clone(),
             ("0", _) => (-rhs.0.clone()).into(),
             (x, y) if x == y => sugg::ZERO.into(),
@@ -245,7 +247,7 @@ impl std::ops::Sub for &MinifyingSugg<'static> {
 impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
     type Output = MinifyingSugg<'static>;
     fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
-        match (self.as_str(), rhs.as_str()) {
+        match (self.to_string().as_str(), rhs.to_string().as_str()) {
             ("0", _) => rhs.clone(),
             (_, "0") => self,
             (_, _) => (self.0 + &rhs.0).into(),
@@ -256,7 +258,7 @@ impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
 impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
     type Output = MinifyingSugg<'static>;
     fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
-        match (self.as_str(), rhs.as_str()) {
+        match (self.to_string().as_str(), rhs.to_string().as_str()) {
             (_, "0") => self,
             ("0", _) => (-rhs.0.clone()).into(),
             (x, y) if x == y => sugg::ZERO.into(),
diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs
index d391fbecf82..778d49cb4b6 100644
--- a/clippy_lints/src/needless_bool.rs
+++ b/clippy_lints/src/needless_bool.rs
@@ -187,14 +187,14 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison {
                 BinOpKind::Eq => {
                     let true_case = Some((|h| h, "equality checks against true are unnecessary"));
                     let false_case = Some((
-                        |h: Sugg<'_>| !h,
+                        |h: Sugg<'tcx>| !h,
                         "equality checks against false can be replaced by a negation",
                     ));
                     check_comparison(cx, e, true_case, false_case, true_case, false_case, ignore_no_literal);
                 },
                 BinOpKind::Ne => {
                     let true_case = Some((
-                        |h: Sugg<'_>| !h,
+                        |h: Sugg<'tcx>| !h,
                         "inequality checks against true can be replaced by a negation",
                     ));
                     let false_case = Some((|h| h, "inequality checks against false are unnecessary"));
@@ -206,12 +206,12 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison {
                     ignore_case,
                     Some((|h| h, "greater than checks against false are unnecessary")),
                     Some((
-                        |h: Sugg<'_>| !h,
+                        |h: Sugg<'tcx>| !h,
                         "less than comparison against true can be replaced by a negation",
                     )),
                     ignore_case,
                     Some((
-                        |l: Sugg<'_>, r: Sugg<'_>| (!l).bit_and(&r),
+                        |l: Sugg<'tcx>, r: Sugg<'tcx>| (!l).bit_and(&r),
                         "order comparisons between booleans can be simplified",
                     )),
                 ),
@@ -219,14 +219,14 @@ impl<'tcx> LateLintPass<'tcx> for BoolComparison {
                     cx,
                     e,
                     Some((
-                        |h: Sugg<'_>| !h,
+                        |h: Sugg<'tcx>| !h,
                         "less than comparison against true can be replaced by a negation",
                     )),
                     ignore_case,
                     ignore_case,
                     Some((|h| h, "greater than checks against false are unnecessary")),
                     Some((
-                        |l: Sugg<'_>, r: Sugg<'_>| l.bit_and(&(!r)),
+                        |l: Sugg<'tcx>, r: Sugg<'tcx>| l.bit_and(&(!r)),
                         "order comparisons between booleans can be simplified",
                     )),
                 ),
diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs
index 8989872cbce..0a240a8b763 100644
--- a/clippy_utils/src/sugg.rs
+++ b/clippy_utils/src/sugg.rs
@@ -1,9 +1,7 @@
 //! Contains utility functions to generate suggestions.
 #![deny(clippy::missing_docs_in_private_items)]
 
-use crate::source::{
-    snippet, snippet_opt, snippet_with_applicability, snippet_with_context, snippet_with_macro_callsite,
-};
+use crate::source::{snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite};
 use crate::{get_parent_expr_for_hir, higher};
 use rustc_ast::util::parser::AssocOp;
 use rustc_ast::{ast, token};
@@ -33,7 +31,7 @@ pub enum Sugg<'a> {
     MaybeParen(Cow<'a, str>),
     /// A binary operator expression, including `as`-casts and explicit type
     /// coercion.
-    BinOp(AssocOp, Cow<'a, str>),
+    BinOp(AssocOp, Cow<'a, str>, Cow<'a, str>),
 }
 
 /// Literal constant `0`, for convenience.
@@ -46,7 +44,8 @@ pub const EMPTY: Sugg<'static> = Sugg::NonParen(Cow::Borrowed(""));
 impl Display for Sugg<'_> {
     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),
+            Sugg::NonParen(ref s) | Sugg::MaybeParen(ref s) => s.fmt(f),
+            Sugg::BinOp(op, ref lhs, ref rhs) => binop_to_string(op, lhs, rhs).fmt(f),
         }
     }
 }
@@ -55,10 +54,8 @@ impl Display for Sugg<'_> {
 impl<'a> Sugg<'a> {
     /// Prepare a suggestion from an expression.
     pub fn hir_opt(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Self> {
-        snippet_opt(cx, expr.span).map(|snippet| {
-            let snippet = Cow::Owned(snippet);
-            Self::hir_from_snippet(expr, snippet)
-        })
+        let get_snippet = |span| snippet(cx, span, "");
+        snippet_opt(cx, expr.span).map(|_| Self::hir_from_snippet(expr, get_snippet))
     }
 
     /// Convenience function around `hir_opt` for suggestions with a default
@@ -93,9 +90,8 @@ impl<'a> Sugg<'a> {
 
     /// Same as `hir`, but will use the pre expansion span if the `expr` was in a macro.
     pub fn hir_with_macro_callsite(cx: &LateContext<'_>, expr: &hir::Expr<'_>, default: &'a str) -> Self {
-        let snippet = snippet_with_macro_callsite(cx, expr.span, default);
-
-        Self::hir_from_snippet(expr, snippet)
+        let get_snippet = |span| snippet_with_macro_callsite(cx, span, default);
+        Self::hir_from_snippet(expr, get_snippet)
     }
 
     /// Same as `hir`, but first walks the span up to the given context. This will result in the
@@ -112,24 +108,26 @@ impl<'a> Sugg<'a> {
         default: &'a str,
         applicability: &mut Applicability,
     ) -> Self {
-        let (snippet, in_macro) = snippet_with_context(cx, expr.span, ctxt, default, applicability);
-
-        if in_macro {
-            Sugg::NonParen(snippet)
+        if expr.span.ctxt() == ctxt {
+            Self::hir_from_snippet(expr, |span| snippet(cx, span, default))
         } else {
-            Self::hir_from_snippet(expr, snippet)
+            let snip = snippet_with_applicability(cx, expr.span, default, applicability);
+            Sugg::NonParen(snip)
         }
     }
 
     /// Generate a suggestion for an expression with the given snippet. This is used by the `hir_*`
     /// function variants of `Sugg`, since these use different snippet functions.
-    fn hir_from_snippet(expr: &hir::Expr<'_>, snippet: Cow<'a, str>) -> Self {
+    fn hir_from_snippet(expr: &hir::Expr<'_>, get_snippet: impl Fn(Span) -> Cow<'a, str>) -> Self {
         if let Some(range) = higher::Range::hir(expr) {
             let op = match range.limits {
                 ast::RangeLimits::HalfOpen => AssocOp::DotDot,
                 ast::RangeLimits::Closed => AssocOp::DotDotEq,
             };
-            return Sugg::BinOp(op, snippet);
+            let start = range.start.map_or("".into(), |expr| get_snippet(expr.span));
+            let end = range.end.map_or("".into(), |expr| get_snippet(expr.span));
+
+            return Sugg::BinOp(op, start, end);
         }
 
         match expr.kind {
@@ -139,7 +137,7 @@ impl<'a> Sugg<'a> {
             | hir::ExprKind::Let(..)
             | hir::ExprKind::Closure(..)
             | hir::ExprKind::Unary(..)
-            | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet),
+            | hir::ExprKind::Match(..) => Sugg::MaybeParen(get_snippet(expr.span)),
             hir::ExprKind::Continue(..)
             | hir::ExprKind::Yield(..)
             | hir::ExprKind::Array(..)
@@ -160,12 +158,20 @@ impl<'a> Sugg<'a> {
             | hir::ExprKind::Struct(..)
             | hir::ExprKind::Tup(..)
             | hir::ExprKind::DropTemps(_)
-            | hir::ExprKind::Err => Sugg::NonParen(snippet),
-            hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet),
-            hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet),
-            hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(op.node.into()), snippet),
-            hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet),
-            hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet),
+            | hir::ExprKind::Err => Sugg::NonParen(get_snippet(expr.span)),
+            hir::ExprKind::Assign(lhs, rhs, _) => {
+                Sugg::BinOp(AssocOp::Assign, get_snippet(lhs.span), get_snippet(rhs.span))
+            },
+            hir::ExprKind::AssignOp(op, lhs, rhs) => {
+                Sugg::BinOp(hirbinop2assignop(op), get_snippet(lhs.span), get_snippet(rhs.span))
+            },
+            hir::ExprKind::Binary(op, lhs, rhs) => Sugg::BinOp(
+                AssocOp::from_ast_binop(op.node.into()),
+                get_snippet(lhs.span),
+                get_snippet(rhs.span),
+            ),
+            hir::ExprKind::Cast(lhs, ty) => Sugg::BinOp(AssocOp::As, get_snippet(lhs.span), get_snippet(ty.span)),
+            hir::ExprKind::Type(lhs, ty) => Sugg::BinOp(AssocOp::Colon, get_snippet(lhs.span), get_snippet(ty.span)),
         }
     }
 
@@ -173,10 +179,12 @@ impl<'a> Sugg<'a> {
     pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self {
         use rustc_ast::ast::RangeLimits;
 
-        let snippet = if expr.span.from_expansion() {
-            snippet_with_macro_callsite(cx, expr.span, default)
-        } else {
-            snippet(cx, expr.span, default)
+        let get_whole_snippet = || {
+            if expr.span.from_expansion() {
+                snippet_with_macro_callsite(cx, expr.span, default)
+            } else {
+                snippet(cx, expr.span, default)
+            }
         };
 
         match expr.kind {
@@ -186,7 +194,7 @@ impl<'a> Sugg<'a> {
             | ast::ExprKind::If(..)
             | ast::ExprKind::Let(..)
             | ast::ExprKind::Unary(..)
-            | ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet),
+            | ast::ExprKind::Match(..) => Sugg::MaybeParen(get_whole_snippet()),
             ast::ExprKind::Async(..)
             | ast::ExprKind::Block(..)
             | ast::ExprKind::Break(..)
@@ -215,14 +223,42 @@ impl<'a> Sugg<'a> {
             | ast::ExprKind::Array(..)
             | ast::ExprKind::While(..)
             | ast::ExprKind::Await(..)
-            | ast::ExprKind::Err => Sugg::NonParen(snippet),
-            ast::ExprKind::Range(.., RangeLimits::HalfOpen) => Sugg::BinOp(AssocOp::DotDot, snippet),
-            ast::ExprKind::Range(.., RangeLimits::Closed) => Sugg::BinOp(AssocOp::DotDotEq, 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),
+            | ast::ExprKind::Err => Sugg::NonParen(get_whole_snippet()),
+            ast::ExprKind::Range(ref lhs, ref rhs, RangeLimits::HalfOpen) => Sugg::BinOp(
+                AssocOp::DotDot,
+                lhs.as_ref().map_or("".into(), |lhs| snippet(cx, lhs.span, default)),
+                rhs.as_ref().map_or("".into(), |rhs| snippet(cx, rhs.span, default)),
+            ),
+            ast::ExprKind::Range(ref lhs, ref rhs, RangeLimits::Closed) => Sugg::BinOp(
+                AssocOp::DotDotEq,
+                lhs.as_ref().map_or("".into(), |lhs| snippet(cx, lhs.span, default)),
+                rhs.as_ref().map_or("".into(), |rhs| snippet(cx, rhs.span, default)),
+            ),
+            ast::ExprKind::Assign(ref lhs, ref rhs, _) => Sugg::BinOp(
+                AssocOp::Assign,
+                snippet(cx, lhs.span, default),
+                snippet(cx, rhs.span, default),
+            ),
+            ast::ExprKind::AssignOp(op, ref lhs, ref rhs) => Sugg::BinOp(
+                astbinop2assignop(op),
+                snippet(cx, lhs.span, default),
+                snippet(cx, rhs.span, default),
+            ),
+            ast::ExprKind::Binary(op, ref lhs, ref rhs) => Sugg::BinOp(
+                AssocOp::from_ast_binop(op.node),
+                snippet(cx, lhs.span, default),
+                snippet(cx, rhs.span, default),
+            ),
+            ast::ExprKind::Cast(ref lhs, ref ty) => Sugg::BinOp(
+                AssocOp::As,
+                snippet(cx, lhs.span, default),
+                snippet(cx, ty.span, default),
+            ),
+            ast::ExprKind::Type(ref lhs, ref ty) => Sugg::BinOp(
+                AssocOp::Colon,
+                snippet(cx, lhs.span, default),
+                snippet(cx, ty.span, default),
+            ),
         }
     }
 
@@ -306,17 +342,51 @@ impl<'a> Sugg<'a> {
                     Sugg::NonParen(format!("({})", sugg).into())
                 }
             },
-            Sugg::BinOp(_, sugg) => {
-                if has_enclosing_paren(&sugg) {
-                    Sugg::NonParen(sugg)
-                } else {
-                    Sugg::NonParen(format!("({})", sugg).into())
-                }
+            Sugg::BinOp(op, lhs, rhs) => {
+                let sugg = binop_to_string(op, &lhs, &rhs);
+                Sugg::NonParen(format!("({})", sugg).into())
             },
         }
     }
 }
 
+/// Generates a string from the operator and both sides.
+fn binop_to_string(op: AssocOp, lhs: &str, rhs: &str) -> String {
+    match op {
+        AssocOp::Add
+        | AssocOp::Subtract
+        | AssocOp::Multiply
+        | AssocOp::Divide
+        | AssocOp::Modulus
+        | AssocOp::LAnd
+        | AssocOp::LOr
+        | AssocOp::BitXor
+        | AssocOp::BitAnd
+        | AssocOp::BitOr
+        | AssocOp::ShiftLeft
+        | AssocOp::ShiftRight
+        | AssocOp::Equal
+        | AssocOp::Less
+        | AssocOp::LessEqual
+        | AssocOp::NotEqual
+        | AssocOp::Greater
+        | AssocOp::GreaterEqual => format!(
+            "{} {} {}",
+            lhs,
+            op.to_ast_binop().expect("Those are AST ops").to_string(),
+            rhs
+        ),
+        AssocOp::Assign => format!("{} = {}", lhs, rhs),
+        AssocOp::AssignOp(op) => {
+            format!("{} {}= {}", lhs, token_kind_to_string(&token::BinOp(op)), rhs)
+        },
+        AssocOp::As => format!("{} as {}", lhs, rhs),
+        AssocOp::DotDot => format!("{}..{}", lhs, rhs),
+        AssocOp::DotDotEq => format!("{}..={}", lhs, rhs),
+        AssocOp::Colon => format!("{}: {}", lhs, rhs),
+    }
+}
+
 /// Return `true` if `sugg` is enclosed in parenthesis.
 fn has_enclosing_paren(sugg: impl AsRef<str>) -> bool {
     let mut chars = sugg.as_ref().chars();
@@ -391,34 +461,22 @@ impl Neg for Sugg<'_> {
     }
 }
 
-impl Not for Sugg<'_> {
-    type Output = Sugg<'static>;
-    fn not(self) -> Sugg<'static> {
+impl Not for Sugg<'a> {
+    type Output = Sugg<'a>;
+    fn not(self) -> Sugg<'a> {
         use AssocOp::{Equal, Greater, GreaterEqual, Less, LessEqual, NotEqual};
 
-        /// Convert ```AssocOp``` to a string of operators.
-        fn op_as_str(op: AssocOp) -> &'static str {
-            op.to_ast_binop().unwrap().to_string()
-        }
-
-        /// Replace the operator in the Snippet.
-        fn replace_op(from_op: AssocOp, to_op: AssocOp, snip: Cow<'_, str>) -> Sugg<'static> {
-            let from = op_as_str(from_op);
-            let to = op_as_str(to_op);
-            let snip = snip.into_owned().replace(from, to);
-            Sugg::BinOp(to_op, Cow::Owned(snip))
-        }
-
-        if let Sugg::BinOp(op, snip) = self {
-            match op {
-                Equal => replace_op(op, NotEqual, snip),
-                NotEqual => replace_op(op, Equal, snip),
-                Less => replace_op(op, GreaterEqual, snip),
-                GreaterEqual => replace_op(op, Less, snip),
-                Greater => replace_op(op, LessEqual, snip),
-                LessEqual => replace_op(op, Greater, snip),
-                _ => make_unop("!", Sugg::BinOp(op, snip)),
-            }
+        if let Sugg::BinOp(op, lhs, rhs) = self {
+            let to_op = match op {
+                Equal => NotEqual,
+                NotEqual => Equal,
+                Less => GreaterEqual,
+                GreaterEqual => Less,
+                Greater => LessEqual,
+                LessEqual => Greater,
+                _ => return make_unop("!", Sugg::BinOp(op, lhs, rhs)),
+            };
+            Sugg::BinOp(to_op, lhs, rhs)
         } else {
             make_unop("!", self)
         }
@@ -490,53 +548,21 @@ pub fn make_assoc(op: AssocOp, lhs: &Sugg<'_>, rhs: &Sugg<'_>) -> Sugg<'static>
             || is_shift(other) && is_arith(op)
     }
 
-    let lhs_paren = if let Sugg::BinOp(lop, _) = *lhs {
+    let lhs_paren = if let Sugg::BinOp(lop, _, _) = *lhs {
         needs_paren(op, lop, Associativity::Left)
     } else {
         false
     };
 
-    let rhs_paren = if let Sugg::BinOp(rop, _) = *rhs {
+    let rhs_paren = if let Sugg::BinOp(rop, _, _) = *rhs {
         needs_paren(op, rop, Associativity::Right)
     } else {
         false
     };
 
-    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::Assign => format!("{} = {}", lhs, rhs),
-        AssocOp::AssignOp(op) => format!("{} {}= {}", lhs, token_kind_to_string(&token::BinOp(op)), rhs),
-        AssocOp::As => format!("{} as {}", lhs, rhs),
-        AssocOp::DotDot => format!("{}..{}", lhs, rhs),
-        AssocOp::DotDotEq => format!("{}..={}", lhs, rhs),
-        AssocOp::Colon => format!("{}: {}", lhs, rhs),
-    };
-
-    Sugg::BinOp(op, sugg.into())
+    let lhs = ParenHelper::new(lhs_paren, lhs).to_string();
+    let rhs = ParenHelper::new(rhs_paren, rhs).to_string();
+    Sugg::BinOp(op, lhs.into(), rhs.into())
 }
 
 /// Convenience wrapper around `make_assoc` and `AssocOp::from_ast_binop`.
@@ -1034,10 +1060,10 @@ mod test {
 
     #[test]
     fn binop_maybe_par() {
-        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into());
+        let sugg = Sugg::BinOp(AssocOp::Add, "1".into(), "1".into());
         assert_eq!("(1 + 1)", sugg.maybe_par().to_string());
 
-        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1) + (1 + 1)".into());
+        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into(), "(1 + 1)".into());
         assert_eq!("((1 + 1) + (1 + 1))", sugg.maybe_par().to_string());
     }
     #[test]
@@ -1045,32 +1071,32 @@ mod test {
         use AssocOp::{Add, Equal, Greater, GreaterEqual, LAnd, LOr, Less, LessEqual, NotEqual};
 
         // Invert the comparison operator.
-        let sugg = Sugg::BinOp(Equal, "1 == 1".into());
+        let sugg = Sugg::BinOp(Equal, "1".into(), "1".into());
         assert_eq!("1 != 1", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(NotEqual, "1 != 1".into());
+        let sugg = Sugg::BinOp(NotEqual, "1".into(), "1".into());
         assert_eq!("1 == 1", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(Less, "1 < 1".into());
+        let sugg = Sugg::BinOp(Less, "1".into(), "1".into());
         assert_eq!("1 >= 1", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(LessEqual, "1 <= 1".into());
+        let sugg = Sugg::BinOp(LessEqual, "1".into(), "1".into());
         assert_eq!("1 > 1", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(Greater, "1 > 1".into());
+        let sugg = Sugg::BinOp(Greater, "1".into(), "1".into());
         assert_eq!("1 <= 1", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(GreaterEqual, "1 >= 1".into());
+        let sugg = Sugg::BinOp(GreaterEqual, "1".into(), "1".into());
         assert_eq!("1 < 1", (!sugg).to_string());
 
         // Other operators are inverted like !(..).
-        let sugg = Sugg::BinOp(Add, "1 + 1".into());
+        let sugg = Sugg::BinOp(Add, "1".into(), "1".into());
         assert_eq!("!(1 + 1)", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(LAnd, "1 && 1".into());
+        let sugg = Sugg::BinOp(LAnd, "1".into(), "1".into());
         assert_eq!("!(1 && 1)", (!sugg).to_string());
 
-        let sugg = Sugg::BinOp(LOr, "1 || 1".into());
+        let sugg = Sugg::BinOp(LOr, "1".into(), "1".into());
         assert_eq!("!(1 || 1)", (!sugg).to_string());
     }
 }
diff --git a/tests/ui/manual_memcpy/with_loop_counters.stderr b/tests/ui/manual_memcpy/with_loop_counters.stderr
index 0243158dec5..2e3ebadd7b5 100644
--- a/tests/ui/manual_memcpy/with_loop_counters.stderr
+++ b/tests/ui/manual_memcpy/with_loop_counters.stderr
@@ -43,7 +43,7 @@ LL | /     for i in 3..(3 + src.len()) {
 LL | |         dst[i] = src[count];
 LL | |         count += 1;
 LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
+   | |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..(3 + src.len() - 3)]);`
 
 error: it looks like you're manually copying between slices
   --> $DIR/with_loop_counters.rs:35:5