about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-12-28 22:15:53 +0000
committerbors <bors@rust-lang.org>2021-12-28 22:15:53 +0000
commitc1cd64b9c6b29ed2a577fc174a50c76267a966e2 (patch)
treeac76694aa0fac911bd2da7f00fa84548deb0fca9
parent16ef044e72ef8db08307c4fdaa1c0dd8b24d0861 (diff)
parenta172439f29301d7c34645406f439f064edc58d69 (diff)
downloadrust-c1cd64b9c6b29ed2a577fc174a50c76267a966e2.tar.gz
rust-c1cd64b9c6b29ed2a577fc174a50c76267a966e2.zip
Auto merge of #8117 - hotate29:issue7320, r=camsteffen
update: ```Sugg::not()``` replacing the comparison operator. #7320

fixes #7320

changelog: ```needless_bool```: Changed to make a smart suggestion.
-rw-r--r--clippy_lints/src/floating_point_arithmetic.rs4
-rw-r--r--clippy_lints/src/loops/manual_memcpy.rs26
-rw-r--r--clippy_lints/src/needless_bool.rs12
-rw-r--r--clippy_lints/src/ranges.rs8
-rw-r--r--clippy_utils/src/sugg.rs251
-rw-r--r--tests/ui/manual_memcpy/with_loop_counters.stderr2
-rw-r--r--tests/ui/needless_bool/fixable.fixed9
-rw-r--r--tests/ui/needless_bool/fixable.rs33
-rw-r--r--tests/ui/needless_bool/fixable.stderr86
-rw-r--r--tests/ui/short_circuit_statement.fixed2
-rw-r--r--tests/ui/short_circuit_statement.stderr2
11 files changed, 307 insertions, 128 deletions
diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs
index f801d95563d..4fefd9fd18a 100644
--- a/clippy_lints/src/floating_point_arithmetic.rs
+++ b/clippy_lints/src/floating_point_arithmetic.rs
@@ -356,7 +356,7 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option<String> {
             if eq_expr_value(cx, lmul_lhs, lmul_rhs);
             if eq_expr_value(cx, rmul_lhs, rmul_rhs);
             then {
-                return Some(format!("{}.hypot({})", Sugg::hir(cx, lmul_lhs, ".."), Sugg::hir(cx, rmul_lhs, "..")));
+                return Some(format!("{}.hypot({})", Sugg::hir(cx, lmul_lhs, "..").maybe_par(), Sugg::hir(cx, rmul_lhs, "..")));
             }
         }
 
@@ -379,7 +379,7 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option<String> {
             if let Some((rvalue, _)) = constant(cx, cx.typeck_results(), rargs_1);
             if Int(2) == lvalue && Int(2) == rvalue;
             then {
-                return Some(format!("{}.hypot({})", Sugg::hir(cx, largs_0, ".."), Sugg::hir(cx, rargs_0, "..")));
+                return Some(format!("{}.hypot({})", Sugg::hir(cx, largs_0, "..").maybe_par(), Sugg::hir(cx, rargs_0, "..")));
             }
         }
     }
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_lints/src/ranges.rs b/clippy_lints/src/ranges.rs
index 09c64485bbb..a1e67c33abf 100644
--- a/clippy_lints/src/ranges.rs
+++ b/clippy_lints/src/ranges.rs
@@ -378,8 +378,8 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
                 span,
                 "an inclusive range would be more readable",
                 |diag| {
-                    let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
-                    let end = Sugg::hir(cx, y, "y");
+                    let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string());
+                    let end = Sugg::hir(cx, y, "y").maybe_par();
                     if let Some(is_wrapped) = &snippet_opt(cx, span) {
                         if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') {
                             diag.span_suggestion(
@@ -415,8 +415,8 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
                 expr.span,
                 "an exclusive range would be more readable",
                 |diag| {
-                    let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
-                    let end = Sugg::hir(cx, y, "y");
+                    let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string());
+                    let end = Sugg::hir(cx, y, "y").maybe_par();
                     diag.span_suggestion(
                         expr.span,
                         "use",
diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs
index 586934df460..92662c59226 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,10 +461,25 @@ impl Neg for Sugg<'_> {
     }
 }
 
-impl Not for Sugg<'_> {
-    type Output = Sugg<'static>;
-    fn not(self) -> Sugg<'static> {
-        make_unop("!", self)
+impl Not for Sugg<'a> {
+    type Output = Sugg<'a>;
+    fn not(self) -> Sugg<'a> {
+        use AssocOp::{Equal, Greater, GreaterEqual, Less, LessEqual, NotEqual};
+
+        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)
+        }
     }
 }
 
