From e3151247982f47a7fdcf225c066c18acea229d30 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 4 May 2022 00:55:00 +0400 Subject: [PATCH] Remove "Sort methods by trait definition" assist It was replaced by the "Sort items by trait definition" assist. --- .../ide-assists/src/handlers/reorder_impl.rs | 217 ------------------ .../src/handlers/reorder_impl_items.rs | 39 ++-- crates/ide-assists/src/lib.rs | 2 - crates/ide-assists/src/tests/generated.rs | 35 --- 4 files changed, 25 insertions(+), 268 deletions(-) delete mode 100644 crates/ide-assists/src/handlers/reorder_impl.rs diff --git a/crates/ide-assists/src/handlers/reorder_impl.rs b/crates/ide-assists/src/handlers/reorder_impl.rs deleted file mode 100644 index c0991c6080d..00000000000 --- a/crates/ide-assists/src/handlers/reorder_impl.rs +++ /dev/null @@ -1,217 +0,0 @@ -use hir::{PathResolution, Semantics}; -use ide_db::{FxHashMap, RootDatabase}; -use itertools::Itertools; -use syntax::{ - ast::{self, HasName}, - ted, AstNode, -}; - -use crate::{utils::get_methods, AssistContext, AssistId, AssistKind, Assists}; - -// Assist: reorder_impl -// -// Reorder the methods of an `impl Trait`. The methods will be ordered -// in the same order as in the trait definition. -// -// ``` -// trait Foo { -// fn a() {} -// fn b() {} -// fn c() {} -// } -// -// struct Bar; -// $0impl Foo for Bar { -// fn b() {} -// fn c() {} -// fn a() {} -// } -// ``` -// -> -// ``` -// trait Foo { -// fn a() {} -// fn b() {} -// fn c() {} -// } -// -// struct Bar; -// impl Foo for Bar { -// fn a() {} -// fn b() {} -// fn c() {} -// } -// ``` -pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let impl_ast = ctx.find_node_at_offset::()?; - let items = impl_ast.assoc_item_list()?; - let methods = get_methods(&items); - - let path = impl_ast - .trait_() - .and_then(|t| match t { - ast::Type::PathType(path) => Some(path), - _ => None, - })? - .path()?; - - let ranks = compute_method_ranks(&path, ctx)?; - let sorted: Vec<_> = methods - .iter() - .cloned() - .sorted_by_key(|f| { - f.name().and_then(|n| ranks.get(&n.to_string()).copied()).unwrap_or(usize::max_value()) - }) - .collect(); - - // Don't edit already sorted methods: - if methods == sorted { - cov_mark::hit!(not_applicable_if_sorted); - return None; - } - - let target = items.syntax().text_range(); - acc.add( - AssistId("reorder_impl", AssistKind::RefactorRewrite), - "Sort methods by trait definition", - target, - |builder| { - let methods = methods.into_iter().map(|fn_| builder.make_mut(fn_)).collect::>(); - methods - .into_iter() - .zip(sorted) - .for_each(|(old, new)| ted::replace(old.syntax(), new.clone_for_update().syntax())); - }, - ) -} - -fn compute_method_ranks(path: &ast::Path, ctx: &AssistContext) -> Option> { - let td = trait_definition(path, &ctx.sema)?; - - Some( - td.items(ctx.db()) - .iter() - .flat_map(|i| match i { - hir::AssocItem::Function(f) => Some(f), - _ => None, - }) - .enumerate() - .map(|(idx, func)| (func.name(ctx.db()).to_string(), idx)) - .collect(), - ) -} - -fn trait_definition(path: &ast::Path, sema: &Semantics) -> Option { - match sema.resolve_path(path)? { - PathResolution::Def(hir::ModuleDef::Trait(trait_)) => Some(trait_), - _ => None, - } -} - -#[cfg(test)] -mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; - - use super::*; - - #[test] - fn not_applicable_if_sorted() { - cov_mark::check!(not_applicable_if_sorted); - check_assist_not_applicable( - reorder_impl, - r#" -trait Bar { - fn a() {} - fn z() {} - fn b() {} -} -struct Foo; -$0impl Bar for Foo { - fn a() {} - fn z() {} - fn b() {} -} - "#, - ) - } - - #[test] - fn not_applicable_if_empty() { - check_assist_not_applicable( - reorder_impl, - r#" -trait Bar {}; -struct Foo; -$0impl Bar for Foo {} - "#, - ) - } - - #[test] - fn reorder_impl_trait_functions() { - check_assist( - reorder_impl, - r#" -trait Bar { - fn a() {} - fn c() {} - fn b() {} - fn d() {} -} - -struct Foo; -$0impl Bar for Foo { - fn d() {} - fn b() {} - fn c() {} - fn a() {} -} - "#, - r#" -trait Bar { - fn a() {} - fn c() {} - fn b() {} - fn d() {} -} - -struct Foo; -impl Bar for Foo { - fn a() {} - fn c() {} - fn b() {} - fn d() {} -} - "#, - ) - } - - #[test] - fn reorder_impl_trait_methods_uneven_ident_lengths() { - check_assist( - reorder_impl, - r#" -trait Bar { - fn foo(&mut self) {} - fn fooo(&mut self) {} -} - -struct Foo; -impl Bar for Foo { - fn fooo(&mut self) {} - fn foo(&mut self) {$0} -}"#, - r#" -trait Bar { - fn foo(&mut self) {} - fn fooo(&mut self) {} -} - -struct Foo; -impl Bar for Foo { - fn foo(&mut self) {} - fn fooo(&mut self) {} -}"#, - ) - } -} diff --git a/crates/ide-assists/src/handlers/reorder_impl_items.rs b/crates/ide-assists/src/handlers/reorder_impl_items.rs index 87bee6c1215..2bda3a18170 100644 --- a/crates/ide-assists/src/handlers/reorder_impl_items.rs +++ b/crates/ide-assists/src/handlers/reorder_impl_items.rs @@ -47,13 +47,6 @@ pub(crate) fn reorder_impl_items(acc: &mut Assists, ctx: &AssistContext) -> Opti let items = impl_ast.assoc_item_list()?; let assoc_items = items.assoc_items().collect::>(); - // If all items are either function or macro calls, then reorder_impl assist can be used - if assoc_items.iter().all(|i| matches!(i, ast::AssocItem::Fn(_) | ast::AssocItem::MacroCall(_))) - { - cov_mark::hit!(not_applicable_if_all_functions); - return None; - } - let path = impl_ast .trait_() .and_then(|t| match t { @@ -152,23 +145,41 @@ fn b() {} } #[test] - fn not_applicable_if_all_functions() { - cov_mark::check!(not_applicable_if_all_functions); - check_assist_not_applicable( + fn reorder_impl_trait_functions() { + check_assist( reorder_impl_items, r#" trait Bar { fn a() {} - fn z() {} + fn c() {} fn b() {} + fn d() {} } + struct Foo; $0impl Bar for Foo { - fn a() {} - fn z() {} + fn d() {} fn b() {} + fn c() {} + fn a() {} } - "#, +"#, + r#" +trait Bar { + fn a() {} + fn c() {} + fn b() {} + fn d() {} +} + +struct Foo; +impl Bar for Foo { + fn a() {} + fn c() {} + fn b() {} + fn d() {} +} +"#, ) } diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 7b2f733567c..42bbc70b532 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -169,7 +169,6 @@ mod handlers { mod remove_mut; mod remove_unused_param; mod reorder_fields; - mod reorder_impl; mod reorder_impl_items; mod replace_try_expr_with_match; mod replace_derive_with_manual_impl; @@ -257,7 +256,6 @@ pub(crate) fn all() -> &'static [Handler] { remove_mut::remove_mut, remove_unused_param::remove_unused_param, reorder_fields::reorder_fields, - reorder_impl::reorder_impl, reorder_impl_items::reorder_impl_items, replace_try_expr_with_match::replace_try_expr_with_match, replace_derive_with_manual_impl::replace_derive_with_manual_impl, diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index b629364b880..eed50a8562d 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1736,41 +1736,6 @@ fn doctest_reorder_fields() { ) } -#[test] -fn doctest_reorder_impl() { - check_doc_test( - "reorder_impl", - r#####" -trait Foo { - fn a() {} - fn b() {} - fn c() {} -} - -struct Bar; -$0impl Foo for Bar { - fn b() {} - fn c() {} - fn a() {} -} -"#####, - r#####" -trait Foo { - fn a() {} - fn b() {} - fn c() {} -} - -struct Bar; -impl Foo for Bar { - fn a() {} - fn b() {} - fn c() {} -} -"#####, - ) -} - #[test] fn doctest_reorder_impl_items() { check_doc_test(