diff --git a/crates/ide_assists/src/handlers/introduce_named_lifetime.rs b/crates/ide_assists/src/handlers/introduce_named_lifetime.rs index 16f8f9d70ba6..aa32c698a2b0 100644 --- a/crates/ide_assists/src/handlers/introduce_named_lifetime.rs +++ b/crates/ide_assists/src/handlers/introduce_named_lifetime.rs @@ -33,9 +33,9 @@ static ASSIST_LABEL: &str = "Introduce named lifetime"; // } // } // ``` -// FIXME: How can we handle renaming any one of multiple anonymous lifetimes? -// FIXME: should also add support for the case fun(f: &Foo) -> &$0Foo pub(crate) fn introduce_named_lifetime(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + // FIXME: How can we handle renaming any one of multiple anonymous lifetimes? + // FIXME: should also add support for the case fun(f: &Foo) -> &$0Foo let lifetime = ctx.find_node_at_offset::().filter(|lifetime| lifetime.text() == "'_")?; let lifetime_loc = lifetime.lifetime_ident_token()?.text_range(); diff --git a/crates/ide_assists/src/handlers/reorder_fields.rs b/crates/ide_assists/src/handlers/reorder_fields.rs index f6a926042c13..cd4eb7c15e90 100644 --- a/crates/ide_assists/src/handlers/reorder_fields.rs +++ b/crates/ide_assists/src/handlers/reorder_fields.rs @@ -20,7 +20,6 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // struct Foo {foo: i32, bar: i32}; // const test: Foo = Foo {foo: 1, bar: 0} // ``` -// pub(crate) fn reorder_fields(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let record = ctx .find_node_at_offset::() diff --git a/crates/ide_assists/src/handlers/reorder_impl.rs b/crates/ide_assists/src/handlers/reorder_impl.rs index 5a6a9f158ea9..d398373c3413 100644 --- a/crates/ide_assists/src/handlers/reorder_impl.rs +++ b/crates/ide_assists/src/handlers/reorder_impl.rs @@ -8,7 +8,7 @@ use syntax::{ ted, AstNode, }; -use crate::{AssistContext, AssistId, AssistKind, Assists}; +use crate::{utils::get_methods, AssistContext, AssistId, AssistKind, Assists}; // Assist: reorder_impl // @@ -44,7 +44,6 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // 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()?; @@ -76,7 +75,7 @@ pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let target = items.syntax().text_range(); acc.add( AssistId("reorder_impl", AssistKind::RefactorRewrite), - "Sort methods", + "Sort methods by trait definition", target, |builder| { let methods = methods.into_iter().map(|fn_| builder.make_mut(fn_)).collect::>(); @@ -111,17 +110,6 @@ fn trait_definition(path: &ast::Path, sema: &Semantics) -> Option< } } -fn get_methods(items: &ast::AssocItemList) -> Vec { - items - .assoc_items() - .flat_map(|i| match i { - ast::AssocItem::Fn(f) => Some(f), - _ => None, - }) - .filter(|f| f.name().is_some()) - .collect() -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; diff --git a/crates/ide_assists/src/handlers/sort_items.rs b/crates/ide_assists/src/handlers/sort_items.rs new file mode 100644 index 000000000000..2ca4a19ec409 --- /dev/null +++ b/crates/ide_assists/src/handlers/sort_items.rs @@ -0,0 +1,565 @@ +use std::cmp::Ordering; + +use itertools::Itertools; + +use syntax::{ + ast::{self, NameOwner}, + ted, AstNode, TextRange, +}; + +use crate::{utils::get_methods, AssistContext, AssistId, AssistKind, Assists}; + +// Assist: sort_items +// +// Sorts item members alphabetically: fields, enum variants and methods. +// +// ``` +// struct $0Foo$0 { second: u32, first: String } +// ``` +// -> +// ``` +// struct Foo { first: String, second: u32 } +// ``` +// --- +// ``` +// trait $0Bar$0 { +// fn second(&self) -> u32; +// fn first(&self) -> String; +// } +// ``` +// -> +// ``` +// trait Bar { +// fn first(&self) -> String; +// fn second(&self) -> u32; +// } +// ``` +// --- +// ``` +// struct Baz; +// impl $0Baz$0 { +// fn second(&self) -> u32; +// fn first(&self) -> String; +// } +// ``` +// -> +// ``` +// struct Baz; +// impl Baz { +// fn first(&self) -> String; +// fn second(&self) -> u32; +// } +// ``` +// --- +// There is a difference between sorting enum variants: +// +// ``` +// enum $0Animal$0 { +// Dog(String, f64), +// Cat { weight: f64, name: String }, +// } +// ``` +// -> +// ``` +// enum Animal { +// Cat { weight: f64, name: String }, +// Dog(String, f64), +// } +// ``` +// and sorting a single enum struct variant: +// +// ``` +// enum Animal { +// Dog(String, f64), +// Cat $0{ weight: f64, name: String }$0, +// } +// ``` +// -> +// ``` +// enum Animal { +// Dog(String, f64), +// Cat { name: String, weight: f64 }, +// } +// ``` +pub(crate) fn sort_items(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + if ctx.frange.range.is_empty() { + cov_mark::hit!(not_applicable_if_no_selection); + return None; + } + + if let Some(trait_ast) = ctx.find_node_at_offset::() { + add_sort_methods_assist(acc, trait_ast.assoc_item_list()?) + } else if let Some(impl_ast) = ctx.find_node_at_offset::() { + add_sort_methods_assist(acc, impl_ast.assoc_item_list()?) + } else if let Some(struct_ast) = ctx.find_node_at_offset::() { + match struct_ast.field_list() { + Some(ast::FieldList::RecordFieldList(it)) => add_sort_fields_assist(acc, it), + _ => { + cov_mark::hit!(not_applicable_if_sorted_or_empty_or_single); + None + } + } + } else if let Some(union_ast) = ctx.find_node_at_offset::() { + add_sort_fields_assist(acc, union_ast.record_field_list()?) + } else if let Some(enum_struct_variant_ast) = ctx.find_node_at_offset::() + { + // should be above enum and below struct + add_sort_fields_assist(acc, enum_struct_variant_ast) + } else if let Some(enum_ast) = ctx.find_node_at_offset::() { + add_sort_variants_assist(acc, enum_ast.variant_list()?) + } else { + None + } +} + +trait AddRewrite { + fn add_rewrite( + &mut self, + label: &str, + old: Vec, + new: Vec, + target: TextRange, + ) -> Option<()>; +} + +impl AddRewrite for Assists { + fn add_rewrite( + &mut self, + label: &str, + old: Vec, + new: Vec, + target: TextRange, + ) -> Option<()> { + self.add(AssistId("sort_items", AssistKind::RefactorRewrite), label, target, |builder| { + let mutable: Vec<_> = old.into_iter().map(|it| builder.make_mut(it)).collect(); + mutable + .into_iter() + .zip(new) + .for_each(|(old, new)| ted::replace(old.syntax(), new.clone_for_update().syntax())); + }) + } +} + +fn add_sort_methods_assist(acc: &mut Assists, item_list: ast::AssocItemList) -> Option<()> { + let methods = get_methods(&item_list); + let sorted = sort_by_name(&methods); + + if methods == sorted { + cov_mark::hit!(not_applicable_if_sorted_or_empty_or_single); + return None; + } + + acc.add_rewrite("Sort methods alphabetically", methods, sorted, item_list.syntax().text_range()) +} + +fn add_sort_fields_assist( + acc: &mut Assists, + record_field_list: ast::RecordFieldList, +) -> Option<()> { + let fields: Vec<_> = record_field_list.fields().collect(); + let sorted = sort_by_name(&fields); + + if fields == sorted { + cov_mark::hit!(not_applicable_if_sorted_or_empty_or_single); + return None; + } + + acc.add_rewrite( + "Sort fields alphabetically", + fields, + sorted, + record_field_list.syntax().text_range(), + ) +} + +fn add_sort_variants_assist(acc: &mut Assists, variant_list: ast::VariantList) -> Option<()> { + let variants: Vec<_> = variant_list.variants().collect(); + let sorted = sort_by_name(&variants); + + if variants == sorted { + cov_mark::hit!(not_applicable_if_sorted_or_empty_or_single); + return None; + } + + acc.add_rewrite( + "Sort variants alphabetically", + variants, + sorted, + variant_list.syntax().text_range(), + ) +} + +fn sort_by_name(initial: &[T]) -> Vec { + initial + .iter() + .cloned() + .sorted_by(|a, b| match (a.name(), b.name()) { + (Some(a), Some(b)) => Ord::cmp(&a.to_string(), &b.to_string()), + + // unexpected, but just in case + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + }) + .collect() +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn not_applicable_if_no_selection() { + cov_mark::check!(not_applicable_if_no_selection); + + check_assist_not_applicable( + sort_items, + r#" +t$0rait Bar { + fn b(); + fn a(); +} + "#, + ) + } + + #[test] + fn not_applicable_if_trait_empty() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +t$0rait Bar$0 { +} + "#, + ) + } + + #[test] + fn not_applicable_if_impl_empty() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +struct Bar; +$0impl Bar$0 { +} + "#, + ) + } + + #[test] + fn not_applicable_if_struct_empty() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +$0struct Bar$0 ; + "#, + ) + } + + #[test] + fn not_applicable_if_struct_empty2() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +$0struct Bar$0 { }; + "#, + ) + } + + #[test] + fn not_applicable_if_enum_empty() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +$0enum ZeroVariants$0 {}; + "#, + ) + } + + #[test] + fn not_applicable_if_trait_sorted() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +t$0rait Bar$0 { + fn a() {} + fn b() {} + fn c() {} +} + "#, + ) + } + + #[test] + fn not_applicable_if_impl_sorted() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +struct Bar; +$0impl Bar$0 { + fn a() {} + fn b() {} + fn c() {} +} + "#, + ) + } + + #[test] + fn not_applicable_if_struct_sorted() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +$0struct Bar$0 { + a: u32, + b: u8, + c: u64, +} + "#, + ) + } + + #[test] + fn not_applicable_if_union_sorted() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +$0union Bar$0 { + a: u32, + b: u8, + c: u64, +} + "#, + ) + } + + #[test] + fn not_applicable_if_enum_sorted() { + cov_mark::check!(not_applicable_if_sorted_or_empty_or_single); + + check_assist_not_applicable( + sort_items, + r#" +$0enum Bar$0 { + a, + b, + c, +} + "#, + ) + } + + #[test] + fn sort_trait() { + check_assist( + sort_items, + r#" +$0trait Bar$0 { + fn a() { + + } + + // comment for c + fn c() {} + fn z() {} + fn b() {} +} + "#, + r#" +trait Bar { + fn a() { + + } + + fn b() {} + // comment for c + fn c() {} + fn z() {} +} + "#, + ) + } + + #[test] + fn sort_impl() { + check_assist( + sort_items, + r#" +struct Bar; +$0impl Bar$0 { + fn c() {} + fn a() {} + /// long + /// doc + /// comment + fn z() {} + fn d() {} +} + "#, + r#" +struct Bar; +impl Bar { + fn a() {} + fn c() {} + fn d() {} + /// long + /// doc + /// comment + fn z() {} +} + "#, + ) + } + + #[test] + fn sort_struct() { + check_assist( + sort_items, + r#" +$0struct Bar$0 { + b: u8, + a: u32, + c: u64, +} + "#, + r#" +struct Bar { + a: u32, + b: u8, + c: u64, +} + "#, + ) + } + + #[test] + fn sort_generic_struct_with_lifetime() { + check_assist( + sort_items, + r#" +$0struct Bar<'a,$0 T> { + d: &'a str, + b: u8, + a: T, + c: u64, +} + "#, + r#" +struct Bar<'a, T> { + a: T, + b: u8, + c: u64, + d: &'a str, +} + "#, + ) + } + + #[test] + fn sort_struct_fields_diff_len() { + check_assist( + sort_items, + r#" +$0struct Bar $0{ + aaa: u8, + a: usize, + b: u8, +} + "#, + r#" +struct Bar { + a: usize, + aaa: u8, + b: u8, +} + "#, + ) + } + + #[test] + fn sort_union() { + check_assist( + sort_items, + r#" +$0union Bar$0 { + b: u8, + a: u32, + c: u64, +} + "#, + r#" +union Bar { + a: u32, + b: u8, + c: u64, +} + "#, + ) + } + + #[test] + fn sort_enum() { + check_assist( + sort_items, + r#" +$0enum Bar $0{ + d{ first: u32, second: usize}, + b = 14, + a, + c(u32, usize), +} + "#, + r#" +enum Bar { + a, + b = 14, + c(u32, usize), + d{ first: u32, second: usize}, +} + "#, + ) + } + + #[test] + fn sort_struct_enum_variant() { + check_assist( + sort_items, + r#" +enum Bar { + d$0{ second: usize, first: u32 }$0, + b = 14, + a, + c(u32, usize), +} + "#, + r#" +enum Bar { + d{ first: u32, second: usize }, + b = 14, + a, + c(u32, usize), +} + "#, + ) + } +} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 71a200860951..bb9d5dd9915d 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -111,6 +111,7 @@ mod handlers { mod replace_qualified_name_with_use; mod replace_string_with_char; mod split_import; + mod sort_items; mod toggle_ignore; mod unmerge_use; mod unwrap_block; @@ -183,6 +184,7 @@ mod handlers { replace_impl_trait_with_generic::replace_impl_trait_with_generic, replace_let_with_if_let::replace_let_with_if_let, replace_qualified_name_with_use::replace_qualified_name_with_use, + sort_items::sort_items, split_import::split_import, toggle_ignore::toggle_ignore, unmerge_use::unmerge_use, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index bda28510d0e9..794bd0f3d7b1 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -1545,6 +1545,97 @@ fn main() { ) } +#[test] +fn doctest_sort_items() { + check_doc_test( + "sort_items", + r#####" +struct $0Foo$0 { second: u32, first: String } +"#####, + r#####" +struct Foo { first: String, second: u32 } +"#####, + ) +} + +#[test] +fn doctest_sort_items_1() { + check_doc_test( + "sort_items", + r#####" +trait $0Bar$0 { + fn second(&self) -> u32; + fn first(&self) -> String; +} +"#####, + r#####" +trait Bar { + fn first(&self) -> String; + fn second(&self) -> u32; +} +"#####, + ) +} + +#[test] +fn doctest_sort_items_2() { + check_doc_test( + "sort_items", + r#####" +struct Baz; +impl $0Baz$0 { + fn second(&self) -> u32; + fn first(&self) -> String; +} +"#####, + r#####" +struct Baz; +impl Baz { + fn first(&self) -> String; + fn second(&self) -> u32; +} +"#####, + ) +} + +#[test] +fn doctest_sort_items_3() { + check_doc_test( + "sort_items", + r#####" +enum $0Animal$0 { + Dog(String, f64), + Cat { weight: f64, name: String }, +} +"#####, + r#####" +enum Animal { + Cat { weight: f64, name: String }, + Dog(String, f64), +} +"#####, + ) +} + +#[test] +fn doctest_sort_items_4() { + check_doc_test( + "sort_items", + r#####" +enum Animal { + Dog(String, f64), + Cat $0{ weight: f64, name: String }$0, +} +"#####, + r#####" +enum Animal { + Dog(String, f64), + Cat { name: String, weight: f64 }, +} +"#####, + ) +} + #[test] fn doctest_split_import() { check_doc_test( diff --git a/crates/ide_assists/src/tests/sourcegen.rs b/crates/ide_assists/src/tests/sourcegen.rs index bf29c5e536f2..2af1de66fadb 100644 --- a/crates/ide_assists/src/tests/sourcegen.rs +++ b/crates/ide_assists/src/tests/sourcegen.rs @@ -16,8 +16,11 @@ use super::check_doc_test; " .to_string(); for assist in assists.iter() { - let test = format!( - r######" + for (idx, section) in assist.sections.iter().enumerate() { + let test_id = + if idx == 0 { assist.id.clone() } else { format!("{}_{}", &assist.id, idx) }; + let test = format!( + r######" #[test] fn doctest_{}() {{ check_doc_test( @@ -27,13 +30,14 @@ r#####" {}"#####) }} "######, - assist.id, - assist.id, - reveal_hash_comments(&assist.before), - reveal_hash_comments(&assist.after) - ); + &test_id, + &assist.id, + reveal_hash_comments(§ion.before), + reveal_hash_comments(§ion.after) + ); - buf.push_str(&test) + buf.push_str(&test) + } } let buf = sourcegen::add_preamble("sourcegen_assists_docs", sourcegen::reformat(buf)); sourcegen::ensure_file_contents( @@ -55,14 +59,18 @@ r#####" fs::write(dst, contents).unwrap(); } } +#[derive(Debug)] +struct Section { + doc: String, + before: String, + after: String, +} #[derive(Debug)] struct Assist { id: String, location: sourcegen::Location, - doc: String, - before: String, - after: String, + sections: Vec
, } impl Assist { @@ -89,22 +97,28 @@ impl Assist { "invalid assist id: {:?}", id ); - let mut lines = block.contents.iter(); - - let doc = take_until(lines.by_ref(), "```").trim().to_string(); - assert!( - doc.chars().next().unwrap().is_ascii_uppercase() && doc.ends_with('.'), - "\n\n{}: assist docs should be proper sentences, with capitalization and a full stop at the end.\n\n{}\n\n", - id, doc, - ); - - let before = take_until(lines.by_ref(), "```"); - - assert_eq!(lines.next().unwrap().as_str(), "->"); - assert_eq!(lines.next().unwrap().as_str(), "```"); - let after = take_until(lines.by_ref(), "```"); + let mut lines = block.contents.iter().peekable(); let location = sourcegen::Location { file: path.to_path_buf(), line: block.line }; - acc.push(Assist { id, location, doc, before, after }) + let mut assist = Assist { id, location, sections: Vec::new() }; + + while lines.peek().is_some() { + let doc = take_until(lines.by_ref(), "```").trim().to_string(); + assert!( + (doc.chars().next().unwrap().is_ascii_uppercase() && doc.ends_with('.')) || assist.sections.len() > 0, + "\n\n{}: assist docs should be proper sentences, with capitalization and a full stop at the end.\n\n{}\n\n", + &assist.id, doc, + ); + + let before = take_until(lines.by_ref(), "```"); + + assert_eq!(lines.next().unwrap().as_str(), "->"); + assert_eq!(lines.next().unwrap().as_str(), "```"); + let after = take_until(lines.by_ref(), "```"); + + assist.sections.push(Section { doc, before, after }); + } + + acc.push(assist) } } @@ -123,13 +137,19 @@ impl Assist { impl fmt::Display for Assist { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let before = self.before.replace("$0", "┃"); // Unicode pseudo-graphics bar - let after = self.after.replace("$0", "┃"); - writeln!( + let _ = writeln!( f, "[discrete]\n=== `{}` -**Source:** {} +**Source:** {}", + self.id, self.location, + ); + for section in &self.sections { + let before = section.before.replace("$0", "┃"); // Unicode pseudo-graphics bar + let after = section.after.replace("$0", "┃"); + let _ = writeln!( + f, + " {} .Before @@ -139,12 +159,13 @@ impl fmt::Display for Assist { .After ```rust {}```", - self.id, - self.location, - self.doc, - hide_hash_comments(&before), - hide_hash_comments(&after) - ) + section.doc, + hide_hash_comments(&before), + hide_hash_comments(&after) + ); + } + + Ok(()) } } diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 35a997efc936..bcd7501724ff 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -516,3 +516,14 @@ fn ty_ctor(ty: &String, ctor: &str) -> Option { let res = ty.to_string().strip_prefix(ctor)?.strip_prefix('<')?.strip_suffix('>')?.to_string(); Some(res) } + +pub(crate) fn get_methods(items: &ast::AssocItemList) -> Vec { + items + .assoc_items() + .flat_map(|i| match i { + ast::AssocItem::Fn(f) => Some(f), + _ => None, + }) + .filter(|f| f.name().is_some()) + .collect() +}