Skip to content

internal: rewrite assoc item manipulaion to use mutable trees #8837

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.

Sign up for GitHub

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 2 commits into from
May 14, 2021
Merged

internal: rewrite assoc item manipulaion to use mutable trees #8837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cea589b
internal: rewrite assoc item manipulaion to use mutable trees
matklad May 14, 2021
6c21d04
internal: use standard style for tests
matklad May 14, 2021
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
3 changes: 2 additions & 1 deletion crates/ide_assists/src/handlers/add_missing_impl_members.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -
// impl Trait for () {
// type X = ();
// fn foo(&self) {}$0
//
// }
// ```
// ->
Expand Down Expand Up @@ -195,6 +194,7 @@ impl Foo for S {
fn baz(&self) {
todo!()
}
}"#,
);
}
Expand Down Expand Up @@ -231,6 +231,7 @@ impl Foo for S {
fn foo(&self) {
${0:todo!()}
}
}"#,
);
}
Expand Down
153 changes: 99 additions & 54 deletions crates/ide_assists/src/handlers/generate_new.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use ast::Adt;
use itertools::Itertools;
use stdx::format_to;
use syntax::ast::{self, AstNode, NameOwner, StructKind, VisibilityOwner};
Expand Down Expand Up @@ -37,7 +36,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
};

// Return early if we've found an existing new fn
let impl_def = find_struct_impl(&ctx, &Adt::Struct(strukt.clone()), "new")?;
let impl_def = find_struct_impl(&ctx, &ast::Adt::Struct(strukt.clone()), "new")?;

let target = strukt.syntax().text_range();
acc.add(AssistId("generate_new", AssistKind::Generate), "Generate `new`", target, |builder| {
Expand All @@ -60,7 +59,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
let start_offset = impl_def
.and_then(|impl_def| find_impl_block_start(impl_def, &mut buf))
.unwrap_or_else(|| {
buf = generate_impl_text(&Adt::Struct(strukt.clone()), &buf);
buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf);
strukt.syntax().text_range().end()
});

Expand All @@ -81,110 +80,142 @@ mod tests {
use super::*;

#[test]
#[rustfmt::skip]
fn test_generate_new() {
// Check output of generation
check_assist(
generate_new,
"struct Foo {$0}",
"struct Foo {}
r#"
struct Foo {$0}
"#,
r#"
struct Foo {}

impl Foo {
fn $0new() -> Self { Self { } }
}",
}
"#,
);
check_assist(
generate_new,
"struct Foo<T: Clone> {$0}",
"struct Foo<T: Clone> {}
r#"
struct Foo<T: Clone> {$0}
"#,
r#"
struct Foo<T: Clone> {}

impl<T: Clone> Foo<T> {
fn $0new() -> Self { Self { } }
}",
}
"#,
);
check_assist(
generate_new,
"struct Foo<'a, T: Foo<'a>> {$0}",
"struct Foo<'a, T: Foo<'a>> {}
r#"
struct Foo<'a, T: Foo<'a>> {$0}
"#,
r#"
struct Foo<'a, T: Foo<'a>> {}

impl<'a, T: Foo<'a>> Foo<'a, T> {
fn $0new() -> Self { Self { } }
}",
}
"#,
);
check_assist(
generate_new,
"struct Foo { baz: String $0}",
"struct Foo { baz: String }
r#"
struct Foo { baz: String $0}
"#,
r#"
struct Foo { baz: String }

impl Foo {
fn $0new(baz: String) -> Self { Self { baz } }
}",
}
"#,
);
check_assist(
generate_new,
"struct Foo { baz: String, qux: Vec<i32> $0}",
"struct Foo { baz: String, qux: Vec<i32> }
r#"
struct Foo { baz: String, qux: Vec<i32> $0}
"#,
r#"
struct Foo { baz: String, qux: Vec<i32> }

impl Foo {
fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
}",
}
"#,
);
}

// Check that visibility modifiers don't get brought in for fields
#[test]
fn check_that_visibility_modifiers_dont_get_brought_in() {
check_assist(
generate_new,
"struct Foo { pub baz: String, pub qux: Vec<i32> $0}",
"struct Foo { pub baz: String, pub qux: Vec<i32> }
r#"
struct Foo { pub baz: String, pub qux: Vec<i32> $0}
"#,
r#"
struct Foo { pub baz: String, pub qux: Vec<i32> }

impl Foo {
fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
}",
}
"#,
);
}

// Check that it reuses existing impls
#[test]
fn check_it_reuses_existing_impls() {
check_assist(
generate_new,
"struct Foo {$0}
r#"
struct Foo {$0}

impl Foo {}
",
"struct Foo {}
"#,
r#"
struct Foo {}

impl Foo {
fn $0new() -> Self { Self { } }
}
",
"#,
);
check_assist(
generate_new,
"struct Foo {$0}
r#"
struct Foo {$0}

impl Foo {
fn qux(&self) {}
}
",
"struct Foo {}
"#,
r#"
struct Foo {}

impl Foo {
fn $0new() -> Self { Self { } }

fn qux(&self) {}
}
",
"#,
);

check_assist(
generate_new,
"struct Foo {$0}
r#"
struct Foo {$0}

impl Foo {
fn qux(&self) {}
fn baz() -> i32 {
5
}
}
",
"struct Foo {}
"#,
r#"
struct Foo {}

impl Foo {
fn $0new() -> Self { Self { } }
Expand All @@ -194,67 +225,79 @@ impl Foo {
5
}
}
",
"#,
);
}

