From 9f41f074be3a600a40b398dd3f4e57a0d69db256 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 14 Oct 2020 19:56:20 +0200 Subject: Add qualify path assist --- crates/assists/src/handlers/qualify_path.rs | 1020 +++++++++++++++++++++++++++ crates/assists/src/lib.rs | 2 + 2 files changed, 1022 insertions(+) create mode 100644 crates/assists/src/handlers/qualify_path.rs diff --git a/crates/assists/src/handlers/qualify_path.rs b/crates/assists/src/handlers/qualify_path.rs new file mode 100644 index 000000000..cff207889 --- /dev/null +++ b/crates/assists/src/handlers/qualify_path.rs @@ -0,0 +1,1020 @@ +use std::collections::BTreeSet; + +use syntax::{ast, AstNode, TextRange}; + +use crate::{ + assist_context::{AssistContext, Assists}, + utils::import_assets::{ImportAssets, ImportCandidate}, + utils::mod_path_to_ast, + AssistId, AssistKind, GroupLabel, +}; + +// Assist: qualify_path +// +// If the name is unresolved, provides all possible qualified paths for it. +// +// ``` +// fn main() { +// let map = HashMap<|>::new(); +// } +// # pub mod std { pub mod collections { pub struct HashMap { } } } +// ``` +// -> +// ``` +// fn main() { +// let map = std::collections::HashMap::new(); +// } +// # pub mod std { pub mod collections { pub struct HashMap { } } } +// ``` +pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let import_assets = + if let Some(path_under_caret) = ctx.find_node_at_offset_with_descend::() { + ImportAssets::for_regular_path(path_under_caret, &ctx.sema) + } else if let Some(method_under_caret) = + ctx.find_node_at_offset_with_descend::() + { + ImportAssets::for_method_call(method_under_caret, &ctx.sema) + } else { + None + }?; + let proposed_imports = import_assets.search_for_relative_paths(&ctx.sema); + if proposed_imports.is_empty() { + return None; + } + + let range = ctx.sema.original_range(import_assets.syntax_under_caret()).range; + match import_assets.import_candidate() { + ImportCandidate::QualifierStart(candidate) => { + let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; + let segment = path.segment()?; + qualify_path_qualifier_start(acc, proposed_imports, range, segment, &candidate.name) + } + ImportCandidate::UnqualifiedName(candidate) => { + qualify_path_unqualified_name(acc, proposed_imports, range, &candidate.name) + } + ImportCandidate::TraitAssocItem(candidate) => { + let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; + let (qualifier, segment) = (path.qualifier()?, path.segment()?); + qualify_path_trait_assoc_item( + acc, + proposed_imports, + range, + qualifier, + segment, + &candidate.name, + ) + } + ImportCandidate::TraitMethod(candidate) => { + let mcall_expr = ast::MethodCallExpr::cast(import_assets.syntax_under_caret().clone())?; + let receiver = mcall_expr.receiver()?; + let name_ref = mcall_expr.name_ref()?; + qualify_path_trait_method( + acc, + proposed_imports, + range, + receiver, + name_ref, + &candidate.name, + ) + } + }; + Some(()) +} + +// a test that covers this -> `associated_struct_const` +fn qualify_path_qualifier_start( + acc: &mut Assists, + proposed_imports: BTreeSet, + range: TextRange, + segment: ast::PathSegment, + qualifier_start: &str, +) { + let group_label = GroupLabel(format!("Qualify {}", qualifier_start)); + for import in proposed_imports { + acc.add_group( + &group_label, + AssistId("qualify_path", AssistKind::QuickFix), + format!("Qualify with `{}`", &import), + range, + |builder| { + let import = mod_path_to_ast(&import); + builder.replace(range, format!("{}::{}", import, segment)); + }, + ); + } +} + +// a test that covers this -> `applicable_when_found_an_import_partial` +fn qualify_path_unqualified_name( + acc: &mut Assists, + proposed_imports: BTreeSet, + range: TextRange, + name: &str, +) { + let group_label = GroupLabel(format!("Qualify {}", name)); + for import in proposed_imports { + acc.add_group( + &group_label, + AssistId("qualify_path", AssistKind::QuickFix), + format!("Qualify as `{}`", &import), + range, + |builder| builder.replace(range, mod_path_to_ast(&import).to_string()), + ); + } +} + +// a test that covers this -> `associated_trait_const` +fn qualify_path_trait_assoc_item( + acc: &mut Assists, + proposed_imports: BTreeSet, + range: TextRange, + qualifier: ast::Path, + segment: ast::PathSegment, + trait_assoc_item_name: &str, +) { + let group_label = GroupLabel(format!("Qualify {}", trait_assoc_item_name)); + for import in proposed_imports { + acc.add_group( + &group_label, + AssistId("qualify_path", AssistKind::QuickFix), + format!("Qualify with cast as `{}`", &import), + range, + |builder| { + let import = mod_path_to_ast(&import); + builder.replace(range, format!("<{} as {}>::{}", qualifier, import, segment)); + }, + ); + } +} + +// a test that covers this -> `trait_method` +fn qualify_path_trait_method( + acc: &mut Assists, + proposed_imports: BTreeSet, + range: TextRange, + receiver: ast::Expr, + name_ref: ast::NameRef, + trait_method_name: &str, +) { + let group_label = GroupLabel(format!("Qualify {}", trait_method_name)); + for import in proposed_imports { + acc.add_group( + &group_label, + AssistId("qualify_path", AssistKind::QuickFix), // < Does this still count as quickfix? + format!("Qualify `{}`", &import), + range, + |builder| { + let import = mod_path_to_ast(&import); + // TODO: check the receiver self type and emit refs accordingly, don't discard other function parameters + builder.replace(range, format!("{}::{}(&{})", import, name_ref, receiver)); + }, + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; + #[test] + fn applicable_when_found_an_import_partial() { + check_assist( + qualify_path, + r" + mod std { + pub mod fmt { + pub struct Formatter; + } + } + + use std::fmt; + + <|>Formatter + ", + r" + mod std { + pub mod fmt { + pub struct Formatter; + } + } + + use std::fmt; + + fmt::Formatter + ", + ); + } + + #[test] + fn applicable_when_found_an_import() { + check_assist( + qualify_path, + r" + <|>PubStruct + + pub mod PubMod { + pub struct PubStruct; + } + ", + r" + PubMod::PubStruct + + pub mod PubMod { + pub struct PubStruct; + } + ", + ); + } + + #[test] + fn applicable_in_macros() { + check_assist( + qualify_path, + r" + macro_rules! foo { + ($i:ident) => { fn foo(a: $i) {} } + } + foo!(Pub<|>Struct); + + pub mod PubMod { + pub struct PubStruct; + } + ", + r" + macro_rules! foo { + ($i:ident) => { fn foo(a: $i) {} } + } + foo!(PubMod::PubStruct); + + pub mod PubMod { + pub struct PubStruct; + } + ", + ); + } + + #[test] + fn applicable_when_found_multiple_imports() { + check_assist( + qualify_path, + r" + PubSt<|>ruct + + pub mod PubMod1 { + pub struct PubStruct; + } + pub mod PubMod2 { + pub struct PubStruct; + } + pub mod PubMod3 { + pub struct PubStruct; + } + ", + r" + PubMod3::PubStruct + + pub mod PubMod1 { + pub struct PubStruct; + } + pub mod PubMod2 { + pub struct PubStruct; + } + pub mod PubMod3 { + pub struct PubStruct; + } + ", + ); + } + + #[test] + fn not_applicable_for_already_imported_types() { + check_assist_not_applicable( + qualify_path, + r" + use PubMod::PubStruct; + + PubStruct<|> + + pub mod PubMod { + pub struct PubStruct; + } + ", + ); + } + + #[test] + fn not_applicable_for_types_with_private_paths() { + check_assist_not_applicable( + qualify_path, + r" + PrivateStruct<|> + + pub mod PubMod { + struct PrivateStruct; + } + ", + ); + } + + #[test] + fn not_applicable_when_no_imports_found() { + check_assist_not_applicable( + qualify_path, + " + PubStruct<|>", + ); + } + + #[test] + fn not_applicable_in_import_statements() { + check_assist_not_applicable( + qualify_path, + r" + use PubStruct<|>; + + pub mod PubMod { + pub struct PubStruct; + }", + ); + } + + #[test] + fn qualify_function() { + check_assist( + qualify_path, + r" + test_function<|> + + pub mod PubMod { + pub fn test_function() {}; + } + ", + r" + PubMod::test_function + + pub mod PubMod { + pub fn test_function() {}; + } + ", + ); + } + + #[test] + fn qualify_macro() { + check_assist( + qualify_path, + r" +//- /lib.rs crate:crate_with_macro +#[macro_export] +macro_rules! foo { + () => () +} + +//- /main.rs crate:main deps:crate_with_macro +fn main() { + foo<|> +} +", + r" +fn main() { + crate_with_macro::foo +} +", + ); + } + + #[test] + fn qualify_path_target() { + check_assist_target( + qualify_path, + r" + struct AssistInfo { + group_label: Option<<|>GroupLabel>, + } + + mod m { pub struct GroupLabel; } + ", + "GroupLabel", + ) + } + + #[test] + fn not_applicable_when_path_start_is_imported() { + check_assist_not_applicable( + qualify_path, + r" + pub mod mod1 { + pub mod mod2 { + pub mod mod3 { + pub struct TestStruct; + } + } + } + + use mod1::mod2; + fn main() { + mod2::mod3::TestStruct<|> + } + ", + ); + } + + #[test] + fn not_applicable_for_imported_function() { + check_assist_not_applicable( + qualify_path, + r" + pub mod test_mod { + pub fn test_function() {} + } + + use test_mod::test_function; + fn main() { + test_function<|> + } + ", + ); + } + + #[test] + fn associated_struct_function() { + check_assist( + qualify_path, + r" + mod test_mod { + pub struct TestStruct {} + impl TestStruct { + pub fn test_function() {} + } + } + + fn main() { + TestStruct::test_function<|> + } + ", + r" + mod test_mod { + pub struct TestStruct {} + impl TestStruct { + pub fn test_function() {} + } + } + + fn main() { + test_mod::TestStruct::test_function + } + ", + ); + } + + #[test] + fn associated_struct_const() { + check_assist( + qualify_path, + r" + mod test_mod { + pub struct TestStruct {} + impl TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + TestStruct::TEST_CONST<|> + } + ", + r" + mod test_mod { + pub struct TestStruct {} + impl TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + test_mod::TestStruct::TEST_CONST + } + ", + ); + } + + #[test] + fn associated_trait_function() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_function(); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_function() {} + } + } + + fn main() { + test_mod::TestStruct::test_function<|> + } + ", + r" + mod test_mod { + pub trait TestTrait { + fn test_function(); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_function() {} + } + } + + fn main() { + ::test_function + } + ", + ); + } + + #[test] + fn not_applicable_for_imported_trait_for_function() { + check_assist_not_applicable( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_function(); + } + pub trait TestTrait2 { + fn test_function(); + } + pub enum TestEnum { + One, + Two, + } + impl TestTrait2 for TestEnum { + fn test_function() {} + } + impl TestTrait for TestEnum { + fn test_function() {} + } + } + + use test_mod::TestTrait2; + fn main() { + test_mod::TestEnum::test_function<|>; + } + ", + ) + } + + #[test] + fn associated_trait_const() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + const TEST_CONST: u8; + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + test_mod::TestStruct::TEST_CONST<|> + } + ", + r" + mod test_mod { + pub trait TestTrait { + const TEST_CONST: u8; + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + const TEST_CONST: u8 = 42; + } + } + + fn main() { + ::TEST_CONST + } + ", + ); + } + + #[test] + fn not_applicable_for_imported_trait_for_const() { + check_assist_not_applicable( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + const TEST_CONST: u8; + } + pub trait TestTrait2 { + const TEST_CONST: f64; + } + pub enum TestEnum { + One, + Two, + } + impl TestTrait2 for TestEnum { + const TEST_CONST: f64 = 42.0; + } + impl TestTrait for TestEnum { + const TEST_CONST: u8 = 42; + } + } + + use test_mod::TestTrait2; + fn main() { + test_mod::TestEnum::TEST_CONST<|>; + } + ", + ) + } + + #[test] + fn trait_method() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_struct.test_meth<|>od() + } + ", + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_mod::TestTrait::test_method(&test_struct) + } + ", + ); + } + + #[test] + fn trait_method_multi_params() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self, test: i32); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self, test: i32) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_struct.test_meth<|>od(42) + } + ", + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self, test: i32); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self, test: i32) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_mod::TestTrait::test_method(&test_struct, 42) + } + ", + ); + } + + #[test] + fn trait_method_consume() { + check_assist( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_method(self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(self) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_struct.test_meth<|>od() + } + ", + r" + mod test_mod { + pub trait TestTrait { + fn test_method(self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(self) {} + } + } + + fn main() { + let test_struct = test_mod::TestStruct {}; + test_mod::TestTrait::test_method(test_struct) + } + ", + ); + } + + #[test] + fn trait_method_cross_crate() { + check_assist( + qualify_path, + r" + //- /main.rs crate:main deps:dep + fn main() { + let test_struct = dep::test_mod::TestStruct {}; + test_struct.test_meth<|>od() + } + //- /dep.rs crate:dep + pub mod test_mod { + pub trait TestTrait { + fn test_method(&self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self) {} + } + } + ", + r" + fn main() { + let test_struct = dep::test_mod::TestStruct {}; + dep::test_mod::TestTrait::test_method(&test_struct) + } + ", + ); + } + + #[test] + fn assoc_fn_cross_crate() { + check_assist( + qualify_path, + r" + //- /main.rs crate:main deps:dep + fn main() { + dep::test_mod::TestStruct::test_func<|>tion + } + //- /dep.rs crate:dep + pub mod test_mod { + pub trait TestTrait { + fn test_function(); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_function() {} + } + } + ", + r" + fn main() { + ::test_function + } + ", + ); + } + + #[test] + fn assoc_const_cross_crate() { + check_assist( + qualify_path, + r" + //- /main.rs crate:main deps:dep + fn main() { + dep::test_mod::TestStruct::CONST<|> + } + //- /dep.rs crate:dep + pub mod test_mod { + pub trait TestTrait { + const CONST: bool; + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + const CONST: bool = true; + } + } + ", + r" + fn main() { + ::CONST + } + ", + ); + } + + #[test] + fn assoc_fn_as_method_cross_crate() { + check_assist_not_applicable( + qualify_path, + r" + //- /main.rs crate:main deps:dep + fn main() { + let test_struct = dep::test_mod::TestStruct {}; + test_struct.test_func<|>tion() + } + //- /dep.rs crate:dep + pub mod test_mod { + pub trait TestTrait { + fn test_function(); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_function() {} + } + } + ", + ); + } + + #[test] + fn private_trait_cross_crate() { + check_assist_not_applicable( + qualify_path, + r" + //- /main.rs crate:main deps:dep + fn main() { + let test_struct = dep::test_mod::TestStruct {}; + test_struct.test_meth<|>od() + } + //- /dep.rs crate:dep + pub mod test_mod { + trait TestTrait { + fn test_method(&self); + } + pub struct TestStruct {} + impl TestTrait for TestStruct { + fn test_method(&self) {} + } + } + ", + ); + } + + #[test] + fn not_applicable_for_imported_trait_for_method() { + check_assist_not_applicable( + qualify_path, + r" + mod test_mod { + pub trait TestTrait { + fn test_method(&self); + } + pub trait TestTrait2 { + fn test_method(&self); + } + pub enum TestEnum { + One, + Two, + } + impl TestTrait2 for TestEnum { + fn test_method(&self) {} + } + impl TestTrait for TestEnum { + fn test_method(&self) {} + } + } + + use test_mod::TestTrait2; + fn main() { + let one = test_mod::TestEnum::One; + one.test<|>_method(); + } + ", + ) + } + + #[test] + fn dep_import() { + check_assist( + qualify_path, + r" +//- /lib.rs crate:dep +pub struct Struct; + +//- /main.rs crate:main deps:dep +fn main() { + Struct<|> +} +", + r" +fn main() { + dep::Struct +} +", + ); + } + + #[test] + fn whole_segment() { + // Tests that only imports whose last segment matches the identifier get suggested. + check_assist( + qualify_path, + r" +//- /lib.rs crate:dep +pub mod fmt { + pub trait Display {} +} + +pub fn panic_fmt() {} + +//- /main.rs crate:main deps:dep +struct S; + +impl f<|>mt::Display for S {} +", + r" +struct S; + +impl dep::fmt::Display for S {} +", + ); + } + + #[test] + fn macro_generated() { + // Tests that macro-generated items are suggested from external crates. + check_assist( + qualify_path, + r" +//- /lib.rs crate:dep +macro_rules! mac { + () => { + pub struct Cheese; + }; +} + +mac!(); + +//- /main.rs crate:main deps:dep +fn main() { + Cheese<|>; +} +", + r" +fn main() { + dep::Cheese; +} +", + ); + } + + #[test] + fn casing() { + // Tests that differently cased names don't interfere and we only suggest the matching one. + check_assist( + qualify_path, + r" +//- /lib.rs crate:dep +pub struct FMT; +pub struct fmt; + +//- /main.rs crate:main deps:dep +fn main() { + FMT<|>; +} +", + r" +fn main() { + dep::FMT; +} +", + ); + } +} diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index a2bec818c..f29b8212f 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -150,6 +150,7 @@ mod handlers { mod merge_match_arms; mod move_bounds; mod move_guard; + mod qualify_path; mod raw_string; mod remove_dbg; mod remove_mut; @@ -196,6 +197,7 @@ mod handlers { move_bounds::move_bounds_to_where_clause, move_guard::move_arm_cond_to_match_guard, move_guard::move_guard_to_arm_body, + qualify_path::qualify_path, raw_string::add_hash, raw_string::make_raw_string, raw_string::make_usual_string, -- cgit v1.2.3 From d983f18df7dd484ec43510111169180d7abe038d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 14 Oct 2020 20:04:52 +0200 Subject: Add mark tests to qualify_path assist --- crates/assists/src/handlers/qualify_path.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/assists/src/handlers/qualify_path.rs b/crates/assists/src/handlers/qualify_path.rs index cff207889..bbbf47bef 100644 --- a/crates/assists/src/handlers/qualify_path.rs +++ b/crates/assists/src/handlers/qualify_path.rs @@ -1,6 +1,7 @@ use std::collections::BTreeSet; use syntax::{ast, AstNode, TextRange}; +use test_utils::mark; use crate::{ assist_context::{AssistContext, Assists}, @@ -89,6 +90,7 @@ fn qualify_path_qualifier_start( segment: ast::PathSegment, qualifier_start: &str, ) { + mark::hit!(qualify_path_qualifier_start); let group_label = GroupLabel(format!("Qualify {}", qualifier_start)); for import in proposed_imports { acc.add_group( @@ -111,6 +113,7 @@ fn qualify_path_unqualified_name( range: TextRange, name: &str, ) { + mark::hit!(qualify_path_unqualified_name); let group_label = GroupLabel(format!("Qualify {}", name)); for import in proposed_imports { acc.add_group( @@ -132,6 +135,7 @@ fn qualify_path_trait_assoc_item( segment: ast::PathSegment, trait_assoc_item_name: &str, ) { + mark::hit!(qualify_path_trait_assoc_item); let group_label = GroupLabel(format!("Qualify {}", trait_assoc_item_name)); for import in proposed_imports { acc.add_group( @@ -156,6 +160,7 @@ fn qualify_path_trait_method( name_ref: ast::NameRef, trait_method_name: &str, ) { + mark::hit!(qualify_path_trait_method); let group_label = GroupLabel(format!("Qualify {}", trait_method_name)); for import in proposed_imports { acc.add_group( @@ -178,6 +183,7 @@ mod tests { use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; #[test] fn applicable_when_found_an_import_partial() { + mark::check!(qualify_path_unqualified_name); check_assist( qualify_path, r" @@ -469,6 +475,7 @@ fn main() { #[test] fn associated_struct_const() { + mark::check!(qualify_path_qualifier_start); check_assist( qualify_path, r" @@ -569,6 +576,7 @@ fn main() { #[test] fn associated_trait_const() { + mark::check!(qualify_path_trait_assoc_item); check_assist( qualify_path, r" @@ -638,6 +646,7 @@ fn main() { #[test] fn trait_method() { + mark::check!(qualify_path_trait_method); check_assist( qualify_path, r" -- cgit v1.2.3 From bc11475a2a0f06d5083a7032764a50e5f8ade130 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 14 Oct 2020 21:40:51 +0200 Subject: Properly qualify trait methods in qualify_path assist --- crates/assists/src/handlers/auto_import.rs | 2 +- crates/assists/src/handlers/qualify_path.rs | 124 +++++++++++++++++----------- crates/assists/src/tests/generated.rs | 19 +++++ crates/assists/src/utils/import_assets.rs | 40 ++++----- crates/syntax/src/ast/make.rs | 3 + 5 files changed, 118 insertions(+), 70 deletions(-) diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs index e595b5b93..13e390a1f 100644 --- a/crates/assists/src/handlers/auto_import.rs +++ b/crates/assists/src/handlers/auto_import.rs @@ -45,7 +45,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let group = import_group_message(import_assets.import_candidate()); let scope = ImportScope::find_insert_use_container(import_assets.syntax_under_caret(), ctx)?; let syntax = scope.as_syntax_node(); - for import in proposed_imports { + for (import, _) in proposed_imports { acc.add_group( &group, AssistId("auto_import", AssistKind::QuickFix), diff --git a/crates/assists/src/handlers/qualify_path.rs b/crates/assists/src/handlers/qualify_path.rs index bbbf47bef..479ff498c 100644 --- a/crates/assists/src/handlers/qualify_path.rs +++ b/crates/assists/src/handlers/qualify_path.rs @@ -1,6 +1,12 @@ -use std::collections::BTreeSet; - -use syntax::{ast, AstNode, TextRange}; +use std::iter; + +use hir::AsName; +use ide_db::RootDatabase; +use syntax::{ + ast, + ast::{make, ArgListOwner}, + AstNode, TextRange, +}; use test_utils::mark; use crate::{ @@ -10,6 +16,8 @@ use crate::{ AssistId, AssistKind, GroupLabel, }; +const ASSIST_ID: AssistId = AssistId("qualify_path", AssistKind::QuickFix); + // Assist: qualify_path // // If the name is unresolved, provides all possible qualified paths for it. @@ -53,30 +61,14 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> ImportCandidate::UnqualifiedName(candidate) => { qualify_path_unqualified_name(acc, proposed_imports, range, &candidate.name) } - ImportCandidate::TraitAssocItem(candidate) => { + ImportCandidate::TraitAssocItem(_) => { let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; let (qualifier, segment) = (path.qualifier()?, path.segment()?); - qualify_path_trait_assoc_item( - acc, - proposed_imports, - range, - qualifier, - segment, - &candidate.name, - ) + qualify_path_trait_assoc_item(acc, proposed_imports, range, qualifier, segment) } - ImportCandidate::TraitMethod(candidate) => { + ImportCandidate::TraitMethod(_) => { let mcall_expr = ast::MethodCallExpr::cast(import_assets.syntax_under_caret().clone())?; - let receiver = mcall_expr.receiver()?; - let name_ref = mcall_expr.name_ref()?; - qualify_path_trait_method( - acc, - proposed_imports, - range, - receiver, - name_ref, - &candidate.name, - ) + qualify_path_trait_method(acc, ctx.sema.db, proposed_imports, range, mcall_expr)?; } }; Some(()) @@ -85,17 +77,17 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> // a test that covers this -> `associated_struct_const` fn qualify_path_qualifier_start( acc: &mut Assists, - proposed_imports: BTreeSet, + proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, range: TextRange, segment: ast::PathSegment, - qualifier_start: &str, + qualifier_start: &ast::NameRef, ) { mark::hit!(qualify_path_qualifier_start); let group_label = GroupLabel(format!("Qualify {}", qualifier_start)); - for import in proposed_imports { + for (import, _) in proposed_imports { acc.add_group( &group_label, - AssistId("qualify_path", AssistKind::QuickFix), + ASSIST_ID, format!("Qualify with `{}`", &import), range, |builder| { @@ -109,16 +101,16 @@ fn qualify_path_qualifier_start( // a test that covers this -> `applicable_when_found_an_import_partial` fn qualify_path_unqualified_name( acc: &mut Assists, - proposed_imports: BTreeSet, + proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, range: TextRange, - name: &str, + name: &ast::NameRef, ) { mark::hit!(qualify_path_unqualified_name); let group_label = GroupLabel(format!("Qualify {}", name)); - for import in proposed_imports { + for (import, _) in proposed_imports { acc.add_group( &group_label, - AssistId("qualify_path", AssistKind::QuickFix), + ASSIST_ID, format!("Qualify as `{}`", &import), range, |builder| builder.replace(range, mod_path_to_ast(&import).to_string()), @@ -129,18 +121,17 @@ fn qualify_path_unqualified_name( // a test that covers this -> `associated_trait_const` fn qualify_path_trait_assoc_item( acc: &mut Assists, - proposed_imports: BTreeSet, + proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, range: TextRange, qualifier: ast::Path, segment: ast::PathSegment, - trait_assoc_item_name: &str, ) { mark::hit!(qualify_path_trait_assoc_item); - let group_label = GroupLabel(format!("Qualify {}", trait_assoc_item_name)); - for import in proposed_imports { + let group_label = GroupLabel(format!("Qualify {}", &segment)); + for (import, _) in proposed_imports { acc.add_group( &group_label, - AssistId("qualify_path", AssistKind::QuickFix), + ASSIST_ID, format!("Qualify with cast as `{}`", &import), range, |builder| { @@ -154,33 +145,74 @@ fn qualify_path_trait_assoc_item( // a test that covers this -> `trait_method` fn qualify_path_trait_method( acc: &mut Assists, - proposed_imports: BTreeSet, + db: &RootDatabase, + proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, range: TextRange, - receiver: ast::Expr, - name_ref: ast::NameRef, - trait_method_name: &str, -) { + mcall_expr: ast::MethodCallExpr, +) -> Option<()> { mark::hit!(qualify_path_trait_method); + + let receiver = mcall_expr.receiver()?; + let trait_method_name = mcall_expr.name_ref()?; + let arg_list = mcall_expr.arg_list().map(|arg_list| arg_list.args()); let group_label = GroupLabel(format!("Qualify {}", trait_method_name)); - for import in proposed_imports { + let find_method = |item: &hir::AssocItem| { + item.name(db).map(|name| name == trait_method_name.as_name()).unwrap_or(false) + }; + for (import, trait_) in proposed_imports.into_iter().filter_map(filter_trait) { acc.add_group( &group_label, - AssistId("qualify_path", AssistKind::QuickFix), // < Does this still count as quickfix? + ASSIST_ID, format!("Qualify `{}`", &import), range, |builder| { let import = mod_path_to_ast(&import); - // TODO: check the receiver self type and emit refs accordingly, don't discard other function parameters - builder.replace(range, format!("{}::{}(&{})", import, name_ref, receiver)); + if let Some(hir::AssocItem::Function(method)) = + trait_.items(db).into_iter().find(find_method) + { + if let Some(self_access) = method.self_param(db).map(|sp| sp.access(db)) { + let receiver = receiver.clone(); + let receiver = match self_access { + hir::Access::Shared => make::expr_ref(receiver, false), + hir::Access::Exclusive => make::expr_ref(receiver, true), + hir::Access::Owned => receiver, + }; + builder.replace( + range, + format!( + "{}::{}{}", + import, + trait_method_name, + match arg_list.clone() { + Some(args) => make::arg_list(iter::once(receiver).chain(args)), + None => make::arg_list(iter::once(receiver)), + } + ), + ); + } + } }, ); } + Some(()) +} + +fn filter_trait( + (import, trait_): (hir::ModPath, hir::ItemInNs), +) -> Option<(hir::ModPath, hir::Trait)> { + if let hir::ModuleDef::Trait(trait_) = hir::ModuleDef::from(trait_.as_module_def_id()?) { + Some((import, trait_)) + } else { + None + } } #[cfg(test)] mod tests { - use super::*; use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; + + use super::*; + #[test] fn applicable_when_found_an_import_partial() { mark::check!(qualify_path_unqualified_name); diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index 41f536574..7d5618263 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs @@ -712,6 +712,25 @@ fn handle(action: Action) { ) } +#[test] +fn doctest_qualify_path() { + check_doc_test( + "qualify_path", + r#####" +fn main() { + let map = HashMap<|>::new(); +} +pub mod std { pub mod collections { pub struct HashMap { } } } +"#####, + r#####" +fn main() { + let map = std::collections::HashMap::new(); +} +pub mod std { pub mod collections { pub struct HashMap { } } } +"#####, + ) +} + #[test] fn doctest_remove_dbg() { check_doc_test( diff --git a/crates/assists/src/utils/import_assets.rs b/crates/assists/src/utils/import_assets.rs index 601f51098..23db3a74b 100644 --- a/crates/assists/src/utils/import_assets.rs +++ b/crates/assists/src/utils/import_assets.rs @@ -1,6 +1,4 @@ //! Look up accessible paths for items. -use std::collections::BTreeSet; - use either::Either; use hir::{AsAssocItem, AssocItemContainer, ModuleDef, Semantics}; use ide_db::{imports_locator, RootDatabase}; @@ -29,12 +27,12 @@ pub(crate) enum ImportCandidate { #[derive(Debug)] pub(crate) struct TraitImportCandidate { pub ty: hir::Type, - pub name: String, + pub name: ast::NameRef, } #[derive(Debug)] pub(crate) struct PathImportCandidate { - pub name: String, + pub name: ast::NameRef, } #[derive(Debug)] @@ -86,9 +84,9 @@ impl ImportAssets { fn get_search_query(&self) -> &str { match &self.import_candidate { ImportCandidate::UnqualifiedName(candidate) - | ImportCandidate::QualifierStart(candidate) => &candidate.name, + | ImportCandidate::QualifierStart(candidate) => candidate.name.text(), ImportCandidate::TraitAssocItem(candidate) - | ImportCandidate::TraitMethod(candidate) => &candidate.name, + | ImportCandidate::TraitMethod(candidate) => candidate.name.text(), } } @@ -96,7 +94,7 @@ impl ImportAssets { &self, sema: &Semantics, config: &InsertUseConfig, - ) -> BTreeSet { + ) -> Vec<(hir::ModPath, hir::ItemInNs)> { let _p = profile::span("import_assists::search_for_imports"); self.search_for(sema, Some(config.prefix_kind)) } @@ -106,7 +104,7 @@ impl ImportAssets { pub(crate) fn search_for_relative_paths( &self, sema: &Semantics, - ) -> BTreeSet { + ) -> Vec<(hir::ModPath, hir::ItemInNs)> { let _p = profile::span("import_assists::search_for_relative_paths"); self.search_for(sema, None) } @@ -115,7 +113,7 @@ impl ImportAssets { &self, sema: &Semantics, prefixed: Option, - ) -> BTreeSet { + ) -> Vec<(hir::ModPath, hir::ItemInNs)> { let db = sema.db; let mut trait_candidates = FxHashSet::default(); let current_crate = self.module_with_name_to_import.krate(); @@ -181,7 +179,7 @@ impl ImportAssets { } }; - imports_locator::find_imports(sema, current_crate, &self.get_search_query()) + let mut res = imports_locator::find_imports(sema, current_crate, &self.get_search_query()) .into_iter() .filter_map(filter) .filter_map(|candidate| { @@ -191,10 +189,13 @@ impl ImportAssets { } else { self.module_with_name_to_import.find_use_path(db, item) } + .map(|path| (path, item)) }) - .filter(|use_path| !use_path.segments.is_empty()) + .filter(|(use_path, _)| !use_path.segments.is_empty()) .take(20) - .collect::>() + .collect::>(); + res.sort_by_key(|(path, _)| path.clone()); + res } fn assoc_to_trait(assoc: AssocItemContainer) -> Option { @@ -215,7 +216,7 @@ impl ImportCandidate { Some(_) => None, None => Some(Self::TraitMethod(TraitImportCandidate { ty: sema.type_of_expr(&method_call.receiver()?)?, - name: method_call.name_ref()?.syntax().to_string(), + name: method_call.name_ref()?, })), } } @@ -243,24 +244,17 @@ impl ImportCandidate { hir::PathResolution::Def(hir::ModuleDef::Adt(assoc_item_path)) => { ImportCandidate::TraitAssocItem(TraitImportCandidate { ty: assoc_item_path.ty(sema.db), - name: segment.syntax().to_string(), + name: segment.name_ref()?, }) } _ => return None, } } else { - ImportCandidate::QualifierStart(PathImportCandidate { - name: qualifier_start.syntax().to_string(), - }) + ImportCandidate::QualifierStart(PathImportCandidate { name: qualifier_start }) } } else { ImportCandidate::UnqualifiedName(PathImportCandidate { - name: segment - .syntax() - .descendants() - .find_map(ast::NameRef::cast)? - .syntax() - .to_string(), + name: segment.syntax().descendants().find_map(ast::NameRef::cast)?, }) }; Some(candidate) diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 74dbdfaf7..5b06cb767 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -172,6 +172,9 @@ pub fn expr_call(f: ast::Expr, arg_list: ast::ArgList) -> ast::Expr { pub fn expr_method_call(receiver: ast::Expr, method: &str, arg_list: ast::ArgList) -> ast::Expr { expr_from_text(&format!("{}.{}{}", receiver, method, arg_list)) } +pub fn expr_ref(expr: ast::Expr, exclusive: bool) -> ast::Expr { + expr_from_text(&if exclusive { format!("&mut {}", expr) } else { format!("&{}", expr) }) +} fn expr_from_text(text: &str) -> ast::Expr { ast_from_text(&format!("const C: () = {};", text)) } -- cgit v1.2.3 From 1d612a6ec48390c38d577cd35369e0891fe6cb6e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 16 Oct 2020 15:56:26 +0200 Subject: De-duplicate `add_group` callsite in qualify_path --- crates/assists/src/handlers/qualify_path.rs | 229 +++++++++++++--------------- 1 file changed, 108 insertions(+), 121 deletions(-) diff --git a/crates/assists/src/handlers/qualify_path.rs b/crates/assists/src/handlers/qualify_path.rs index 479ff498c..f436bdbbf 100644 --- a/crates/assists/src/handlers/qualify_path.rs +++ b/crates/assists/src/handlers/qualify_path.rs @@ -5,7 +5,7 @@ use ide_db::RootDatabase; use syntax::{ ast, ast::{make, ArgListOwner}, - AstNode, TextRange, + AstNode, }; use test_utils::mark; @@ -16,8 +16,6 @@ use crate::{ AssistId, AssistKind, GroupLabel, }; -const ASSIST_ID: AssistId = AssistId("qualify_path", AssistKind::QuickFix); - // Assist: qualify_path // // If the name is unresolved, provides all possible qualified paths for it. @@ -51,162 +49,151 @@ pub(crate) fn qualify_path(acc: &mut Assists, ctx: &AssistContext) -> Option<()> return None; } + let candidate = import_assets.import_candidate(); let range = ctx.sema.original_range(import_assets.syntax_under_caret()).range; - match import_assets.import_candidate() { - ImportCandidate::QualifierStart(candidate) => { + + let qualify_candidate = match candidate { + ImportCandidate::QualifierStart(_) => { + mark::hit!(qualify_path_qualifier_start); let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; let segment = path.segment()?; - qualify_path_qualifier_start(acc, proposed_imports, range, segment, &candidate.name) + QualifyCandidate::QualifierStart(segment) } - ImportCandidate::UnqualifiedName(candidate) => { - qualify_path_unqualified_name(acc, proposed_imports, range, &candidate.name) + ImportCandidate::UnqualifiedName(_) => { + mark::hit!(qualify_path_unqualified_name); + QualifyCandidate::UnqualifiedName } ImportCandidate::TraitAssocItem(_) => { + mark::hit!(qualify_path_trait_assoc_item); let path = ast::Path::cast(import_assets.syntax_under_caret().clone())?; let (qualifier, segment) = (path.qualifier()?, path.segment()?); - qualify_path_trait_assoc_item(acc, proposed_imports, range, qualifier, segment) + QualifyCandidate::TraitAssocItem(qualifier, segment) } ImportCandidate::TraitMethod(_) => { + mark::hit!(qualify_path_trait_method); let mcall_expr = ast::MethodCallExpr::cast(import_assets.syntax_under_caret().clone())?; - qualify_path_trait_method(acc, ctx.sema.db, proposed_imports, range, mcall_expr)?; + QualifyCandidate::TraitMethod(ctx.sema.db, mcall_expr) } }; - Some(()) -} -// a test that covers this -> `associated_struct_const` -fn qualify_path_qualifier_start( - acc: &mut Assists, - proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, - range: TextRange, - segment: ast::PathSegment, - qualifier_start: &ast::NameRef, -) { - mark::hit!(qualify_path_qualifier_start); - let group_label = GroupLabel(format!("Qualify {}", qualifier_start)); - for (import, _) in proposed_imports { + let group_label = group_label(candidate); + for (import, item) in proposed_imports { acc.add_group( &group_label, - ASSIST_ID, - format!("Qualify with `{}`", &import), + AssistId("qualify_path", AssistKind::QuickFix), + label(candidate, &import), range, |builder| { - let import = mod_path_to_ast(&import); - builder.replace(range, format!("{}::{}", import, segment)); + qualify_candidate.qualify( + |replace_with: String| builder.replace(range, replace_with), + import, + item, + ) }, ); } + Some(()) } -// a test that covers this -> `applicable_when_found_an_import_partial` -fn qualify_path_unqualified_name( - acc: &mut Assists, - proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, - range: TextRange, - name: &ast::NameRef, -) { - mark::hit!(qualify_path_unqualified_name); - let group_label = GroupLabel(format!("Qualify {}", name)); - for (import, _) in proposed_imports { - acc.add_group( - &group_label, - ASSIST_ID, - format!("Qualify as `{}`", &import), - range, - |builder| builder.replace(range, mod_path_to_ast(&import).to_string()), - ); - } +enum QualifyCandidate<'db> { + QualifierStart(ast::PathSegment), + UnqualifiedName, + TraitAssocItem(ast::Path, ast::PathSegment), + TraitMethod(&'db RootDatabase, ast::MethodCallExpr), } -// a test that covers this -> `associated_trait_const` -fn qualify_path_trait_assoc_item( - acc: &mut Assists, - proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, - range: TextRange, - qualifier: ast::Path, - segment: ast::PathSegment, -) { - mark::hit!(qualify_path_trait_assoc_item); - let group_label = GroupLabel(format!("Qualify {}", &segment)); - for (import, _) in proposed_imports { - acc.add_group( - &group_label, - ASSIST_ID, - format!("Qualify with cast as `{}`", &import), - range, - |builder| { +impl QualifyCandidate<'_> { + fn qualify(&self, mut replacer: impl FnMut(String), import: hir::ModPath, item: hir::ItemInNs) { + match self { + QualifyCandidate::QualifierStart(segment) => { let import = mod_path_to_ast(&import); - builder.replace(range, format!("<{} as {}>::{}", qualifier, import, segment)); - }, - ); + replacer(format!("{}::{}", import, segment)); + } + QualifyCandidate::UnqualifiedName => replacer(mod_path_to_ast(&import).to_string()), + QualifyCandidate::TraitAssocItem(qualifier, segment) => { + let import = mod_path_to_ast(&import); + replacer(format!("<{} as {}>::{}", qualifier, import, segment)); + } + &QualifyCandidate::TraitMethod(db, ref mcall_expr) => { + Self::qualify_trait_method(db, mcall_expr, replacer, import, item); + } + } + } + + fn qualify_trait_method( + db: &RootDatabase, + mcall_expr: &ast::MethodCallExpr, + mut replacer: impl FnMut(String), + import: hir::ModPath, + item: hir::ItemInNs, + ) -> Option<()> { + let receiver = mcall_expr.receiver()?; + let trait_method_name = mcall_expr.name_ref()?; + let arg_list = mcall_expr.arg_list().map(|arg_list| arg_list.args()); + let trait_ = item_as_trait(item)?; + let method = find_trait_method(db, trait_, &trait_method_name)?; + if let Some(self_access) = method.self_param(db).map(|sp| sp.access(db)) { + let import = mod_path_to_ast(&import); + let receiver = match self_access { + hir::Access::Shared => make::expr_ref(receiver, false), + hir::Access::Exclusive => make::expr_ref(receiver, true), + hir::Access::Owned => receiver, + }; + replacer(format!( + "{}::{}{}", + import, + trait_method_name, + match arg_list.clone() { + Some(args) => make::arg_list(iter::once(receiver).chain(args)), + None => make::arg_list(iter::once(receiver)), + } + )); + } + Some(()) } } -// a test that covers this -> `trait_method` -fn qualify_path_trait_method( - acc: &mut Assists, +fn find_trait_method( db: &RootDatabase, - proposed_imports: Vec<(hir::ModPath, hir::ItemInNs)>, - range: TextRange, - mcall_expr: ast::MethodCallExpr, -) -> Option<()> { - mark::hit!(qualify_path_trait_method); - - let receiver = mcall_expr.receiver()?; - let trait_method_name = mcall_expr.name_ref()?; - let arg_list = mcall_expr.arg_list().map(|arg_list| arg_list.args()); - let group_label = GroupLabel(format!("Qualify {}", trait_method_name)); - let find_method = |item: &hir::AssocItem| { - item.name(db).map(|name| name == trait_method_name.as_name()).unwrap_or(false) - }; - for (import, trait_) in proposed_imports.into_iter().filter_map(filter_trait) { - acc.add_group( - &group_label, - ASSIST_ID, - format!("Qualify `{}`", &import), - range, - |builder| { - let import = mod_path_to_ast(&import); - if let Some(hir::AssocItem::Function(method)) = - trait_.items(db).into_iter().find(find_method) - { - if let Some(self_access) = method.self_param(db).map(|sp| sp.access(db)) { - let receiver = receiver.clone(); - let receiver = match self_access { - hir::Access::Shared => make::expr_ref(receiver, false), - hir::Access::Exclusive => make::expr_ref(receiver, true), - hir::Access::Owned => receiver, - }; - builder.replace( - range, - format!( - "{}::{}{}", - import, - trait_method_name, - match arg_list.clone() { - Some(args) => make::arg_list(iter::once(receiver).chain(args)), - None => make::arg_list(iter::once(receiver)), - } - ), - ); - } - } - }, - ); + trait_: hir::Trait, + trait_method_name: &ast::NameRef, +) -> Option { + if let Some(hir::AssocItem::Function(method)) = + trait_.items(db).into_iter().find(|item: &hir::AssocItem| { + item.name(db).map(|name| name == trait_method_name.as_name()).unwrap_or(false) + }) + { + Some(method) + } else { + None } - Some(()) } -fn filter_trait( - (import, trait_): (hir::ModPath, hir::ItemInNs), -) -> Option<(hir::ModPath, hir::Trait)> { - if let hir::ModuleDef::Trait(trait_) = hir::ModuleDef::from(trait_.as_module_def_id()?) { - Some((import, trait_)) +fn item_as_trait(item: hir::ItemInNs) -> Option { + if let hir::ModuleDef::Trait(trait_) = hir::ModuleDef::from(item.as_module_def_id()?) { + Some(trait_) } else { None } } +fn group_label(candidate: &ImportCandidate) -> GroupLabel { + let name = match candidate { + ImportCandidate::UnqualifiedName(it) | ImportCandidate::QualifierStart(it) => &it.name, + ImportCandidate::TraitAssocItem(it) | ImportCandidate::TraitMethod(it) => &it.name, + }; + GroupLabel(format!("Qualify {}", name)) +} + +fn label(candidate: &ImportCandidate, import: &hir::ModPath) -> String { + match candidate { + ImportCandidate::UnqualifiedName(_) => format!("Qualify as `{}`", &import), + ImportCandidate::QualifierStart(_) => format!("Qualify with `{}`", &import), + ImportCandidate::TraitAssocItem(_) => format!("Qualify `{}`", &import), + ImportCandidate::TraitMethod(_) => format!("Qualify with cast as `{}`", &import), + } +} + #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; -- cgit v1.2.3