@@ -463,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`.
@@ -1007,10 +1060,32 @@ 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]
+    fn not_op() {
+        use AssocOp::{Add, Equal, Greater, GreaterEqual, LAnd, LOr, Less, LessEqual, NotEqual};
+
+        fn test_not(op: AssocOp, correct: &str) {
+            let sugg = Sugg::BinOp(op, "x".into(), "y".into());
+            assert_eq!((!sugg).to_string(), correct);
+        }
+
+        // Invert the comparison operator.
+        test_not(Equal, "x != y");
+        test_not(NotEqual, "x == y");
+        test_not(Less, "x >= y");
+        test_not(LessEqual, "x > y");
+        test_not(Greater, "x <= y");
+        test_not(GreaterEqual, "x < y");
+
+        // Other operators are inverted like !(..).
+        test_not(Add, "!(x + y)");
+        test_not(LAnd, "!(x && y)");
+        test_not(LOr, "!(x || y)");
+    }
 }
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
diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed
index 85da1f4e104..89dc13fd5b1 100644
--- a/tests/ui/needless_bool/fixable.fixed
+++ b/tests/ui/needless_bool/fixable.fixed
@@ -41,6 +41,15 @@ fn main() {
     x;
     !x;
     !(x && y);
+    let a = 0;
+    let b = 1;
+
+    a != b;
+    a == b;
+    a >= b;
+    a > b;
+    a <= b;
+    a < b;
     if x {
         x
     } else {
diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs
index add60630251..c11d9472e8d 100644
--- a/tests/ui/needless_bool/fixable.rs
+++ b/tests/ui/needless_bool/fixable.rs
@@ -53,6 +53,39 @@ fn main() {
     } else {
         true
     };
+    let a = 0;
+    let b = 1;
+
+    if a == b {
+        false
+    } else {
+        true
+    };
+    if a != b {
+        false
+    } else {
+        true
+    };
+    if a < b {
+        false
+    } else {
+        true
+    };
+    if a <= b {
+        false
+    } else {
+        true
+    };
+    if a > b {
+        false
+    } else {
+        true
+    };
+    if a >= b {
+        false
+    } else {
+        true
+    };
     if x {
         x
     } else {
diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr
index 22c0a7bb491..d2c48376f76 100644
--- a/tests/ui/needless_bool/fixable.stderr
+++ b/tests/ui/needless_bool/fixable.stderr
@@ -31,7 +31,67 @@ LL | |     };
    | |_____^ help: you can reduce it to: `!(x && y)`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:72:5
+  --> $DIR/fixable.rs:59:5
+   |
+LL | /     if a == b {
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `a != b`
+
+error: this if-then-else expression returns a bool literal
+  --> $DIR/fixable.rs:64:5
+   |
+LL | /     if a != b {
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `a == b`
+
+error: this if-then-else expression returns a bool literal
+  --> $DIR/fixable.rs:69:5
+   |
+LL | /     if a < b {
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `a >= b`
+
+error: this if-then-else expression returns a bool literal
+  --> $DIR/fixable.rs:74:5
+   |
+LL | /     if a <= b {
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `a > b`
+
+error: this if-then-else expression returns a bool literal
+  --> $DIR/fixable.rs:79:5
+   |
+LL | /     if a > b {
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `a <= b`
+
+error: this if-then-else expression returns a bool literal
+  --> $DIR/fixable.rs:84:5
+   |
+LL | /     if a >= b {
+LL | |         false
+LL | |     } else {
+LL | |         true
+LL | |     };
+   | |_____^ help: you can reduce it to: `a < b`
+
+error: this if-then-else expression returns a bool literal
+  --> $DIR/fixable.rs:105:5
    |
 LL | /     if x {
 LL | |         return true;
@@ -41,7 +101,7 @@ LL | |     };
    | |_____^ help: you can reduce it to: `return x`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:80:5
+  --> $DIR/fixable.rs:113:5
    |
 LL | /     if x {
 LL | |         return false;
@@ -51,7 +111,7 @@ LL | |     };
    | |_____^ help: you can reduce it to: `return !x`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:88:5
+  --> $DIR/fixable.rs:121:5
    |
 LL | /     if x && y {
 LL | |         return true;
@@ -61,7 +121,7 @@ LL | |     };
    | |_____^ help: you can reduce it to: `return x && y`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:96:5
+  --> $DIR/fixable.rs:129:5
    |
 LL | /     if x && y {
 LL | |         return false;
@@ -71,7 +131,7 @@ LL | |     };
    | |_____^ help: you can reduce it to: `return !(x && y)`
 
 error: equality checks against true are unnecessary
-  --> $DIR/fixable.rs:104:8
+  --> $DIR/fixable.rs:137:8
    |
 LL |     if x == true {};
    |        ^^^^^^^^^ help: try simplifying it as shown: `x`
@@ -79,25 +139,25 @@ LL |     if x == true {};
    = note: `-D clippy::bool-comparison` implied by `-D warnings`
 
 error: equality checks against false can be replaced by a negation
-  --> $DIR/fixable.rs:108:8
+  --> $DIR/fixable.rs:141:8
    |
 LL |     if x == false {};
    |        ^^^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: equality checks against true are unnecessary
-  --> $DIR/fixable.rs:118:8
+  --> $DIR/fixable.rs:151:8
    |
 LL |     if x == true {};
    |        ^^^^^^^^^ help: try simplifying it as shown: `x`
 
 error: equality checks against false can be replaced by a negation
-  --> $DIR/fixable.rs:119:8
+  --> $DIR/fixable.rs:152:8
    |
 LL |     if x == false {};
    |        ^^^^^^^^^^ help: try simplifying it as shown: `!x`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:128:12
+  --> $DIR/fixable.rs:161:12
    |
 LL |       } else if returns_bool() {
    |  ____________^
@@ -108,7 +168,7 @@ LL | |     };
    | |_____^ help: you can reduce it to: `{ !returns_bool() }`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:141:5
+  --> $DIR/fixable.rs:174:5
    |
 LL | /     if unsafe { no(4) } & 1 != 0 {
 LL | |         true
@@ -118,16 +178,16 @@ LL | |     };
    | |_____^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:146:30
+  --> $DIR/fixable.rs:179:30
    |
 LL |     let _brackets_unneeded = if unsafe { no(4) } & 1 != 0 { true } else { false };
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `unsafe { no(4) } & 1 != 0`
 
 error: this if-then-else expression returns a bool literal
-  --> $DIR/fixable.rs:149:9
+  --> $DIR/fixable.rs:182:9
    |
 LL |         if unsafe { no(4) } & 1 != 0 { true } else { false }
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)`
 
