Migrate apply_demorgan to SyntaxEditor

ShoyuVanilla · ShoyuVanilla · commit bf0c3944f8c4 · 2025-02-24T22:41:53.000+09:00
diff --git a/crates/ide-assists/src/handlers/apply_demorgan.rs b/crates/ide-assists/src/handlers/apply_demorgan.rs
@@ -3,12 +3,12 @@ use std::collections::VecDeque;
 use ide_db::{
     assists::GroupLabel,
     famous_defs::FamousDefs,
-    source_change::SourceChangeBuilder,
     syntax_helpers::node_ext::{for_each_tail_expr, walk_expr},
 };
 use syntax::{
-    ast::{self, make, AstNode, Expr::BinExpr, HasArgList},
-    ted, SyntaxKind, T,
+    ast::{self, syntax_factory::SyntaxFactory, AstNode, Expr::BinExpr, HasArgList},
+    syntax_editor::{Position, SyntaxEditor},
+    SyntaxKind, SyntaxNode, T,
 };
 
 use crate::{utils::invert_boolean_expression, AssistContext, AssistId, AssistKind, Assists};
@@ -58,9 +58,12 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
         _ => return None,
     };
 
-    let demorganed = bin_expr.clone_subtree().clone_for_update();
+    let make = SyntaxFactory::new();
+
+    let demorganed = bin_expr.clone_subtree();
+    let mut editor = SyntaxEditor::new(demorganed.syntax().clone());
+    editor.replace(demorganed.op_token()?, make.token(inv_token));
 