// Check visibility of new fn based on struct
#[test]
fn check_visibility_of_new_fn_based_on_struct() {
check_assist(
generate_new,
"pub struct Foo {$0}",
"pub struct Foo {}
r#"
pub struct Foo {$0}
"#,
r#"
pub struct Foo {}

impl Foo {
pub fn $0new() -> Self { Self { } }
}",
}
"#,
);
check_assist(
generate_new,
"pub(crate) struct Foo {$0}",
"pub(crate) struct Foo {}
r#"
pub(crate) struct Foo {$0}
"#,
r#"
pub(crate) struct Foo {}

impl Foo {
pub(crate) fn $0new() -> Self { Self { } }
}",
}
"#,
);
}

#[test]
fn generate_new_not_applicable_if_fn_exists() {
check_assist_not_applicable(
generate_new,
"
r#"
struct Foo {$0}

impl Foo {
fn new() -> Self {
Self
}
}",
}
"#,
);

check_assist_not_applicable(
generate_new,
"
r#"
struct Foo {$0}

impl Foo {
fn New() -> Self {
Self
}
}",
}
"#,
);
}

#[test]
fn generate_new_target() {
check_assist_target(
generate_new,
"
r#"
struct SomeThingIrrelevant;
/// Has a lifetime parameter
struct Foo<'a, T: Foo<'a>> {$0}
struct EvenMoreIrrelevant;
",
"#,
"/// Has a lifetime parameter
struct Foo<'a, T: Foo<'a>> {}",
);
Expand All @@ -264,7 +307,7 @@ struct Foo<'a, T: Foo<'a>> {}",
fn test_unrelated_new() {
check_assist(
generate_new,
r##"
r#"
pub struct AstId<N: AstNode> {
file_id: HirFileId,
file_ast_id: FileAstId<N>,
Expand All @@ -285,8 +328,9 @@ impl<T> Source<T> {
pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
Source { file_id: self.file_id, ast: f(self.ast) }
}
}"##,
r##"
}
"#,
r#"
pub struct AstId<N: AstNode> {
file_id: HirFileId,
file_ast_id: FileAstId<N>,
Expand All @@ -309,7 +353,8 @@ impl<T> Source<T> {
pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
Source { file_id: self.file_id, ast: f(self.ast) }
}
}"##,
}
"#,
);
}
}
1 change: 0 additions & 1 deletion crates/ide_assists/src/tests/generated.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ trait Trait {
impl Trait for () {
type X = ();
fn foo(&self) {}$0
}
"#####,
r#####"
Expand Down
26 changes: 14 additions & 12 deletions crates/ide_assists/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,12 @@ pub fn add_trait_assoc_items_to_impl(
sema: &hir::Semantics<ide_db::RootDatabase>,
items: Vec<ast::AssocItem>,
trait_: hir::Trait,
impl_def: ast::Impl,
impl_: ast::Impl,
target_scope: hir::SemanticsScope,
) -> (ast::Impl, ast::AssocItem) {
let impl_item_list = impl_def.assoc_item_list().unwrap_or_else(make::assoc_item_list);

let n_existing_items = impl_item_list.assoc_items().count();
let source_scope = sema.scope_for_def(trait_);
let ast_transform = QualifyPaths::new(&target_scope, &source_scope)
.or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def.clone()));
.or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_.clone()));

let items = items
.into_iter()
Expand All @@ -147,13 +144,18 @@ pub fn add_trait_assoc_items_to_impl(
ast::AssocItem::TypeAlias(def) => ast::AssocItem::TypeAlias(def.remove_bounds()),
_ => it,
})
.map(|it| edit::remove_attrs_and_docs(&it));

let new_impl_item_list = impl_item_list.append_items(items);
let new_impl_def = impl_def.with_assoc_item_list(new_impl_item_list);
let first_new_item =
new_impl_def.assoc_item_list().unwrap().assoc_items().nth(n_existing_items).unwrap();
return (new_impl_def, first_new_item);
.map(|it| edit::remove_attrs_and_docs(&it).clone_subtree().clone_for_update());

let res = impl_.clone_for_update();
let assoc_item_list = res.get_or_create_assoc_item_list();
let mut first_item = None;
for item in items {
if first_item.is_none() {
first_item = Some(item.clone())
}
assoc_item_list.add_item(item)
}
return (res, first_item.unwrap());

fn add_body(fn_def: ast::Fn) -> ast::Fn {
match fn_def.body() {
Expand Down
Loading