From 58be1edfbb86658dcf9ce58800f53ef2a333b7da Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 22 Feb 2021 15:18:11 +0300 Subject: [PATCH] Make more common assist easier to ues --- Cargo.lock | 1 + crates/assists/Cargo.toml | 3 + crates/assists/src/lib.rs | 10 +++- crates/assists/src/tests.rs | 107 +++++++++++++++++++++------------- crates/base_db/src/fixture.rs | 11 +++- 5 files changed, 89 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a8ef7ef371..6139679fab7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,6 +58,7 @@ name = "assists" version = "0.0.0" dependencies = [ "either", + "expect-test", "hir", "ide_db", "itertools 0.10.0", diff --git a/crates/assists/Cargo.toml b/crates/assists/Cargo.toml index ed8ad666f75..c78cb99afe2 100644 --- a/crates/assists/Cargo.toml +++ b/crates/assists/Cargo.toml @@ -21,3 +21,6 @@ profile = { path = "../profile", version = "0.0.0" } ide_db = { path = "../ide_db", version = "0.0.0" } hir = { path = "../hir", version = "0.0.0" } test_utils = { path = "../test_utils", version = "0.0.0" } + +[dev-dependencies] +expect-test = "1.1" diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index 957efa6b9c5..7067cf8b698 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -179,9 +179,7 @@ pub(crate) fn all() -> &'static [Handler] { early_return::convert_to_guarded_return, expand_glob_import::expand_glob_import, move_module_to_file::move_module_to_file, - extract_function::extract_function, extract_struct_from_enum_variant::extract_struct_from_enum_variant, - extract_variable::extract_variable, fill_match_arms::fill_match_arms, fix_visibility::fix_visibility, flip_binexpr::flip_binexpr, @@ -229,12 +227,18 @@ pub(crate) fn all() -> &'static [Handler] { unmerge_use::unmerge_use, unwrap_block::unwrap_block, wrap_return_type_in_result::wrap_return_type_in_result, - // These are manually sorted for better priorities + // These are manually sorted for better priorities. By default, + // priority is determined by the size of the target range (smaller + // target wins). If the ranges are equal, position in this list is + // used as a tie-breaker. add_missing_impl_members::add_missing_impl_members, add_missing_impl_members::add_missing_default_members, // replace_string_with_char::replace_string_with_char, raw_string::make_raw_string, + // + extract_variable::extract_variable, + extract_function::extract_function, // Are you sure you want to add new assist here, and not to the // sorted list above? ] diff --git a/crates/assists/src/tests.rs b/crates/assists/src/tests.rs index 720f561a172..384eb7eeea2 100644 --- a/crates/assists/src/tests.rs +++ b/crates/assists/src/tests.rs @@ -1,5 +1,6 @@ mod generated; +use expect_test::expect; use hir::Semantics; use ide_db::{ base_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt}, @@ -10,11 +11,11 @@ source_change::FileSystemEdit, RootDatabase, }; +use stdx::{format_to, trim_indent}; use syntax::TextRange; -use test_utils::{assert_eq_text, extract_offset, extract_range}; +use test_utils::{assert_eq_text, extract_offset}; use crate::{handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, Assists}; -use stdx::{format_to, trim_indent}; pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { snippet_cap: SnippetCap::new(true), @@ -163,6 +164,22 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label: }; } +fn labels(assists: &[Assist]) -> String { + let mut labels = assists + .iter() + .map(|assist| { + let mut label = match &assist.group { + Some(g) => g.0.clone(), + None => assist.label.to_string(), + }; + label.push('\n'); + label + }) + .collect::>(); + labels.dedup(); + labels.into_iter().collect::() +} + #[test] fn assist_order_field_struct() { let before = "struct Foo { $0bar: u32 }"; @@ -181,66 +198,78 @@ fn assist_order_field_struct() { #[test] fn assist_order_if_expr() { - let before = " - pub fn test_some_range(a: int) -> bool { - if let 2..6 = $05$0 { - true - } else { - false - } - }"; - let (range, before) = extract_range(before); - let (db, file_id) = with_single_file(&before); - let frange = FileRange { file_id, range }; - let assists = Assist::get(&db, &TEST_CONFIG, false, frange); - let mut assists = assists.iter(); + let (db, frange) = RootDatabase::with_range( + r#" +pub fn test_some_range(a: int) -> bool { + if let 2..6 = $05$0 { + true + } else { + false + } +} +"#, + ); - assert_eq!(assists.next().expect("expected assist").label, "Extract into function"); - assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); - assert_eq!(assists.next().expect("expected assist").label, "Replace with match"); + let assists = Assist::get(&db, &TEST_CONFIG, false, frange); + let expected = labels(&assists); + + expect![[r#" + Convert integer base + Extract into variable + Extract into function + Replace with match + "#]] + .assert_eq(&expected); } #[test] fn assist_filter_works() { - let before = " - pub fn test_some_range(a: int) -> bool { - if let 2..6 = $05$0 { - true - } else { - false - } - }"; - let (range, before) = extract_range(before); - let (db, file_id) = with_single_file(&before); - let frange = FileRange { file_id, range }; - + let (db, frange) = RootDatabase::with_range( + r#" +pub fn test_some_range(a: int) -> bool { + if let 2..6 = $05$0 { + true + } else { + false + } +} +"#, + ); { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::Refactor]); let assists = Assist::get(&db, &cfg, false, frange); - let mut assists = assists.iter(); + let expected = labels(&assists); - assert_eq!(assists.next().expect("expected assist").label, "Extract into function"); - assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); - assert_eq!(assists.next().expect("expected assist").label, "Replace with match"); + expect![[r#" + Convert integer base + Extract into variable + Extract into function + Replace with match + "#]] + .assert_eq(&expected); } { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::RefactorExtract]); let assists = Assist::get(&db, &cfg, false, frange); - assert_eq!(assists.len(), 2); + let expected = labels(&assists); - let mut assists = assists.iter(); - assert_eq!(assists.next().expect("expected assist").label, "Extract into function"); - assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); + expect![[r#" + Extract into variable + Extract into function + "#]] + .assert_eq(&expected); } { let mut cfg = TEST_CONFIG; cfg.allowed = Some(vec![AssistKind::QuickFix]); let assists = Assist::get(&db, &cfg, false, frange); - assert!(assists.is_empty(), "All asserts but quickfixes should be filtered out"); + let expected = labels(&assists); + + expect![[r#""#]].assert_eq(&expected); } } diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs index 98acd61b126..5c9824814e8 100644 --- a/crates/base_db/src/fixture.rs +++ b/crates/base_db/src/fixture.rs @@ -67,7 +67,7 @@ use vfs::{file_set::FileSet, VfsPath}; use crate::{ - input::CrateName, Change, CrateGraph, CrateId, Edition, Env, FileId, FilePosition, + input::CrateName, Change, CrateGraph, CrateId, Edition, Env, FileId, FilePosition, FileRange, SourceDatabaseExt, SourceRoot, SourceRootId, }; @@ -99,6 +99,15 @@ fn with_position(ra_fixture: &str) -> (Self, FilePosition) { (db, FilePosition { file_id, offset }) } + fn with_range(ra_fixture: &str) -> (Self, FileRange) { + let (db, file_id, range_or_offset) = Self::with_range_or_offset(ra_fixture); + let range = match range_or_offset { + RangeOrOffset::Range(it) => it, + RangeOrOffset::Offset(_) => panic!(), + }; + (db, FileRange { file_id, range }) + } + fn with_range_or_offset(ra_fixture: &str) -> (Self, FileId, RangeOrOffset) { let fixture = ChangeFixture::parse(ra_fixture); let mut db = Self::default();