From 128eef779f572a8120cb475d86a373ac4c9e5559 Mon Sep 17 00:00:00 2001 From: unexge Date: Thu, 13 Aug 2020 00:51:15 +0300 Subject: Improve AST replacing in expand glob import --- crates/assists/src/handlers/expand_glob_import.rs | 267 +++++++++++++++++++--- 1 file changed, 229 insertions(+), 38 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index 81d0af2f3..0288d1c77 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -4,7 +4,7 @@ use ide_db::{ defs::{classify_name_ref, Definition, NameRefClass}, RootDatabase, }; -use syntax::{algo, ast, match_ast, AstNode, SyntaxNode, SyntaxToken, T}; +use syntax::{ast, AstNode, SyntaxToken, T}; use crate::{ assist_context::{AssistBuilder, AssistContext, Assists}, @@ -38,7 +38,7 @@ use crate::{ // ``` pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let star = ctx.find_token_at_offset(T![*])?; - let mod_path = find_mod_path(&star)?; + let (parent, mod_path) = find_parent_and_path(&star)?; let module = match ctx.sema.resolve_path(&mod_path)? { PathResolution::Def(ModuleDef::Module(it)) => it, _ => return None, @@ -52,19 +52,23 @@ pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Opti source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect(); let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file); - let parent = star.parent().parent()?; + let target = parent.syntax(); acc.add( AssistId("expand_glob_import", AssistKind::RefactorRewrite), "Expand glob import", - parent.text_range(), + target.text_range(), |builder| { - replace_ast(builder, &parent, mod_path, used_names); + replace_ast(builder, parent, mod_path, used_names); }, ) } -fn find_mod_path(star: &SyntaxToken) -> Option { - star.ancestors().find_map(|n| ast::UseTree::cast(n).and_then(|u| u.path())) +fn find_parent_and_path(star: &SyntaxToken) -> Option<(ast::UseTree, ast::Path)> { + star.ancestors().find_map(|n| { + let use_tree = ast::UseTree::cast(n)?; + let path = use_tree.path()?; + Some((use_tree, path)) + }) } #[derive(PartialEq)] @@ -137,41 +141,28 @@ fn find_used_names( fn replace_ast( builder: &mut AssistBuilder, - node: &SyntaxNode, + parent: ast::UseTree, path: ast::Path, used_names: Vec, ) { - let replacement: Either = match used_names.as_slice() { - [name] => Either::Left(ast::make::use_tree( + let replacement = match used_names.as_slice() { + [name] => ast::make::use_tree( ast::make::path_from_text(&format!("{}::{}", path, name)), None, None, false, - )), - names => Either::Right(ast::make::use_tree_list(names.iter().map(|n| { - ast::make::use_tree(ast::make::path_from_text(&n.to_string()), None, None, false) - }))), - }; - - let mut replace_node = |replacement: Either| { - algo::diff(node, &replacement.either(|u| u.syntax().clone(), |ut| ut.syntax().clone())) - .into_text_edit(builder.text_edit_builder()); + ), + names => ast::make::use_tree( + path, + Some(ast::make::use_tree_list(names.iter().map(|n| { + ast::make::use_tree(ast::make::path_from_text(&n.to_string()), None, None, false) + }))), + None, + false, + ), }; - match_ast! { - match node { - ast::UseTree(use_tree) => { - replace_node(replacement); - }, - ast::UseTreeList(use_tree_list) => { - replace_node(replacement); - }, - ast::Use(use_item) => { - builder.replace_ast(use_item, ast::make::use_(replacement.left_or_else(|ut| ast::make::use_tree(path, Some(ut), None, false)))); - }, - _ => {}, - } - } + builder.replace_ast(parent, replacement); } #[cfg(test)] @@ -260,7 +251,7 @@ fn qux(bar: Bar, baz: Baz) { expand_glob_import, r" mod foo { - mod bar { + pub mod bar { pub struct Bar; pub struct Baz; pub struct Qux; @@ -268,7 +259,7 @@ mod foo { pub fn f() {} } - mod baz { + pub mod baz { pub fn g() {} } } @@ -282,7 +273,7 @@ fn qux(bar: Bar, baz: Baz) { ", r" mod foo { - mod bar { + pub mod bar { pub struct Bar; pub struct Baz; pub struct Qux; @@ -290,7 +281,7 @@ mod foo { pub fn f() {} } - mod baz { + pub mod baz { pub fn g() {} } } @@ -302,7 +293,207 @@ fn qux(bar: Bar, baz: Baz) { g(); } ", - ) + ); + + check_assist( + expand_glob_import, + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + } +} + +use foo::{bar::{Bar, Baz, f}, baz::*<|>}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); +} +", + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + } +} + +use foo::{bar::{Bar, Baz, f}, baz::g}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); +} +", + ); + + check_assist( + expand_glob_import, + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + + pub mod qux { + pub fn h() {} + pub fn m() {} + + pub mod q { + pub fn j() {} + } + } + } +} + +use foo::{ + bar::{*, f}, + baz::{g, qux::*<|>} +}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); + h(); + q::j(); +} +", + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + + pub mod qux { + pub fn h() {} + pub fn m() {} + + pub mod q { + pub fn j() {} + } + } + } +} + +use foo::{ + bar::{*, f}, + baz::{g, qux::{q, h}} +}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); + h(); + q::j(); +} +", + ); + + check_assist( + expand_glob_import, + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + + pub mod qux { + pub fn h() {} + pub fn m() {} + + pub mod q { + pub fn j() {} + } + } + } +} + +use foo::{ + bar::{*, f}, + baz::{g, qux::{h, q::*<|>}} +}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); + h(); + j(); +} +", + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + + pub mod qux { + pub fn h() {} + pub fn m() {} + + pub mod q { + pub fn j() {} + } + } + } +} + +use foo::{ + bar::{*, f}, + baz::{g, qux::{h, q::j}} +}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); + h(); + j(); +} +", + ); } #[test] -- cgit v1.2.3 From bb72150f0214c9395c48fdd3ee93650f0a507112 Mon Sep 17 00:00:00 2001 From: unexge Date: Fri, 14 Aug 2020 22:10:49 +0300 Subject: Handle more cases in AST replacing in expand glob import --- crates/assists/src/handlers/expand_glob_import.rs | 253 +++++++++++++++++++--- 1 file changed, 219 insertions(+), 34 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index 0288d1c77..ff9c80d49 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -1,10 +1,12 @@ use either::Either; +use std::iter::successors; + use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; use ide_db::{ defs::{classify_name_ref, Definition, NameRefClass}, RootDatabase, }; -use syntax::{ast, AstNode, SyntaxToken, T}; +use syntax::{algo, ast, AstNode, SourceFile, SyntaxNode, SyntaxToken, T}; use crate::{ assist_context::{AssistBuilder, AssistContext, Assists}, @@ -48,27 +50,39 @@ pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Opti let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset()); let defs_in_mod = find_defs_in_mod(ctx, scope, module)?; - let name_refs_in_source_file = - source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect(); - let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file); + let names_to_import = find_names_to_import(ctx, source_file, defs_in_mod); - let target = parent.syntax(); + let target = parent.clone().either(|n| n.syntax().clone(), |n| n.syntax().clone()); acc.add( AssistId("expand_glob_import", AssistKind::RefactorRewrite), "Expand glob import", target.text_range(), |builder| { - replace_ast(builder, parent, mod_path, used_names); + replace_ast(builder, parent, mod_path, names_to_import); }, ) } -fn find_parent_and_path(star: &SyntaxToken) -> Option<(ast::UseTree, ast::Path)> { - star.ancestors().find_map(|n| { +fn find_parent_and_path( + star: &SyntaxToken, +) -> Option<(Either, ast::Path)> { + return star.ancestors().find_map(|n| { + find_use_tree_list(n.clone()) + .and_then(|(u, p)| Some((Either::Right(u), p))) + .or_else(|| find_use_tree(n).and_then(|(u, p)| Some((Either::Left(u), p)))) + }); + + fn find_use_tree_list(n: SyntaxNode) -> Option<(ast::UseTreeList, ast::Path)> { + let use_tree_list = ast::UseTreeList::cast(n)?; + let path = use_tree_list.parent_use_tree().path()?; + Some((use_tree_list, path)) + } + + fn find_use_tree(n: SyntaxNode) -> Option<(ast::UseTree, ast::Path)> { let use_tree = ast::UseTree::cast(n)?; let path = use_tree.path()?; Some((use_tree, path)) - }) + } } #[derive(PartialEq)] @@ -105,14 +119,36 @@ fn find_defs_in_mod( Some(defs) } -fn find_used_names( +fn find_names_to_import( ctx: &AssistContext, + source_file: &SourceFile, defs_in_mod: Vec, - name_refs_in_source_file: Vec, ) -> Vec { - let defs_in_source_file = name_refs_in_source_file - .iter() - .filter_map(|r| classify_name_ref(&ctx.sema, r)) + let (name_refs_in_use_item, name_refs_in_source) = source_file + .syntax() + .descendants() + .filter_map(|n| { + let name_ref = ast::NameRef::cast(n.clone())?; + let name_ref_class = classify_name_ref(&ctx.sema, &name_ref)?; + let is_in_use_item = + successors(n.parent(), |n| n.parent()).find_map(ast::Use::cast).is_some(); + Some((name_ref_class, is_in_use_item)) + }) + .partition::, _>(|&(_, is_in_use_item)| is_in_use_item); + + let name_refs_to_import: Vec = name_refs_in_source + .into_iter() + .filter_map(|(r, _)| { + if name_refs_in_use_item.contains(&(r.clone(), true)) { + // already imported + return None; + } + Some(r) + }) + .collect(); + + let defs_in_source_file = name_refs_to_import + .into_iter() .filter_map(|rc| match rc { NameRefClass::Definition(Definition::ModuleDef(def)) => Some(Def::ModuleDef(def)), NameRefClass::Definition(Definition::Macro(def)) => Some(Def::MacroDef(def)), @@ -141,28 +177,62 @@ fn find_used_names( fn replace_ast( builder: &mut AssistBuilder, - parent: ast::UseTree, + parent: Either, path: ast::Path, - used_names: Vec, + names_to_import: Vec, ) { - let replacement = match used_names.as_slice() { - [name] => ast::make::use_tree( - ast::make::path_from_text(&format!("{}::{}", path, name)), - None, - None, - false, - ), - names => ast::make::use_tree( - path, - Some(ast::make::use_tree_list(names.iter().map(|n| { - ast::make::use_tree(ast::make::path_from_text(&n.to_string()), None, None, false) - }))), - None, - false, - ), + let existing_use_trees = match parent.clone() { + Either::Left(_) => vec![], + Either::Right(u) => u.use_trees().filter(|n| + // filter out star + n.star_token().is_none() + ).collect(), }; - builder.replace_ast(parent, replacement); + let new_use_trees: Vec = names_to_import + .iter() + .map(|n| ast::make::use_tree(ast::make::path_from_text(&n.to_string()), None, None, false)) + .collect(); + + let use_trees = [&existing_use_trees[..], &new_use_trees[..]].concat(); + + match use_trees.as_slice() { + [name] => { + if let Some(end_path) = name.path() { + let replacement = ast::make::use_tree( + ast::make::path_from_text(&format!("{}::{}", path, end_path)), + None, + None, + false, + ); + + algo::diff( + &parent.either(|n| n.syntax().clone(), |n| n.syntax().clone()), + replacement.syntax(), + ) + .into_text_edit(builder.text_edit_builder()); + } + } + names => { + let replacement = match parent { + Either::Left(_) => ast::make::use_tree( + path, + Some(ast::make::use_tree_list(names.to_owned())), + None, + false, + ) + .syntax() + .clone(), + Either::Right(_) => ast::make::use_tree_list(names.to_owned()).syntax().clone(), + }; + + algo::diff( + &parent.either(|n| n.syntax().clone(), |n| n.syntax().clone()), + &replacement, + ) + .into_text_edit(builder.text_edit_builder()); + } + }; } #[cfg(test)] @@ -236,7 +306,46 @@ mod foo { pub fn f() {} } -use foo::{Baz, Bar, f}; +use foo::{f, Baz, Bar}; + +fn qux(bar: Bar, baz: Baz) { + f(); +} +", + ) + } + + #[test] + fn expanding_glob_import_with_existing_uses_in_same_module() { + check_assist( + expand_glob_import, + r" +mod foo { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} +} + +use foo::Bar; +use foo::{*<|>, f}; + +fn qux(bar: Bar, baz: Baz) { + f(); +} +", + r" +mod foo { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} +} + +use foo::Bar; +use foo::{f, Baz}; fn qux(bar: Bar, baz: Baz) { f(); @@ -286,7 +395,7 @@ mod foo { } } -use foo::{bar::{Baz, Bar, f}, baz::*}; +use foo::{bar::{f, Baz, Bar}, baz::*}; fn qux(bar: Bar, baz: Baz) { f(); @@ -486,6 +595,82 @@ use foo::{ baz::{g, qux::{h, q::j}} }; +fn qux(bar: Bar, baz: Baz) { + f(); + g(); + h(); + j(); +} +", + ); + + check_assist( + expand_glob_import, + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + + pub mod qux { + pub fn h() {} + pub fn m() {} + + pub mod q { + pub fn j() {} + } + } + } +} + +use foo::{ + bar::{*, f}, + baz::{g, qux::{q::j, *<|>}} +}; + +fn qux(bar: Bar, baz: Baz) { + f(); + g(); + h(); + j(); +} +", + r" +mod foo { + pub mod bar { + pub struct Bar; + pub struct Baz; + pub struct Qux; + + pub fn f() {} + } + + pub mod baz { + pub fn g() {} + + pub mod qux { + pub fn h() {} + pub fn m() {} + + pub mod q { + pub fn j() {} + } + } + } +} + +use foo::{ + bar::{*, f}, + baz::{g, qux::{q::j, h}} +}; + fn qux(bar: Bar, baz: Baz) { f(); g(); -- cgit v1.2.3 From 11d048af03e0d9e07c5c67c9a644af5fbf94ed57 Mon Sep 17 00:00:00 2001 From: unexge Date: Fri, 14 Aug 2020 22:17:26 +0300 Subject: Run rustfmt --- crates/assists/src/handlers/expand_glob_import.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index ff9c80d49..f9a08e84e 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -183,10 +183,12 @@ fn replace_ast( ) { let existing_use_trees = match parent.clone() { Either::Left(_) => vec![], - Either::Right(u) => u.use_trees().filter(|n| + Either::Right(u) => u + .use_trees() + .filter(|n| // filter out star - n.star_token().is_none() - ).collect(), + n.star_token().is_none()) + .collect(), }; let new_use_trees: Vec = names_to_import -- cgit v1.2.3 From 0847bc801eab10ec32792fdc546007a8a1cfbdde Mon Sep 17 00:00:00 2001 From: unexge Date: Sun, 16 Aug 2020 16:25:10 +0300 Subject: Use `Definition::find_usages` for finding used items in expand glob import --- crates/assists/src/assist_context.rs | 4 - crates/assists/src/handlers/expand_glob_import.rs | 247 ++++++++++++---------- 2 files changed, 137 insertions(+), 114 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/src/assist_context.rs b/crates/assists/src/assist_context.rs index 11c171fc2..bf520069e 100644 --- a/crates/assists/src/assist_context.rs +++ b/crates/assists/src/assist_context.rs @@ -73,10 +73,6 @@ impl<'a> AssistContext<'a> { self.sema.db } - pub(crate) fn source_file(&self) -> &SourceFile { - &self.source_file - } - // NB, this ignores active selection. pub(crate) fn offset(&self) -> TextSize { self.frange.range.start() diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index f9a08e84e..64267a03f 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -1,12 +1,10 @@ use either::Either; -use std::iter::successors; - -use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; +use hir::{MacroDef, Module, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; use ide_db::{ defs::{classify_name_ref, Definition, NameRefClass}, - RootDatabase, + search::SearchScope, }; -use syntax::{algo, ast, AstNode, SourceFile, SyntaxNode, SyntaxToken, T}; +use syntax::{algo, ast, AstNode, Direction, SyntaxNode, SyntaxToken, T}; use crate::{ assist_context::{AssistBuilder, AssistContext, Assists}, @@ -41,16 +39,17 @@ use crate::{ pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let star = ctx.find_token_at_offset(T![*])?; let (parent, mod_path) = find_parent_and_path(&star)?; - let module = match ctx.sema.resolve_path(&mod_path)? { + let target_module = match ctx.sema.resolve_path(&mod_path)? { PathResolution::Def(ModuleDef::Module(it)) => it, _ => return None, }; - let source_file = ctx.source_file(); - let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset()); + let current_scope = ctx.sema.scope(&star.parent()); + let current_module = current_scope.module()?; - let defs_in_mod = find_defs_in_mod(ctx, scope, module)?; - let names_to_import = find_names_to_import(ctx, source_file, defs_in_mod); + let refs_in_target = find_refs_in_mod(ctx, target_module, Some(current_module))?; + let imported_defs = find_imported_defs(ctx, star)?; + let names_to_import = find_names_to_import(ctx, current_scope, refs_in_target, imported_defs); let target = parent.clone().either(|n| n.syntax().clone(), |n| n.syntax().clone()); acc.add( @@ -85,94 +84,119 @@ fn find_parent_and_path( } } -#[derive(PartialEq)] +#[derive(Debug, PartialEq, Clone)] enum Def { ModuleDef(ModuleDef), MacroDef(MacroDef), } -impl Def { - fn name(&self, db: &RootDatabase) -> Option { - match self { - Def::ModuleDef(def) => def.name(db), - Def::MacroDef(def) => def.name(db), +#[derive(Debug, Clone)] +struct Ref { + // could be alias + visible_name: Name, + def: Def, +} + +impl Ref { + fn from_scope_def(name: Name, scope_def: ScopeDef) -> Option { + match scope_def { + ScopeDef::ModuleDef(def) => Some(Ref { visible_name: name, def: Def::ModuleDef(def) }), + ScopeDef::MacroDef(def) => Some(Ref { visible_name: name, def: Def::MacroDef(def) }), + _ => None, } } -} -fn find_defs_in_mod( - ctx: &AssistContext, - from: SemanticsScope<'_>, - module: hir::Module, -) -> Option> { - let module_scope = module.scope(ctx.db(), from.module()); - - let mut defs = vec![]; - for (_, def) in module_scope { - match def { - ScopeDef::ModuleDef(def) => defs.push(Def::ModuleDef(def)), - ScopeDef::MacroDef(def) => defs.push(Def::MacroDef(def)), - _ => continue, + fn is_referenced_in(&self, ctx: &AssistContext, scope: &SemanticsScope) -> bool { + let def = match self.def { + Def::ModuleDef(def) => Definition::ModuleDef(def), + Def::MacroDef(def) => Definition::Macro(def), + }; + + if let Definition::ModuleDef(ModuleDef::Trait(tr)) = def { + if scope + .traits_in_scope() + .into_iter() + .find(|other_tr_id| tr == other_tr_id.to_owned().into()) + .is_some() + { + return true; + } } + + let search_scope = SearchScope::single_file(ctx.frange.file_id); + !def.find_usages(&ctx.sema, Some(search_scope)).is_empty() + } +} + +#[derive(Debug, Clone)] +struct Refs(Vec); + +impl Refs { + fn used_refs(&self, ctx: &AssistContext, scope: &SemanticsScope) -> Refs { + Refs(self.0.clone().into_iter().filter(|r| r.is_referenced_in(ctx, scope)).collect()) } - Some(defs) + fn filter_out_by_defs(&self, defs: Vec) -> Refs { + Refs(self.0.clone().into_iter().filter(|r| !defs.contains(&r.def)).collect()) + } } -fn find_names_to_import( +fn find_refs_in_mod( ctx: &AssistContext, - source_file: &SourceFile, - defs_in_mod: Vec, -) -> Vec { - let (name_refs_in_use_item, name_refs_in_source) = source_file - .syntax() - .descendants() - .filter_map(|n| { - let name_ref = ast::NameRef::cast(n.clone())?; - let name_ref_class = classify_name_ref(&ctx.sema, &name_ref)?; - let is_in_use_item = - successors(n.parent(), |n| n.parent()).find_map(ast::Use::cast).is_some(); - Some((name_ref_class, is_in_use_item)) - }) - .partition::, _>(|&(_, is_in_use_item)| is_in_use_item); - - let name_refs_to_import: Vec = name_refs_in_source - .into_iter() - .filter_map(|(r, _)| { - if name_refs_in_use_item.contains(&(r.clone(), true)) { - // already imported - return None; - } - Some(r) - }) - .collect(); - - let defs_in_source_file = name_refs_to_import - .into_iter() - .filter_map(|rc| match rc { - NameRefClass::Definition(Definition::ModuleDef(def)) => Some(Def::ModuleDef(def)), - NameRefClass::Definition(Definition::Macro(def)) => Some(Def::MacroDef(def)), - _ => None, - }) - .collect::>(); + module: Module, + visible_from: Option, +) -> Option { + let module_scope = module.scope(ctx.db(), visible_from); + let refs = module_scope.into_iter().filter_map(|(n, d)| Ref::from_scope_def(n, d)).collect(); + Some(Refs(refs)) +} - defs_in_mod - .iter() - .filter(|def| { - if let Def::ModuleDef(ModuleDef::Trait(tr)) = def { - for item in tr.items(ctx.db()) { - if let AssocItem::Function(f) = item { - if defs_in_source_file.contains(&Def::ModuleDef(ModuleDef::Function(f))) { - return true; - } - } - } - } +// looks for name refs in parent use block's siblings +// +// mod bar { +// mod qux { +// struct Qux; +// } +// +// pub use qux::Qux; +// } +// +// ↓ --------------- +// use foo::*<|>; +// use baz::Baz; +// ↑ --------------- +fn find_imported_defs(ctx: &AssistContext, star: SyntaxToken) -> Option> { + let parent_use_item_syntax = + star.ancestors().find_map(|n| if ast::Use::can_cast(n.kind()) { Some(n) } else { None })?; + + Some( + [Direction::Prev, Direction::Next] + .iter() + .map(|dir| { + parent_use_item_syntax + .siblings(dir.to_owned()) + .filter(|n| ast::Use::can_cast(n.kind())) + }) + .flatten() + .filter_map(|n| Some(n.descendants().filter_map(ast::NameRef::cast))) + .flatten() + .filter_map(|r| match classify_name_ref(&ctx.sema, &r)? { + NameRefClass::Definition(Definition::ModuleDef(def)) => Some(Def::ModuleDef(def)), + NameRefClass::Definition(Definition::Macro(def)) => Some(Def::MacroDef(def)), + _ => None, + }) + .collect(), + ) +} - defs_in_source_file.contains(def) - }) - .filter_map(|d| d.name(ctx.db())) - .collect() +fn find_names_to_import( + ctx: &AssistContext, + current_scope: SemanticsScope, + refs_in_target: Refs, + imported_defs: Vec, +) -> Vec { + let used_refs = refs_in_target.used_refs(ctx, ¤t_scope).filter_out_by_defs(imported_defs); + used_refs.0.iter().map(|r| r.visible_name.clone()).collect() } fn replace_ast( @@ -685,34 +709,37 @@ fn qux(bar: Bar, baz: Baz) { #[test] fn expanding_glob_import_with_macro_defs() { - check_assist( - expand_glob_import, - r" -//- /lib.rs crate:foo -#[macro_export] -macro_rules! bar { - () => () -} - -pub fn baz() {} - -//- /main.rs crate:main deps:foo -use foo::*<|>; - -fn main() { - bar!(); - baz(); -} -", - r" -use foo::{bar, baz}; - -fn main() { - bar!(); - baz(); -} -", - ) + // TODO: this is currently fails because `Definition::find_usages` ignores macros + // https://github.com/rust-analyzer/rust-analyzer/issues/3484 + // + // check_assist( + // expand_glob_import, + // r" + // //- /lib.rs crate:foo + // #[macro_export] + // macro_rules! bar { + // () => () + // } + + // pub fn baz() {} + + // //- /main.rs crate:main deps:foo + // use foo::*<|>; + + // fn main() { + // bar!(); + // baz(); + // } + // ", + // r" + // use foo::{bar, baz}; + + // fn main() { + // bar!(); + // baz(); + // } + // ", + // ) } #[test] -- cgit v1.2.3 From 585f5d4901f3ddc0bec6aface8a86707d216886e Mon Sep 17 00:00:00 2001 From: unexge Date: Sun, 16 Aug 2020 16:42:44 +0300 Subject: Use fixme instead of todo --- crates/assists/src/handlers/expand_glob_import.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index 64267a03f..0e80cde77 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -709,7 +709,7 @@ fn qux(bar: Bar, baz: Baz) { #[test] fn expanding_glob_import_with_macro_defs() { - // TODO: this is currently fails because `Definition::find_usages` ignores macros + // FIXME: this is currently fails because `Definition::find_usages` ignores macros // https://github.com/rust-analyzer/rust-analyzer/issues/3484 // // check_assist( -- cgit v1.2.3 From 5d28dec7b93aad54866c80213358fb3eb28153be Mon Sep 17 00:00:00 2001 From: unexge Date: Sun, 16 Aug 2020 18:37:15 +0300 Subject: Fix importing unused traits in expand glob import --- crates/assists/src/handlers/expand_glob_import.rs | 101 ++++++++++++++++------ 1 file changed, 73 insertions(+), 28 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index 0e80cde77..194fae63e 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -1,5 +1,5 @@ use either::Either; -use hir::{MacroDef, Module, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; +use hir::{AssocItem, MacroDef, Module, ModuleDef, Name, PathResolution, ScopeDef}; use ide_db::{ defs::{classify_name_ref, Definition, NameRefClass}, search::SearchScope, @@ -49,7 +49,7 @@ pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Opti let refs_in_target = find_refs_in_mod(ctx, target_module, Some(current_module))?; let imported_defs = find_imported_defs(ctx, star)?; - let names_to_import = find_names_to_import(ctx, current_scope, refs_in_target, imported_defs); + let names_to_import = find_names_to_import(ctx, refs_in_target, imported_defs); let target = parent.clone().either(|n| n.syntax().clone(), |n| n.syntax().clone()); acc.add( @@ -90,6 +90,18 @@ enum Def { MacroDef(MacroDef), } +impl Def { + fn is_referenced_in(&self, ctx: &AssistContext) -> bool { + let def = match self { + Def::ModuleDef(def) => Definition::ModuleDef(*def), + Def::MacroDef(def) => Definition::Macro(*def), + }; + + let search_scope = SearchScope::single_file(ctx.frange.file_id); + !def.find_usages(&ctx.sema, Some(search_scope)).is_empty() + } +} + #[derive(Debug, Clone)] struct Ref { // could be alias @@ -105,35 +117,39 @@ impl Ref { _ => None, } } - - fn is_referenced_in(&self, ctx: &AssistContext, scope: &SemanticsScope) -> bool { - let def = match self.def { - Def::ModuleDef(def) => Definition::ModuleDef(def), - Def::MacroDef(def) => Definition::Macro(def), - }; - - if let Definition::ModuleDef(ModuleDef::Trait(tr)) = def { - if scope - .traits_in_scope() - .into_iter() - .find(|other_tr_id| tr == other_tr_id.to_owned().into()) - .is_some() - { - return true; - } - } - - let search_scope = SearchScope::single_file(ctx.frange.file_id); - !def.find_usages(&ctx.sema, Some(search_scope)).is_empty() - } } #[derive(Debug, Clone)] struct Refs(Vec); impl Refs { - fn used_refs(&self, ctx: &AssistContext, scope: &SemanticsScope) -> Refs { - Refs(self.0.clone().into_iter().filter(|r| r.is_referenced_in(ctx, scope)).collect()) + fn used_refs(&self, ctx: &AssistContext) -> Refs { + Refs( + self.0 + .clone() + .into_iter() + .filter(|r| { + if let Def::ModuleDef(ModuleDef::Trait(tr)) = r.def { + if tr + .items(ctx.db()) + .into_iter() + .find(|ai| { + if let AssocItem::Function(f) = *ai { + Def::ModuleDef(ModuleDef::Function(f)).is_referenced_in(ctx) + } else { + false + } + }) + .is_some() + { + return true; + } + } + + r.def.is_referenced_in(ctx) + }) + .collect(), + ) } fn filter_out_by_defs(&self, defs: Vec) -> Refs { @@ -191,11 +207,10 @@ fn find_imported_defs(ctx: &AssistContext, star: SyntaxToken) -> Option fn find_names_to_import( ctx: &AssistContext, - current_scope: SemanticsScope, refs_in_target: Refs, imported_defs: Vec, ) -> Vec { - let used_refs = refs_in_target.used_refs(ctx, ¤t_scope).filter_out_by_defs(imported_defs); + let used_refs = refs_in_target.used_refs(ctx).filter_out_by_defs(imported_defs); used_refs.0.iter().map(|r| r.visible_name.clone()).collect() } @@ -767,7 +782,37 @@ fn main() { ().method(); } ", - ) + ); + + check_assist( + expand_glob_import, + r" +//- /lib.rs crate:foo +pub trait Tr { + fn method(&self) {} +} +impl Tr for () {} + +pub trait Tr2 { + fn method2(&self) {} +} +impl Tr2 for () {} + +//- /main.rs crate:main deps:foo +use foo::*<|>; + +fn main() { + ().method(); +} +", + r" +use foo::Tr; + +fn main() { + ().method(); +} +", + ); } #[test] -- cgit v1.2.3 From 5cff4b60bedf4d0b2e30d08aa19686584202a560 Mon Sep 17 00:00:00 2001 From: unexge Date: Mon, 17 Aug 2020 20:22:14 +0300 Subject: Fix importing private modules in expand glob import --- crates/assists/src/handlers/expand_glob_import.rs | 52 +++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index 194fae63e..eb6dd68bb 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -162,11 +162,28 @@ fn find_refs_in_mod( module: Module, visible_from: Option, ) -> Option { + if let Some(from) = visible_from { + if !is_mod_visible_from(ctx, module, from) { + return None; + } + } + let module_scope = module.scope(ctx.db(), visible_from); let refs = module_scope.into_iter().filter_map(|(n, d)| Ref::from_scope_def(n, d)).collect(); Some(Refs(refs)) } +fn is_mod_visible_from(ctx: &AssistContext, module: Module, from: Module) -> bool { + match module.parent(ctx.db()) { + Some(parent) => { + parent.visibility_of(ctx.db(), &ModuleDef::Module(module)).map_or(true, |vis| { + vis.is_visible_from(ctx.db(), from.into()) && is_mod_visible_from(ctx, parent, from) + }) + } + None => true, + } +} + // looks for name refs in parent use block's siblings // // mod bar { @@ -815,6 +832,41 @@ fn main() { ); } + #[test] + fn expanding_is_not_applicable_if_target_module_is_not_accessible_from_current_scope() { + check_assist_not_applicable( + expand_glob_import, + r" +mod foo { + mod bar { + pub struct Bar; + } +} + +use foo::bar::*<|>; + +fn baz(bar: Bar) {} +", + ); + + check_assist_not_applicable( + expand_glob_import, + r" +mod foo { + mod bar { + pub mod baz { + pub struct Baz; + } + } +} + +use foo::bar::baz::*<|>; + +fn qux(baz: Baz) {} +", + ); + } + #[test] fn expanding_is_not_applicable_if_cursor_is_not_in_star_token() { check_assist_not_applicable( -- cgit v1.2.3 From ef54e8451d2ebe20e900f9bea4545795ebd035c7 Mon Sep 17 00:00:00 2001 From: unexge Date: Wed, 19 Aug 2020 20:16:03 +0300 Subject: Use new `Definition::usages` API in expand glob import --- crates/assists/src/handlers/expand_glob_import.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs index eb6dd68bb..b39d040f6 100644 --- a/crates/assists/src/handlers/expand_glob_import.rs +++ b/crates/assists/src/handlers/expand_glob_import.rs @@ -98,7 +98,7 @@ impl Def { }; let search_scope = SearchScope::single_file(ctx.frange.file_id); - !def.find_usages(&ctx.sema, Some(search_scope)).is_empty() + def.usages(&ctx.sema).in_scope(search_scope).at_least_one() } } -- cgit v1.2.3 From 1d129a7172bbe502182be6cc3b50b3250cb6f3a9 Mon Sep 17 00:00:00 2001 From: dragfire Date: Sun, 23 Aug 2020 14:30:07 -0600 Subject: Invert if should be smart about is_some, is_none, is_ok, is_err --- crates/assists/src/handlers/invert_if.rs | 18 ++++++++++++++++++ crates/assists/src/utils.rs | 21 ++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/invert_if.rs b/crates/assists/src/handlers/invert_if.rs index f0e047538..294256297 100644 --- a/crates/assists/src/handlers/invert_if.rs +++ b/crates/assists/src/handlers/invert_if.rs @@ -106,4 +106,22 @@ mod tests { "fn f() { i<|>f let Some(_) = Some(1) { 1 } else { 0 } }", ) } + + #[test] + fn invert_if_option_case() { + check_assist( + invert_if, + "fn f() { if<|> doc_style.is_some() { Class::DocComment } else { Class::Comment } }", + "fn f() { if doc_style.is_none() { Class::Comment } else { Class::DocComment } }", + ) + } + + #[test] + fn invert_if_result_case() { + check_assist( + invert_if, + "fn f() { i<|>f doc_style.is_err() { Class::Err } else { Class::Ok } }", + "fn f() { if doc_style.is_ok() { Class::Ok } else { Class::Err } }", + ) + } } diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index d071d6502..e15c982e7 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs @@ -11,7 +11,7 @@ use syntax::{ ast::{self, make, NameOwner}, AstNode, Direction, SyntaxKind::*, - SyntaxNode, TextSize, T, + SyntaxNode, SyntaxText, TextSize, T, }; use crate::assist_config::SnippetCap; @@ -179,6 +179,25 @@ fn invert_special_case(expr: &ast::Expr) -> Option { ast::BinOp::EqualityTest => bin.replace_op(T![!=]).map(|it| it.into()), _ => None, }, + ast::Expr::MethodCallExpr(mce) => { + const IS_SOME_TEXT: &str = "is_some"; + const IS_NONE_TEXT: &str = "is_none"; + const IS_OK_TEXT: &str = "is_ok"; + const IS_ERR_TEXT: &str = "is_err"; + + let name = mce.name_ref()?; + let name_text = name.text(); + + let caller = || -> Option { Some(mce.receiver()?.syntax().text()) }; + + match name_text { + x if x == IS_SOME_TEXT => make::expr_method_call(IS_NONE_TEXT, caller), + x if x == IS_NONE_TEXT => make::expr_method_call(IS_SOME_TEXT, caller), + x if x == IS_OK_TEXT => make::expr_method_call(IS_ERR_TEXT, caller), + x if x == IS_ERR_TEXT => make::expr_method_call(IS_OK_TEXT, caller), + _ => None, + } + } ast::Expr::PrefixExpr(pe) if pe.op_kind()? == ast::PrefixOp::Not => pe.expr(), // FIXME: // ast::Expr::Literal(true | false ) -- cgit v1.2.3 From a8fa5cd42e3cfa131121a46b289ee919e495316e Mon Sep 17 00:00:00 2001 From: Pavan Kumar Sunkara Date: Mon, 24 Aug 2020 10:25:19 +0200 Subject: Add version to deps in cargo.toml --- crates/assists/Cargo.toml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'crates/assists') diff --git a/crates/assists/Cargo.toml b/crates/assists/Cargo.toml index a560a35c7..aab901684 100644 --- a/crates/assists/Cargo.toml +++ b/crates/assists/Cargo.toml @@ -13,11 +13,11 @@ rustc-hash = "1.1.0" itertools = "0.9.0" either = "1.5.3" -stdx = { path = "../stdx" } -syntax = { path = "../syntax" } -text_edit = { path = "../text_edit" } -profile = { path = "../profile" } -base_db = { path = "../base_db" } -ide_db = { path = "../ide_db" } -hir = { path = "../hir" } -test_utils = { path = "../test_utils" } +stdx = { path = "../stdx", version = "0.0.0" } +syntax = { path = "../syntax", version = "0.0.0" } +text_edit = { path = "../text_edit", version = "0.0.0" } +profile = { path = "../profile", version = "0.0.0" } +base_db = { path = "../base_db", 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" } -- cgit v1.2.3 From 335add49dbd98c13a8dd2b0284cefda00da13ec9 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Sunkara Date: Mon, 24 Aug 2020 13:06:30 +0200 Subject: Add description for crates that will be published --- crates/assists/Cargo.toml | 1 + 1 file changed, 1 insertion(+) (limited to 'crates/assists') diff --git a/crates/assists/Cargo.toml b/crates/assists/Cargo.toml index aab901684..264125651 100644 --- a/crates/assists/Cargo.toml +++ b/crates/assists/Cargo.toml @@ -1,6 +1,7 @@ [package] name = "assists" version = "0.0.0" +description = "TBD" license = "MIT OR Apache-2.0" authors = ["rust-analyzer developers"] edition = "2018" -- cgit v1.2.3