Skip to content

internal: Migrate some low-hanging remove_* assists to SyntaxEditor #19225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions crates/ide-assists/src/handlers/remove_mut.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use syntax::{SyntaxKind, TextRange, T};
use syntax::{SyntaxKind, T};

use crate::{AssistContext, AssistId, AssistKind, Assists};

Expand All @@ -19,19 +19,20 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
// ```
pub(crate) fn remove_mut(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let mut_token = ctx.find_token_syntax_at_offset(T![mut])?;
let delete_from = mut_token.text_range().start();
let delete_to = match mut_token.next_token() {
Some(it) if it.kind() == SyntaxKind::WHITESPACE => it.text_range().end(),
_ => mut_token.text_range().end(),
};

let target = mut_token.text_range();
acc.add(
AssistId("remove_mut", AssistKind::Refactor),
"Remove `mut` keyword",
target,
|builder| {
builder.delete(TextRange::new(delete_from, delete_to));
let mut editor = builder.make_editor(&mut_token.parent().unwrap());
match mut_token.next_token() {
Some(it) if it.kind() == SyntaxKind::WHITESPACE => editor.delete(it),
_ => (),
}
editor.delete(mut_token);
builder.add_file_edits(ctx.file_id(), editor);
},
)
}
17 changes: 13 additions & 4 deletions crates/ide-assists/src/handlers/remove_parentheses.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use syntax::{ast, AstNode, SyntaxKind, T};
use syntax::{
ast::{self, syntax_factory::SyntaxFactory},
syntax_editor::Position,
AstNode, SyntaxKind, T,
};

use crate::{AssistContext, AssistId, AssistKind, Assists};

Expand Down Expand Up @@ -40,6 +44,7 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) ->
"Remove redundant parentheses",
target,
|builder| {
let mut editor = builder.make_editor(parens.syntax());
let prev_token = parens.syntax().first_token().and_then(|it| it.prev_token());
let need_to_add_ws = match prev_token {
Some(it) => {
Expand All @@ -48,9 +53,13 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) ->
}
None => false,
};
let expr = if need_to_add_ws { format!(" {expr}") } else { expr.to_string() };

builder.replace(parens.syntax().text_range(), expr)
if need_to_add_ws {
let make = SyntaxFactory::new();
editor.insert(Position::before(parens.syntax()), make.whitespace(" "));
editor.add_mappings(make.finish_with_mappings());
}
editor.replace(parens.syntax(), expr.syntax());
builder.add_file_edits(ctx.file_id(), editor);
},
)
}
Expand Down
62 changes: 48 additions & 14 deletions crates/ide-assists/src/handlers/remove_unused_param.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use ide_db::{defs::Definition, search::FileReference, EditionedFileId};
use syntax::{
algo::find_node_at_range,
algo::{find_node_at_range, least_common_ancestor_element},
ast::{self, HasArgList},
AstNode, SourceFile, SyntaxKind, SyntaxNode, TextRange, T,
syntax_editor::Element,
AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, TextRange, T,
};