-    ted::replace(demorganed.op_token()?, ast::make::token(inv_token));
     let mut exprs = VecDeque::from([
         (bin_expr.lhs()?, demorganed.lhs()?),
         (bin_expr.rhs()?, demorganed.rhs()?),
@@ -70,35 +73,39 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
         if let BinExpr(bin_expr) = &expr {
             if let BinExpr(cbin_expr) = &dm {
                 if op == bin_expr.op_kind()? {
-                    ted::replace(cbin_expr.op_token()?, ast::make::token(inv_token));
+                    editor.replace(cbin_expr.op_token()?, make.token(inv_token));
                     exprs.push_back((bin_expr.lhs()?, cbin_expr.lhs()?));
                     exprs.push_back((bin_expr.rhs()?, cbin_expr.rhs()?));
                 } else {
-                    let mut inv = invert_boolean_expression(expr);
-                    if inv.needs_parens_in(dm.syntax().parent()?) {
-                        inv = ast::make::expr_paren(inv).clone_for_update();
+                    let mut inv = invert_boolean_expression(&make, expr);
+                    if needs_parens_in_place_of(&inv, &dm.syntax().parent()?, &dm) {
+                        inv = make.expr_paren(inv).into();
                     }
-                    ted::replace(dm.syntax(), inv.syntax());
+                    editor.replace(dm.syntax(), inv.syntax());
                 }
             } else {
                 return None;
             }
         } else {
-            let mut inv = invert_boolean_expression(dm.clone_subtree()).clone_for_update();
-            if inv.needs_parens_in(dm.syntax().parent()?) {
-                inv = ast::make::expr_paren(inv).clone_for_update();
+            let mut inv = invert_boolean_expression(&make, dm.clone());
+            if needs_parens_in_place_of(&inv, &dm.syntax().parent()?, &dm) {
+                inv = make.expr_paren(inv).into();
             }
-            ted::replace(dm.syntax(), inv.syntax());
+            editor.replace(dm.syntax(), inv.syntax());
         }
     }
 
+    editor.add_mappings(make.finish_with_mappings());
+    let edit = editor.finish();
+    let demorganed = ast::Expr::cast(edit.new_root().clone())?;
+
     acc.add_group(
         &GroupLabel("Apply De Morgan's law".to_owned()),
         AssistId("apply_demorgan", AssistKind::RefactorRewrite),
         "Apply De Morgan's law",
         op_range,
-        |edit| {
-            let demorganed = ast::Expr::BinExpr(demorganed);
+        |builder| {
+            let make = SyntaxFactory::new();
             let paren_expr = bin_expr.syntax().parent().and_then(ast::ParenExpr::cast);
             let neg_expr = paren_expr
                 .clone()
@@ -107,24 +114,32 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
                 .filter(|prefix_expr| matches!(prefix_expr.op_kind(), Some(ast::UnaryOp::Not)))
                 .map(ast::Expr::PrefixExpr);
 
+            let mut editor;
             if let Some(paren_expr) = paren_expr {
                 if let Some(neg_expr) = neg_expr {
                     cov_mark::hit!(demorgan_double_negation);
                     let parent = neg_expr.syntax().parent();
+                    editor = builder.make_editor(neg_expr.syntax());
 
                     if parent.is_some_and(|parent| demorganed.needs_parens_in(parent)) {
                         cov_mark::hit!(demorgan_keep_parens_for_op_precedence2);
-                        edit.replace_ast(neg_expr, make::expr_paren(demorganed));
+                        editor.replace(neg_expr.syntax(), make.expr_paren(demorganed).syntax());
                     } else {
-                        edit.replace_ast(neg_expr, demorganed);
+                        editor.replace(neg_expr.syntax(), demorganed.syntax());
                     };
                 } else {
                     cov_mark::hit!(demorgan_double_parens);
-                    edit.replace_ast(paren_expr.into(), add_bang_paren(demorganed));
+                    editor = builder.make_editor(paren_expr.syntax());
+
+                    editor.replace(paren_expr.syntax(), add_bang_paren(&make, demorganed).syntax());
                 }
             } else {
-                edit.replace_ast(bin_expr.into(), add_bang_paren(demorganed));
+                editor = builder.make_editor(bin_expr.syntax());
+                editor.replace(bin_expr.syntax(), add_bang_paren(&make, demorganed).syntax());
             }
+
+            editor.add_mappings(make.finish_with_mappings());
+            builder.add_file_edits(ctx.file_id(), editor);
         },
     )
 }
@@ -161,7 +176,7 @@ pub(crate) fn apply_demorgan_iterator(acc: &mut Assists, ctx: &AssistContext<'_>
     let (name, arg_expr) = validate_method_call_expr(ctx, &method_call)?;
 
     let ast::Expr::ClosureExpr(closure_expr) = arg_expr else { return None };
-    let closure_body = closure_expr.body()?;
+    let closure_body = closure_expr.body()?.clone_for_update();
 
     let op_range = method_call.syntax().text_range();
     let label = format!("Apply De Morgan's law to `Iterator::{}`", name.text().as_str());
@@ -170,18 +185,19 @@ pub(crate) fn apply_demorgan_iterator(acc: &mut Assists, ctx: &AssistContext<'_>
         AssistId("apply_demorgan_iterator", AssistKind::RefactorRewrite),
         label,
         op_range,
-        |edit| {
+        |builder| {
+            let make = SyntaxFactory::new();
+            let mut editor = builder.make_editor(method_call.syntax());
             // replace the method name
             let new_name = match name.text().as_str() {
-                "all" => make::name_ref("any"),
-                "any" => make::name_ref("all"),
+                "all" => make.name_ref("any"),
+                "any" => make.name_ref("all"),
                 _ => unreachable!(),
-            }
-            .clone_for_update();
-            edit.replace_ast(name, new_name);
+            };
+            editor.replace(name.syntax(), new_name.syntax());
 
             // negate all tail expressions in the closure body
-            let tail_cb = &mut |e: &_| tail_cb_impl(edit, e);
+            let tail_cb = &mut |e: &_| tail_cb_impl(&mut editor, &make, e);
             walk_expr(&closure_body, &mut |expr| {
                 if let ast::Expr::ReturnExpr(ret_expr) = expr {
                     if let Some(ret_expr_arg) = &ret_expr.expr() {
@@ -198,15 +214,15 @@ pub(crate) fn apply_demorgan_iterator(acc: &mut Assists, ctx: &AssistContext<'_>
                 .and_then(ast::PrefixExpr::cast)
                 .filter(|prefix_expr| matches!(prefix_expr.op_kind(), Some(ast::UnaryOp::Not)))
             {
-                edit.delete(
-                    prefix_expr
-                        .op_token()
-                        .expect("prefix expression always has an operator")
-                        .text_range(),
+                editor.delete(
+                    prefix_expr.op_token().expect("prefix expression always has an operator"),
                 );
             } else {
-                edit.insert(method_call.syntax().text_range().start(), "!");
+                editor.insert(Position::before(method_call.syntax()), make.token(SyntaxKind::BANG));
             }
+
+            editor.add_mappings(make.finish_with_mappings());
+            builder.add_file_edits(ctx.file_id(), editor);
         },
     )
 }
@@ -233,26 +249,50 @@ fn validate_method_call_expr(
     it_type.impls_trait(sema.db, iter_trait, &[]).then_some((name_ref, arg_expr))
 }
 
-fn tail_cb_impl(edit: &mut SourceChangeBuilder, e: &ast::Expr) {
+fn tail_cb_impl(editor: &mut SyntaxEditor, make: &SyntaxFactory, e: &ast::Expr) {
     match e {
         ast::Expr::BreakExpr(break_expr) => {
             if let Some(break_expr_arg) = break_expr.expr() {
-                for_each_tail_expr(&break_expr_arg, &mut |e| tail_cb_impl(edit, e))
+                for_each_tail_expr(&break_expr_arg, &mut |e| tail_cb_impl(editor, make, e))
             }
         }
         ast::Expr::ReturnExpr(_) => {
             // all return expressions have already been handled by the walk loop
         }
         e => {
-            let inverted_body = invert_boolean_expression(e.clone());
-            edit.replace(e.syntax().text_range(), inverted_body.syntax().text());
+            let inverted_body = invert_boolean_expression(make, e.clone());
+            editor.replace(e.syntax(), inverted_body.syntax());
         }
     }
 }
 
 /// Add bang and parentheses to the expression.
-fn add_bang_paren(expr: ast::Expr) -> ast::Expr {
-    make::expr_prefix(T![!], make::expr_paren(expr)).into()
+fn add_bang_paren(make: &SyntaxFactory, expr: ast::Expr) -> ast::Expr {
+    make.expr_prefix(T![!], make.expr_paren(expr).into()).into()
+}
+
+fn needs_parens_in_place_of(
+    this: &ast::Expr,
+    parent: &SyntaxNode,
+    in_place_of: &ast::Expr,
+) -> bool {
+    assert_eq!(Some(parent), in_place_of.syntax().parent().as_ref());
+
+    let child_idx = parent
+        .children()
+        .enumerate()
+        .find_map(|(i, it)| if &it == in_place_of.syntax() { Some(i) } else { None })
+        .unwrap();
+    let parent = parent.clone_subtree();
+    let subtree_place = parent.children().nth(child_idx).unwrap();
+
+    let mut editor = SyntaxEditor::new(parent);
+    editor.replace(subtree_place, this.syntax());
+    let edit = editor.finish();
+
+    let replaced = edit.new_root().children().nth(child_idx).unwrap();
+    let replaced = ast::Expr::cast(replaced).unwrap();
+    replaced.needs_parens_in(edit.new_root().clone())
 }
 
 #[cfg(test)]
diff --git a/crates/ide-assists/src/handlers/convert_bool_then.rs b/crates/ide-assists/src/handlers/convert_bool_then.rs
@@ -13,7 +13,7 @@ use syntax::{
 };
 
 use crate::{
-    utils::{invert_boolean_expression, unwrap_trivial_block},
+    utils::{invert_boolean_expression_legacy, unwrap_trivial_block},
     AssistContext, AssistId, AssistKind, Assists,
 };
 
@@ -119,7 +119,7 @@ pub(crate) fn convert_if_to_bool_then(acc: &mut Assists, ctx: &AssistContext<'_>
                     | ast::Expr::WhileExpr(_)
                     | ast::Expr::YieldExpr(_)
             );
-            let cond = if invert_cond { invert_boolean_expression(cond) } else { cond };
+            let cond = if invert_cond { invert_boolean_expression_legacy(cond) } else { cond };
             let cond = if parenthesize { make::expr_paren(cond) } else { cond };
             let arg_list = make::arg_list(Some(make::expr_closure(None, closure_body)));
             let mcall = make::expr_method_call(cond, make::name_ref("then"), arg_list);
diff --git a/crates/ide-assists/src/handlers/convert_to_guarded_return.rs b/crates/ide-assists/src/handlers/convert_to_guarded_return.rs
@@ -17,7 +17,7 @@ use syntax::{
 
 use crate::{
     assist_context::{AssistContext, Assists},
-    utils::invert_boolean_expression,
+    utils::invert_boolean_expression_legacy,
     AssistId, AssistKind,
 };
 
@@ -139,7 +139,7 @@ fn if_expr_to_guarded_return(
                     let new_expr = {
                         let then_branch =
                             make::block_expr(once(make::expr_stmt(early_expression).into()), None);
-                        let cond = invert_boolean_expression(cond_expr);
+                        let cond = invert_boolean_expression_legacy(cond_expr);
                         make::expr_if(cond, then_branch, None).indent(if_indent_level)
                     };
                     new_expr.syntax().clone_for_update()
diff --git a/crates/ide-assists/src/handlers/convert_while_to_loop.rs b/crates/ide-assists/src/handlers/convert_while_to_loop.rs
@@ -13,7 +13,7 @@ use syntax::{
 
 use crate::{
     assist_context::{AssistContext, Assists},
-    utils::invert_boolean_expression,
+    utils::invert_boolean_expression_legacy,
     AssistId, AssistKind,
 };
 
@@ -63,7 +63,7 @@ pub(crate) fn convert_while_to_loop(acc: &mut Assists, ctx: &AssistContext<'_>)
                 let stmts = iter::once(make::expr_stmt(if_expr.into()).into());
                 make::block_expr(stmts, None)
             } else {
-                let if_cond = invert_boolean_expression(while_cond);
+                let if_cond = invert_boolean_expression_legacy(while_cond);
                 let if_expr = make::expr_if(if_cond, break_block, None).syntax().clone().into();
                 let elements = while_body.stmt_list().map_or_else(
                     || Either::Left(iter::empty()),
diff --git a/crates/ide-assists/src/handlers/invert_if.rs b/crates/ide-assists/src/handlers/invert_if.rs
@@ -6,7 +6,7 @@ use syntax::{
 
 use crate::{
     assist_context::{AssistContext, Assists},
-    utils::invert_boolean_expression,
+    utils::invert_boolean_expression_legacy,
     AssistId, AssistKind,
 };
 
@@ -48,7 +48,7 @@ pub(crate) fn invert_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()
     };
 
     acc.add(AssistId("invert_if", AssistKind::RefactorRewrite), "Invert if", if_range, |edit| {
-        let flip_cond = invert_boolean_expression(cond.clone());
+        let flip_cond = invert_boolean_expression_legacy(cond.clone());
         edit.replace_ast(cond, flip_cond);
 
         let else_node = else_block.syntax();
diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs
@@ -17,7 +17,9 @@ use syntax::{
         self,
         edit::{AstNodeEdit, IndentLevel},
         edit_in_place::{AttrsOwnerEdit, Indent, Removable},
-        make, HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace,
+        make,
+        syntax_factory::SyntaxFactory,
+        HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace,
     },
     ted, AstNode, AstToken, Direction, Edition, NodeOrToken, SourceFile,
     SyntaxKind::*,
@@ -245,11 +247,79 @@ 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 {
-    invert_special_case(&expr).unwrap_or_else(|| make::expr_prefix(T![!], expr).into())
+pub(crate) fn invert_boolean_expression(make: &SyntaxFactory, expr: ast::Expr) -> ast::Expr {
+    invert_special_case(make, &expr).unwrap_or_else(|| make.expr_prefix(T![!], expr).into())
 }
 
-fn invert_special_case(expr: &ast::Expr) -> Option<ast::Expr> {
+// FIXME: Migrate usages of this function to the above function and remove this.
+pub(crate) fn invert_boolean_expression_legacy(expr: ast::Expr) -> ast::Expr {
+    invert_special_case_legacy(&expr).unwrap_or_else(|| make::expr_prefix(T![!], expr).into())
+}
+
+fn invert_special_case(make: &SyntaxFactory, expr: &ast::Expr) -> Option<ast::Expr> {
+    match expr {
+        ast::Expr::BinExpr(bin) => {
+            let op_kind = bin.op_kind()?;
+            let rev_kind = match op_kind {
+                ast::BinaryOp::CmpOp(ast::CmpOp::Eq { negated }) => {
+                    ast::BinaryOp::CmpOp(ast::CmpOp::Eq { negated: !negated })
+                }
+                ast::BinaryOp::CmpOp(ast::CmpOp::Ord { ordering: ast::Ordering::Less, strict }) => {
+                    ast::BinaryOp::CmpOp(ast::CmpOp::Ord {
+                        ordering: ast::Ordering::Greater,
+                        strict: !strict,
+                    })
+                }
+                ast::BinaryOp::CmpOp(ast::CmpOp::Ord {
+                    ordering: ast::Ordering::Greater,
+                    strict,
+                }) => ast::BinaryOp::CmpOp(ast::CmpOp::Ord {
+                    ordering: ast::Ordering::Less,
+                    strict: !strict,
+                }),
+                // Parenthesize other expressions before prefixing `!`
+                _ => {
+                    return Some(
+                        make.expr_prefix(T![!], make.expr_paren(expr.clone()).into()).into(),
+                    );
+                }
+            };
+
+            Some(make.expr_bin(bin.lhs()?, rev_kind, bin.rhs()?).into())
+        }
+        ast::Expr::MethodCallExpr(mce) => {
+            let receiver = mce.receiver()?;
+            let method = mce.name_ref()?;
+            let arg_list = mce.arg_list()?;
+
+            let method = match method.text().as_str() {
+                "is_some" => "is_none",
+                "is_none" => "is_some",
+                "is_ok" => "is_err",
+                "is_err" => "is_ok",
+                _ => return None,
+            };
+
+            Some(make.expr_method_call(receiver, make.name_ref(method), arg_list).into())
+        }
+        ast::Expr::PrefixExpr(pe) if pe.op_kind()? == ast::UnaryOp::Not => match pe.expr()? {
+            ast::Expr::ParenExpr(parexpr) => {
+                parexpr.expr().map(|e| e.clone_subtree().clone_for_update())
+            }
+            _ => pe.expr().map(|e| e.clone_subtree().clone_for_update()),
+        },
+        ast::Expr::Literal(lit) => match lit.kind() {
+            ast::LiteralKind::Bool(b) => match b {
+                true => Some(ast::Expr::Literal(make.expr_literal("false"))),
+                false => Some(ast::Expr::Literal(make.expr_literal("true"))),
+            },
+            _ => None,
+        },
+        _ => None,
+    }
+}
+
+fn invert_special_case_legacy(expr: &ast::Expr) -> Option<ast::Expr> {
     match expr {
         ast::Expr::BinExpr(bin) => {
             let bin = bin.clone_for_update();
