From fc060573f9374af3b3a44343d303ef6e26f116a8 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 6 Mar 2019 14:41:22 +0100 Subject: Add 'add_missing_impl_members' assist stub --- crates/ra_assists/src/add_missing_impl_members.rs | 41 +++++++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 ++ 2 files changed, 43 insertions(+) create mode 100644 crates/ra_assists/src/add_missing_impl_members.rs (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs new file mode 100644 index 000000000..a0b656f8f --- /dev/null +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -0,0 +1,41 @@ +use crate::assist_ctx::{Assist, AssistCtx}; +use hir::db::HirDatabase; + +pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> Option { + unimplemented!() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::helpers::{ check_assist }; + + #[test] + fn test_add_missing_impl_members() { + check_assist( + add_missing_impl_members, + " +trait Foo { + fn foo(&self); +} + +struct S; + +impl Foo for S { + <|> +}", + " +trait Foo { + fn foo(&self); +} + +struct S; + +impl Foo for S { + fn foo(&self) { + <|> + } +}", + ); + } +} diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 6c3d75d79..0c4abb450 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -95,6 +95,7 @@ mod replace_if_let_with_match; mod split_import; mod remove_dbg; mod auto_import; +mod add_missing_impl_members; fn all_assists() -> &'static [fn(AssistCtx) -> Option] { &[ @@ -108,6 +109,7 @@ fn all_assists() -> &'static [fn(AssistCtx) -> Option Date: Wed, 6 Mar 2019 23:45:11 +0100 Subject: Calculate missing functions from impl body --- crates/ra_assists/src/add_missing_impl_members.rs | 63 ++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index a0b656f8f..120109d4b 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -1,7 +1,68 @@ +use std::collections::HashSet; + use crate::assist_ctx::{Assist, AssistCtx}; + +use hir::Resolver; use hir::db::HirDatabase; +use ra_syntax::{SmolStr, SyntaxKind, SyntaxNode, TreeArc}; +use ra_syntax::ast::{self, AstNode, FnDef, ImplItem, ImplItemKind, NameOwner}; +use ra_db::FilePosition; + +/// Given an `ast::ImplBlock`, resolves the target trait (the one being +/// implemented) to a `ast::TraitDef`. +pub(crate) fn resolve_target_trait_def( + db: &impl HirDatabase, + resolver: &Resolver, + impl_block: &ast::ImplBlock, +) -> Option> { + let ast_path = impl_block.target_trait().map(AstNode::syntax).and_then(ast::PathType::cast)?; + let hir_path = ast_path.path().and_then(hir::Path::from_ast)?; + + match resolver.resolve_path(db, &hir_path).take_types() { + Some(hir::Resolution::Def(hir::ModuleDef::Trait(def))) => Some(def.source(db).1), + _ => None, + } +} + +pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Option { + use SyntaxKind::{IMPL_BLOCK, ITEM_LIST, WHITESPACE}; + + let node = ctx.covering_node(); + let kinds = node.ancestors().take(3).map(SyntaxNode::kind); + // Only suggest this in `impl Foo for S { [Item...] <|> }` cursor position + if !Iterator::eq(kinds, [WHITESPACE, ITEM_LIST, IMPL_BLOCK].iter().cloned()) { + return None; + } + + let impl_node = node.ancestors().find_map(ast::ImplBlock::cast)?; + + let trait_def = { + let db = ctx.db; + // TODO: Can we get the position of cursor itself rather than supplied range? + let range = ctx.frange; + let position = FilePosition { file_id: range.file_id, offset: range.range.start() }; + let resolver = hir::source_binder::resolver_for_position(db, position); + + resolve_target_trait_def(db, &resolver, impl_node)? + }; + + let fn_def_opt = |kind| if let ImplItemKind::FnDef(def) = kind { Some(def) } else { None }; + let def_name = |&def| -> Option<&SmolStr> { FnDef::name(def).map(ast::Name::text) }; + + let trait_items = trait_def.syntax().descendants().find_map(ast::ItemList::cast)?.impl_items(); + let impl_items = impl_node.item_list()?.impl_items(); + + let trait_fns = trait_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); + let impl_fns = impl_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); + + let trait_fn_names = trait_fns.iter().filter_map(def_name).collect::>(); + let impl_fn_names = impl_fns.iter().filter_map(def_name).collect::>(); + + let missing_fn_names = trait_fn_names.difference(&impl_fn_names).collect::>(); + let missing_fns = trait_fns + .iter() + .filter(|&t| def_name(t).map(|n| missing_fn_names.contains(&n)).unwrap_or(false)); -pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> Option { unimplemented!() } -- cgit v1.2.3 From 2f616eea9c21ebbd217a42785eae1af3173689c2 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 01:48:31 +0100 Subject: Implement a simple working assist --- crates/ra_assists/src/add_missing_impl_members.rs | 72 +++++++++++++++++++---- 1 file changed, 59 insertions(+), 13 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 120109d4b..4926a9b24 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -1,12 +1,15 @@ use std::collections::HashSet; -use crate::assist_ctx::{Assist, AssistCtx}; +use crate::{Assist, AssistId, AssistCtx}; use hir::Resolver; use hir::db::HirDatabase; -use ra_syntax::{SmolStr, SyntaxKind, SyntaxNode, TreeArc}; +use ra_syntax::{SmolStr, SyntaxKind, SyntaxNode, TextUnit, TreeArc}; use ra_syntax::ast::{self, AstNode, FnDef, ImplItem, ImplItemKind, NameOwner}; use ra_db::FilePosition; +use ra_fmt::{leading_indent, reindent}; + +use itertools::Itertools; /// Given an `ast::ImplBlock`, resolves the target trait (the one being /// implemented) to a `ast::TraitDef`. @@ -24,7 +27,21 @@ pub(crate) fn resolve_target_trait_def( } } -pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Option { +pub(crate) fn build_func_body(def: &ast::FnDef) -> String { + let mut buf = String::new(); + + for child in def.syntax().children() { + if child.kind() == SyntaxKind::SEMI { + buf.push_str(" { unimplemented!() }") + } else { + child.text().push_to(&mut buf); + } + } + + buf.trim_end().to_string() +} + +pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> Option { use SyntaxKind::{IMPL_BLOCK, ITEM_LIST, WHITESPACE}; let node = ctx.covering_node(); @@ -35,6 +52,7 @@ pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Opti } let impl_node = node.ancestors().find_map(ast::ImplBlock::cast)?; + let impl_item_list = impl_node.item_list()?; let trait_def = { let db = ctx.db; @@ -47,23 +65,49 @@ pub(crate) fn add_missing_impl_members(ctx: AssistCtx) -> Opti }; let fn_def_opt = |kind| if let ImplItemKind::FnDef(def) = kind { Some(def) } else { None }; - let def_name = |&def| -> Option<&SmolStr> { FnDef::name(def).map(ast::Name::text) }; + let def_name = |def| -> Option<&SmolStr> { FnDef::name(def).map(ast::Name::text) }; let trait_items = trait_def.syntax().descendants().find_map(ast::ItemList::cast)?.impl_items(); - let impl_items = impl_node.item_list()?.impl_items(); + let impl_items = impl_item_list.impl_items(); let trait_fns = trait_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); let impl_fns = impl_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); - let trait_fn_names = trait_fns.iter().filter_map(def_name).collect::>(); - let impl_fn_names = impl_fns.iter().filter_map(def_name).collect::>(); + let trait_fn_names = trait_fns.iter().cloned().filter_map(def_name).collect::>(); + let impl_fn_names = impl_fns.iter().cloned().filter_map(def_name).collect::>(); let missing_fn_names = trait_fn_names.difference(&impl_fn_names).collect::>(); - let missing_fns = trait_fns + let missing_fns: Vec<_> = trait_fns .iter() - .filter(|&t| def_name(t).map(|n| missing_fn_names.contains(&n)).unwrap_or(false)); + .cloned() + .filter(|t| def_name(t).map(|n| missing_fn_names.contains(&n)).unwrap_or(false)) + .collect(); - unimplemented!() + if missing_fns.is_empty() { + return None; + } + + let last_whitespace_node = + impl_item_list.syntax().children().filter_map(ast::Whitespace::cast).last()?.syntax(); + + ctx.add_action(AssistId("add_impl_missing_members"), "add impl missing members", |edit| { + let func_bodies = missing_fns.into_iter().map(build_func_body).join("\n"); + let func_bodies = String::from("\n") + &func_bodies; + + let first_impl_item = impl_item_list.impl_items().next(); + // FIXME: We should respect the indent of the first item from the item list or the indent of leading block + some default indent (4?) + // Another approach is to not indent at all if there are no items here + let indent = first_impl_item.and_then(|i| leading_indent(i.syntax())).unwrap_or_default(); + let func_bodies = reindent(&func_bodies, indent) + "\n"; + + let changed_range = last_whitespace_node.range(); + let replaced_text_range = TextUnit::of_str(&func_bodies); + + edit.replace(changed_range, func_bodies); + edit.set_cursor(changed_range.start() + replaced_text_range - TextUnit::of_str("\n")); + }); + + ctx.build() } #[cfg(test)] @@ -78,24 +122,26 @@ mod tests { " trait Foo { fn foo(&self); + fn bar(&self); } struct S; impl Foo for S { + fn bar(&self) {} <|> }", " trait Foo { fn foo(&self); + fn bar(&self); } struct S; impl Foo for S { - fn foo(&self) { - <|> - } + fn bar(&self) {} + fn foo(&self) { unimplemented!() }<|> }", ); } -- cgit v1.2.3 From 38eece97ecc801811a8847cfd230e97d838398cd Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 12:02:53 +0100 Subject: Redo indent calculation when adding missing impl members --- crates/ra_assists/src/add_missing_impl_members.rs | 30 ++++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 4926a9b24..0888268e4 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -90,15 +90,24 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> let last_whitespace_node = impl_item_list.syntax().children().filter_map(ast::Whitespace::cast).last()?.syntax(); - ctx.add_action(AssistId("add_impl_missing_members"), "add impl missing members", |edit| { - let func_bodies = missing_fns.into_iter().map(build_func_body).join("\n"); + ctx.add_action(AssistId("add_impl_missing_members"), "add missing impl members", |edit| { + let indent = { + // FIXME: Find a way to get the indent already used in the file. + // Now, we copy the indent of first item or indent with 4 spaces relative to impl block + const DEFAULT_INDENT: &str = " "; + let first_item = impl_item_list.impl_items().next(); + let first_item_indent = first_item.and_then(|i| leading_indent(i.syntax())); + let impl_block_indent = || leading_indent(impl_node.syntax()).unwrap_or_default(); + + first_item_indent + .map(ToOwned::to_owned) + .unwrap_or_else(|| impl_block_indent().to_owned() + DEFAULT_INDENT) + }; + + let mut func_bodies = missing_fns.into_iter().map(build_func_body); + let func_bodies = func_bodies.join("\n"); let func_bodies = String::from("\n") + &func_bodies; - - let first_impl_item = impl_item_list.impl_items().next(); - // FIXME: We should respect the indent of the first item from the item list or the indent of leading block + some default indent (4?) - // Another approach is to not indent at all if there are no items here - let indent = first_impl_item.and_then(|i| leading_indent(i.syntax())).unwrap_or_default(); - let func_bodies = reindent(&func_bodies, indent) + "\n"; + let func_bodies = reindent(&func_bodies, &indent) + "\n"; let changed_range = last_whitespace_node.range(); let replaced_text_range = TextUnit::of_str(&func_bodies); @@ -123,6 +132,7 @@ mod tests { trait Foo { fn foo(&self); fn bar(&self); + fn baz(&self); } struct S; @@ -135,13 +145,15 @@ impl Foo for S { trait Foo { fn foo(&self); fn bar(&self); + fn baz(&self); } struct S; impl Foo for S { fn bar(&self) {} - fn foo(&self) { unimplemented!() }<|> + fn foo(&self) { unimplemented!() } + fn baz(&self) { unimplemented!() }<|> }", ); } -- cgit v1.2.3 From 713975b1c1584bb141dfca84818bf5a7ca91a8ee Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 13:21:56 +0100 Subject: Properly support the case when the cursor is inside an empty block or outside --- crates/ra_assists/src/add_missing_impl_members.rs | 59 +++++++++++++++++------ 1 file changed, 44 insertions(+), 15 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 0888268e4..f121dafb2 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -4,7 +4,7 @@ use crate::{Assist, AssistId, AssistCtx}; use hir::Resolver; use hir::db::HirDatabase; -use ra_syntax::{SmolStr, SyntaxKind, SyntaxNode, TextUnit, TreeArc}; +use ra_syntax::{SmolStr, SyntaxKind, TextRange, TextUnit, TreeArc}; use ra_syntax::ast::{self, AstNode, FnDef, ImplItem, ImplItemKind, NameOwner}; use ra_db::FilePosition; use ra_fmt::{leading_indent, reindent}; @@ -42,17 +42,13 @@ pub(crate) fn build_func_body(def: &ast::FnDef) -> String { } pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> Option { - use SyntaxKind::{IMPL_BLOCK, ITEM_LIST, WHITESPACE}; - let node = ctx.covering_node(); - let kinds = node.ancestors().take(3).map(SyntaxNode::kind); - // Only suggest this in `impl Foo for S { [Item...] <|> }` cursor position - if !Iterator::eq(kinds, [WHITESPACE, ITEM_LIST, IMPL_BLOCK].iter().cloned()) { - return None; - } - let impl_node = node.ancestors().find_map(ast::ImplBlock::cast)?; let impl_item_list = impl_node.item_list()?; + // Don't offer the assist when cursor is at the end, outside the block itself. + if node.range().end() == impl_node.syntax().range().end() { + return None; + } let trait_def = { let db = ctx.db; @@ -82,14 +78,10 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> .cloned() .filter(|t| def_name(t).map(|n| missing_fn_names.contains(&n)).unwrap_or(false)) .collect(); - if missing_fns.is_empty() { return None; } - let last_whitespace_node = - impl_item_list.syntax().children().filter_map(ast::Whitespace::cast).last()?.syntax(); - ctx.add_action(AssistId("add_impl_missing_members"), "add missing impl members", |edit| { let indent = { // FIXME: Find a way to get the indent already used in the file. @@ -109,7 +101,16 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> let func_bodies = String::from("\n") + &func_bodies; let func_bodies = reindent(&func_bodies, &indent) + "\n"; - let changed_range = last_whitespace_node.range(); + let changed_range = { + let last_whitespace = impl_item_list.syntax().children(); + let last_whitespace = last_whitespace.filter_map(ast::Whitespace::cast).last(); + let last_whitespace = last_whitespace.map(|w| w.syntax()); + + let cursor_range = TextRange::from_to(node.range().end(), node.range().end()); + + last_whitespace.map(|x| x.range()).unwrap_or(cursor_range) + }; + let replaced_text_range = TextUnit::of_str(&func_bodies); edit.replace(changed_range, func_bodies); @@ -122,7 +123,7 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> #[cfg(test)] mod tests { use super::*; - use crate::helpers::{ check_assist }; + use crate::helpers::{check_assist, check_assist_not_applicable}; #[test] fn test_add_missing_impl_members() { @@ -157,4 +158,32 @@ impl Foo for S { }", ); } + + #[test] + fn test_empty_impl_block() { + check_assist( + add_missing_impl_members, + " +trait Foo { fn foo(&self); } +struct S; +impl Foo for S {<|>}", + " +trait Foo { fn foo(&self); } +struct S; +impl Foo for S { + fn foo(&self) { unimplemented!() }<|> +}", + ); + } + + #[test] + fn test_cursor_after_empty_impl_block() { + check_assist_not_applicable( + add_missing_impl_members, + " +trait Foo { fn foo(&self); } +struct S; +impl Foo for S {}<|>", + ) + } } -- cgit v1.2.3 From 406343492c9f1741fcc3eafbb13efa42691fe0db Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 13:36:35 +0100 Subject: Simplify calculation of missing functions Asymptotically computing a set difference is faster but in the average case we won't have more than ~10 functions. Also prefer not using hash sets as these may yield nondeterministic results. --- crates/ra_assists/src/add_missing_impl_members.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index f121dafb2..97af6362c 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -1,5 +1,3 @@ -use std::collections::HashSet; - use crate::{Assist, AssistId, AssistCtx}; use hir::Resolver; @@ -69,14 +67,9 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> let trait_fns = trait_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); let impl_fns = impl_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); - let trait_fn_names = trait_fns.iter().cloned().filter_map(def_name).collect::>(); - let impl_fn_names = impl_fns.iter().cloned().filter_map(def_name).collect::>(); - - let missing_fn_names = trait_fn_names.difference(&impl_fn_names).collect::>(); let missing_fns: Vec<_> = trait_fns - .iter() - .cloned() - .filter(|t| def_name(t).map(|n| missing_fn_names.contains(&n)).unwrap_or(false)) + .into_iter() + .filter(|t| impl_fns.iter().all(|i| def_name(i) != def_name(t))) .collect(); if missing_fns.is_empty() { return None; -- cgit v1.2.3 From 0e47c371fdea2bc898f4fed210782047937af208 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 13:44:01 +0100 Subject: Ignore unnamed trait fns and add more tests --- crates/ra_assists/src/add_missing_impl_members.rs | 72 ++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 97af6362c..5000d0122 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -69,6 +69,7 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> let missing_fns: Vec<_> = trait_fns .into_iter() + .filter(|t| def_name(t).is_some()) .filter(|t| impl_fns.iter().all(|i| def_name(i) != def_name(t))) .collect(); if missing_fns.is_empty() { @@ -89,8 +90,7 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> .unwrap_or_else(|| impl_block_indent().to_owned() + DEFAULT_INDENT) }; - let mut func_bodies = missing_fns.into_iter().map(build_func_body); - let func_bodies = func_bodies.join("\n"); + let func_bodies = missing_fns.into_iter().map(build_func_body).join("\n"); let func_bodies = String::from("\n") + &func_bodies; let func_bodies = reindent(&func_bodies, &indent) + "\n"; @@ -152,6 +152,40 @@ impl Foo for S { ); } + #[test] + fn test_copied_overriden_members() { + check_assist( + add_missing_impl_members, + " +trait Foo { + fn foo(&self); + fn bar(&self) -> bool { true } + fn baz(&self) -> u32 { 42 } +} + +struct S; + +impl Foo for S { + fn bar(&self) {} + <|> +}", + " +trait Foo { + fn foo(&self); + fn bar(&self) -> bool { true } + fn baz(&self) -> u32 { 42 } +} + +struct S; + +impl Foo for S { + fn bar(&self) {} + fn foo(&self) { unimplemented!() } + fn baz(&self) -> u32 { 42 }<|> +}", + ); + } + #[test] fn test_empty_impl_block() { check_assist( @@ -179,4 +213,38 @@ struct S; impl Foo for S {}<|>", ) } + + #[test] + fn test_empty_trait() { + check_assist_not_applicable( + add_missing_impl_members, + " +trait Foo; +struct S; +impl Foo for S { <|> }", + ) + } + + #[test] + fn test_ignore_unnamed_trait_members() { + check_assist( + add_missing_impl_members, + " +trait Foo { + fn (arg: u32); + fn valid(some: u32) -> bool { false } +} +struct S; +impl Foo for S { <|> }", + " +trait Foo { + fn (arg: u32); + fn valid(some: u32) -> bool { false } +} +struct S; +impl Foo for S { + fn valid(some: u32) -> bool { false }<|> +}", + ) + } } -- cgit v1.2.3 From b3742873d9e675bda917a7d0c766a06f294cf604 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 16:10:03 +0100 Subject: Simplify trait resolution fragment --- crates/ra_assists/src/add_missing_impl_members.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 5000d0122..e682ca055 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -49,13 +49,10 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> } let trait_def = { - let db = ctx.db; - // TODO: Can we get the position of cursor itself rather than supplied range? - let range = ctx.frange; - let position = FilePosition { file_id: range.file_id, offset: range.range.start() }; - let resolver = hir::source_binder::resolver_for_position(db, position); + let position = FilePosition { file_id: ctx.frange.file_id, offset: node.range().end() }; + let resolver = hir::source_binder::resolver_for_position(ctx.db, position); - resolve_target_trait_def(db, &resolver, impl_node)? + resolve_target_trait_def(ctx.db, &resolver, impl_node)? }; let fn_def_opt = |kind| if let ImplItemKind::FnDef(def) = kind { Some(def) } else { None }; -- cgit v1.2.3 From 1df81f3d659926078b73dfb7cc246e13ef308429 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 16:10:21 +0100 Subject: Take into account parent indent when filling trait members --- crates/ra_assists/src/add_missing_impl_members.rs | 50 +++++++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index e682ca055..8e8520c59 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -74,22 +74,25 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> } ctx.add_action(AssistId("add_impl_missing_members"), "add missing impl members", |edit| { - let indent = { + let (parent_indent, indent) = { // FIXME: Find a way to get the indent already used in the file. // Now, we copy the indent of first item or indent with 4 spaces relative to impl block const DEFAULT_INDENT: &str = " "; let first_item = impl_item_list.impl_items().next(); - let first_item_indent = first_item.and_then(|i| leading_indent(i.syntax())); - let impl_block_indent = || leading_indent(impl_node.syntax()).unwrap_or_default(); - - first_item_indent - .map(ToOwned::to_owned) - .unwrap_or_else(|| impl_block_indent().to_owned() + DEFAULT_INDENT) + let first_item_indent = + first_item.and_then(|i| leading_indent(i.syntax())).map(ToOwned::to_owned); + let impl_block_indent = leading_indent(impl_node.syntax()).unwrap_or_default(); + + ( + impl_block_indent.to_owned(), + first_item_indent.unwrap_or_else(|| impl_block_indent.to_owned() + DEFAULT_INDENT), + ) }; let func_bodies = missing_fns.into_iter().map(build_func_body).join("\n"); let func_bodies = String::from("\n") + &func_bodies; - let func_bodies = reindent(&func_bodies, &indent) + "\n"; + let trailing_whitespace = format!("\n{}", parent_indent); + let func_bodies = reindent(&func_bodies, &indent) + &trailing_whitespace; let changed_range = { let last_whitespace = impl_item_list.syntax().children(); @@ -104,7 +107,9 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> let replaced_text_range = TextUnit::of_str(&func_bodies); edit.replace(changed_range, func_bodies); - edit.set_cursor(changed_range.start() + replaced_text_range - TextUnit::of_str("\n")); + edit.set_cursor( + changed_range.start() + replaced_text_range - TextUnit::of_str(&trailing_whitespace), + ); }); ctx.build() @@ -241,6 +246,33 @@ trait Foo { struct S; impl Foo for S { fn valid(some: u32) -> bool { false }<|> +}", + ) + } + + #[test] + fn test_indented_impl_block() { + check_assist( + add_missing_impl_members, + " +trait Foo { + fn valid(some: u32) -> bool { false } +} +struct S; + +mod my_mod { + impl crate::Foo for S { <|> } +}", + " +trait Foo { + fn valid(some: u32) -> bool { false } +} +struct S; + +mod my_mod { + impl crate::Foo for S { + fn valid(some: u32) -> bool { false }<|> + } }", ) } -- cgit v1.2.3 From 2f36f47dab59886f2f82252f427c4a3d3ee2c83b Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Thu, 7 Mar 2019 16:20:37 +0100 Subject: Do a cleanup/legibility pass --- crates/ra_assists/src/add_missing_impl_members.rs | 42 +++++++++++------------ 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 8e8520c59..567fc9586 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -44,31 +44,35 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> let impl_node = node.ancestors().find_map(ast::ImplBlock::cast)?; let impl_item_list = impl_node.item_list()?; // Don't offer the assist when cursor is at the end, outside the block itself. + let cursor_range = TextRange::from_to(node.range().end(), node.range().end()); if node.range().end() == impl_node.syntax().range().end() { return None; } let trait_def = { - let position = FilePosition { file_id: ctx.frange.file_id, offset: node.range().end() }; + let position = FilePosition { file_id: ctx.frange.file_id, offset: cursor_range.end() }; let resolver = hir::source_binder::resolver_for_position(ctx.db, position); resolve_target_trait_def(ctx.db, &resolver, impl_node)? }; - let fn_def_opt = |kind| if let ImplItemKind::FnDef(def) = kind { Some(def) } else { None }; - let def_name = |def| -> Option<&SmolStr> { FnDef::name(def).map(ast::Name::text) }; + let missing_fns: Vec<_> = { + let fn_def_opt = |kind| if let ImplItemKind::FnDef(def) = kind { Some(def) } else { None }; + let def_name = |def| -> Option<&SmolStr> { FnDef::name(def).map(ast::Name::text) }; - let trait_items = trait_def.syntax().descendants().find_map(ast::ItemList::cast)?.impl_items(); - let impl_items = impl_item_list.impl_items(); + let trait_items = + trait_def.syntax().descendants().find_map(ast::ItemList::cast)?.impl_items(); + let impl_items = impl_item_list.impl_items(); - let trait_fns = trait_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); - let impl_fns = impl_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); + let trait_fns = trait_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); + let impl_fns = impl_items.map(ImplItem::kind).filter_map(fn_def_opt).collect::>(); - let missing_fns: Vec<_> = trait_fns - .into_iter() - .filter(|t| def_name(t).is_some()) - .filter(|t| impl_fns.iter().all(|i| def_name(i) != def_name(t))) - .collect(); + trait_fns + .into_iter() + .filter(|t| def_name(t).is_some()) + .filter(|t| impl_fns.iter().all(|i| def_name(i) != def_name(t))) + .collect() + }; if missing_fns.is_empty() { return None; } @@ -89,21 +93,17 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> ) }; - let func_bodies = missing_fns.into_iter().map(build_func_body).join("\n"); - let func_bodies = String::from("\n") + &func_bodies; - let trailing_whitespace = format!("\n{}", parent_indent); - let func_bodies = reindent(&func_bodies, &indent) + &trailing_whitespace; - let changed_range = { let last_whitespace = impl_item_list.syntax().children(); let last_whitespace = last_whitespace.filter_map(ast::Whitespace::cast).last(); - let last_whitespace = last_whitespace.map(|w| w.syntax()); - let cursor_range = TextRange::from_to(node.range().end(), node.range().end()); - - last_whitespace.map(|x| x.range()).unwrap_or(cursor_range) + last_whitespace.map(|w| w.syntax().range()).unwrap_or(cursor_range) }; + let func_bodies = format!("\n{}", missing_fns.into_iter().map(build_func_body).join("\n")); + let trailing_whitespace = format!("\n{}", parent_indent); + let func_bodies = reindent(&func_bodies, &indent) + &trailing_whitespace; + let replaced_text_range = TextUnit::of_str(&func_bodies); edit.replace(changed_range, func_bodies); -- cgit v1.2.3 From 5b0b87f8deded258585d161c3077280e7d8112e9 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sat, 16 Mar 2019 23:19:14 +0100 Subject: Provide assist when cursor is immediately outside impl item block --- crates/ra_assists/src/add_missing_impl_members.rs | 32 +++++++++++++---------- 1 file changed, 18 insertions(+), 14 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 567fc9586..96dde3c4b 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -11,7 +11,7 @@ use itertools::Itertools; /// Given an `ast::ImplBlock`, resolves the target trait (the one being /// implemented) to a `ast::TraitDef`. -pub(crate) fn resolve_target_trait_def( +fn resolve_target_trait_def( db: &impl HirDatabase, resolver: &Resolver, impl_block: &ast::ImplBlock, @@ -25,7 +25,7 @@ pub(crate) fn resolve_target_trait_def( } } -pub(crate) fn build_func_body(def: &ast::FnDef) -> String { +fn build_func_body(def: &ast::FnDef) -> String { let mut buf = String::new(); for child in def.syntax().children() { @@ -40,17 +40,12 @@ pub(crate) fn build_func_body(def: &ast::FnDef) -> String { } pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> Option { - let node = ctx.covering_node(); - let impl_node = node.ancestors().find_map(ast::ImplBlock::cast)?; + let impl_node = ctx.node_at_offset::()?; let impl_item_list = impl_node.item_list()?; - // Don't offer the assist when cursor is at the end, outside the block itself. - let cursor_range = TextRange::from_to(node.range().end(), node.range().end()); - if node.range().end() == impl_node.syntax().range().end() { - return None; - } let trait_def = { - let position = FilePosition { file_id: ctx.frange.file_id, offset: cursor_range.end() }; + let file_id = ctx.frange.file_id; + let position = FilePosition { file_id, offset: impl_node.syntax().range().start() }; let resolver = hir::source_binder::resolver_for_position(ctx.db, position); resolve_target_trait_def(ctx.db, &resolver, impl_node)? @@ -94,10 +89,13 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> }; let changed_range = { - let last_whitespace = impl_item_list.syntax().children(); - let last_whitespace = last_whitespace.filter_map(ast::Whitespace::cast).last(); + let children = impl_item_list.syntax().children(); + let last_whitespace = children.filter_map(ast::Whitespace::cast).last(); - last_whitespace.map(|w| w.syntax().range()).unwrap_or(cursor_range) + last_whitespace.map(|w| w.syntax().range()).unwrap_or_else(|| { + let in_brackets = impl_item_list.syntax().range().end() - TextUnit::of_str("}"); + TextRange::from_to(in_brackets, in_brackets) + }) }; let func_bodies = format!("\n{}", missing_fns.into_iter().map(build_func_body).join("\n")); @@ -207,12 +205,18 @@ impl Foo for S { #[test] fn test_cursor_after_empty_impl_block() { - check_assist_not_applicable( + check_assist( add_missing_impl_members, " trait Foo { fn foo(&self); } struct S; impl Foo for S {}<|>", + " +trait Foo { fn foo(&self); } +struct S; +impl Foo for S { + fn foo(&self) { unimplemented!() }<|> +}", ) } -- cgit v1.2.3 From 30a226c72532f04033d4c283ba7c94790e2da541 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sat, 16 Mar 2019 23:24:17 +0100 Subject: Move the primary assist fn to the top of the file --- crates/ra_assists/src/add_missing_impl_members.rs | 60 +++++++++++------------ 1 file changed, 30 insertions(+), 30 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/add_missing_impl_members.rs b/crates/ra_assists/src/add_missing_impl_members.rs index 96dde3c4b..4435c4b5d 100644 --- a/crates/ra_assists/src/add_missing_impl_members.rs +++ b/crates/ra_assists/src/add_missing_impl_members.rs @@ -9,36 +9,6 @@ use ra_fmt::{leading_indent, reindent}; use itertools::Itertools; -/// Given an `ast::ImplBlock`, resolves the target trait (the one being -/// implemented) to a `ast::TraitDef`. -fn resolve_target_trait_def( - db: &impl HirDatabase, - resolver: &Resolver, - impl_block: &ast::ImplBlock, -) -> Option> { - let ast_path = impl_block.target_trait().map(AstNode::syntax).and_then(ast::PathType::cast)?; - let hir_path = ast_path.path().and_then(hir::Path::from_ast)?; - - match resolver.resolve_path(db, &hir_path).take_types() { - Some(hir::Resolution::Def(hir::ModuleDef::Trait(def))) => Some(def.source(db).1), - _ => None, - } -} - -fn build_func_body(def: &ast::FnDef) -> String { - let mut buf = String::new(); - - for child in def.syntax().children() { - if child.kind() == SyntaxKind::SEMI { - buf.push_str(" { unimplemented!() }") - } else { - child.text().push_to(&mut buf); - } - } - - buf.trim_end().to_string() -} - pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> Option { let impl_node = ctx.node_at_offset::()?; let impl_item_list = impl_node.item_list()?; @@ -113,6 +83,36 @@ pub(crate) fn add_missing_impl_members(mut ctx: AssistCtx) -> ctx.build() } +/// Given an `ast::ImplBlock`, resolves the target trait (the one being +/// implemented) to a `ast::TraitDef`. +fn resolve_target_trait_def( + db: &impl HirDatabase, + resolver: &Resolver, + impl_block: &ast::ImplBlock, +) -> Option> { + let ast_path = impl_block.target_trait().map(AstNode::syntax).and_then(ast::PathType::cast)?; + let hir_path = ast_path.path().and_then(hir::Path::from_ast)?; + + match resolver.resolve_path(db, &hir_path).take_types() { + Some(hir::Resolution::Def(hir::ModuleDef::Trait(def))) => Some(def.source(db).1), + _ => None, + } +} + +fn build_func_body(def: &ast::FnDef) -> String { + let mut buf = String::new(); + + for child in def.syntax().children() { + if child.kind() == SyntaxKind::SEMI { + buf.push_str(" { unimplemented!() }") + } else { + child.text().push_to(&mut buf); + } + } + + buf.trim_end().to_string() +} + #[cfg(test)] mod tests { use super::*; -- cgit v1.2.3