Merge #7719

7719: De Morgan's Law assist now correctly parenthesizes binary expressions. r=Veykril a=lbrande

Closes #7694 by parenthesizing binary expressions that are negated.

Co-authored-by: lbrande <[email protected]>
Co-authored-by: Lukas Wirth <[email protected]>

1 files changed, 41 insertions, 9 deletions

diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs
index 0074da741..276792bc1 100644
--- a/crates/ide_assists/src/utils.rs
+++ b/crates/ide_assists/src/utils.rs
@@ -3,8 +3,11 @@
 use std::ops;
 
 use ast::TypeBoundsOwner;
-use hir::{Adt, HasSource};
-use ide_db::{helpers::SnippetCap, RootDatabase};
+use hir::{Adt, HasSource, Semantics};
+use ide_db::{
+    helpers::{FamousDefs, SnippetCap},
+    RootDatabase,
+};
 use itertools::Itertools;
 use stdx::format_to;
 use syntax::{
@@ -205,23 +208,36 @@ pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize {
         .unwrap_or_else(|| node.text_range().start())
 }
 
-pub(crate) fn invert_boolean_expression(expr: ast::Expr) -> ast::Expr {
-    if let Some(expr) = invert_special_case(&expr) {
+pub(crate) fn invert_boolean_expression(
+    sema: &Semantics<RootDatabase>,
+    expr: ast::Expr,
+) -> ast::Expr {
+    if let Some(expr) = invert_special_case(sema, &expr) {
         return expr;
     }
     make::expr_prefix(T![!], expr)
 }
 
-fn invert_special_case(expr: &ast::Expr) -> Option<ast::Expr> {
+fn invert_special_case(sema: &Semantics<RootDatabase>, expr: &ast::Expr) -> Option<ast::Expr> {
     match expr {
         ast::Expr::BinExpr(bin) => match bin.op_kind()? {
             ast::BinOp::NegatedEqualityTest => bin.replace_op(T![==]).map(|it| it.into()),
             ast::BinOp::EqualityTest => bin.replace_op(T![!=]).map(|it| it.into()),
-            // Parenthesize composite boolean expressions before prefixing `!`
-            ast::BinOp::BooleanAnd | ast::BinOp::BooleanOr => {
-                Some(make::expr_prefix(T![!], make::expr_paren(expr.clone())))
+            // Swap `<` with `>=`, `<=` with `>`, ... if operands `impl Ord`
+            ast::BinOp::LesserTest if bin_impls_ord(sema, bin) => {
+                bin.replace_op(T![>=]).map(|it| it.into())
+            }
+            ast::BinOp::LesserEqualTest if bin_impls_ord(sema, bin) => {
+                bin.replace_op(T![>]).map(|it| it.into())
+            }
+            ast::BinOp::GreaterTest if bin_impls_ord(sema, bin) => {
+                bin.replace_op(T![<=]).map(|it| it.into())
+            }
+            ast::BinOp::GreaterEqualTest if bin_impls_ord(sema, bin) => {
+                bin.replace_op(T![<]).map(|it| it.into())
             }
-            _ => None,
+            // Parenthesize other expressions before prefixing `!`
+            _ => Some(make::expr_prefix(T![!], make::expr_paren(expr.clone()))),
         },
         ast::Expr::MethodCallExpr(mce) => {
             let receiver = mce.receiver()?;
@@ -250,6 +266,22 @@ fn invert_special_case(expr: &ast::Expr) -> Option<ast::Expr> {
     }
 }
 
+fn bin_impls_ord(sema: &Semantics<RootDatabase>, bin: &ast::BinExpr) -> bool {
+    match (
+        bin.lhs().and_then(|lhs| sema.type_of_expr(&lhs)),
+        bin.rhs().and_then(|rhs| sema.type_of_expr(&rhs)),
+    ) {
+        (Some(lhs_ty), Some(rhs_ty)) if lhs_ty == rhs_ty => {
+            let krate = sema.scope(bin.syntax()).module().map(|it| it.krate());
+            let ord_trait = FamousDefs(sema, krate).core_cmp_Ord();
+            ord_trait.map_or(false, |ord_trait| {
+                lhs_ty.autoderef(sema.db).any(|ty| ty.impls_trait(sema.db, ord_trait, &[]))
+            })
+        }
+        _ => false,
+    }
+}
+
 pub(crate) fn next_prev() -> impl Iterator<Item = Direction> {
     [Direction::Next, Direction::Prev].iter().copied()
 }