Skip to content

Commit 82dfdac

Browse files
committed
Modify around add_trait_assoc_items_to_impl to migrate add_missing_impl_members
Signed-off-by: Hayashi Mikihiro <[email protected]>
1 parent 827e3f7 commit 82dfdac

File tree

6 files changed

+229
-110
lines changed

6 files changed

+229
-110
lines changed

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

Lines changed: 81 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use hir::HasSource;
22
use syntax::{
33
Edition,
44
ast::{self, AstNode, make},
5-
ted,
5+
syntax_editor::{Position, SyntaxEditor},
66
};
77

88
use crate::{
@@ -148,31 +148,62 @@ fn add_missing_impl_members_inner(
148148

149149
let target = impl_def.syntax().text_range();
150150
acc.add(AssistId::quick_fix(assist_id), label, target, |edit| {
151-
let new_impl_def = edit.make_mut(impl_def.clone());
152-
let first_new_item = add_trait_assoc_items_to_impl(
151+
let new_item = add_trait_assoc_items_to_impl(
153152
&ctx.sema,
154153
ctx.config,
155154
&missing_items,
156155
trait_,
157-
&new_impl_def,
156+
&impl_def,
158157
&target_scope,
159158
);
160159

160+
let Some((first_new_item, other_items)) = new_item.split_first() else {
161+
return;
162+
};
163+
164+
let mut first_new_item = if let DefaultMethods::No = mode
165+
&& let ast::AssocItem::Fn(func) = &first_new_item
166+
&& let Some(body) = try_gen_trait_body(
167+
ctx,
168+
func,
169+
trait_ref,
170+
&impl_def,
171+
target_scope.krate().edition(ctx.sema.db),
172+
)
173+
&& let Some(func_body) = func.body()
174+
{
175+
let mut func_editor = SyntaxEditor::new(first_new_item.syntax().clone_subtree());
176+
func_editor.replace(func_body.syntax(), body.syntax());
177+
ast::AssocItem::cast(func_editor.finish().new_root().clone())
178+
} else {
179+
Some(first_new_item.clone())
180+
};
181+
182+
let new_assoc_items = first_new_item
183+
.clone()
184+
.into_iter()
185+
.chain(other_items.iter().cloned())
186+
.map(either::Either::Right)
187+
.collect::<Vec<_>>();
188+
189+
let mut editor = edit.make_editor(impl_def.syntax());
190+
if let Some(assoc_item_list) = impl_def.assoc_item_list() {
191+
let items = new_assoc_items.into_iter().filter_map(either::Either::right).collect();
192+
assoc_item_list.add_items(&mut editor, items);
193+
} else {
194+
let assoc_item_list = make::assoc_item_list(Some(new_assoc_items)).clone_for_update();
195+
editor.insert_all(
196+
Position::after(impl_def.syntax()),
197+
vec![make::tokens::whitespace(" ").into(), assoc_item_list.syntax().clone().into()],
198+
);
199+
first_new_item = assoc_item_list.assoc_items().next();
200+
}
201+
161202
if let Some(cap) = ctx.config.snippet_cap {
162203
let mut placeholder = None;
163204
if let DefaultMethods::No = mode {
164-
if let ast::AssocItem::Fn(func) = &first_new_item {
165-
if let Some(body) = try_gen_trait_body(
166-
ctx,
167-
func,
168-
trait_ref,
169-
&impl_def,
170-
target_scope.krate().edition(ctx.sema.db),
171-
) && let Some(func_body) = func.body()
172-
{
173-
ted::replace(func_body.syntax(), body.syntax());
174-
} else if let Some(m) =
175-
func.syntax().descendants().find_map(ast::MacroCall::cast)
205+
if let Some(ast::AssocItem::Fn(func)) = &first_new_item {
206+
if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast)
176207
&& m.syntax().text() == "todo!()"
177208
{
178209
placeholder = Some(m);
@@ -181,11 +212,14 @@ fn add_missing_impl_members_inner(
181212
}
182213

183214
if let Some(macro_call) = placeholder {
184-
edit.add_placeholder_snippet(cap, macro_call);
185-
} else {
186-
edit.add_tabstop_before(cap, first_new_item);
215+
let placeholder = edit.make_placeholder_snippet(cap);
216+
editor.add_annotation(macro_call.syntax(), placeholder);
217+
} else if let Some(first_new_item) = first_new_item {
218+
let tabstop = edit.make_tabstop_before(cap);
219+
editor.add_annotation(first_new_item.syntax(), tabstop);
187220
};
188221
};
222+
edit.add_file_edits(ctx.vfs_file_id(), editor);
189223
})
190224
}
191225

@@ -322,7 +356,7 @@ impl Foo for S {
322356
}
323357

324358
#[test]
325-
fn test_impl_def_without_braces() {
359+
fn test_impl_def_without_braces_macro() {
326360
check_assist(
327361
add_missing_impl_members,
328362
r#"
@@ -340,6 +374,33 @@ impl Foo for S {
340374
);
341375
}
342376

377+
#[test]
378+
fn test_impl_def_without_braces_tabstop_first_item() {
379+
check_assist(
380+
add_missing_impl_members,
381+
r#"
382+
trait Foo {
383+
type Output;
384+
fn foo(&self);
385+
}
386+
struct S;
387+
impl Foo for S { $0 }"#,
388+
r#"
389+
trait Foo {
390+
type Output;
391+
fn foo(&self);
392+
}
393+
struct S;
394+
impl Foo for S {
395+
$0type Output;
396+
397+
fn foo(&self) {
398+
todo!()
399+
}
400+
}"#,
401+
);
402+
}
403+
343404
#[test]
344405
fn fill_in_type_params_1() {
345406
check_assist(

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

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -167,33 +167,44 @@ pub(crate) fn generate_impl_trait(acc: &mut Assists, ctx: &AssistContext<'_>) ->
167167
DefaultMethods::No,
168168
IgnoreAssocItems::DocHiddenAttrPresent,
169169
);
170-
let impl_ = make::impl_trait(
171-
trait_.unsafe_token().is_some(),
172-
None,
173-
trait_.generic_param_list().map(|list| {
174-
make::generic_arg_list(list.generic_params().map(|_| holder_arg.clone()))
175-
}),
176-
None,
177-
None,
178-
false,
179-
make::ty(&name.text()),
180-
make::ty_placeholder(),
181-
None,
182-
None,
183-
None,
184-
)
185-
.clone_for_update();
186-
187-
if !missing_items.is_empty() {
188-
utils::add_trait_assoc_items_to_impl(
170+
171+
let trait_gen_args = trait_.generic_param_list().map(|list| {
172+
make::generic_arg_list(list.generic_params().map(|_| holder_arg.clone()))
173+
});
174+
175+
let make_impl_ = |body| {
176+
make::impl_trait(
177+
trait_.unsafe_token().is_some(),
178+
None,
179+
trait_gen_args.clone(),
180+
None,
181+
None,
182+
false,
183+
make::ty(&name.text()),
184+
make::ty_placeholder(),
185+
None,
186+
None,
187+
body,
188+
)
189+
.clone_for_update()
190+
};
191+
192+
let impl_ = if missing_items.is_empty() {
193+
make_impl_(None)
194+
} else {
195+
let impl_ = make_impl_(None);
196+
let assoc_items = utils::add_trait_assoc_items_to_impl(
189197
&ctx.sema,
190198
ctx.config,
191199
&missing_items,
192200
hir_trait,
193201
&impl_,
194202
&target_scope,
195203
);
196-
}
204+
let assoc_items = assoc_items.into_iter().map(either::Either::Right).collect();
205+
let assoc_item_list = make::assoc_item_list(Some(assoc_items));
206+
make_impl_(Some(assoc_item_list))
207+
};
197208

198209
if let Some(cap) = ctx.config.snippet_cap {
199210
if let Some(generics) = impl_.trait_().and_then(|it| it.generic_arg_list()) {

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -206,19 +206,35 @@ fn impl_def_from_trait(
206206
}
207207
let impl_def = generate_trait_impl(impl_is_unsafe, adt, make::ty_path(trait_path.clone()));
208208

209-
let _ =
209+
let assoc_items =
210210
add_trait_assoc_items_to_impl(sema, config, &trait_items, trait_, &impl_def, &target_scope);
211-
let impl_def = impl_def.clone_subtree();
212-
let mut editor = SyntaxEditor::new(impl_def.syntax().clone());
213-
let first_assoc_item = impl_def.assoc_item_list().and_then(|item| item.assoc_items().next())?;
214-
// Generate a default `impl` function body for the derived trait.
215-
if let ast::AssocItem::Fn(ref func) = first_assoc_item {
216-
if let Some(body) = gen_trait_fn_body(func, trait_path, adt, None)
211+
let assoc_item_list = if let Some((first, other)) =
212+
assoc_items.split_first().map(|(first, other)| (first.clone_subtree(), other))
213+
{
214+
let first_item = if let ast::AssocItem::Fn(ref func) = first
215+
&& let Some(body) = gen_trait_fn_body(func, trait_path, adt, None)
217216
&& let Some(func_body) = func.body()
218217
{
218+
let mut editor = SyntaxEditor::new(first.syntax().clone());
219219
editor.replace(func_body.syntax(), body.syntax());
220-
}
221-
};
220+
ast::AssocItem::cast(editor.finish().new_root().clone())
221+
} else {
222+
Some(first.clone())
223+
};
224+
let items = first_item
225+
.into_iter()
226+
.chain(other.iter().cloned())
227+
.map(either::Either::Right)
228+
.collect();
229+
make::assoc_item_list(Some(items))
230+
} else {
231+
make::assoc_item_list(None)
232+
}
233+
.clone_for_update();
234+
235+
let impl_def = impl_def.clone_subtree();
236+
let mut editor = SyntaxEditor::new(impl_def.syntax().clone());
237+
editor.replace(impl_def.assoc_item_list()?.syntax(), assoc_item_list.syntax());
222238
let impl_def = ast::Impl::cast(editor.finish().new_root().clone())?;
223239
Some(impl_def)
224240
}

crates/ide-assists/src/utils.rs

Lines changed: 55 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use syntax::{
2323
ast::{
2424
self, HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace,
2525
edit::{AstNodeEdit, IndentLevel},
26-
edit_in_place::{AttrsOwnerEdit, Indent, Removable},
26+
edit_in_place::{AttrsOwnerEdit, Removable},
2727
make,
2828
syntax_factory::SyntaxFactory,
2929
},
@@ -178,78 +178,73 @@ pub fn filter_assoc_items(
178178
/// [`filter_assoc_items()`]), clones each item for update and applies path transformation to it,
179179
/// then inserts into `impl_`. Returns the modified `impl_` and the first associated item that got
180180
/// inserted.
181+
#[must_use]
181182
pub fn add_trait_assoc_items_to_impl(
182183
sema: &Semantics<'_, RootDatabase>,
183184
config: &AssistConfig,
184185
original_items: &[InFile<ast::AssocItem>],
185186
trait_: hir::Trait,
186187
impl_: &ast::Impl,
187188
target_scope: &hir::SemanticsScope<'_>,
188-
) -> ast::AssocItem {
189+
) -> Vec<ast::AssocItem> {
189190
let new_indent_level = IndentLevel::from_node(impl_.syntax()) + 1;
190-
let items = original_items.iter().map(|InFile { file_id, value: original_item }| {
191-
let cloned_item = {
192-
if let Some(macro_file) = file_id.macro_file() {
193-
let span_map = sema.db.expansion_span_map(macro_file);
194-
let item_prettified = prettify_macro_expansion(
195-
sema.db,
196-
original_item.syntax().clone(),
197-
&span_map,
198-
target_scope.krate().into(),
199-
);
200-
if let Some(formatted) = ast::AssocItem::cast(item_prettified) {
201-
return formatted;
202-
} else {
203-
stdx::never!("formatted `AssocItem` could not be cast back to `AssocItem`");
191+
original_items
192+
.iter()
193+
.map(|InFile { file_id, value: original_item }| {
194+
let cloned_item = {
195+
if let Some(macro_file) = file_id.macro_file() {
196+
let span_map = sema.db.expansion_span_map(macro_file);
197+
let item_prettified = prettify_macro_expansion(
198+
sema.db,
199+
original_item.syntax().clone(),
200+
&span_map,
201+
target_scope.krate().into(),
202+
);
203+
if let Some(formatted) = ast::AssocItem::cast(item_prettified) {
204+
return formatted;
205+
} else {
206+
stdx::never!("formatted `AssocItem` could not be cast back to `AssocItem`");
207+
}
204208
}
205-
}
206-
original_item.clone_for_update()
207-
};
208-
209-
if let Some(source_scope) = sema.scope(original_item.syntax()) {
210-
// FIXME: Paths in nested macros are not handled well. See
211-
// `add_missing_impl_members::paths_in_nested_macro_should_get_transformed` test.
212-
let transform =
213-
PathTransform::trait_impl(target_scope, &source_scope, trait_, impl_.clone());
214-
transform.apply(cloned_item.syntax());
215-
}
216-
cloned_item.remove_attrs_and_docs();
217-
cloned_item.reindent_to(new_indent_level);
218-
cloned_item
219-
});
209+
original_item.clone_for_update()
210+
};
220211

221-
let assoc_item_list = impl_.get_or_create_assoc_item_list();
222-
223-
let mut first_item = None;
224-
for item in items {
225-
first_item.get_or_insert_with(|| item.clone());
226-
match &item {
227-
ast::AssocItem::Fn(fn_) if fn_.body().is_none() => {
228-
let body = AstNodeEdit::indent(
229-
&make::block_expr(
230-
None,
231-
Some(match config.expr_fill_default {
232-
ExprFillDefaultMode::Todo => make::ext::expr_todo(),
233-
ExprFillDefaultMode::Underscore => make::ext::expr_underscore(),
234-
ExprFillDefaultMode::Default => make::ext::expr_todo(),
235-
}),
236-
),
237-
new_indent_level,
238-
);
239-
ted::replace(fn_.get_or_create_body().syntax(), body.syntax())
212+
if let Some(source_scope) = sema.scope(original_item.syntax()) {
213+
// FIXME: Paths in nested macros are not handled well. See
214+
// `add_missing_impl_members::paths_in_nested_macro_should_get_transformed` test.
215+
let transform =
216+
PathTransform::trait_impl(target_scope, &source_scope, trait_, impl_.clone());
217+
transform.apply(cloned_item.syntax());
240218
}
241-
ast::AssocItem::TypeAlias(type_alias) => {
242-
if let Some(type_bound_list) = type_alias.type_bound_list() {
243-
type_bound_list.remove()
219+
cloned_item.remove_attrs_and_docs();
220+
cloned_item.reset_indent()
221+
})
222+
.map(|item| {
223+
match &item {
224+
ast::AssocItem::Fn(fn_) if fn_.body().is_none() => {
225+
let body = AstNodeEdit::indent(
226+
&make::block_expr(
227+
None,
228+
Some(match config.expr_fill_default {
229+
ExprFillDefaultMode::Todo => make::ext::expr_todo(),
230+
ExprFillDefaultMode::Underscore => make::ext::expr_underscore(),
231+
ExprFillDefaultMode::Default => make::ext::expr_todo(),
232+
}),
233+
),
234+
IndentLevel::single(),
235+
);
236+
ted::replace(fn_.get_or_create_body().syntax(), body.syntax());
244237
}
238+
ast::AssocItem::TypeAlias(type_alias) => {
239+
if let Some(type_bound_list) = type_alias.type_bound_list() {
240+
type_bound_list.remove()
241+
}
242+
}
243+
_ => {}
245244
}
246-
_ => {}
247-
}
248-
249-
assoc_item_list.add_item(item)
250-
}
251-
252-
first_item.unwrap()
245+
AstNodeEdit::indent(&item, new_indent_level)
246+
})
247+
.collect()
253248
}
254249

255250
pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize {

0 commit comments

Comments
 (0)