use SyntaxKind::WHITESPACE;
Expand Down Expand Up @@ -74,15 +75,21 @@ pub(crate) fn remove_unused_param(acc: &mut Assists, ctx: &AssistContext<'_>) ->
cov_mark::hit!(keep_used);
return None;
}
let parent = param.syntax().parent()?;
acc.add(
AssistId("remove_unused_param", AssistKind::Refactor),
"Remove unused parameter",
param.syntax().text_range(),
|builder| {
builder.delete(range_to_remove(param.syntax()));
let mut editor = builder.make_editor(&parent);
let elements = elements_to_remove(param.syntax());
for element in elements {
editor.delete(element);
}
for (file_id, references) in fn_def.usages(&ctx.sema).all() {
process_usages(ctx, builder, file_id, references, param_position, is_self_present);
}
builder.add_file_edits(ctx.file_id(), editor);
},
)
}
Expand All @@ -96,20 +103,24 @@ fn process_usages(
is_self_present: bool,
) {
let source_file = ctx.sema.parse(file_id);
builder.edit_file(file_id);
let possible_ranges = references
.into_iter()
.filter_map(|usage| process_usage(&source_file, usage, arg_to_remove, is_self_present));

let mut ranges_to_delete: Vec<TextRange> = vec![];
for range in possible_ranges {
if !ranges_to_delete.iter().any(|it| it.contains_range(range)) {
ranges_to_delete.push(range)
for element_range in possible_ranges {
let Some(SyntaxElement::Node(parent)) = element_range
.iter()
.cloned()
.reduce(|a, b| least_common_ancestor_element(&a, &b).unwrap().syntax_element())
else {
continue;
};
let mut editor = builder.make_editor(&parent);
for element in element_range {
editor.delete(element);
}
}

for range in ranges_to_delete {
builder.delete(range)
builder.add_file_edits(file_id, editor);
}
}

Expand All @@ -118,7 +129,7 @@ fn process_usage(
FileReference { range, .. }: FileReference,
mut arg_to_remove: usize,
is_self_present: bool,
) -> Option<TextRange> {
) -> Option<Vec<SyntaxElement>> {
let call_expr_opt: Option<ast::CallExpr> = find_node_at_range(source_file.syntax(), range);
if let Some(call_expr) = call_expr_opt {
let call_expr_range = call_expr.expr()?.syntax().text_range();
Expand All @@ -127,7 +138,7 @@ fn process_usage(
}

let arg = call_expr.arg_list()?.args().nth(arg_to_remove)?;
return Some(range_to_remove(arg.syntax()));
return Some(elements_to_remove(arg.syntax()));
}

let method_call_expr_opt: Option<ast::MethodCallExpr> =
Expand All @@ -143,7 +154,7 @@ fn process_usage(
}

let arg = method_call_expr.arg_list()?.args().nth(arg_to_remove)?;
return Some(range_to_remove(arg.syntax()));
return Some(elements_to_remove(arg.syntax()));
}

None
Expand Down Expand Up @@ -174,6 +185,29 @@ pub(crate) fn range_to_remove(node: &SyntaxNode) -> TextRange {
}
}

pub(crate) fn elements_to_remove(node: &SyntaxNode) -> Vec<SyntaxElement> {
let up_to_comma = next_prev().find_map(|dir| {
node.siblings_with_tokens(dir)
.filter_map(|it| it.into_token())
.find(|it| it.kind() == T![,])
.map(|it| (dir, it))
});
if let Some((dir, token)) = up_to_comma {
let after = token.siblings_with_tokens(dir).nth(1).unwrap();
let mut result: Vec<_> =
node.siblings_with_tokens(dir).take_while(|it| it != &after).collect();
if node.next_sibling().is_some() {
result.extend(
token.siblings_with_tokens(dir).skip(1).take_while(|it| it.kind() == WHITESPACE),
);
}

result
} else {
vec![node.syntax_element()]
}
}

#[cfg(test)]
mod tests {
use crate::tests::{check_assist, check_assist_not_applicable};
Expand Down
24 changes: 22 additions & 2 deletions crates/syntax/src/algo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
use itertools::Itertools;

use crate::{
AstNode, Direction, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange,
TextSize,
syntax_editor::Element, AstNode, Direction, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode,
SyntaxToken, TextRange, TextSize,
};

/// Returns ancestors of the node at the offset, sorted by length. This should
Expand Down Expand Up @@ -89,6 +89,26 @@ pub fn least_common_ancestor(u: &SyntaxNode, v: &SyntaxNode) -> Option<SyntaxNod
Some(res)
}

pub fn least_common_ancestor_element(u: impl Element, v: impl Element) -> Option<SyntaxNode> {
let u = u.syntax_element();
let v = v.syntax_element();
if u == v {
return match u {
NodeOrToken::Node(node) => Some(node),
NodeOrToken::Token(token) => token.parent(),
};
}

let u_depth = u.ancestors().count();
let v_depth = v.ancestors().count();
let keep = u_depth.min(v_depth);

let u_candidates = u.ancestors().skip(u_depth - keep);
let v_candidates = v.ancestors().skip(v_depth - keep);
let (res, _) = u_candidates.zip(v_candidates).find(|(x, y)| x == y)?;
Some(res)
}

pub fn neighbor<T: AstNode>(me: &T, direction: Direction) -> Option<T> {
me.syntax().siblings(direction).skip(1).find_map(T::cast)
}
Expand Down
59 changes: 59 additions & 0 deletions crates/syntax/src/syntax_editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//! [`SyntaxEditor`]: https://github.com/dotnet/roslyn/blob/43b0b05cc4f492fd5de00f6f6717409091df8daa/src/Workspaces/Core/Portable/Editing/SyntaxEditor.cs

use std::{
fmt,
num::NonZeroU32,
ops::RangeInclusive,
sync::atomic::{AtomicU32, Ordering},
Expand Down Expand Up @@ -282,6 +283,64 @@ enum ChangeKind {
Replace,
}

impl fmt::Display for Change {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Change::Insert(position, node_or_token) => {
let parent = position.parent();
let mut parent_str = parent.to_string();
let target_range = self.target_range().start() - parent.text_range().start();

parent_str.insert_str(
target_range.into(),
&format!("\x1b[42m{node_or_token}\x1b[0m\x1b[K"),
);
f.write_str(&parent_str)
}
Change::InsertAll(position, vec) => {
let parent = position.parent();
let mut parent_str = parent.to_string();
let target_range = self.target_range().start() - parent.text_range().start();
let insertion: String = vec.iter().map(|it| it.to_string()).collect();

parent_str
.insert_str(target_range.into(), &format!("\x1b[42m{insertion}\x1b[0m\x1b[K"));
f.write_str(&parent_str)
}
Change::Replace(old, new) => {
if let Some(new) = new {
write!(f, "\x1b[41m{old}\x1b[42m{new}\x1b[0m\x1b[K")
} else {
write!(f, "\x1b[41m{old}\x1b[0m\x1b[K")
}
}
Change::ReplaceWithMany(old, vec) => {
let new: String = vec.iter().map(|it| it.to_string()).collect();
write!(f, "\x1b[41m{old}\x1b[42m{new}\x1b[0m\x1b[K")
}
Change::ReplaceAll(range, vec) => {
let parent = range.start().parent().unwrap();
let parent_str = parent.to_string();
let pre_range =
TextRange::new(parent.text_range().start(), range.start().text_range().start());
let old_range = TextRange::new(
range.start().text_range().start(),
range.end().text_range().end(),
);
let post_range =
TextRange::new(range.end().text_range().end(), parent.text_range().end());

let pre_str = &parent_str[pre_range - parent.text_range().start()];
let old_str = &parent_str[old_range - parent.text_range().start()];
let post_str = &parent_str[post_range - parent.text_range().start()];
let new: String = vec.iter().map(|it| it.to_string()).collect();

write!(f, "{pre_str}\x1b[41m{old_str}\x1b[42m{new}\x1b[0m\x1b[K{post_str}")
}
}
}
}

/// Utility trait to allow calling syntax editor functions with references or owned
/// nodes. Do not use outside of this module.
pub trait Element {
Expand Down
Loading