From 32b95ea3100d34e87e50ec2d2d42786b2927de9c Mon Sep 17 00:00:00 2001 From: vi_mi Date: Mon, 16 Aug 2021 13:45:10 +0530 Subject: [PATCH 1/5] feat: Adding extract_module assist --- .../src/handlers/extract_module.rs | 1409 +++++++++++++++++ .../src/handlers/remove_unused_param.rs | 2 +- crates/ide_assists/src/lib.rs | 2 + crates/ide_assists/src/tests/generated.rs | 29 + crates/syntax/src/ast/make.rs | 6 + 5 files changed, 1447 insertions(+), 1 deletion(-) create mode 100644 crates/ide_assists/src/handlers/extract_module.rs diff --git a/crates/ide_assists/src/handlers/extract_module.rs b/crates/ide_assists/src/handlers/extract_module.rs new file mode 100644 index 0000000000..79c5354f84 --- /dev/null +++ b/crates/ide_assists/src/handlers/extract_module.rs @@ -0,0 +1,1409 @@ +use std::collections::{HashMap, HashSet}; + +use hir::{HasSource, ModuleDef, ModuleSource}; +use ide_db::{ + assists::{AssistId, AssistKind}, + base_db::FileId, + defs::{Definition, NameClass, NameRefClass}, + search::{FileReference, SearchScope}, +}; +use stdx::format_to; +use syntax::{ + algo::find_node_at_range, + ast::{ + self, + edit::{AstNodeEdit, IndentLevel}, + make, HasName, HasVisibility, + }, + match_ast, ted, AstNode, SourceFile, SyntaxNode, TextRange, +}; + +use crate::{AssistContext, Assists}; + +use super::remove_unused_param::range_to_remove; + +// Assist: extract_module +// +// Extracts a selected region as seperate module. All the references, visibility and imports are +// resolved. +// +// ``` +// $0 +// fn foo(name: i32) -> i32 { +// name + 1 +// } +// $0 +// +// fn bar(name: i32) -> i32 { +// name + 2 +// } +// ``` +// -> +// ``` +// mod modname { +// pub(crate) fn foo(name: i32) -> i32 { +// name + 1 +// } +// } +// +// fn bar(name: i32) -> i32 { +// name + 2 +// } +// ``` +pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + if ctx.frange.range.is_empty() { + return None; + } + + let node = ctx.covering_element(); + let node = match node { + syntax::NodeOrToken::Node(n) => n, + syntax::NodeOrToken::Token(t) => t.parent()?, + }; + + let mut curr_parent_module: Option = None; + if let Some(mod_syn_opt) = node.ancestors().find(|it| ast::Module::can_cast(it.kind())) { + curr_parent_module = ast::Module::cast(mod_syn_opt); + } + + let mut module = extract_target(&node, ctx.frange.range)?; + if module.body_items.len() == 0 { + return None; + } + + let old_item_indent = module.body_items[0].indent_level(); + + acc.add( + AssistId("extract_module", AssistKind::RefactorExtract), + "Extract Module", + module.text_range, + |builder| { + //This takes place in three steps: + // + //- Firstly, we will update the references(usages) e.g. converting a + // function call bar() to modname::bar(), and similarly for other items + // + //- Secondly, changing the visibility of each item inside the newly selected module + // i.e. making a fn a() {} to pub(crate) fn a() {} + // + //- Thirdly, resolving all the imports this includes removing paths from imports + // outside the module, shifting/cloning them inside new module, or shifting the imports, or making + // new import statemnts + + //We are getting item usages and record_fields together, record_fields + //for change_visibility and usages for first point mentioned above in the process + let (usages_to_be_processed, record_fields) = module.get_usages_and_record_fields(ctx); + + let import_paths_to_be_removed = module.resolve_imports(curr_parent_module, &ctx); + + if let Some(block_items) = module.change_visibility(record_fields) { + module.body_items = block_items; + if module.body_items.len() == 0 { + return; + } + + let mut body_items = Vec::new(); + let new_item_indent = old_item_indent + 1; + for item in module.body_items { + let item = item.indent(IndentLevel(1)); + let mut indented_item = String::new(); + format_to!(indented_item, "{}{}", new_item_indent, item.to_string()); + body_items.push(indented_item); + } + + let body = body_items.join("\n\n"); + + let mut module_def = String::new(); + + format_to!(module_def, "mod {} {{\n{}\n{}}}", module.name, body, old_item_indent); + + for usages_to_be_updated_for_file in usages_to_be_processed { + builder.edit_file(usages_to_be_updated_for_file.0); + for usage_to_be_processed in usages_to_be_updated_for_file.1 { + builder.replace(usage_to_be_processed.0, usage_to_be_processed.1) + } + } + + builder.edit_file(ctx.frange.file_id); + for import_path_text_range in import_paths_to_be_removed { + builder.delete(import_path_text_range); + } + builder.replace(module.text_range, module_def) + } + }, + ) +} + +#[derive(Debug)] +struct Module { + text_range: TextRange, + name: String, + body_items: Vec, +} + +fn extract_target(node: &SyntaxNode, selection_range: TextRange) -> Option { + let body_items: Vec = node + .children() + .filter_map(|child| { + if let Some(item) = ast::Item::cast(child.clone()) { + if selection_range.contains_range(item.syntax().text_range()) { + return Some(item); + } + return None; + } + None + }) + .collect(); + + Some(Module { text_range: selection_range, name: "modname".to_string(), body_items }) +} + +impl Module { + fn get_usages_and_record_fields( + &self, + ctx: &AssistContext, + ) -> (HashMap>, Vec) { + let mut record_fields = Vec::new(); + let mut refs: HashMap> = HashMap::new(); + + //Here impl is not included as each item inside impl will be tied to the parent of + //implementing block(a struct, enum, etc), if the parent is in selected module, it will + //get updated by ADT section given below or if it is not, then we dont need to do any operation + self.body_items.clone().into_iter().for_each(|item| { + match_ast! { + match (item.syntax()) { + ast::Adt(it) => { + if let Some( nod ) = ctx.sema.to_def(&it) { + let node_def = Definition::ModuleDef(nod.into()); + self.expand_and_group_usages_file_wise(ctx, node_def, &mut refs); + + let mut get_record_fields = |it: ast::Adt| { + for desc in it.syntax().descendants() { + if let Some(record_field) = ast::RecordField::cast(desc) { + record_fields.push(record_field.syntax().clone()); + } + } + }; + + //Enum Fields are not allowed to explicitly specify pub, it is implied + match it { + ast::Adt::Struct(_) => get_record_fields(it), + ast::Adt::Union(_) => get_record_fields(it), + ast::Adt::Enum(_) => {}, + } + } + }, + ast::TypeAlias(it) => { + if let Some( nod ) = ctx.sema.to_def(&it) { + let node_def = Definition::ModuleDef(nod.into()); + self.expand_and_group_usages_file_wise(ctx, node_def, &mut refs); + } + }, + ast::Const(it) => { + if let Some( nod ) = ctx.sema.to_def(&it) { + let node_def = Definition::ModuleDef(nod.into()); + self.expand_and_group_usages_file_wise(ctx, node_def, &mut refs); + } + }, + ast::Static(it) => { + if let Some( nod ) = ctx.sema.to_def(&it) { + let node_def = Definition::ModuleDef(nod.into()); + self.expand_and_group_usages_file_wise(ctx, node_def, &mut refs); + } + }, + ast::Fn(it) => { + if let Some( nod ) = ctx.sema.to_def(&it) { + let node_def = Definition::ModuleDef(nod.into()); + self.expand_and_group_usages_file_wise(ctx, node_def, &mut refs); + } + }, + _ => (), + } + } + }); + + return (refs, record_fields); + } + + fn expand_and_group_usages_file_wise( + &self, + ctx: &AssistContext, + node_def: Definition, + refs: &mut HashMap>, + ) { + for (file_id, references) in node_def.usages(&ctx.sema).all() { + if let Some(file_refs) = refs.get_mut(&file_id) { + let mut usages = self.expand_ref_to_usages(references, ctx, file_id); + file_refs.append(&mut usages); + } else { + refs.insert(file_id, self.expand_ref_to_usages(references, ctx, file_id)); + } + } + } + + fn expand_ref_to_usages( + &self, + refs: Vec, + ctx: &AssistContext, + file_id: FileId, + ) -> Vec<(TextRange, String)> { + let source_file = ctx.sema.parse(file_id); + + let mut usages_to_be_processed_for_file = Vec::new(); + for usage in refs { + if let Some(x) = self.get_usage_to_be_processed(&source_file, usage) { + usages_to_be_processed_for_file.push(x); + } + } + + usages_to_be_processed_for_file + } + + fn get_usage_to_be_processed( + &self, + source_file: &SourceFile, + FileReference { range, name, .. }: FileReference, + ) -> Option<(TextRange, String)> { + let path: Option = find_node_at_range(source_file.syntax(), range); + + let path = path?; + + for desc in path.syntax().descendants() { + if desc.to_string() == name.syntax().to_string() + && !self.text_range.contains_range(desc.text_range()) + { + if let Some(name_ref) = ast::NameRef::cast(desc) { + return Some(( + name_ref.syntax().text_range(), + format!("{}::{}", self.name, name_ref.to_string()), + )); + } + } + } + + None + } + + fn change_visibility(&self, record_fields: Vec) -> Option> { + let (body_items, mut replacements, record_field_parents, impls) = + get_replacements_for_visibilty_change(self.body_items.clone(), false); + + let impl_items = impls.into_iter().fold(Vec::new(), |mut impl_items, x| { + let this_impl_items = + x.syntax().descendants().fold(Vec::new(), |mut this_impl_items, x| { + if let Some(item) = ast::Item::cast(x.clone()) { + this_impl_items.push(item); + } + return this_impl_items; + }); + + impl_items.append(&mut this_impl_items.clone()); + return impl_items; + }); + + let (_, mut impl_item_replacements, _, _) = + get_replacements_for_visibilty_change(impl_items.clone(), true); + + replacements.append(&mut impl_item_replacements); + + record_field_parents.into_iter().for_each(|x| { + x.1.descendants().filter_map(|x| ast::RecordField::cast(x)).for_each(|desc| { + let is_record_field_present = record_fields + .clone() + .into_iter() + .find(|x| x.to_string() == desc.to_string()) + .is_some(); + if is_record_field_present { + replacements.push((desc.visibility().clone(), desc.syntax().clone())); + } + }); + }); + + replacements.into_iter().for_each(|(vis, syntax)| { + add_change_vis(vis, syntax.first_child_or_token()); + }); + + Some(body_items) + } + + fn resolve_imports( + &mut self, + curr_parent_module: Option, + ctx: &AssistContext, + ) -> Vec { + let mut import_paths_to_be_removed: Vec = vec![]; + let mut node_set: HashSet = HashSet::new(); + + self.body_items.clone().into_iter().for_each(|item| { + item.syntax().descendants().for_each(|x| { + if let Some(name) = ast::Name::cast(x.clone()) { + if let Some(name_classify) = NameClass::classify(&ctx.sema, &name) { + //Necessary to avoid two same names going through + if !node_set.contains(&name.syntax().to_string()) { + node_set.insert(name.syntax().to_string()); + let def_opt: Option = match name_classify { + NameClass::Definition(def) => Some(def), + _ => None, + }; + + if let Some(def) = def_opt { + if let Some(import_path) = self + .process_names_and_namerefs_for_import_resolve( + def, + name.syntax(), + &curr_parent_module, + ctx, + ) + { + import_paths_to_be_removed.push(import_path); + } + } + } + } + } + + if let Some(name_ref) = ast::NameRef::cast(x) { + if let Some(name_classify) = NameRefClass::classify(&ctx.sema, &name_ref) { + //Necessary to avoid two same names going through + if !node_set.contains(&name_ref.syntax().to_string()) { + node_set.insert(name_ref.syntax().to_string()); + let def_opt: Option = match name_classify { + NameRefClass::Definition(def) => Some(def), + _ => None, + }; + + if let Some(def) = def_opt { + if let Some(import_path) = self + .process_names_and_namerefs_for_import_resolve( + def, + name_ref.syntax(), + &curr_parent_module, + ctx, + ) + { + import_paths_to_be_removed.push(import_path); + } + } + } + } + } + }); + }); + + import_paths_to_be_removed + } + + fn process_names_and_namerefs_for_import_resolve( + &mut self, + def: Definition, + node_syntax: &SyntaxNode, + curr_parent_module: &Option, + ctx: &AssistContext, + ) -> Option { + //We only need to find in the current file + let selection_range = ctx.frange.range; + let search_scope = SearchScope::single_file(ctx.frange.file_id); + let usage_res = def.usages(&ctx.sema).in_scope(search_scope).all(); + let curr_file_id = ctx.frange.file_id; + let file = ctx.sema.parse(ctx.frange.file_id); + + let mut exists_inside_sel = false; + let mut exists_outside_sel = false; + usage_res.clone().into_iter().for_each(|x| { + let mut non_use_nodes_itr = (&x.1).into_iter().filter_map(|x| { + if find_node_at_range::(file.syntax(), x.range).is_none() { + let path_opt = find_node_at_range::(file.syntax(), x.range); + return path_opt; + } + + None + }); + + if non_use_nodes_itr + .clone() + .find(|x| !selection_range.contains_range(x.syntax().text_range())) + .is_some() + { + exists_outside_sel = true; + } + if non_use_nodes_itr + .find(|x| selection_range.contains_range(x.syntax().text_range())) + .is_some() + { + exists_inside_sel = true; + } + }); + + let source_exists_outside_sel_in_same_mod = does_source_exists_outside_sel_in_same_mod( + def, + ctx, + curr_parent_module, + selection_range, + curr_file_id, + ); + + let use_stmt_opt: Option = usage_res.into_iter().find_map(|x| { + let file_id = x.0; + let mut use_opt: Option = None; + if file_id == ctx.frange.file_id { + (&x.1).into_iter().for_each(|x| { + let node_opt: Option = find_node_at_range(file.syntax(), x.range); + if let Some(node) = node_opt { + use_opt = Some(node.clone()); + } + }); + } + return use_opt; + }); + + let mut use_tree_str_opt: Option> = None; + //Exists inside and outside selection + // - Use stmt for item is present -> get the use_tree_str and reconstruct the path in new + // module + // - Use stmt for item is not present -> + //If it is not found, the definition is either ported inside new module or it stays + //outside: + //- Def is inside: Nothing to import + //- Def is outside: Import it inside with super + + //Exists inside selection but not outside -> Check for the import of it in original module, + //get the use_tree_str, reconstruct the use stmt in new module + + let mut import_path_to_be_removed: Option = None; + if exists_inside_sel && exists_outside_sel { + //Changes to be made only inside new module + + //If use_stmt exists, find the use_tree_str, reconstruct it inside new module + //If not, insert a use stmt with super and the given nameref + if let Some((use_tree_str, _)) = + self.process_use_stmt_for_import_resolve(use_stmt_opt, node_syntax) + { + use_tree_str_opt = Some(use_tree_str); + } else if source_exists_outside_sel_in_same_mod { + //Considered only after use_stmt is not present + //source_exists_outside_sel_in_same_mod | exists_outside_sel(exists_inside_sel = + //true for all cases) + // false | false -> Do nothing + // false | true -> If source is in selection -> nothing to do, If source is outside + // mod -> ust_stmt transversal + // true | false -> super import insertion + // true | true -> super import insertion + self.make_use_stmt_of_node_with_super(node_syntax); + } + } else if exists_inside_sel && !exists_outside_sel { + //Changes to be made inside new module, and remove import from outside + + if let Some((use_tree_str, text_range_opt)) = + self.process_use_stmt_for_import_resolve(use_stmt_opt, node_syntax) + { + if let Some(text_range) = text_range_opt { + import_path_to_be_removed = Some(text_range); + } + use_tree_str_opt = Some(use_tree_str); + } else if source_exists_outside_sel_in_same_mod { + self.make_use_stmt_of_node_with_super(node_syntax); + } + } + + if let Some(use_tree_str) = use_tree_str_opt { + let mut use_tree_str = use_tree_str.clone(); + use_tree_str.reverse(); + if use_tree_str[0].to_string().contains("super") { + let super_path = make::ext::ident_path("super"); + use_tree_str.insert(0, super_path) + } + + let use_ = + make::use_(None, make::use_tree(make::join_paths(use_tree_str), None, None, false)); + if let Some(item) = ast::Item::cast(use_.syntax().clone()) { + self.body_items.insert(0, item); + } + } + + import_path_to_be_removed + } + + fn make_use_stmt_of_node_with_super(&mut self, node_syntax: &SyntaxNode) { + let super_path = make::ext::ident_path("super"); + let node_path = make::ext::ident_path(&node_syntax.to_string()); + let use_ = make::use_( + None, + make::use_tree(make::join_paths(vec![super_path, node_path]), None, None, false), + ); + if let Some(item) = ast::Item::cast(use_.syntax().clone()) { + self.body_items.insert(0, item); + } + } + + fn process_use_stmt_for_import_resolve( + &self, + use_stmt_opt: Option, + node_syntax: &SyntaxNode, + ) -> Option<(Vec, Option)> { + if let Some(use_stmt) = use_stmt_opt { + for desc in use_stmt.syntax().descendants() { + if let Some(path_seg) = ast::PathSegment::cast(desc) { + if path_seg.syntax().to_string() == node_syntax.to_string() { + let mut use_tree_str = vec![path_seg.parent_path()]; + get_use_tree_paths_from_path(path_seg.parent_path(), &mut use_tree_str); + for ancs in path_seg.syntax().ancestors() { + //Here we are looking for use_tree with same string value as node + //passed above as the range_to_remove function looks for a comma and + //then includes it in the text range to remove it. But the comma only + //appears at the use_tree level + if let Some(use_tree) = ast::UseTree::cast(ancs) { + if use_tree.syntax().to_string() == node_syntax.to_string() { + return Some(( + use_tree_str, + Some(range_to_remove(use_tree.syntax())), + )); + } + } + } + + return Some((use_tree_str, None)); + } + } + } + } + + None + } +} + +fn does_source_exists_outside_sel_in_same_mod( + def: Definition, + ctx: &AssistContext, + curr_parent_module: &Option, + selection_range: TextRange, + curr_file_id: FileId, +) -> bool { + let mut source_exists_outside_sel_in_same_mod = false; + match def { + Definition::ModuleDef(it) => match it { + ModuleDef::Module(x) => { + let source = x.definition_source(ctx.db()); + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + if let Some(hir_module) = x.parent(ctx.db()) { + have_same_parent = + compare_hir_and_ast_module(&ast_module, hir_module, ctx).is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + match source.value { + ModuleSource::Module(module_) => { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(module_.syntax().text_range()); + } + _ => {} + } + } + } + ModuleDef::Function(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + ModuleDef::Adt(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + ModuleDef::Variant(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + ModuleDef::Const(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + ModuleDef::Static(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + ModuleDef::Trait(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + ModuleDef::TypeAlias(x) => { + if let Some(source) = x.source(ctx.db()) { + let have_same_parent; + if let Some(ast_module) = &curr_parent_module { + have_same_parent = + compare_hir_and_ast_module(&ast_module, x.module(ctx.db()), ctx) + .is_some(); + } else { + let source_file_id = source.file_id.original_file(ctx.db()); + have_same_parent = source_file_id == curr_file_id; + } + + if have_same_parent { + source_exists_outside_sel_in_same_mod = + !selection_range.contains_range(source.value.syntax().text_range()); + } + } + } + _ => {} + }, + _ => {} + } + + return source_exists_outside_sel_in_same_mod; +} + +fn get_replacements_for_visibilty_change( + items: Vec, + is_clone_for_updated: bool, +) -> ( + Vec, + Vec<(Option, SyntaxNode)>, + Vec<(Option, SyntaxNode)>, + Vec, +) { + let mut replacements = Vec::new(); + let mut record_field_parents = Vec::new(); + let mut impls = Vec::new(); + let mut body_items = Vec::new(); + + items.into_iter().for_each(|item| { + let mut item = item; + if !is_clone_for_updated { + item = item.clone_for_update(); + } + body_items.push(item.clone()); + //Use stmts are ignored + match item { + ast::Item::Const(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::Enum(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::ExternCrate(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::Fn(it) => replacements.push((it.visibility().clone(), it.syntax().clone())), + ast::Item::Impl(it) => impls.push(it), + ast::Item::MacroRules(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::MacroDef(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::Module(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::Static(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::Struct(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())); + record_field_parents.push((it.visibility().clone(), it.syntax().clone())); + } + ast::Item::Trait(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::TypeAlias(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())) + } + ast::Item::Union(it) => { + replacements.push((it.visibility().clone(), it.syntax().clone())); + record_field_parents.push((it.visibility().clone(), it.syntax().clone())); + } + _ => (), + } + }); + + return (body_items, replacements, record_field_parents, impls); +} + +fn get_use_tree_paths_from_path( + path: ast::Path, + use_tree_str: &mut Vec, +) -> Option<&mut Vec> { + path.syntax().ancestors().filter(|x| x.to_string() != path.to_string()).find_map(|x| { + if let Some(use_tree) = ast::UseTree::cast(x.clone()) { + if let Some(upper_tree_path) = use_tree.path() { + if upper_tree_path.to_string() != path.to_string() { + use_tree_str.push(upper_tree_path.clone()); + get_use_tree_paths_from_path(upper_tree_path, use_tree_str); + return Some(use_tree); + } + } + } + None + })?; + + Some(use_tree_str) +} + +fn add_change_vis( + vis: Option, + node_or_token_opt: Option, +) -> Option<()> { + if let Some(vis) = vis { + if vis.syntax().text() == "pub" { + ted::replace(vis.syntax(), make::visibility_pub_crate().syntax().clone_for_update()); + } + } else { + if let Some(node_or_token) = node_or_token_opt { + let pub_crate_vis = make::visibility_pub_crate().clone_for_update(); + if let Some(node) = node_or_token.as_node() { + ted::insert(ted::Position::before(node), pub_crate_vis.syntax()); + } + if let Some(token) = node_or_token.as_token() { + ted::insert(ted::Position::before(token), pub_crate_vis.syntax()); + } + } + } + + Some(()) +} + +fn compare_hir_and_ast_module( + ast_module: &ast::Module, + hir_module: hir::Module, + ctx: &AssistContext, +) -> Option<()> { + let hir_mod_name = hir_module.name(ctx.db())?; + let ast_mod_name = ast_module.name()?; + if hir_mod_name.to_string() != ast_mod_name.to_string() { + return None; + } + + return Some(()); +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn test_not_applicable_without_selection() { + check_assist_not_applicable( + extract_module, + r" + $0pub struct PublicStruct { + field: i32, + } + ", + ) + } + + #[test] + fn test_extract_module() { + check_assist( + extract_module, + r" + mod thirdpartycrate { + pub mod nest { + pub struct SomeType; + pub struct SomeType2; + } + pub struct SomeType1; + } + + mod bar { + use crate::thirdpartycrate::{nest::{SomeType, SomeType2}, SomeType1}; + + pub struct PublicStruct { + field: PrivateStruct, + field1: SomeType1, + } + + impl PublicStruct { + pub fn new() -> Self { + Self { field: PrivateStruct::new(), field1: SomeType1 } + } + } + + fn foo() { + let _s = PrivateStruct::new(); + let _a = bar(); + } + + $0 + struct PrivateStruct { + inner: SomeType, + } + + pub struct PrivateStruct1 { + pub inner: i32, + } + + impl PrivateStruct { + fn new() -> Self { + PrivateStruct { inner: SomeType } + } + } + + fn bar() -> i32 { + 2 + } + $0 + } + ", + r" + mod thirdpartycrate { + pub mod nest { + pub struct SomeType; + pub struct SomeType2; + } + pub struct SomeType1; + } + + mod bar { + use crate::thirdpartycrate::{nest::{SomeType2}, SomeType1}; + + pub struct PublicStruct { + field: modname::PrivateStruct, + field1: SomeType1, + } + + impl PublicStruct { + pub fn new() -> Self { + Self { field: modname::PrivateStruct::new(), field1: SomeType1 } + } + } + + fn foo() { + let _s = modname::PrivateStruct::new(); + let _a = modname::bar(); + } + + mod modname { + use crate::thirdpartycrate::nest::SomeType; + + pub(crate) struct PrivateStruct { + pub(crate) inner: SomeType, + } + + pub(crate) struct PrivateStruct1 { + pub(crate) inner: i32, + } + + impl PrivateStruct { + pub(crate) fn new() -> Self { + PrivateStruct { inner: SomeType } + } + } + + pub(crate) fn bar() -> i32 { + 2 + } + } + } + ", + ); + } + + #[test] + fn test_extract_module_for_function_only() { + check_assist( + extract_module, + r" + $0 + fn foo(name: i32) -> i32 { + name + 1 + } + $0 + + fn bar(name: i32) -> i32 { + name + 2 + } + ", + r" + mod modname { + pub(crate) fn foo(name: i32) -> i32 { + name + 1 + } + } + + fn bar(name: i32) -> i32 { + name + 2 + } + ", + ) + } + + #[test] + fn test_extract_module_for_impl_having_corresponding_adt_in_selection() { + check_assist( + extract_module, + r" + mod impl_play { + $0 + struct A {} + + impl A { + pub fn new_a() -> i32 { + 2 + } + } + $0 + + fn a() { + let _a = A::new_a(); + } + } + ", + r" + mod impl_play { + mod modname { + pub(crate) struct A {} + + impl A { + pub(crate) fn new_a() -> i32 { + 2 + } + } + } + + fn a() { + let _a = modname::A::new_a(); + } + } + ", + ) + } + + #[test] + fn test_import_resolve_when_its_only_inside_selection() { + check_assist( + extract_module, + r" + mod foo { + pub struct PrivateStruct; + pub struct PrivateStruct1; + } + + mod bar { + use super::foo::{PrivateStruct, PrivateStruct1}; + + $0 + struct Strukt { + field: PrivateStruct, + } + $0 + + struct Strukt1 { + field: PrivateStruct1, + } + } + ", + r" + mod foo { + pub struct PrivateStruct; + pub struct PrivateStruct1; + } + + mod bar { + use super::foo::{PrivateStruct1}; + + mod modname { + use super::super::foo::PrivateStruct; + + pub(crate) struct Strukt { + pub(crate) field: PrivateStruct, + } + } + + struct Strukt1 { + field: PrivateStruct1, + } + } + ", + ) + } + + #[test] + fn test_import_resolve_when_its_inside_and_outside_selection_and_source_not_in_same_mod() { + check_assist( + extract_module, + r" + mod foo { + pub struct PrivateStruct; + } + + mod bar { + use super::foo::PrivateStruct; + + $0 + struct Strukt { + field: PrivateStruct, + } + $0 + + struct Strukt1 { + field: PrivateStruct, + } + } + ", + r" + mod foo { + pub struct PrivateStruct; + } + + mod bar { + use super::foo::PrivateStruct; + + mod modname { + use super::super::foo::PrivateStruct; + + pub(crate) struct Strukt { + pub(crate) field: PrivateStruct, + } + } + + struct Strukt1 { + field: PrivateStruct, + } + } + ", + ) + } + + #[test] + fn test_import_resolve_when_its_inside_and_outside_selection_and_source_is_in_same_mod() { + check_assist( + extract_module, + r" + mod bar { + pub struct PrivateStruct; + + $0 + struct Strukt { + field: PrivateStruct, + } + $0 + + struct Strukt1 { + field: PrivateStruct, + } + } + ", + r" + mod bar { + pub struct PrivateStruct; + + mod modname { + use super::PrivateStruct; + + pub(crate) struct Strukt { + pub(crate) field: PrivateStruct, + } + } + + struct Strukt1 { + field: PrivateStruct, + } + } + ", + ) + } + + #[test] + fn test_extract_module_for_correspoding_adt_of_impl_present_in_same_mod_but_not_in_selection() { + check_assist( + extract_module, + r" + mod impl_play { + struct A {} + + $0 + impl A { + pub fn new_a() -> i32 { + 2 + } + } + $0 + + fn a() { + let _a = A::new_a(); + } + } + ", + r" + mod impl_play { + struct A {} + + mod modname { + use super::A; + + impl A { + pub(crate) fn new_a() -> i32 { + 2 + } + } + } + + fn a() { + let _a = A::new_a(); + } + } + ", + ) + } + + #[test] + fn test_extract_module_for_impl_not_having_corresponding_adt_in_selection_and_not_in_same_mod_but_with_super( + ) { + check_assist( + extract_module, + r" + mod foo { + pub struct A {} + } + mod impl_play { + use super::foo::A; + + $0 + impl A { + pub fn new_a() -> i32 { + 2 + } + } + $0 + + fn a() { + let _a = A::new_a(); + } + } + ", + r" + mod foo { + pub struct A {} + } + mod impl_play { + use super::foo::A; + + mod modname { + use super::super::foo::A; + + impl A { + pub(crate) fn new_a() -> i32 { + 2 + } + } + } + + fn a() { + let _a = A::new_a(); + } + } + ", + ) + } + + #[test] + fn test_import_resolve_for_trait_bounds_on_function() { + check_assist( + extract_module, + r" + mod impl_play2 { + trait JustATrait {} + + $0 + struct A {} + + fn foo(arg: T) -> T { + arg + } + + impl JustATrait for A {} + + fn bar() { + let a = A {}; + foo(a); + } + $0 + } + ", + r" + mod impl_play2 { + trait JustATrait {} + + mod modname { + use super::JustATrait; + + pub(crate) struct A {} + + pub(crate) fn foo(arg: T) -> T { + arg + } + + impl JustATrait for A {} + + pub(crate) fn bar() { + let a = A {}; + foo(a); + } + } + } + ", + ) + } + + #[test] + fn test_extract_module_for_module() { + check_assist( + extract_module, + r" + mod impl_play2 { + $0 + mod impl_play { + pub struct A {} + } + $0 + } + ", + r" + mod impl_play2 { + mod modname { + pub(crate) mod impl_play { + pub struct A {} + } + } + } + ", + ) + } + + #[test] + fn test_extract_module_with_multiple_files() { + check_assist( + extract_module, + r" + //- /main.rs + mod foo; + + use foo::PrivateStruct; + + pub struct Strukt { + field: PrivateStruct, + } + + fn main() { + $0 + struct Strukt1 { + field: Strukt, + } + $0 + } + //- /foo.rs + pub struct PrivateStruct; + ", + r" + mod foo; + + use foo::PrivateStruct; + + pub struct Strukt { + field: PrivateStruct, + } + + fn main() { + mod modname { + use super::Strukt; + + pub(crate) struct Strukt1 { + pub(crate) field: Strukt, + } + } + } + ", + ) + } +} diff --git a/crates/ide_assists/src/handlers/remove_unused_param.rs b/crates/ide_assists/src/handlers/remove_unused_param.rs index 545809a71d..5210166738 100644 --- a/crates/ide_assists/src/handlers/remove_unused_param.rs +++ b/crates/ide_assists/src/handlers/remove_unused_param.rs @@ -140,7 +140,7 @@ fn process_usage( None } -fn range_to_remove(node: &SyntaxNode) -> TextRange { +pub(crate) fn range_to_remove(node: &SyntaxNode) -> TextRange { let up_to_comma = next_prev().find_map(|dir| { node.siblings_with_tokens(dir) .filter_map(|it| it.into_token()) diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index ea2c19b508..1d72b5c0f9 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -122,6 +122,7 @@ mod handlers { mod destructure_tuple_binding; mod expand_glob_import; mod extract_function; + mod extract_module; mod extract_struct_from_enum_variant; mod extract_type_alias; mod extract_variable; @@ -273,6 +274,7 @@ mod handlers { // extract_variable::extract_variable, extract_function::extract_function, + extract_module::extract_module, // generate_getter::generate_getter, generate_getter::generate_getter_mut, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 25acd53482..9cf20e716e 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -526,6 +526,35 @@ fn $0fun_name(n: i32) { ) } +#[test] +fn doctest_extract_module() { + check_doc_test( + "extract_module", + r#####" +$0 +fn foo(name: i32) -> i32 { + name + 1 +} +$0 + +fn bar(name: i32) -> i32 { + name + 2 +} +"#####, + r#####" +mod modname { + pub(crate) fn foo(name: i32) -> i32 { + name + 1 + } +} + +fn bar(name: i32) -> i32 { + name + 2 +} +"#####, + ) +} + #[test] fn doctest_extract_struct_from_enum_variant() { check_doc_test( diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 14faf9182d..ed69973af9 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -196,6 +196,12 @@ pub fn path_from_segments( format!("use {};", segments) }) } + +pub fn join_paths(paths: impl IntoIterator) -> ast::Path { + let paths = paths.into_iter().map(|it| it.syntax().clone()).join("::"); + ast_from_text(&format!("use {};", paths)) +} + // FIXME: should not be pub pub fn path_from_text(text: &str) -> ast::Path { ast_from_text(&format!("fn main() {{ let test = {}; }}", text)) From 227490c06935d7f901b027621ef77d85aa7cac8a Mon Sep 17 00:00:00 2001 From: vi_mi Date: Tue, 12 Oct 2021 06:00:15 +0000 Subject: [PATCH 2/5] fix: arbitary noop of assist and same file double writes --- .../src/handlers/extract_module.rs | 108 +++++++++--------- 1 file changed, 57 insertions(+), 51 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_module.rs b/crates/ide_assists/src/handlers/extract_module.rs index 79c5354f84..fbc66d1bf4 100644 --- a/crates/ide_assists/src/handlers/extract_module.rs +++ b/crates/ide_assists/src/handlers/extract_module.rs @@ -73,63 +73,69 @@ pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<( let old_item_indent = module.body_items[0].indent_level(); + //This takes place in three steps: + // + //- Firstly, we will update the references(usages) e.g. converting a + // function call bar() to modname::bar(), and similarly for other items + // + //- Secondly, changing the visibility of each item inside the newly selected module + // i.e. making a fn a() {} to pub(crate) fn a() {} + // + //- Thirdly, resolving all the imports this includes removing paths from imports + // outside the module, shifting/cloning them inside new module, or shifting the imports, or making + // new import statemnts + + //We are getting item usages and record_fields together, record_fields + //for change_visibility and usages for first point mentioned above in the process + let (usages_to_be_processed, record_fields) = module.get_usages_and_record_fields(ctx); + + let import_paths_to_be_removed = module.resolve_imports(curr_parent_module, &ctx); + module.body_items = module.change_visibility(record_fields)?; + if module.body_items.len() == 0 { + return None; + } + acc.add( AssistId("extract_module", AssistKind::RefactorExtract), "Extract Module", module.text_range, |builder| { - //This takes place in three steps: - // - //- Firstly, we will update the references(usages) e.g. converting a - // function call bar() to modname::bar(), and similarly for other items - // - //- Secondly, changing the visibility of each item inside the newly selected module - // i.e. making a fn a() {} to pub(crate) fn a() {} - // - //- Thirdly, resolving all the imports this includes removing paths from imports - // outside the module, shifting/cloning them inside new module, or shifting the imports, or making - // new import statemnts - - //We are getting item usages and record_fields together, record_fields - //for change_visibility and usages for first point mentioned above in the process - let (usages_to_be_processed, record_fields) = module.get_usages_and_record_fields(ctx); - - let import_paths_to_be_removed = module.resolve_imports(curr_parent_module, &ctx); - - if let Some(block_items) = module.change_visibility(record_fields) { - module.body_items = block_items; - if module.body_items.len() == 0 { - return; - } - - let mut body_items = Vec::new(); - let new_item_indent = old_item_indent + 1; - for item in module.body_items { - let item = item.indent(IndentLevel(1)); - let mut indented_item = String::new(); - format_to!(indented_item, "{}{}", new_item_indent, item.to_string()); - body_items.push(indented_item); - } - - let body = body_items.join("\n\n"); - - let mut module_def = String::new(); - - format_to!(module_def, "mod {} {{\n{}\n{}}}", module.name, body, old_item_indent); - - for usages_to_be_updated_for_file in usages_to_be_processed { - builder.edit_file(usages_to_be_updated_for_file.0); - for usage_to_be_processed in usages_to_be_updated_for_file.1 { - builder.replace(usage_to_be_processed.0, usage_to_be_processed.1) - } - } - - builder.edit_file(ctx.frange.file_id); - for import_path_text_range in import_paths_to_be_removed { - builder.delete(import_path_text_range); - } - builder.replace(module.text_range, module_def) + let mut body_items = Vec::new(); + let new_item_indent = old_item_indent + 1; + for item in module.body_items { + let item = item.indent(IndentLevel(1)); + let mut indented_item = String::new(); + format_to!(indented_item, "{}{}", new_item_indent, item.to_string()); + body_items.push(indented_item); } + + let body = body_items.join("\n\n"); + + let mut module_def = String::new(); + + format_to!(module_def, "mod {} {{\n{}\n{}}}", module.name, body, old_item_indent); + + let mut usages_to_be_updated_for_curr_file = vec![]; + for usages_to_be_updated_for_file in usages_to_be_processed { + if usages_to_be_updated_for_file.0 == ctx.frange.file_id { + usages_to_be_updated_for_curr_file = usages_to_be_updated_for_file.1; + continue; + } + builder.edit_file(usages_to_be_updated_for_file.0); + for usage_to_be_processed in usages_to_be_updated_for_file.1 { + builder.replace(usage_to_be_processed.0, usage_to_be_processed.1) + } + } + + builder.edit_file(ctx.frange.file_id); + for usage_to_be_processed in usages_to_be_updated_for_curr_file { + builder.replace(usage_to_be_processed.0, usage_to_be_processed.1) + } + + for import_path_text_range in import_paths_to_be_removed { + builder.delete(import_path_text_range); + } + builder.replace(module.text_range, module_def) }, ) } From 2efcff7f75a9a88d50600d4208db6b33be2705e3 Mon Sep 17 00:00:00 2001 From: vi_mi Date: Tue, 12 Oct 2021 07:58:10 +0000 Subject: [PATCH 3/5] fix: Adding tuple fields in ADT, chore: test action section unindentation --- .../src/handlers/extract_module.rs | 361 +++++++++--------- 1 file changed, 190 insertions(+), 171 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_module.rs b/crates/ide_assists/src/handlers/extract_module.rs index fbc66d1bf4..e466b3990e 100644 --- a/crates/ide_assists/src/handlers/extract_module.rs +++ b/crates/ide_assists/src/handlers/extract_module.rs @@ -100,6 +100,8 @@ pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<( "Extract Module", module.text_range, |builder| { + let _ = &module; + let mut body_items = Vec::new(); let new_item_indent = old_item_indent + 1; for item in module.body_items { @@ -148,7 +150,7 @@ struct Module { } fn extract_target(node: &SyntaxNode, selection_range: TextRange) -> Option { - let body_items: Vec = node + let mut body_items: Vec = node .children() .filter_map(|child| { if let Some(item) = ast::Item::cast(child.clone()) { @@ -161,6 +163,10 @@ fn extract_target(node: &SyntaxNode, selection_range: TextRange) -> Option (HashMap>, Vec) { - let mut record_fields = Vec::new(); + let mut adt_fields = Vec::new(); let mut refs: HashMap> = HashMap::new(); //Here impl is not included as each item inside impl will be tied to the parent of @@ -183,18 +189,31 @@ impl Module { let node_def = Definition::ModuleDef(nod.into()); self.expand_and_group_usages_file_wise(ctx, node_def, &mut refs); - let mut get_record_fields = |it: ast::Adt| { - for desc in it.syntax().descendants() { - if let Some(record_field) = ast::RecordField::cast(desc) { - record_fields.push(record_field.syntax().clone()); - } - } - }; - //Enum Fields are not allowed to explicitly specify pub, it is implied match it { - ast::Adt::Struct(_) => get_record_fields(it), - ast::Adt::Union(_) => get_record_fields(it), + ast::Adt::Struct(x) => { + if let Some(field_list) = x.field_list() { + match field_list { + ast::FieldList::RecordFieldList(record_field_list) => { + record_field_list.fields().for_each(|record_field| { + adt_fields.push(record_field.syntax().clone()); + }); + }, + ast::FieldList::TupleFieldList(tuple_field_list) => { + tuple_field_list.fields().for_each(|tuple_field| { + adt_fields.push(tuple_field.syntax().clone()); + }); + }, + } + } + }, + ast::Adt::Union(x) => { + if let Some(record_field_list) = x.record_field_list() { + record_field_list.fields().for_each(|record_field| { + adt_fields.push(record_field.syntax().clone()); + }); + } + }, ast::Adt::Enum(_) => {}, } } @@ -228,7 +247,7 @@ impl Module { } }); - return (refs, record_fields); + return (refs, adt_fields); } fn expand_and_group_usages_file_wise( @@ -881,9 +900,9 @@ mod tests { check_assist_not_applicable( extract_module, r" - $0pub struct PublicStruct { - field: i32, - } +$0pub struct PublicStruct { + field: i32, +} ", ) } @@ -920,25 +939,25 @@ mod tests { let _a = bar(); } - $0 - struct PrivateStruct { - inner: SomeType, - } +$0 +struct PrivateStruct { + inner: SomeType, +} - pub struct PrivateStruct1 { - pub inner: i32, - } +pub struct PrivateStruct1 { + pub inner: i32, +} - impl PrivateStruct { - fn new() -> Self { - PrivateStruct { inner: SomeType } - } - } +impl PrivateStruct { + fn new() -> Self { + PrivateStruct { inner: SomeType } + } +} - fn bar() -> i32 { - 2 - } - $0 +fn bar() -> i32 { + 2 +} +$0 } ", r" @@ -969,27 +988,27 @@ mod tests { let _a = modname::bar(); } - mod modname { - use crate::thirdpartycrate::nest::SomeType; +mod modname { + use crate::thirdpartycrate::nest::SomeType; - pub(crate) struct PrivateStruct { - pub(crate) inner: SomeType, - } + pub(crate) struct PrivateStruct { + pub(crate) inner: SomeType, + } - pub(crate) struct PrivateStruct1 { - pub(crate) inner: i32, - } + pub(crate) struct PrivateStruct1 { + pub(crate) inner: i32, + } - impl PrivateStruct { - pub(crate) fn new() -> Self { - PrivateStruct { inner: SomeType } - } - } + impl PrivateStruct { + pub(crate) fn new() -> Self { + PrivateStruct { inner: SomeType } + } + } - pub(crate) fn bar() -> i32 { - 2 - } - } + pub(crate) fn bar() -> i32 { + 2 + } +} } ", ); @@ -1000,22 +1019,22 @@ mod tests { check_assist( extract_module, r" - $0 - fn foo(name: i32) -> i32 { - name + 1 - } - $0 +$0 +fn foo(name: i32) -> i32 { + name + 1 +} +$0 fn bar(name: i32) -> i32 { name + 2 } ", r" - mod modname { - pub(crate) fn foo(name: i32) -> i32 { - name + 1 - } - } +mod modname { + pub(crate) fn foo(name: i32) -> i32 { + name + 1 + } +} fn bar(name: i32) -> i32 { name + 2 @@ -1030,15 +1049,15 @@ mod tests { extract_module, r" mod impl_play { - $0 - struct A {} +$0 +struct A {} - impl A { - pub fn new_a() -> i32 { - 2 - } - } - $0 +impl A { + pub fn new_a() -> i32 { + 2 + } +} +$0 fn a() { let _a = A::new_a(); @@ -1047,15 +1066,15 @@ mod tests { ", r" mod impl_play { - mod modname { - pub(crate) struct A {} +mod modname { + pub(crate) struct A {} - impl A { - pub(crate) fn new_a() -> i32 { - 2 - } - } - } + impl A { + pub(crate) fn new_a() -> i32 { + 2 + } + } +} fn a() { let _a = modname::A::new_a(); @@ -1078,11 +1097,11 @@ mod tests { mod bar { use super::foo::{PrivateStruct, PrivateStruct1}; - $0 - struct Strukt { - field: PrivateStruct, - } - $0 +$0 +struct Strukt { + field: PrivateStruct, +} +$0 struct Strukt1 { field: PrivateStruct1, @@ -1098,13 +1117,13 @@ mod tests { mod bar { use super::foo::{PrivateStruct1}; - mod modname { - use super::super::foo::PrivateStruct; +mod modname { + use super::super::foo::PrivateStruct; - pub(crate) struct Strukt { - pub(crate) field: PrivateStruct, - } - } + pub(crate) struct Strukt { + pub(crate) field: PrivateStruct, + } +} struct Strukt1 { field: PrivateStruct1, @@ -1126,11 +1145,11 @@ mod tests { mod bar { use super::foo::PrivateStruct; - $0 - struct Strukt { - field: PrivateStruct, - } - $0 +$0 +struct Strukt { + field: PrivateStruct, +} +$0 struct Strukt1 { field: PrivateStruct, @@ -1145,13 +1164,13 @@ mod tests { mod bar { use super::foo::PrivateStruct; - mod modname { - use super::super::foo::PrivateStruct; +mod modname { + use super::super::foo::PrivateStruct; - pub(crate) struct Strukt { - pub(crate) field: PrivateStruct, - } - } + pub(crate) struct Strukt { + pub(crate) field: PrivateStruct, + } +} struct Strukt1 { field: PrivateStruct, @@ -1169,11 +1188,11 @@ mod tests { mod bar { pub struct PrivateStruct; - $0 - struct Strukt { - field: PrivateStruct, - } - $0 +$0 +struct Strukt { + field: PrivateStruct, +} +$0 struct Strukt1 { field: PrivateStruct, @@ -1184,13 +1203,13 @@ mod tests { mod bar { pub struct PrivateStruct; - mod modname { - use super::PrivateStruct; +mod modname { + use super::PrivateStruct; - pub(crate) struct Strukt { - pub(crate) field: PrivateStruct, - } - } + pub(crate) struct Strukt { + pub(crate) field: PrivateStruct, + } +} struct Strukt1 { field: PrivateStruct, @@ -1208,13 +1227,13 @@ mod tests { mod impl_play { struct A {} - $0 - impl A { - pub fn new_a() -> i32 { - 2 - } - } - $0 +$0 +impl A { + pub fn new_a() -> i32 { + 2 + } +} +$0 fn a() { let _a = A::new_a(); @@ -1225,15 +1244,15 @@ mod tests { mod impl_play { struct A {} - mod modname { - use super::A; +mod modname { + use super::A; - impl A { - pub(crate) fn new_a() -> i32 { - 2 - } - } - } + impl A { + pub(crate) fn new_a() -> i32 { + 2 + } + } +} fn a() { let _a = A::new_a(); @@ -1255,13 +1274,13 @@ mod tests { mod impl_play { use super::foo::A; - $0 - impl A { - pub fn new_a() -> i32 { - 2 - } - } - $0 +$0 +impl A { + pub fn new_a() -> i32 { + 2 + } +} +$0 fn a() { let _a = A::new_a(); @@ -1275,15 +1294,15 @@ mod tests { mod impl_play { use super::foo::A; - mod modname { - use super::super::foo::A; +mod modname { + use super::super::foo::A; - impl A { - pub(crate) fn new_a() -> i32 { - 2 - } - } - } + impl A { + pub(crate) fn new_a() -> i32 { + 2 + } + } +} fn a() { let _a = A::new_a(); @@ -1301,42 +1320,42 @@ mod tests { mod impl_play2 { trait JustATrait {} - $0 - struct A {} +$0 +struct A {} - fn foo(arg: T) -> T { - arg - } +fn foo(arg: T) -> T { + arg +} - impl JustATrait for A {} +impl JustATrait for A {} - fn bar() { - let a = A {}; - foo(a); - } - $0 +fn bar() { + let a = A {}; + foo(a); +} +$0 } ", r" mod impl_play2 { trait JustATrait {} - mod modname { - use super::JustATrait; +mod modname { + use super::JustATrait; - pub(crate) struct A {} + pub(crate) struct A {} - pub(crate) fn foo(arg: T) -> T { - arg - } + pub(crate) fn foo(arg: T) -> T { + arg + } - impl JustATrait for A {} + impl JustATrait for A {} - pub(crate) fn bar() { - let a = A {}; - foo(a); - } - } + pub(crate) fn bar() { + let a = A {}; + foo(a); + } +} } ", ) @@ -1348,20 +1367,20 @@ mod tests { extract_module, r" mod impl_play2 { - $0 - mod impl_play { - pub struct A {} - } - $0 +$0 +mod impl_play { + pub struct A {} +} +$0 } ", r" mod impl_play2 { - mod modname { - pub(crate) mod impl_play { - pub struct A {} - } - } +mod modname { + pub(crate) mod impl_play { + pub struct A {} + } +} } ", ) From 2bf5f14666084632e25d1af86cd0fea6792d60e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Sat, 16 Oct 2021 13:39:55 +0300 Subject: [PATCH 4/5] Use trimmed selection range --- .../ide_assists/src/handlers/extract_module.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_module.rs b/crates/ide_assists/src/handlers/extract_module.rs index e466b3990e..dbff8f3d3e 100644 --- a/crates/ide_assists/src/handlers/extract_module.rs +++ b/crates/ide_assists/src/handlers/extract_module.rs @@ -51,7 +51,7 @@ use super::remove_unused_param::range_to_remove; // } // ``` pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - if ctx.frange.range.is_empty() { + if ctx.has_empty_selection() { return None; } @@ -66,7 +66,7 @@ pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<( curr_parent_module = ast::Module::cast(mod_syn_opt); } - let mut module = extract_target(&node, ctx.frange.range)?; + let mut module = extract_target(&node, ctx.selection_trimmed())?; if module.body_items.len() == 0 { return None; } @@ -119,7 +119,7 @@ pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<( let mut usages_to_be_updated_for_curr_file = vec![]; for usages_to_be_updated_for_file in usages_to_be_processed { - if usages_to_be_updated_for_file.0 == ctx.frange.file_id { + if usages_to_be_updated_for_file.0 == ctx.file_id() { usages_to_be_updated_for_curr_file = usages_to_be_updated_for_file.1; continue; } @@ -129,7 +129,7 @@ pub(crate) fn extract_module(acc: &mut Assists, ctx: &AssistContext) -> Option<( } } - builder.edit_file(ctx.frange.file_id); + builder.edit_file(ctx.file_id()); for usage_to_be_processed in usages_to_be_updated_for_curr_file { builder.replace(usage_to_be_processed.0, usage_to_be_processed.1) } @@ -426,11 +426,11 @@ impl Module { ctx: &AssistContext, ) -> Option { //We only need to find in the current file - let selection_range = ctx.frange.range; - let search_scope = SearchScope::single_file(ctx.frange.file_id); + let selection_range = ctx.selection_trimmed(); + let curr_file_id = ctx.file_id(); + let search_scope = SearchScope::single_file(curr_file_id); let usage_res = def.usages(&ctx.sema).in_scope(search_scope).all(); - let curr_file_id = ctx.frange.file_id; - let file = ctx.sema.parse(ctx.frange.file_id); + let file = ctx.sema.parse(curr_file_id); let mut exists_inside_sel = false; let mut exists_outside_sel = false; @@ -470,7 +470,7 @@ impl Module { let use_stmt_opt: Option = usage_res.into_iter().find_map(|x| { let file_id = x.0; let mut use_opt: Option = None; - if file_id == ctx.frange.file_id { + if file_id == curr_file_id { (&x.1).into_iter().for_each(|x| { let node_opt: Option = find_node_at_range(file.syntax(), x.range); if let Some(node) = node_opt { From 3e73a46660b234ef1943a0cbb4348f4c75eb8ed6 Mon Sep 17 00:00:00 2001 From: vi_mi Date: Fri, 22 Oct 2021 08:55:47 +0000 Subject: [PATCH 5/5] fix: making tests compatible with new trimmed sel_range --- .../src/handlers/extract_module.rs | 72 +++++++------------ crates/ide_assists/src/tests/generated.rs | 6 +- 2 files changed, 26 insertions(+), 52 deletions(-) diff --git a/crates/ide_assists/src/handlers/extract_module.rs b/crates/ide_assists/src/handlers/extract_module.rs index dbff8f3d3e..dc0683866c 100644 --- a/crates/ide_assists/src/handlers/extract_module.rs +++ b/crates/ide_assists/src/handlers/extract_module.rs @@ -28,11 +28,9 @@ use super::remove_unused_param::range_to_remove; // resolved. // // ``` -// $0 -// fn foo(name: i32) -> i32 { +// $0fn foo(name: i32) -> i32 { // name + 1 -// } -// $0 +// }$0 // // fn bar(name: i32) -> i32 { // name + 2 @@ -939,8 +937,7 @@ $0pub struct PublicStruct { let _a = bar(); } -$0 -struct PrivateStruct { +$0struct PrivateStruct { inner: SomeType, } @@ -956,8 +953,7 @@ impl PrivateStruct { fn bar() -> i32 { 2 -} -$0 +}$0 } ", r" @@ -1019,11 +1015,9 @@ mod modname { check_assist( extract_module, r" -$0 -fn foo(name: i32) -> i32 { +$0fn foo(name: i32) -> i32 { name + 1 -} -$0 +}$0 fn bar(name: i32) -> i32 { name + 2 @@ -1049,15 +1043,13 @@ mod modname { extract_module, r" mod impl_play { -$0 -struct A {} +$0struct A {} impl A { pub fn new_a() -> i32 { 2 } -} -$0 +}$0 fn a() { let _a = A::new_a(); @@ -1097,11 +1089,9 @@ mod modname { mod bar { use super::foo::{PrivateStruct, PrivateStruct1}; -$0 -struct Strukt { +$0struct Strukt { field: PrivateStruct, -} -$0 +}$0 struct Strukt1 { field: PrivateStruct1, @@ -1145,11 +1135,9 @@ mod modname { mod bar { use super::foo::PrivateStruct; -$0 -struct Strukt { +$0struct Strukt { field: PrivateStruct, -} -$0 +}$0 struct Strukt1 { field: PrivateStruct, @@ -1188,11 +1176,9 @@ mod modname { mod bar { pub struct PrivateStruct; -$0 -struct Strukt { +$0struct Strukt { field: PrivateStruct, -} -$0 +}$0 struct Strukt1 { field: PrivateStruct, @@ -1227,13 +1213,11 @@ mod modname { mod impl_play { struct A {} -$0 -impl A { +$0impl A { pub fn new_a() -> i32 { 2 } -} -$0 +}$0 fn a() { let _a = A::new_a(); @@ -1274,13 +1258,11 @@ mod modname { mod impl_play { use super::foo::A; -$0 -impl A { +$0impl A { pub fn new_a() -> i32 { 2 } -} -$0 +}$0 fn a() { let _a = A::new_a(); @@ -1320,8 +1302,7 @@ mod modname { mod impl_play2 { trait JustATrait {} -$0 -struct A {} +$0struct A {} fn foo(arg: T) -> T { arg @@ -1332,8 +1313,7 @@ impl JustATrait for A {} fn bar() { let a = A {}; foo(a); -} -$0 +}$0 } ", r" @@ -1367,11 +1347,9 @@ mod modname { extract_module, r" mod impl_play2 { -$0 -mod impl_play { +$0mod impl_play { pub struct A {} -} -$0 +}$0 } ", r" @@ -1401,11 +1379,9 @@ mod modname { } fn main() { - $0 - struct Strukt1 { + $0struct Strukt1 { field: Strukt, - } - $0 + }$0 } //- /foo.rs pub struct PrivateStruct; diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 9cf20e716e..5f801e5d1a 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -531,11 +531,9 @@ fn doctest_extract_module() { check_doc_test( "extract_module", r#####" -$0 -fn foo(name: i32) -> i32 { +$0fn foo(name: i32) -> i32 { name + 1 -} -$0 +}$0 fn bar(name: i32) -> i32 { name + 2