Merge pull request #19171 from ShoyuVanilla/migrate-de-morgan-assist

internal: Migrate `apply_demorgan` to `SyntaxEditor`

Author: Veykril

crates/ide-assists/src/handlers/add_missing_match_arms.rs

@@ -76,6 +76,8 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
 
     let cfg = ctx.config.import_path_config();
 
+    let make = SyntaxFactory::new();
+
     let module = ctx.sema.scope(expr.syntax())?.module();
     let (mut missing_pats, is_non_exhaustive, has_hidden_variants): (
         Peekable<Box<dyn Iterator<Item = (ast::Pat, bool)>>>,
@@ -93,7 +95,7 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
             .into_iter()
             .filter_map(|variant| {
                 Some((
-                    build_pat(ctx, module, variant, cfg)?,
+                    build_pat(ctx, &make, module, variant, cfg)?,
                     variant.should_be_hidden(ctx.db(), module.krate()),
                 ))
             })
@@ -144,10 +146,11 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
                 let is_hidden = variants
                     .iter()
                     .any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));
-                let patterns =
-                    variants.into_iter().filter_map(|variant| build_pat(ctx, module, variant, cfg));
+                let patterns = variants
+                    .into_iter()
+                    .filter_map(|variant| build_pat(ctx, &make, module, variant, cfg));
 
-                (ast::Pat::from(make::tuple_pat(patterns)), is_hidden)
+                (ast::Pat::from(make.tuple_pat(patterns)), is_hidden)
             })
             .filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat));
         (
@@ -176,9 +179,11 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
                 let is_hidden = variants
                     .iter()
                     .any(|variant| variant.should_be_hidden(ctx.db(), module.krate()));
-                let patterns =
-                    variants.into_iter().filter_map(|variant| build_pat(ctx, module, variant, cfg));
-                (ast::Pat::from(make::slice_pat(patterns)), is_hidden)
+                let patterns = variants
+                    .into_iter()
+                    .filter_map(|variant| build_pat(ctx, &make, module, variant, cfg));
+
+                (ast::Pat::from(make.slice_pat(patterns)), is_hidden)
             })
             .filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat));
         (
@@ -203,8 +208,6 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
         "Fill match arms",
         ctx.sema.original_range(match_expr.syntax()).range,
         |builder| {
-            let make = SyntaxFactory::new();
-
             // having any hidden variants means that we need a catch-all arm
             needs_catch_all_arm |= has_hidden_variants;
 
@@ -243,7 +246,7 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
 
             if needs_catch_all_arm && !has_catch_all_arm {
                 cov_mark::hit!(added_wildcard_pattern);
-                let arm = make.match_arm(make::wildcard_pat().into(), None, make::ext::expr_todo());
+                let arm = make.match_arm(make.wildcard_pat().into(), None, make::ext::expr_todo());
                 arms.push(arm);
             }
 
@@ -290,7 +293,7 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>)
                 }
             }
 
-            editor.add_mappings(make.finish_with_mappings());
+            editor.add_mappings(make.take());
             builder.add_file_edits(ctx.file_id(), editor);
         },
     )
@@ -445,6 +448,7 @@ fn resolve_array_of_enum_def(
 
 fn build_pat(
     ctx: &AssistContext<'_>,
+    make: &SyntaxFactory,
     module: hir::Module,
     var: ExtendedVariant,
     cfg: ImportPathConfig,
@@ -455,31 +459,32 @@ fn build_pat(
             let edition = module.krate().edition(db);
             let path = mod_path_to_ast(&module.find_path(db, ModuleDef::from(var), cfg)?, edition);
             let fields = var.fields(db);
-            let pat = match var.kind(db) {
+            let pat: ast::Pat = match var.kind(db) {
                 hir::StructKind::Tuple => {
                     let mut name_generator = suggest_name::NameGenerator::new();
                     let pats = fields.into_iter().map(|f| {
                         let name = name_generator.for_type(&f.ty(db), db, edition);
                         match name {
-                            Some(name) => make::ext::simple_ident_pat(make::name(&name)).into(),
-                            None => make::wildcard_pat().into(),
+                            Some(name) => make::ext::simple_ident_pat(make.name(&name)).into(),
+                            None => make.wildcard_pat().into(),
                         }
                     });
-                    make::tuple_struct_pat(path, pats).into()
+                    make.tuple_struct_pat(path, pats).into()
                 }
                 hir::StructKind::Record => {
-                    let pats = fields
+                    let fields = fields
                         .into_iter()
-                        .map(|f| make::name(f.name(db).as_str()))
-                        .map(|name| make::ext::simple_ident_pat(name).into());
-                    make::record_pat(path, pats).into()
+                        .map(|f| make.name_ref(f.name(db).as_str()))
+                        .map(|name_ref| make.record_pat_field_shorthand(name_ref));
+                    let fields = make.record_pat_field_list(fields, None);
+                    make.record_pat_with_fields(path, fields).into()
                 }
-                hir::StructKind::Unit => make::path_pat(path),
+                hir::StructKind::Unit => make.path_pat(path),
             };
             Some(pat)
         }
-        ExtendedVariant::True => Some(ast::Pat::from(make::literal_pat("true"))),
-        ExtendedVariant::False => Some(ast::Pat::from(make::literal_pat("false"))),
+        ExtendedVariant::True => Some(ast::Pat::from(make.literal_pat("true"))),
+        ExtendedVariant::False => Some(ast::Pat::from(make.literal_pat("false"))),
     }
 }
 

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)]

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);

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()