Skip to content

Commit

Permalink
Merge pull request #19171 from ShoyuVanilla/migrate-de-morgan-assist
Browse files Browse the repository at this point in the history
internal: Migrate `apply_demorgan` to `SyntaxEditor`
  • Loading branch information
Veykril authored Feb 24, 2025
2 parents 93bd36d + a0b9931 commit f6edb71
Show file tree
Hide file tree
Showing 10 changed files with 351 additions and 75 deletions.
49 changes: 27 additions & 22 deletions crates/ide-assists/src/handlers/add_missing_match_arms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)>>>,
Expand All @@ -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()),
))
})
Expand Down Expand Up @@ -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));
(
Expand Down Expand Up @@ -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));
(
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
},
)
Expand Down Expand Up @@ -445,6 +448,7 @@ fn resolve_array_of_enum_def(

fn build_pat(
ctx: &AssistContext<'_>,
make: &SyntaxFactory,
module: hir::Module,
var: ExtendedVariant,
cfg: ImportPathConfig,
Expand All @@ -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"))),
}
}

Expand Down
120 changes: 80 additions & 40 deletions crates/ide-assists/src/handlers/apply_demorgan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()?),
Expand All @@ -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()
Expand All @@ -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);
},
)
}
Expand Down Expand Up @@ -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());
Expand All @@ -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() {
Expand All @@ -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);
},
)
}
Expand All @@ -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)]
Expand Down
4 changes: 2 additions & 2 deletions crates/ide-assists/src/handlers/convert_bool_then.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions crates/ide-assists/src/handlers/convert_to_guarded_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use syntax::{

use crate::{
assist_context::{AssistContext, Assists},
utils::invert_boolean_expression,
utils::invert_boolean_expression_legacy,
AssistId, AssistKind,
};

Expand Down Expand Up @@ -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()
Expand Down
Loading

0 comments on commit f6edb71

Please sign in to comment.