-error: aborting due to 15 previous errors
+error: aborting due to 21 previous errors
 
diff --git a/tests/ui/short_circuit_statement.fixed b/tests/ui/short_circuit_statement.fixed
index af0a397bd1a..dd22ecab0b5 100644
--- a/tests/ui/short_circuit_statement.fixed
+++ b/tests/ui/short_circuit_statement.fixed
@@ -6,7 +6,7 @@
 fn main() {
     if f() { g(); }
     if !f() { g(); }
-    if !(1 == 2) { g(); }
+    if 1 != 2 { g(); }
 }
 
 fn f() -> bool {
diff --git a/tests/ui/short_circuit_statement.stderr b/tests/ui/short_circuit_statement.stderr
index 0a3f60c3d13..aa84ac3a792 100644
--- a/tests/ui/short_circuit_statement.stderr
+++ b/tests/ui/short_circuit_statement.stderr
@@ -16,7 +16,7 @@ error: boolean short circuit operator in statement may be clearer using an expli
   --> $DIR/short_circuit_statement.rs:9:5
    |
 LL |     1 == 2 || g();
-   |     ^^^^^^^^^^^^^^ help: replace it with: `if !(1 == 2) { g(); }`
+   |     ^^^^^^^^^^^^^^ help: replace it with: `if 1 != 2 { g(); }`
 
 error: aborting due to 3 previous errors