From 29bf6bed9b65691a54a72f83c6cf3be40ae558e8 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 9 Nov 2020 13:07:18 +0100 Subject: More consistent naming --- crates/assists/src/handlers/add_custom_impl.rs | 385 -------------------- .../handlers/replace_derive_with_manual_impl.rs | 398 +++++++++++++++++++++ crates/assists/src/lib.rs | 4 +- crates/assists/src/tests/generated.rs | 42 ++- xtask/tests/tidy.rs | 8 +- 5 files changed, 428 insertions(+), 409 deletions(-) delete mode 100644 crates/assists/src/handlers/add_custom_impl.rs create mode 100644 crates/assists/src/handlers/replace_derive_with_manual_impl.rs diff --git a/crates/assists/src/handlers/add_custom_impl.rs b/crates/assists/src/handlers/add_custom_impl.rs deleted file mode 100644 index c13493fd8..000000000 --- a/crates/assists/src/handlers/add_custom_impl.rs +++ /dev/null @@ -1,385 +0,0 @@ -use ide_db::imports_locator; -use itertools::Itertools; -use syntax::{ - ast::{self, make, AstNode}, - Direction, SmolStr, - SyntaxKind::{IDENT, WHITESPACE}, - TextSize, -}; - -use crate::{ - assist_context::{AssistBuilder, AssistContext, Assists}, - utils::{ - add_trait_assoc_items_to_impl, filter_assoc_items, mod_path_to_ast, render_snippet, Cursor, - DefaultMethods, - }, - AssistId, AssistKind, -}; - -// Assist: add_custom_impl -// -// Adds impl block for derived trait. -// -// ``` -// #[derive(Deb<|>ug, Display)] -// struct S; -// ``` -// -> -// ``` -// #[derive(Display)] -// struct S; -// -// impl Debug for S { -// $0 -// } -// ``` -pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let attr = ctx.find_node_at_offset::()?; - - let attr_name = attr - .syntax() - .descendants_with_tokens() - .filter(|t| t.kind() == IDENT) - .find_map(syntax::NodeOrToken::into_token) - .filter(|t| t.text() == "derive")? - .text() - .clone(); - - let trait_token = - ctx.token_at_offset().find(|t| t.kind() == IDENT && *t.text() != attr_name)?; - let trait_path = make::path_unqualified(make::path_segment(make::name_ref(trait_token.text()))); - - let annotated_name = attr.syntax().siblings(Direction::Next).find_map(ast::Name::cast)?; - let insert_pos = annotated_name.syntax().parent()?.text_range().end(); - - let current_module = ctx.sema.scope(annotated_name.syntax()).module()?; - let current_crate = current_module.krate(); - - let found_traits = imports_locator::find_imports(&ctx.sema, current_crate, trait_token.text()) - .into_iter() - .filter_map(|candidate: either::Either| match candidate { - either::Either::Left(hir::ModuleDef::Trait(trait_)) => Some(trait_), - _ => None, - }) - .flat_map(|trait_| { - current_module - .find_use_path(ctx.sema.db, hir::ModuleDef::Trait(trait_)) - .as_ref() - .map(mod_path_to_ast) - .zip(Some(trait_)) - }); - - let mut no_traits_found = true; - for (trait_path, trait_) in found_traits.inspect(|_| no_traits_found = false) { - add_assist(acc, ctx, &attr, &trait_path, Some(trait_), &annotated_name, insert_pos)?; - } - if no_traits_found { - add_assist(acc, ctx, &attr, &trait_path, None, &annotated_name, insert_pos)?; - } - Some(()) -} - -fn add_assist( - acc: &mut Assists, - ctx: &AssistContext, - attr: &ast::Attr, - trait_path: &ast::Path, - trait_: Option, - annotated_name: &ast::Name, - insert_pos: TextSize, -) -> Option<()> { - let target = attr.syntax().text_range(); - let input = attr.token_tree()?; - let label = format!("Add custom impl `{}` for `{}`", trait_path, annotated_name); - let trait_name = trait_path.segment().and_then(|seg| seg.name_ref())?; - - acc.add(AssistId("add_custom_impl", AssistKind::Refactor), label, target, |builder| { - let impl_def_with_items = - impl_def_from_trait(&ctx.sema, annotated_name, trait_, trait_path); - update_attribute(builder, &input, &trait_name, &attr); - match (ctx.config.snippet_cap, impl_def_with_items) { - (None, _) => builder.insert( - insert_pos, - format!("\n\nimpl {} for {} {{\n\n}}", trait_path, annotated_name), - ), - (Some(cap), None) => builder.insert_snippet( - cap, - insert_pos, - format!("\n\nimpl {} for {} {{\n $0\n}}", trait_path, annotated_name), - ), - (Some(cap), Some((impl_def, first_assoc_item))) => { - let mut cursor = Cursor::Before(first_assoc_item.syntax()); - let placeholder; - if let ast::AssocItem::Fn(ref func) = first_assoc_item { - if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) { - if m.syntax().text() == "todo!()" { - placeholder = m; - cursor = Cursor::Replace(placeholder.syntax()); - } - } - } - - builder.insert_snippet( - cap, - insert_pos, - format!("\n\n{}", render_snippet(cap, impl_def.syntax(), cursor)), - ) - } - }; - }) -} - -fn impl_def_from_trait( - sema: &hir::Semantics, - annotated_name: &ast::Name, - trait_: Option, - trait_path: &ast::Path, -) -> Option<(ast::Impl, ast::AssocItem)> { - let trait_ = trait_?; - let target_scope = sema.scope(annotated_name.syntax()); - let trait_items = filter_assoc_items(sema.db, &trait_.items(sema.db), DefaultMethods::No); - if trait_items.is_empty() { - return None; - } - let impl_def = make::impl_trait( - trait_path.clone(), - make::path_unqualified(make::path_segment(make::name_ref(annotated_name.text()))), - ); - let (impl_def, first_assoc_item) = - add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope); - Some((impl_def, first_assoc_item)) -} - -fn update_attribute( - builder: &mut AssistBuilder, - input: &ast::TokenTree, - trait_name: &ast::NameRef, - attr: &ast::Attr, -) { - let new_attr_input = input - .syntax() - .descendants_with_tokens() - .filter(|t| t.kind() == IDENT) - .filter_map(|t| t.into_token().map(|t| t.text().clone())) - .filter(|t| t != trait_name.text()) - .collect::>(); - let has_more_derives = !new_attr_input.is_empty(); - - if has_more_derives { - let new_attr_input = format!("({})", new_attr_input.iter().format(", ")); - builder.replace(input.syntax().text_range(), new_attr_input); - } else { - let attr_range = attr.syntax().text_range(); - builder.delete(attr_range); - - if let Some(line_break_range) = attr - .syntax() - .next_sibling_or_token() - .filter(|t| t.kind() == WHITESPACE) - .map(|t| t.text_range()) - { - builder.delete(line_break_range); - } - } -} - -#[cfg(test)] -mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; - - use super::*; - - #[test] - fn add_custom_impl_debug() { - check_assist( - add_custom_impl, - " -mod fmt { - pub struct Error; - pub type Result = Result<(), Error>; - pub struct Formatter<'a>; - pub trait Debug { - fn fmt(&self, f: &mut Formatter<'_>) -> Result; - } -} - -#[derive(Debu<|>g)] -struct Foo { - bar: String, -} -", - " -mod fmt { - pub struct Error; - pub type Result = Result<(), Error>; - pub struct Formatter<'a>; - pub trait Debug { - fn fmt(&self, f: &mut Formatter<'_>) -> Result; - } -} - -struct Foo { - bar: String, -} - -impl fmt::Debug for Foo { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - ${0:todo!()} - } -} -", - ) - } - #[test] - fn add_custom_impl_all() { - check_assist( - add_custom_impl, - " -mod foo { - pub trait Bar { - type Qux; - const Baz: usize = 42; - const Fez: usize; - fn foo(); - fn bar() {} - } -} - -#[derive(<|>Bar)] -struct Foo { - bar: String, -} -", - " -mod foo { - pub trait Bar { - type Qux; - const Baz: usize = 42; - const Fez: usize; - fn foo(); - fn bar() {} - } -} - -struct Foo { - bar: String, -} - -impl foo::Bar for Foo { - $0type Qux; - - const Baz: usize = 42; - - const Fez: usize; - - fn foo() { - todo!() - } -} -", - ) - } - #[test] - fn add_custom_impl_for_unique_input() { - check_assist( - add_custom_impl, - " -#[derive(Debu<|>g)] -struct Foo { - bar: String, -} - ", - " -struct Foo { - bar: String, -} - -impl Debug for Foo { - $0 -} - ", - ) - } - - #[test] - fn add_custom_impl_for_with_visibility_modifier() { - check_assist( - add_custom_impl, - " -#[derive(Debug<|>)] -pub struct Foo { - bar: String, -} - ", - " -pub struct Foo { - bar: String, -} - -impl Debug for Foo { - $0 -} - ", - ) - } - - #[test] - fn add_custom_impl_when_multiple_inputs() { - check_assist( - add_custom_impl, - " -#[derive(Display, Debug<|>, Serialize)] -struct Foo {} - ", - " -#[derive(Display, Serialize)] -struct Foo {} - -impl Debug for Foo { - $0 -} - ", - ) - } - - #[test] - fn test_ignore_derive_macro_without_input() { - check_assist_not_applicable( - add_custom_impl, - " -#[derive(<|>)] -struct Foo {} - ", - ) - } - - #[test] - fn test_ignore_if_cursor_on_param() { - check_assist_not_applicable( - add_custom_impl, - " -#[derive<|>(Debug)] -struct Foo {} - ", - ); - - check_assist_not_applicable( - add_custom_impl, - " -#[derive(Debug)<|>] -struct Foo {} - ", - ) - } - - #[test] - fn test_ignore_if_not_derive() { - check_assist_not_applicable( - add_custom_impl, - " -#[allow(non_camel_<|>case_types)] -struct Foo {} - ", - ) - } -} diff --git a/crates/assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs new file mode 100644 index 000000000..82625516c --- /dev/null +++ b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs @@ -0,0 +1,398 @@ +use ide_db::imports_locator; +use itertools::Itertools; +use syntax::{ + ast::{self, make, AstNode}, + Direction, SmolStr, + SyntaxKind::{IDENT, WHITESPACE}, + TextSize, +}; + +use crate::{ + assist_context::{AssistBuilder, AssistContext, Assists}, + utils::{ + add_trait_assoc_items_to_impl, filter_assoc_items, mod_path_to_ast, render_snippet, Cursor, + DefaultMethods, + }, + AssistId, AssistKind, +}; + +// Assist: replace_derive_with_manual_impl +// +// Converts a `derive` impl into a manual one. +// +// ``` +// # trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } +// #[derive(Deb<|>ug, Display)] +// struct S; +// ``` +// -> +// ``` +// # trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } +// #[derive(Display)] +// struct S; +// +// impl Debug for S { +// fn fmt(&self, f: &mut Formatter) -> Result<()> { +// ${0:todo!()} +// } +// } +// ``` +pub(crate) fn replace_derive_with_manual_impl( + acc: &mut Assists, + ctx: &AssistContext, +) -> Option<()> { + let attr = ctx.find_node_at_offset::()?; + + let attr_name = attr + .syntax() + .descendants_with_tokens() + .filter(|t| t.kind() == IDENT) + .find_map(syntax::NodeOrToken::into_token) + .filter(|t| t.text() == "derive")? + .text() + .clone(); + + let trait_token = + ctx.token_at_offset().find(|t| t.kind() == IDENT && *t.text() != attr_name)?; + let trait_path = make::path_unqualified(make::path_segment(make::name_ref(trait_token.text()))); + + let annotated_name = attr.syntax().siblings(Direction::Next).find_map(ast::Name::cast)?; + let insert_pos = annotated_name.syntax().parent()?.text_range().end(); + + let current_module = ctx.sema.scope(annotated_name.syntax()).module()?; + let current_crate = current_module.krate(); + + let found_traits = imports_locator::find_imports(&ctx.sema, current_crate, trait_token.text()) + .into_iter() + .filter_map(|candidate: either::Either| match candidate { + either::Either::Left(hir::ModuleDef::Trait(trait_)) => Some(trait_), + _ => None, + }) + .flat_map(|trait_| { + current_module + .find_use_path(ctx.sema.db, hir::ModuleDef::Trait(trait_)) + .as_ref() + .map(mod_path_to_ast) + .zip(Some(trait_)) + }); + + let mut no_traits_found = true; + for (trait_path, trait_) in found_traits.inspect(|_| no_traits_found = false) { + add_assist(acc, ctx, &attr, &trait_path, Some(trait_), &annotated_name, insert_pos)?; + } + if no_traits_found { + add_assist(acc, ctx, &attr, &trait_path, None, &annotated_name, insert_pos)?; + } + Some(()) +} + +fn add_assist( + acc: &mut Assists, + ctx: &AssistContext, + attr: &ast::Attr, + trait_path: &ast::Path, + trait_: Option, + annotated_name: &ast::Name, + insert_pos: TextSize, +) -> Option<()> { + let target = attr.syntax().text_range(); + let input = attr.token_tree()?; + let label = format!("Convert to manual `impl {} for {}`", trait_path, annotated_name); + let trait_name = trait_path.segment().and_then(|seg| seg.name_ref())?; + + acc.add( + AssistId("replace_derive_with_manual_impl", AssistKind::Refactor), + label, + target, + |builder| { + let impl_def_with_items = + impl_def_from_trait(&ctx.sema, annotated_name, trait_, trait_path); + update_attribute(builder, &input, &trait_name, &attr); + match (ctx.config.snippet_cap, impl_def_with_items) { + (None, _) => builder.insert( + insert_pos, + format!("\n\nimpl {} for {} {{\n\n}}", trait_path, annotated_name), + ), + (Some(cap), None) => builder.insert_snippet( + cap, + insert_pos, + format!("\n\nimpl {} for {} {{\n $0\n}}", trait_path, annotated_name), + ), + (Some(cap), Some((impl_def, first_assoc_item))) => { + let mut cursor = Cursor::Before(first_assoc_item.syntax()); + let placeholder; + if let ast::AssocItem::Fn(ref func) = first_assoc_item { + if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) + { + if m.syntax().text() == "todo!()" { + placeholder = m; + cursor = Cursor::Replace(placeholder.syntax()); + } + } + } + + builder.insert_snippet( + cap, + insert_pos, + format!("\n\n{}", render_snippet(cap, impl_def.syntax(), cursor)), + ) + } + }; + }, + ) +} + +fn impl_def_from_trait( + sema: &hir::Semantics, + annotated_name: &ast::Name, + trait_: Option, + trait_path: &ast::Path, +) -> Option<(ast::Impl, ast::AssocItem)> { + let trait_ = trait_?; + let target_scope = sema.scope(annotated_name.syntax()); + let trait_items = filter_assoc_items(sema.db, &trait_.items(sema.db), DefaultMethods::No); + if trait_items.is_empty() { + return None; + } + let impl_def = make::impl_trait( + trait_path.clone(), + make::path_unqualified(make::path_segment(make::name_ref(annotated_name.text()))), + ); + let (impl_def, first_assoc_item) = + add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope); + Some((impl_def, first_assoc_item)) +} + +fn update_attribute( + builder: &mut AssistBuilder, + input: &ast::TokenTree, + trait_name: &ast::NameRef, + attr: &ast::Attr, +) { + let new_attr_input = input + .syntax() + .descendants_with_tokens() + .filter(|t| t.kind() == IDENT) + .filter_map(|t| t.into_token().map(|t| t.text().clone())) + .filter(|t| t != trait_name.text()) + .collect::>(); + let has_more_derives = !new_attr_input.is_empty(); + + if has_more_derives { + let new_attr_input = format!("({})", new_attr_input.iter().format(", ")); + builder.replace(input.syntax().text_range(), new_attr_input); + } else { + let attr_range = attr.syntax().text_range(); + builder.delete(attr_range); + + if let Some(line_break_range) = attr + .syntax() + .next_sibling_or_token() + .filter(|t| t.kind() == WHITESPACE) + .map(|t| t.text_range()) + { + builder.delete(line_break_range); + } + } +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn add_custom_impl_debug() { + check_assist( + replace_derive_with_manual_impl, + " +mod fmt { + pub struct Error; + pub type Result = Result<(), Error>; + pub struct Formatter<'a>; + pub trait Debug { + fn fmt(&self, f: &mut Formatter<'_>) -> Result; + } +} + +#[derive(Debu<|>g)] +struct Foo { + bar: String, +} +", + " +mod fmt { + pub struct Error; + pub type Result = Result<(), Error>; + pub struct Formatter<'a>; + pub trait Debug { + fn fmt(&self, f: &mut Formatter<'_>) -> Result; + } +} + +struct Foo { + bar: String, +} + +impl fmt::Debug for Foo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ${0:todo!()} + } +} +", + ) + } + #[test] + fn add_custom_impl_all() { + check_assist( + replace_derive_with_manual_impl, + " +mod foo { + pub trait Bar { + type Qux; + const Baz: usize = 42; + const Fez: usize; + fn foo(); + fn bar() {} + } +} + +#[derive(<|>Bar)] +struct Foo { + bar: String, +} +", + " +mod foo { + pub trait Bar { + type Qux; + const Baz: usize = 42; + const Fez: usize; + fn foo(); + fn bar() {} + } +} + +struct Foo { + bar: String, +} + +impl foo::Bar for Foo { + $0type Qux; + + const Baz: usize = 42; + + const Fez: usize; + + fn foo() { + todo!() + } +} +", + ) + } + #[test] + fn add_custom_impl_for_unique_input() { + check_assist( + replace_derive_with_manual_impl, + " +#[derive(Debu<|>g)] +struct Foo { + bar: String, +} + ", + " +struct Foo { + bar: String, +} + +impl Debug for Foo { + $0 +} + ", + ) + } + + #[test] + fn add_custom_impl_for_with_visibility_modifier() { + check_assist( + replace_derive_with_manual_impl, + " +#[derive(Debug<|>)] +pub struct Foo { + bar: String, +} + ", + " +pub struct Foo { + bar: String, +} + +impl Debug for Foo { + $0 +} + ", + ) + } + + #[test] + fn add_custom_impl_when_multiple_inputs() { + check_assist( + replace_derive_with_manual_impl, + " +#[derive(Display, Debug<|>, Serialize)] +struct Foo {} + ", + " +#[derive(Display, Serialize)] +struct Foo {} + +impl Debug for Foo { + $0 +} + ", + ) + } + + #[test] + fn test_ignore_derive_macro_without_input() { + check_assist_not_applicable( + replace_derive_with_manual_impl, + " +#[derive(<|>)] +struct Foo {} + ", + ) + } + + #[test] + fn test_ignore_if_cursor_on_param() { + check_assist_not_applicable( + replace_derive_with_manual_impl, + " +#[derive<|>(Debug)] +struct Foo {} + ", + ); + + check_assist_not_applicable( + replace_derive_with_manual_impl, + " +#[derive(Debug)<|>] +struct Foo {} + ", + ) + } + + #[test] + fn test_ignore_if_not_derive() { + check_assist_not_applicable( + replace_derive_with_manual_impl, + " +#[allow(non_camel_<|>case_types)] +struct Foo {} + ", + ) + } +} diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index af88b3437..92f764145 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -120,7 +120,6 @@ mod handlers { pub(crate) type Handler = fn(&mut Assists, &AssistContext) -> Option<()>; - mod add_custom_impl; mod add_explicit_type; mod add_missing_impl_members; mod add_turbo_fish; @@ -157,6 +156,7 @@ mod handlers { mod remove_mut; mod remove_unused_param; mod reorder_fields; + mod replace_derive_with_manual_impl; mod replace_if_let_with_match; mod replace_impl_trait_with_generic; mod replace_let_with_if_let; @@ -169,7 +169,6 @@ mod handlers { pub(crate) fn all() -> &'static [Handler] { &[ // These are alphabetic for the foolish consistency - add_custom_impl::add_custom_impl, add_explicit_type::add_explicit_type, add_turbo_fish::add_turbo_fish, apply_demorgan::apply_demorgan, @@ -208,6 +207,7 @@ mod handlers { remove_mut::remove_mut, remove_unused_param::remove_unused_param, reorder_fields::reorder_fields, + replace_derive_with_manual_impl::replace_derive_with_manual_impl, replace_if_let_with_match::replace_if_let_with_match, replace_impl_trait_with_generic::replace_impl_trait_with_generic, replace_let_with_if_let::replace_let_with_if_let, diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index 168e1626a..629788f05 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs @@ -2,25 +2,6 @@ use super::check_doc_test; -#[test] -fn doctest_add_custom_impl() { - check_doc_test( - "add_custom_impl", - r#####" -#[derive(Deb<|>ug, Display)] -struct S; -"#####, - r#####" -#[derive(Display)] -struct S; - -impl Debug for S { - $0 -} -"#####, - ) -} - #[test] fn doctest_add_explicit_type() { check_doc_test( @@ -831,6 +812,29 @@ const test: Foo = Foo {foo: 1, bar: 0} ) } +#[test] +fn doctest_replace_derive_with_manual_impl() { + check_doc_test( + "replace_derive_with_manual_impl", + r#####" +trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } +#[derive(Deb<|>ug, Display)] +struct S; +"#####, + r#####" +trait Debug { fn fmt(&self, f: &mut Formatter) -> Result<()>; } +#[derive(Display)] +struct S; + +impl Debug for S { + fn fmt(&self, f: &mut Formatter) -> Result<()> { + ${0:todo!()} + } +} +"#####, + ) +} + #[test] fn doctest_replace_if_let_with_match() { check_doc_test( diff --git a/xtask/tests/tidy.rs b/xtask/tests/tidy.rs index 4c7db8405..99652e76b 100644 --- a/xtask/tests/tidy.rs +++ b/xtask/tests/tidy.rs @@ -214,9 +214,6 @@ fn check_todo(path: &Path, text: &str) { // This file itself obviously needs to use todo (<- like this!). "tests/cli.rs", // Some of our assists generate `todo!()`. - "tests/generated.rs", - "handlers/add_custom_impl.rs", - "handlers/add_missing_impl_members.rs", "handlers/add_turbo_fish.rs", "handlers/generate_function.rs", // To support generating `todo!()` in assists, we have `expr_todo()` in @@ -229,6 +226,11 @@ fn check_todo(path: &Path, text: &str) { return; } if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") { + // Generated by an assist + if text.contains("${0:todo!()}") { + return; + } + panic!( "\nTODO markers or todo! macros should not be committed to the master branch,\n\ use FIXME instead\n\ -- cgit v1.2.3