Skip to content

Commit a1415f0

Browse files
committed
minor: Migrate remove_unnecessary_wrapper to SyntaxEditor
1 parent 13ce2e2 commit a1415f0

File tree

2 files changed

+64
-20
lines changed

2 files changed

+64
-20
lines changed

crates/ide-diagnostics/src/handlers/type_mismatch.rs

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ use syntax::{
99
ast::{
1010
self,
1111
edit::{AstNodeEdit, IndentLevel},
12-
make, BlockExpr, Expr, ExprStmt, HasArgList,
12+
syntax_factory::SyntaxFactory,
13+
BlockExpr, Expr, ExprStmt, HasArgList,
1314
},
14-
ted, AstNode, AstPtr, TextSize,
15+
AstNode, AstPtr, TextSize,
1516
};
1617

1718
use crate::{adjusted_display_range, fix, Assist, Diagnostic, DiagnosticCode, DiagnosticsContext};
@@ -199,6 +200,7 @@ fn remove_unnecessary_wrapper(
199200
let expr_range = expr.syntax().text_range();
200201
let actual_enum = d.actual.as_adt().and_then(|adt| adt.as_enum())?;
201202
let famous_defs = FamousDefs(&ctx.sema, ctx.sema.scope(expr.syntax())?.krate());
203+
let file_id = expr_ptr.file_id.original_file(ctx.sema.db);
202204

203205
let Expr::CallExpr(call_expr) = expr else {
204206
return None;
@@ -224,8 +226,8 @@ fn remove_unnecessary_wrapper(
224226

225227
let inner_arg = call_expr.arg_list().and_then(|list| list.args().next())?;
226228

227-
let mut builder = SourceChangeBuilder::new(expr_ptr.file_id.original_file(ctx.sema.db));
228-
229+
let mut builder = SourceChangeBuilder::new(file_id);
230+
let mut editor;
229231
match inner_arg {
230232
// We're returning `()`
231233
Expr::TupleExpr(tup) if tup.fields().next().is_none() => {
@@ -234,35 +236,34 @@ fn remove_unnecessary_wrapper(
234236
.parent()
235237
.and_then(Either::<ast::ReturnExpr, ast::StmtList>::cast)?;
236238

239+
editor = builder.make_editor(parent.syntax());
240+
let make = SyntaxFactory::new();
241+
237242
match parent {
238243
Either::Left(ret_expr) => {
239-
let old = builder.make_mut(ret_expr);
240-
let new = make::expr_return(None).clone_for_update();
241-
242-
ted::replace(old.syntax(), new.syntax());
244+
editor.replace(ret_expr.syntax(), make.expr_return(None).syntax());
243245
}
244246
Either::Right(stmt_list) => {
245-
if stmt_list.statements().count() == 0 {
246-
let block = stmt_list.syntax().parent().and_then(ast::BlockExpr::cast)?;
247-
let old = builder.make_mut(block);
248-
let new = make::expr_empty_block().clone_for_update();
249-
250-
ted::replace(old.syntax(), new.syntax());
247+
let new_block = if stmt_list.statements().next().is_none() {
248+
make.expr_empty_block()
251249
} else {
252-
let old = builder.make_syntax_mut(stmt_list.syntax().parent()?);
253-
let new = make::block_expr(stmt_list.statements(), None).clone_for_update();
250+
make.block_expr(stmt_list.statements(), None)
251+
};
254252

255-
ted::replace(old, new.syntax());
256-
}
253+
editor.replace(stmt_list.syntax().parent()?, new_block.syntax());
257254
}
258255
}
256+
257+
editor.add_mappings(make.finish_with_mappings());
259258
}
260259
_ => {
261-
let call_mut = builder.make_mut(call_expr);
262-
ted::replace(call_mut.syntax(), inner_arg.clone_for_update().syntax());
260+
editor = builder.make_editor(call_expr.syntax());
261+
// FIXME: remove `clone_for_update` when `SyntaxEditor` handles it for us
262+
editor.replace(call_expr.syntax(), inner_arg.syntax().clone_for_update());
263263
}
264264
}
265265

266+
builder.add_file_edits(file_id, editor);
266267
let name = format!("Remove unnecessary {}() wrapper", path_str);
267268
acc.push(fix("remove_unnecessary_wrapper", &name, builder.finish(), expr_range));
268269
Some(())
@@ -844,6 +845,29 @@ fn div(x: i32, y: i32) -> i32 {
844845
);
845846
}
846847

848+
#[test]
849+
fn test_unwrap_return_type_option_tail_unit() {
850+
check_fix(
851+
r#"
852+
//- minicore: option, result
853+
fn div(x: i32, y: i32) {
854+
if y == 0 {
855+
panic!();
856+
}
857+
858+
Ok(())$0
859+
}
860+
"#,
861+
r#"
862+
fn div(x: i32, y: i32) {
863+
if y == 0 {
864+
panic!();
865+
}
866+
}
867+
"#,
868+
);
869+
}
870+
847871
#[test]
848872
fn test_unwrap_return_type_handles_generic_functions() {
849873
check_fix(

crates/syntax/src/ast/syntax_factory/constructors.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ impl SyntaxFactory {
6363
ast
6464
}
6565

66+
pub fn expr_empty_block(&self) -> ast::BlockExpr {
67+
ast::BlockExpr { syntax: make::expr_empty_block().syntax().clone_for_update() }
68+
}
69+
6670
pub fn expr_path(&self, path: ast::Path) -> ast::Expr {
6771
let ast::Expr::PathExpr(ast) = make::expr_path(path.clone()).clone_for_update() else {
6872
unreachable!()
@@ -92,6 +96,22 @@ impl SyntaxFactory {
9296
ast.into()
9397
}
9498

99+
pub fn expr_return(&self, expr: Option<ast::Expr>) -> ast::ReturnExpr {
100+
let ast::Expr::ReturnExpr(ast) = make::expr_return(expr.clone()).clone_for_update() else {
101+
unreachable!()
102+
};
103+
104+
if let Some(mut mapping) = self.mappings() {
105+
let mut builder = SyntaxMappingBuilder::new(ast.syntax().clone());
106+
if let Some(input) = expr {
107+
builder.map_node(input.syntax().clone(), ast.expr().unwrap().syntax().clone());
108+
}
109+
builder.finish(&mut mapping);
110+
}
111+
112+
ast
113+
}
114+
95115
pub fn let_stmt(
96116
&self,
97117
pattern: ast::Pat,

0 commit comments

Comments
 (0)