From 29bf6bed9b65691a54a72f83c6cf3be40ae558e8 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 9 Nov 2020 13:07:18 +0100 Subject: [PATCH] More consistent naming --- ....rs => replace_derive_with_manual_impl.rs} | 105 ++++++++++-------- crates/assists/src/lib.rs | 4 +- crates/assists/src/tests/generated.rs | 42 +++---- xtask/tests/tidy.rs | 8 +- 4 files changed, 89 insertions(+), 70 deletions(-) rename crates/assists/src/handlers/{add_custom_impl.rs => replace_derive_with_manual_impl.rs} (74%) diff --git a/crates/assists/src/handlers/add_custom_impl.rs b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs similarity index 74% rename from crates/assists/src/handlers/add_custom_impl.rs rename to crates/assists/src/handlers/replace_derive_with_manual_impl.rs index c13493fd87..82625516c2 100644 --- a/crates/assists/src/handlers/add_custom_impl.rs +++ b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs @@ -16,24 +16,31 @@ use crate::{ AssistId, AssistKind, }; -// Assist: add_custom_impl +// Assist: replace_derive_with_manual_impl // -// Adds impl block for derived trait. +// 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 { -// $0 +// fn fmt(&self, f: &mut Formatter) -> Result<()> { +// ${0:todo!()} +// } // } // ``` -pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { +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 @@ -90,43 +97,49 @@ fn add_assist( ) -> Option<()> { let target = attr.syntax().text_range(); let input = attr.token_tree()?; - let label = format!("Add custom impl `{}` for `{}`", trait_path, annotated_name); + 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("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( + 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\n{}", render_snippet(cap, impl_def.syntax(), cursor)), - ) - } - }; - }) + 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( @@ -192,7 +205,7 @@ mod tests { #[test] fn add_custom_impl_debug() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " mod fmt { pub struct Error; @@ -233,7 +246,7 @@ impl fmt::Debug for Foo { #[test] fn add_custom_impl_all() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " mod foo { pub trait Bar { @@ -282,7 +295,7 @@ impl foo::Bar for Foo { #[test] fn add_custom_impl_for_unique_input() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Debu<|>g)] struct Foo { @@ -304,7 +317,7 @@ impl Debug for Foo { #[test] fn add_custom_impl_for_with_visibility_modifier() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Debug<|>)] pub struct Foo { @@ -326,7 +339,7 @@ impl Debug for Foo { #[test] fn add_custom_impl_when_multiple_inputs() { check_assist( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Display, Debug<|>, Serialize)] struct Foo {} @@ -345,7 +358,7 @@ impl Debug for Foo { #[test] fn test_ignore_derive_macro_without_input() { check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(<|>)] struct Foo {} @@ -356,7 +369,7 @@ struct Foo {} #[test] fn test_ignore_if_cursor_on_param() { check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive<|>(Debug)] struct Foo {} @@ -364,7 +377,7 @@ struct Foo {} ); check_assist_not_applicable( - add_custom_impl, + replace_derive_with_manual_impl, " #[derive(Debug)<|>] struct Foo {} @@ -375,7 +388,7 @@ struct Foo {} #[test] fn test_ignore_if_not_derive() { check_assist_not_applicable( - add_custom_impl, + 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 af88b34374..92f7641454 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 168e1626ab..629788f055 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 4c7db8405e..99652e76b6 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\