From 5cd4eb6dd6d8c733077a6aeea5d2cc0812ded096 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Fri, 22 May 2020 22:28:30 +0200 Subject: Add preliminary implementation of extract struct from enum variant --- crates/ra_assists/Cargo.toml | 1 + crates/ra_assists/src/assist_context.rs | 62 +++- .../handlers/extract_struct_from_enum_variant.rs | 338 +++++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 + 4 files changed, 402 insertions(+), 1 deletion(-) create mode 100644 crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml index 3bcf58ba4..f3481bdeb 100644 --- a/crates/ra_assists/Cargo.toml +++ b/crates/ra_assists/Cargo.toml @@ -20,5 +20,6 @@ ra_fmt = { path = "../ra_fmt" } ra_prof = { path = "../ra_prof" } ra_db = { path = "../ra_db" } ra_ide_db = { path = "../ra_ide_db" } +hir_expand = { path = "../ra_hir_expand", package = "ra_hir_expand" } hir = { path = "../ra_hir", package = "ra_hir" } test_utils = { path = "../test_utils" } diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 5b1a4680b..6291c68de 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -2,7 +2,7 @@ use algo::find_covering_element; use hir::Semantics; -use ra_db::{FileId, FileRange}; +use ra_db::{FileId, FileRange, FilePosition}; use ra_fmt::{leading_indent, reindent}; use ra_ide_db::{ source_change::{SourceChange, SourceFileEdit}, @@ -19,6 +19,7 @@ use crate::{ assist_config::{AssistConfig, SnippetCap}, Assist, AssistId, GroupLabel, ResolvedAssist, }; +use rustc_hash::FxHashMap; /// `AssistContext` allows to apply an assist or check if it could be applied. /// @@ -138,6 +139,16 @@ impl Assists { let label = Assist::new(id, label.into(), None, target); self.add_impl(label, f) } + pub(crate) fn add_in_multiple_files( + &mut self, + id: AssistId, + label: impl Into, + target: TextRange, + f: impl FnOnce(&mut AssistDirector), + ) -> Option<()> { + let label = Assist::new(id, label.into(), None, target); + self.add_impl_multiple_files(label, f) + } pub(crate) fn add_group( &mut self, group: &GroupLabel, @@ -162,6 +173,27 @@ impl Assists { Some(()) } + fn add_impl_multiple_files(&mut self, label: Assist, f: impl FnOnce(&mut AssistDirector)) -> Option<()> { + let change_label = label.label.clone(); + if !self.resolve { + return None + } + let mut director = AssistDirector::new(change_label.clone()); + f(&mut director); + let changes = director.finish(); + let file_edits: Vec = changes.into_iter() + .map(|mut change| change.source_file_edits.pop().unwrap()).collect(); + + let source_change = SourceChange { + source_file_edits: file_edits, + file_system_edits: vec![], + is_snippet: false, + }; + + self.buf.push((label, Some(source_change))); + Some(()) + } + fn finish(mut self) -> Vec<(Assist, Option)> { self.buf.sort_by_key(|(label, _edit)| label.target.len()); self.buf @@ -255,3 +287,31 @@ impl AssistBuilder { res } } + +pub(crate) struct AssistDirector { + source_changes: Vec, + builders: FxHashMap, + change_label: String +} + +impl AssistDirector { + fn new(change_label: String) -> AssistDirector { + AssistDirector { + source_changes: vec![], + builders: FxHashMap::default(), + change_label + } + } + + pub(crate) fn perform(&mut self, file_id: FileId, f: impl FnOnce(&mut AssistBuilder)) { + let mut builder = self.builders.entry(file_id).or_insert(AssistBuilder::new(file_id)); + f(&mut builder); + } + + fn finish(mut self) -> Vec { + for (file_id, builder) in self.builders.into_iter().collect::>() { + self.source_changes.push(builder.finish()); + } + self.source_changes + } +} diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs new file mode 100644 index 000000000..6e19a6feb --- /dev/null +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -0,0 +1,338 @@ +use hir_expand::name::AsName; +use ra_ide_db::{ + defs::Definition, imports_locator::ImportsLocator, search::Reference, RootDatabase, +}; +use ra_syntax::{ + algo::find_node_at_offset, + ast::{self, AstNode, NameOwner}, + SourceFile, SyntaxNode, TextRange, TextSize, +}; +use stdx::format_to; + +use crate::{ + assist_context::{AssistBuilder, AssistDirector}, + utils::insert_use_statement, + AssistContext, AssistId, Assists, +}; +use ast::{ArgListOwner, VisibilityOwner}; +use hir::{EnumVariant, Module, ModuleDef}; +use ra_fmt::leading_indent; +use rustc_hash::FxHashSet; +use ra_db::FileId; + +// Assist extract_struct_from_enum +// +// Extracts a from struct from enum variant +// +// ``` +// enum A { <|>One(u32, u32) } +// ``` +// -> +// ``` +// struct One(pub u32, pub u32); +// +// enum A { One(One) }" +// ``` +pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let variant = ctx.find_node_at_offset::()?; + let field_list = match variant.kind() { + ast::StructKind::Tuple(field_list) => field_list, + _ => return None, + }; + let variant_name = variant.name()?.to_string(); + let enum_ast = variant.parent_enum(); + let enum_name = enum_ast.name().unwrap().to_string(); + let visibility = enum_ast.visibility(); + let variant_hir = ctx.sema.to_def(&variant)?; + + if existing_struct_def(ctx.db, &variant_name, &variant_hir) { + return None; + } + + let target = variant.syntax().text_range(); + return acc.add_in_multiple_files( + AssistId("extract_struct_from_enum_variant"), + "Extract struct from enum variant", + target, + |edit| { + let definition = Definition::ModuleDef(ModuleDef::EnumVariant(variant_hir)); + let res = definition.find_usages(&ctx.db, None); + let module_def = mod_def_for_target_module(ctx, &enum_name); + let start_offset = variant.parent_enum().syntax().text_range().start(); + let mut seen_files_map: FxHashSet = FxHashSet::default(); + seen_files_map.insert(module_def.module(ctx.db).unwrap()); + for reference in res { + let source_file = ctx.sema.parse(reference.file_range.file_id); + update_reference( + ctx, + edit, + reference, + &source_file, + &module_def, + &mut seen_files_map, + ); + } + extract_struct_def( + edit, + enum_ast.syntax(), + &variant_name, + &field_list.to_string(), + start_offset, + ctx.frange.file_id, + &visibility, + ); + let list_range = field_list.syntax().text_range(); + update_variant(edit, &variant_name, ctx.frange.file_id, list_range); + }, + ); +} + +fn existing_struct_def(db: &RootDatabase, variant_name: &str, variant: &EnumVariant) -> bool { + let module_defs = variant.parent_enum(db).module(db).scope(db, None); + for (name, _) in module_defs { + if name.to_string() == variant_name.to_string() { + return true; + } + } + false +} + +fn mod_def_for_target_module(ctx: &AssistContext, enum_name: &str) -> ModuleDef { + ImportsLocator::new(ctx.db).find_imports(enum_name).first().unwrap().left().unwrap() +} + +fn insert_use_import( + ctx: &AssistContext, + builder: &mut AssistBuilder, + path: &ast::PathExpr, + module: &Module, + module_def: &ModuleDef, + path_segment: ast::NameRef, +) -> Option<()> { + let db = ctx.db; + let mod_path = module.find_use_path(db, module_def.clone()); + if let Some(mut mod_path) = mod_path { + mod_path.segments.pop(); + mod_path.segments.push(path_segment.as_name()); + insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder()); + } + Some(()) +} + +fn extract_struct_def( + edit: &mut AssistDirector, + enum_ast: &SyntaxNode, + variant_name: &str, + variant_list: &str, + start_offset: TextSize, + file_id: FileId, + visibility: &Option, +) -> Option<()> { + let visibility_string = if let Some(visibility) = visibility { + format!("{} ", visibility.to_string()) + } else { + "".to_string() + }; + let mut buf = String::new(); + let indent = if let Some(indent) = leading_indent(enum_ast) { + indent.to_string() + } else { + "".to_string() + }; + + format_to!( + buf, + r#"{}struct {}{}; + +{}"#, + visibility_string, + variant_name, + list_with_visibility(variant_list), + indent + ); + edit.perform(file_id, |builder| { + builder.insert(start_offset, buf); + }); + Some(()) +} + +fn update_variant( + edit: &mut AssistDirector, + variant_name: &str, + file_id: FileId, + list_range: TextRange, +) -> Option<()> { + let inside_variant_range = TextRange::new( + list_range.start().checked_add(TextSize::from(1))?, + list_range.end().checked_sub(TextSize::from(1))?, + ); + edit.perform(file_id, |builder| { + builder.set_file(file_id); + builder.replace(inside_variant_range, variant_name); + }); + Some(()) +} + +fn update_reference( + ctx: &AssistContext, + edit: &mut AssistDirector, + reference: Reference, + source_file: &SourceFile, + module_def: &ModuleDef, + seen_files_map: &mut FxHashSet, +) -> Option<()> { + let path_expr: ast::PathExpr = find_node_at_offset::( + source_file.syntax(), + reference.file_range.range.start(), + )?; + let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; + let list = call.arg_list()?; + let segment = path_expr.path()?.segment()?; + let list_range = list.syntax().text_range(); + let inside_list_range = TextRange::new( + list_range.start().checked_add(TextSize::from(1))?, + list_range.end().checked_sub(TextSize::from(1))?, + ); + edit.perform(reference.file_range.file_id, |builder| { + let module = ctx.sema.scope(&path_expr.syntax()).module().unwrap(); + if !seen_files_map.contains(&module) { + if insert_use_import( + ctx, + builder, + &path_expr, + &module, + module_def, + segment.name_ref().unwrap(), + ) + .is_some() + { + seen_files_map.insert(module); + } + } + builder.replace(inside_list_range, format!("{}{}", segment, list)); + }); + Some(()) +} + +fn list_with_visibility(list: &str) -> String { + list.split(',') + .map(|part| { + let index = if part.chars().next().unwrap() == '(' { 1usize } else { 0 }; + let mut mod_part = part.trim().to_string(); + mod_part.insert_str(index, "pub "); + mod_part + }) + .collect::>() + .join(", ") +} + +#[cfg(test)] +mod tests { + + use crate::{utils::FamousDefs, tests::{check_assist, check_assist_not_applicable}}; + + use super::*; + + #[test] + fn test_extract_struct_several_fields() { + check_assist( + extract_struct_from_enum, + "enum A { <|>One(u32, u32) }", + r#"struct One(pub u32, pub u32); + +enum A { One(One) }"#, + ); + } + + #[test] + fn test_extract_struct_one_field() { + check_assist( + extract_struct_from_enum, + "enum A { <|>One(u32) }", + r#"struct One(pub u32); + +enum A { One(One) }"#, + ); + } + + #[test] + fn test_extract_struct_pub_visibility() { + check_assist( + extract_struct_from_enum, + "pub enum A { <|>One(u32, u32) }", + r#"pub struct One(pub u32, pub u32); + +pub enum A { One(One) }"#, + ); + } + + #[test] + fn test_extract_struct_with_complex_imports() { + check_assist( + extract_struct_from_enum, + r#"mod my_mod { + fn another_fn() { + let m = my_other_mod::MyEnum::MyField(1, 1); + } + + pub mod my_other_mod { + fn another_fn() { + let m = MyEnum::MyField(1, 1); + } + + pub enum MyEnum { + <|>MyField(u8, u8), + } + } +} + +fn another_fn() { + let m = my_mod::my_other_mod::MyEnum::MyField(1, 1); +}"#, + r#"use my_mod::my_other_mod::MyField; + +mod my_mod { + use my_other_mod::MyField; + + fn another_fn() { + let m = my_other_mod::MyEnum::MyField(MyField(1, 1)); + } + + pub mod my_other_mod { + fn another_fn() { + let m = MyEnum::MyField(MyField(1, 1)); + } + + pub struct MyField(pub u8, pub u8); + + pub enum MyEnum { + MyField(MyField), + } + } +} + +fn another_fn() { + let m = my_mod::my_other_mod::MyEnum::MyField(MyField(1, 1)); +}"#, + ); + } + + fn check_not_applicable(ra_fixture: &str) { + let fixture = + format!("//- main.rs crate:main deps:core\n{}\n{}", ra_fixture, FamousDefs::FIXTURE); + check_assist_not_applicable(extract_struct_from_enum, &fixture) + } + + #[test] + fn test_extract_enum_not_applicable_for_element_with_no_fields() { + check_not_applicable("enum A { <|>One }"); + } + + #[test] + fn test_extract_enum_not_applicable_if_struct_exists() { + check_not_applicable( + r#"struct One; + enum A { <|>One(u8) }"#, + ); + } +} diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 464bc03dd..9933f7a50 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -115,6 +115,7 @@ mod handlers { mod change_return_type_to_result; mod change_visibility; mod early_return; + mod extract_struct_from_enum_variant; mod fill_match_arms; mod fix_visibility; mod flip_binexpr; @@ -154,6 +155,7 @@ mod handlers { change_return_type_to_result::change_return_type_to_result, change_visibility::change_visibility, early_return::convert_to_guarded_return, + extract_struct_from_enum_variant::extract_struct_from_enum, fill_match_arms::fill_match_arms, fix_visibility::fix_visibility, flip_binexpr::flip_binexpr, -- cgit v1.2.3 From 04a35784df4cf267a8bbce6d5542869ed1a52fcb Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Fri, 22 May 2020 22:43:52 +0200 Subject: Formatting and remove unused imports --- crates/ra_assists/src/assist_context.rs | 30 ++++++++++++---------- .../handlers/extract_struct_from_enum_variant.rs | 7 +++-- 2 files changed, 21 insertions(+), 16 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 6291c68de..52bc7820e 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -2,7 +2,7 @@ use algo::find_covering_element; use hir::Semantics; -use ra_db::{FileId, FileRange, FilePosition}; +use ra_db::{FileId, FileRange}; use ra_fmt::{leading_indent, reindent}; use ra_ide_db::{ source_change::{SourceChange, SourceFileEdit}, @@ -173,16 +173,20 @@ impl Assists { Some(()) } - fn add_impl_multiple_files(&mut self, label: Assist, f: impl FnOnce(&mut AssistDirector)) -> Option<()> { + fn add_impl_multiple_files( + &mut self, + label: Assist, + f: impl FnOnce(&mut AssistDirector), + ) -> Option<()> { let change_label = label.label.clone(); if !self.resolve { - return None + return None; } let mut director = AssistDirector::new(change_label.clone()); f(&mut director); let changes = director.finish(); - let file_edits: Vec = changes.into_iter() - .map(|mut change| change.source_file_edits.pop().unwrap()).collect(); + let file_edits: Vec = + changes.into_iter().map(|mut change| change.source_file_edits.pop().unwrap()).collect(); let source_change = SourceChange { source_file_edits: file_edits, @@ -291,16 +295,12 @@ impl AssistBuilder { pub(crate) struct AssistDirector { source_changes: Vec, builders: FxHashMap, - change_label: String -} + change_label: String, +} impl AssistDirector { fn new(change_label: String) -> AssistDirector { - AssistDirector { - source_changes: vec![], - builders: FxHashMap::default(), - change_label - } + AssistDirector { source_changes: vec![], builders: FxHashMap::default(), change_label } } pub(crate) fn perform(&mut self, file_id: FileId, f: impl FnOnce(&mut AssistBuilder)) { @@ -309,8 +309,10 @@ impl AssistDirector { } fn finish(mut self) -> Vec { - for (file_id, builder) in self.builders.into_iter().collect::>() { - self.source_changes.push(builder.finish()); + for (file_id, builder) in + self.builders.into_iter().collect::>() + { + self.source_changes.push(builder.finish()); } self.source_changes } diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 6e19a6feb..57907a503 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -16,9 +16,9 @@ use crate::{ }; use ast::{ArgListOwner, VisibilityOwner}; use hir::{EnumVariant, Module, ModuleDef}; +use ra_db::FileId; use ra_fmt::leading_indent; use rustc_hash::FxHashSet; -use ra_db::FileId; // Assist extract_struct_from_enum // @@ -229,7 +229,10 @@ fn list_with_visibility(list: &str) -> String { #[cfg(test)] mod tests { - use crate::{utils::FamousDefs, tests::{check_assist, check_assist_not_applicable}}; + use crate::{ + tests::{check_assist, check_assist_not_applicable}, + utils::FamousDefs, + }; use super::*; -- cgit v1.2.3 From 97ffe3c6e8289553e3b3bd22392a22eaa8d61f42 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Fri, 22 May 2020 22:47:25 +0200 Subject: Refactor AssistDirector --- crates/ra_assists/src/assist_context.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 52bc7820e..37de4f590 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -178,11 +178,10 @@ impl Assists { label: Assist, f: impl FnOnce(&mut AssistDirector), ) -> Option<()> { - let change_label = label.label.clone(); if !self.resolve { return None; } - let mut director = AssistDirector::new(change_label.clone()); + let mut director = AssistDirector::new(); f(&mut director); let changes = director.finish(); let file_edits: Vec = @@ -295,12 +294,11 @@ impl AssistBuilder { pub(crate) struct AssistDirector { source_changes: Vec, builders: FxHashMap, - change_label: String, } impl AssistDirector { - fn new(change_label: String) -> AssistDirector { - AssistDirector { source_changes: vec![], builders: FxHashMap::default(), change_label } + fn new() -> AssistDirector { + AssistDirector { source_changes: vec![], builders: FxHashMap::default() } } pub(crate) fn perform(&mut self, file_id: FileId, f: impl FnOnce(&mut AssistBuilder)) { @@ -309,7 +307,7 @@ impl AssistDirector { } fn finish(mut self) -> Vec { - for (file_id, builder) in + for (_, builder) in self.builders.into_iter().collect::>() { self.source_changes.push(builder.finish()); -- cgit v1.2.3 From ef1aaeb59516b16f1b83eb7cdb22f1bcdcc46446 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Fri, 22 May 2020 22:59:17 +0200 Subject: More formatting --- crates/ra_assists/src/assist_context.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 37de4f590..e7220eea9 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -307,9 +307,7 @@ impl AssistDirector { } fn finish(mut self) -> Vec { - for (_, builder) in - self.builders.into_iter().collect::>() - { + for (_, builder) in self.builders.into_iter().collect::>() { self.source_changes.push(builder.finish()); } self.source_changes -- cgit v1.2.3 From fce10200a0d666fbd2e2faa84b0526f586485bb3 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Sat, 23 May 2020 01:23:40 +0200 Subject: Better naming and fix some review comments --- crates/ra_assists/src/assist_context.rs | 13 ++++++------- .../src/handlers/extract_struct_from_enum_variant.rs | 16 ++++++++-------- 2 files changed, 14 insertions(+), 15 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index e7220eea9..94286b497 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -292,13 +292,12 @@ impl AssistBuilder { } pub(crate) struct AssistDirector { - source_changes: Vec, builders: FxHashMap, } impl AssistDirector { fn new() -> AssistDirector { - AssistDirector { source_changes: vec![], builders: FxHashMap::default() } + AssistDirector { builders: FxHashMap::default() } } pub(crate) fn perform(&mut self, file_id: FileId, f: impl FnOnce(&mut AssistBuilder)) { @@ -306,10 +305,10 @@ impl AssistDirector { f(&mut builder); } - fn finish(mut self) -> Vec { - for (_, builder) in self.builders.into_iter().collect::>() { - self.source_changes.push(builder.finish()); - } - self.source_changes + fn finish(self) -> Vec { + self.builders + .into_iter() + .map(|(_, builder)| builder.finish()) + .collect::>() } } diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 57907a503..3250eed5b 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -59,8 +59,8 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - let res = definition.find_usages(&ctx.db, None); let module_def = mod_def_for_target_module(ctx, &enum_name); let start_offset = variant.parent_enum().syntax().text_range().start(); - let mut seen_files_map: FxHashSet = FxHashSet::default(); - seen_files_map.insert(module_def.module(ctx.db).unwrap()); + let mut visited_modules_set: FxHashSet = FxHashSet::default(); + visited_modules_set.insert(module_def.module(ctx.db).unwrap()); for reference in res { let source_file = ctx.sema.parse(reference.file_range.file_id); update_reference( @@ -69,7 +69,7 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - reference, &source_file, &module_def, - &mut seen_files_map, + &mut visited_modules_set, ); } extract_struct_def( @@ -101,7 +101,7 @@ fn mod_def_for_target_module(ctx: &AssistContext, enum_name: &str) -> ModuleDef ImportsLocator::new(ctx.db).find_imports(enum_name).first().unwrap().left().unwrap() } -fn insert_use_import( +fn insert_import( ctx: &AssistContext, builder: &mut AssistBuilder, path: &ast::PathExpr, @@ -179,7 +179,7 @@ fn update_reference( reference: Reference, source_file: &SourceFile, module_def: &ModuleDef, - seen_files_map: &mut FxHashSet, + visited_modules_set: &mut FxHashSet, ) -> Option<()> { let path_expr: ast::PathExpr = find_node_at_offset::( source_file.syntax(), @@ -195,8 +195,8 @@ fn update_reference( ); edit.perform(reference.file_range.file_id, |builder| { let module = ctx.sema.scope(&path_expr.syntax()).module().unwrap(); - if !seen_files_map.contains(&module) { - if insert_use_import( + if !visited_modules_set.contains(&module) { + if insert_import( ctx, builder, &path_expr, @@ -206,7 +206,7 @@ fn update_reference( ) .is_some() { - seen_files_map.insert(module); + visited_modules_set.insert(module); } } builder.replace(inside_list_range, format!("{}{}", segment, list)); -- cgit v1.2.3 From 4984520ef565e926ba08c6512715ed631e4527e4 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Sat, 23 May 2020 01:27:11 +0200 Subject: Use default instead of new in AssistDirector --- crates/ra_assists/src/assist_context.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 94286b497..bc5481494 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -181,7 +181,7 @@ impl Assists { if !self.resolve { return None; } - let mut director = AssistDirector::new(); + let mut director = AssistDirector::default(); f(&mut director); let changes = director.finish(); let file_edits: Vec = @@ -296,10 +296,6 @@ pub(crate) struct AssistDirector { } impl AssistDirector { - fn new() -> AssistDirector { - AssistDirector { builders: FxHashMap::default() } - } - pub(crate) fn perform(&mut self, file_id: FileId, f: impl FnOnce(&mut AssistBuilder)) { let mut builder = self.builders.entry(file_id).or_insert(AssistBuilder::new(file_id)); f(&mut builder); @@ -312,3 +308,9 @@ impl AssistDirector { .collect::>() } } + +impl Default for AssistDirector { + fn default() -> Self { + AssistDirector { builders: FxHashMap::default() } + } +} -- cgit v1.2.3 From 6ee1c60c9cff781e10d6379f68fc951378403f6b Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Sat, 23 May 2020 01:41:08 +0200 Subject: Further review fixes --- crates/ra_assists/Cargo.toml | 1 - .../src/handlers/extract_struct_from_enum_variant.rs | 20 +++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml index f3481bdeb..3bcf58ba4 100644 --- a/crates/ra_assists/Cargo.toml +++ b/crates/ra_assists/Cargo.toml @@ -20,6 +20,5 @@ ra_fmt = { path = "../ra_fmt" } ra_prof = { path = "../ra_prof" } ra_db = { path = "../ra_db" } ra_ide_db = { path = "../ra_ide_db" } -hir_expand = { path = "../ra_hir_expand", package = "ra_hir_expand" } hir = { path = "../ra_hir", package = "ra_hir" } test_utils = { path = "../test_utils" } diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 3250eed5b..359283802 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -1,4 +1,3 @@ -use hir_expand::name::AsName; use ra_ide_db::{ defs::Definition, imports_locator::ImportsLocator, search::Reference, RootDatabase, }; @@ -15,14 +14,14 @@ use crate::{ AssistContext, AssistId, Assists, }; use ast::{ArgListOwner, VisibilityOwner}; -use hir::{EnumVariant, Module, ModuleDef}; +use hir::{AsName, EnumVariant, Module, ModuleDef}; use ra_db::FileId; use ra_fmt::leading_indent; use rustc_hash::FxHashSet; // Assist extract_struct_from_enum // -// Extracts a from struct from enum variant +// Extracts a struct from enum variant // // ``` // enum A { <|>One(u32, u32) } @@ -41,7 +40,7 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - }; let variant_name = variant.name()?.to_string(); let enum_ast = variant.parent_enum(); - let enum_name = enum_ast.name().unwrap().to_string(); + let enum_name = enum_ast.name()?.to_string(); let visibility = enum_ast.visibility(); let variant_hir = ctx.sema.to_def(&variant)?; @@ -88,13 +87,12 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - } fn existing_struct_def(db: &RootDatabase, variant_name: &str, variant: &EnumVariant) -> bool { - let module_defs = variant.parent_enum(db).module(db).scope(db, None); - for (name, _) in module_defs { - if name.to_string() == variant_name.to_string() { - return true; - } - } - false + variant + .parent_enum(db) + .module(db) + .scope(db, None) + .into_iter() + .any(|(name, _)| name.to_string() == variant_name.to_string()) } fn mod_def_for_target_module(ctx: &AssistContext, enum_name: &str) -> ModuleDef { -- cgit v1.2.3 From 3a244e02b53687161d4cc39254cb9a432756017f Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Sat, 23 May 2020 11:53:02 +0200 Subject: Remove unwraps where possible --- .../handlers/extract_struct_from_enum_variant.rs | 33 ++++++++-------------- 1 file changed, 11 insertions(+), 22 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 359283802..d5397bf21 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -39,15 +39,16 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - _ => return None, }; let variant_name = variant.name()?.to_string(); - let enum_ast = variant.parent_enum(); - let enum_name = enum_ast.name()?.to_string(); - let visibility = enum_ast.visibility(); let variant_hir = ctx.sema.to_def(&variant)?; - if existing_struct_def(ctx.db, &variant_name, &variant_hir) { return None; } - + let enum_ast = variant.parent_enum(); + let enum_name = enum_ast.name()?.to_string(); + let visibility = enum_ast.visibility(); + let current_module_def = + ImportsLocator::new(ctx.db).find_imports(&enum_name).first()?.left()?; + let current_module = current_module_def.module(ctx.db)?; let target = variant.syntax().text_range(); return acc.add_in_multiple_files( AssistId("extract_struct_from_enum_variant"), @@ -56,10 +57,9 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - |edit| { let definition = Definition::ModuleDef(ModuleDef::EnumVariant(variant_hir)); let res = definition.find_usages(&ctx.db, None); - let module_def = mod_def_for_target_module(ctx, &enum_name); let start_offset = variant.parent_enum().syntax().text_range().start(); let mut visited_modules_set: FxHashSet = FxHashSet::default(); - visited_modules_set.insert(module_def.module(ctx.db).unwrap()); + visited_modules_set.insert(current_module); for reference in res { let source_file = ctx.sema.parse(reference.file_range.file_id); update_reference( @@ -67,7 +67,7 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - edit, reference, &source_file, - &module_def, + ¤t_module_def, &mut visited_modules_set, ); } @@ -95,10 +95,6 @@ fn existing_struct_def(db: &RootDatabase, variant_name: &str, variant: &EnumVari .any(|(name, _)| name.to_string() == variant_name.to_string()) } -fn mod_def_for_target_module(ctx: &AssistContext, enum_name: &str) -> ModuleDef { - ImportsLocator::new(ctx.db).find_imports(enum_name).first().unwrap().left().unwrap() -} - fn insert_import( ctx: &AssistContext, builder: &mut AssistBuilder, @@ -186,23 +182,16 @@ fn update_reference( let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let list = call.arg_list()?; let segment = path_expr.path()?.segment()?; + let segment_name = segment.name_ref()?; + let module = ctx.sema.scope(&path_expr.syntax()).module()?; let list_range = list.syntax().text_range(); let inside_list_range = TextRange::new( list_range.start().checked_add(TextSize::from(1))?, list_range.end().checked_sub(TextSize::from(1))?, ); edit.perform(reference.file_range.file_id, |builder| { - let module = ctx.sema.scope(&path_expr.syntax()).module().unwrap(); if !visited_modules_set.contains(&module) { - if insert_import( - ctx, - builder, - &path_expr, - &module, - module_def, - segment.name_ref().unwrap(), - ) - .is_some() + if insert_import(ctx, builder, &path_expr, &module, module_def, segment_name).is_some() { visited_modules_set.insert(module); } -- cgit v1.2.3 From e2974ba8f7d862eac084ada2fdf327bbde803bed Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Sat, 23 May 2020 11:57:12 +0200 Subject: Remove unnecessary set_file and change variable positions for better readability --- crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index d5397bf21..dd2fd57a7 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -127,12 +127,12 @@ fn extract_struct_def( } else { "".to_string() }; - let mut buf = String::new(); let indent = if let Some(indent) = leading_indent(enum_ast) { indent.to_string() } else { "".to_string() }; + let mut buf = String::new(); format_to!( buf, @@ -161,7 +161,6 @@ fn update_variant( list_range.end().checked_sub(TextSize::from(1))?, ); edit.perform(file_id, |builder| { - builder.set_file(file_id); builder.replace(inside_variant_range, variant_name); }); Some(()) -- cgit v1.2.3 From 08aa8e1de717425ebf49796df1515d9fbd8b2e3e Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Sun, 24 May 2020 14:53:12 +0200 Subject: Further refactoring under review comments --- .../src/handlers/extract_struct_from_enum_variant.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index dd2fd57a7..a36587633 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -6,7 +6,6 @@ use ra_syntax::{ ast::{self, AstNode, NameOwner}, SourceFile, SyntaxNode, TextRange, TextSize, }; -use stdx::format_to; use crate::{ assist_context::{AssistBuilder, AssistDirector}, @@ -58,7 +57,7 @@ pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) - let definition = Definition::ModuleDef(ModuleDef::EnumVariant(variant_hir)); let res = definition.find_usages(&ctx.db, None); let start_offset = variant.parent_enum().syntax().text_range().start(); - let mut visited_modules_set: FxHashSet = FxHashSet::default(); + let mut visited_modules_set = FxHashSet::default(); visited_modules_set.insert(current_module); for reference in res { let source_file = ctx.sema.parse(reference.file_range.file_id); @@ -132,10 +131,7 @@ fn extract_struct_def( } else { "".to_string() }; - let mut buf = String::new(); - - format_to!( - buf, + let struct_def = format!( r#"{}struct {}{}; {}"#, @@ -145,7 +141,7 @@ fn extract_struct_def( indent ); edit.perform(file_id, |builder| { - builder.insert(start_offset, buf); + builder.insert(start_offset, struct_def); }); Some(()) } -- cgit v1.2.3 From a9d567584857b1be4ca8eaa5ef2c7d85f7b2845e Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Wed, 3 Jun 2020 20:10:33 +0200 Subject: Fix incorrect behaviour if not resolved --- crates/ra_assists/src/assist_context.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index bc5481494..1925db8b2 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -179,6 +179,7 @@ impl Assists { f: impl FnOnce(&mut AssistDirector), ) -> Option<()> { if !self.resolve { + self.buf.push((label, None)); return None; } let mut director = AssistDirector::default(); -- cgit v1.2.3 From a6d3c77bdd998499941a6aceccde85f7a94b804d Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Wed, 3 Jun 2020 20:43:57 +0200 Subject: Fixed tests --- .../handlers/extract_struct_from_enum_variant.rs | 21 ++++++++++++--------- crates/ra_assists/src/lib.rs | 2 +- crates/ra_assists/src/tests/generated.rs | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 10 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index a36587633..2b1951aff 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -18,9 +18,9 @@ use ra_db::FileId; use ra_fmt::leading_indent; use rustc_hash::FxHashSet; -// Assist extract_struct_from_enum +// Assist: extract_struct_from_enum_variant // -// Extracts a struct from enum variant +// Extracts a struct from enum variant. // // ``` // enum A { <|>One(u32, u32) } @@ -29,9 +29,12 @@ use rustc_hash::FxHashSet; // ``` // struct One(pub u32, pub u32); // -// enum A { One(One) }" +// enum A { One(One) } // ``` -pub(crate) fn extract_struct_from_enum(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { +pub(crate) fn extract_struct_from_enum_variant( + acc: &mut Assists, + ctx: &AssistContext, +) -> Option<()> { let variant = ctx.find_node_at_offset::()?; let field_list = match variant.kind() { ast::StructKind::Tuple(field_list) => field_list, @@ -221,7 +224,7 @@ mod tests { #[test] fn test_extract_struct_several_fields() { check_assist( - extract_struct_from_enum, + extract_struct_from_enum_variant, "enum A { <|>One(u32, u32) }", r#"struct One(pub u32, pub u32); @@ -232,7 +235,7 @@ enum A { One(One) }"#, #[test] fn test_extract_struct_one_field() { check_assist( - extract_struct_from_enum, + extract_struct_from_enum_variant, "enum A { <|>One(u32) }", r#"struct One(pub u32); @@ -243,7 +246,7 @@ enum A { One(One) }"#, #[test] fn test_extract_struct_pub_visibility() { check_assist( - extract_struct_from_enum, + extract_struct_from_enum_variant, "pub enum A { <|>One(u32, u32) }", r#"pub struct One(pub u32, pub u32); @@ -254,7 +257,7 @@ pub enum A { One(One) }"#, #[test] fn test_extract_struct_with_complex_imports() { check_assist( - extract_struct_from_enum, + extract_struct_from_enum_variant, r#"mod my_mod { fn another_fn() { let m = my_other_mod::MyEnum::MyField(1, 1); @@ -305,7 +308,7 @@ fn another_fn() { fn check_not_applicable(ra_fixture: &str) { let fixture = format!("//- main.rs crate:main deps:core\n{}\n{}", ra_fixture, FamousDefs::FIXTURE); - check_assist_not_applicable(extract_struct_from_enum, &fixture) + check_assist_not_applicable(extract_struct_from_enum_variant, &fixture) } #[test] diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 88ce9b62e..185428bd5 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -156,7 +156,7 @@ mod handlers { change_return_type_to_result::change_return_type_to_result, change_visibility::change_visibility, early_return::convert_to_guarded_return, - extract_struct_from_enum_variant::extract_struct_from_enum, + extract_struct_from_enum_variant::extract_struct_from_enum_variant, fill_match_arms::fill_match_arms, fix_visibility::fix_visibility, flip_binexpr::flip_binexpr, diff --git a/crates/ra_assists/src/tests/generated.rs b/crates/ra_assists/src/tests/generated.rs index d17504529..40a223727 100644 --- a/crates/ra_assists/src/tests/generated.rs +++ b/crates/ra_assists/src/tests/generated.rs @@ -337,6 +337,21 @@ fn main() { ) } +#[test] +fn doctest_extract_struct_from_enum_variant() { + check_doc_test( + "extract_struct_from_enum_variant", + r#####" +enum A { <|>One(u32, u32) } +"#####, + r#####" +struct One(pub u32, pub u32); + +enum A { One(One) } +"#####, + ) +} + #[test] fn doctest_fill_match_arms() { check_doc_test( -- cgit v1.2.3 From b0c8a2be7bd0d053eb9dd0e02fe2cf08b093a19a Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Thu, 4 Jun 2020 10:03:44 +0200 Subject: Remove AsName import --- .../src/handlers/extract_struct_from_enum_variant.rs | 13 +++++++++---- crates/ra_assists/src/utils/insert_use.rs | 11 ++++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 2b1951aff..72e5dd735 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -9,11 +9,11 @@ use ra_syntax::{ use crate::{ assist_context::{AssistBuilder, AssistDirector}, - utils::insert_use_statement, + utils::insert_use::insert_use_statement_with_string_path, AssistContext, AssistId, Assists, }; use ast::{ArgListOwner, VisibilityOwner}; -use hir::{AsName, EnumVariant, Module, ModuleDef}; +use hir::{EnumVariant, Module, ModuleDef}; use ra_db::FileId; use ra_fmt::leading_indent; use rustc_hash::FxHashSet; @@ -109,8 +109,13 @@ fn insert_import( let mod_path = module.find_use_path(db, module_def.clone()); if let Some(mut mod_path) = mod_path { mod_path.segments.pop(); - mod_path.segments.push(path_segment.as_name()); - insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder()); + let use_path = format!("{}::{}", mod_path.to_string(), path_segment.to_string()); + insert_use_statement_with_string_path( + path.syntax(), + &use_path, + ctx, + builder.text_edit_builder(), + ); } Some(()) } diff --git a/crates/ra_assists/src/utils/insert_use.rs b/crates/ra_assists/src/utils/insert_use.rs index 0ee43482f..114f5949a 100644 --- a/crates/ra_assists/src/utils/insert_use.rs +++ b/crates/ra_assists/src/utils/insert_use.rs @@ -23,7 +23,16 @@ pub(crate) fn insert_use_statement( ctx: &AssistContext, builder: &mut TextEditBuilder, ) { - let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::>(); + insert_use_statement_with_string_path(position, &path_to_import.to_string(), ctx, builder); +} + +pub(crate) fn insert_use_statement_with_string_path( + position: &SyntaxNode, + path_to_import: &str, + ctx: &AssistContext, + builder: &mut TextEditBuilder, +) { + let target = path_to_import.split("::").map(SmolStr::new).collect::>(); let container = ctx.sema.ancestors_with_macros(position.clone()).find_map(|n| { if let Some(module) = ast::Module::cast(n.clone()) { return module.item_list().map(|it| it.syntax().clone()); -- cgit v1.2.3 From 921306757baa636af7872b003d33dc1a8bd2b725 Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Wed, 3 Jun 2020 17:54:23 -0500 Subject: introduce_named_lifetime assist wasn't applicable when type parameter followed anonymous lifetime token (fixes #4684) --- .../src/handlers/introduce_named_lifetime.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/introduce_named_lifetime.rs b/crates/ra_assists/src/handlers/introduce_named_lifetime.rs index beb5b7366..28fcbc9ba 100644 --- a/crates/ra_assists/src/handlers/introduce_named_lifetime.rs +++ b/crates/ra_assists/src/handlers/introduce_named_lifetime.rs @@ -41,8 +41,6 @@ pub(crate) fn introduce_named_lifetime(acc: &mut Assists, ctx: &AssistContext) - if let Some(fn_def) = lifetime_token.ancestors().find_map(ast::FnDef::cast) { generate_fn_def_assist(acc, &fn_def, lifetime_token.text_range()) } else if let Some(impl_def) = lifetime_token.ancestors().find_map(ast::ImplDef::cast) { - // only allow naming the last anonymous lifetime - lifetime_token.next_token().filter(|tok| tok.kind() == SyntaxKind::R_ANGLE)?; generate_impl_def_assist(acc, &impl_def, lifetime_token.text_range()) } else { None @@ -190,6 +188,23 @@ mod tests { ); } + #[test] + fn test_impl_with_other_type_param() { + check_assist( + introduce_named_lifetime, + "impl fmt::Display for SepByBuilder<'_<|>, I> + where + I: Iterator, + I::Item: fmt::Display, + {", + "impl fmt::Display for SepByBuilder<'a, I> + where + I: Iterator, + I::Item: fmt::Display, + {", + ) + } + #[test] fn test_example_case_cursor_before_tick() { check_assist( -- cgit v1.2.3 From 74c3e7a1adf9d14bbac5cbbe9cd893d758f82561 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Fri, 5 Jun 2020 11:45:41 +0200 Subject: Remove unnecessary return --- crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 72e5dd735..ef963ddba 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -52,7 +52,7 @@ pub(crate) fn extract_struct_from_enum_variant( ImportsLocator::new(ctx.db).find_imports(&enum_name).first()?.left()?; let current_module = current_module_def.module(ctx.db)?; let target = variant.syntax().text_range(); - return acc.add_in_multiple_files( + acc.add_in_multiple_files( AssistId("extract_struct_from_enum_variant"), "Extract struct from enum variant", target, @@ -85,7 +85,7 @@ pub(crate) fn extract_struct_from_enum_variant( let list_range = field_list.syntax().text_range(); update_variant(edit, &variant_name, ctx.frange.file_id, list_range); }, - ); + ) } fn existing_struct_def(db: &RootDatabase, variant_name: &str, variant: &EnumVariant) -> bool { -- cgit v1.2.3 From 5dda9955380c6214aa5720ad640b76b870aaa556 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Fri, 5 Jun 2020 13:17:17 +0200 Subject: Fix review comments --- .../handlers/extract_struct_from_enum_variant.rs | 41 ++++++++++------------ crates/ra_assists/src/utils/insert_use.rs | 11 +----- 2 files changed, 19 insertions(+), 33 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index ef963ddba..2c455a1fd 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -1,6 +1,4 @@ -use ra_ide_db::{ - defs::Definition, imports_locator::ImportsLocator, search::Reference, RootDatabase, -}; +use ra_ide_db::{defs::Definition, search::Reference, RootDatabase}; use ra_syntax::{ algo::find_node_at_offset, ast::{self, AstNode, NameOwner}, @@ -9,11 +7,11 @@ use ra_syntax::{ use crate::{ assist_context::{AssistBuilder, AssistDirector}, - utils::insert_use::insert_use_statement_with_string_path, + utils::insert_use_statement, AssistContext, AssistId, Assists, }; use ast::{ArgListOwner, VisibilityOwner}; -use hir::{EnumVariant, Module, ModuleDef}; +use hir::{EnumVariant, Module, ModuleDef, Name}; use ra_db::FileId; use ra_fmt::leading_indent; use rustc_hash::FxHashSet; @@ -46,11 +44,11 @@ pub(crate) fn extract_struct_from_enum_variant( return None; } let enum_ast = variant.parent_enum(); - let enum_name = enum_ast.name()?.to_string(); let visibility = enum_ast.visibility(); - let current_module_def = - ImportsLocator::new(ctx.db).find_imports(&enum_name).first()?.left()?; - let current_module = current_module_def.module(ctx.db)?; + let enum_hir = ctx.sema.to_def(&enum_ast)?; + let variant_hir_name = variant_hir.name(ctx.db); + let enum_module_def = ModuleDef::from(enum_hir); + let current_module = enum_hir.module(ctx.db); let target = variant.syntax().text_range(); acc.add_in_multiple_files( AssistId("extract_struct_from_enum_variant"), @@ -69,7 +67,8 @@ pub(crate) fn extract_struct_from_enum_variant( edit, reference, &source_file, - ¤t_module_def, + &enum_module_def, + &variant_hir_name, &mut visited_modules_set, ); } @@ -102,20 +101,15 @@ fn insert_import( builder: &mut AssistBuilder, path: &ast::PathExpr, module: &Module, - module_def: &ModuleDef, - path_segment: ast::NameRef, + enum_module_def: &ModuleDef, + variant_hir_name: &Name, ) -> Option<()> { let db = ctx.db; - let mod_path = module.find_use_path(db, module_def.clone()); + let mod_path = module.find_use_path(db, enum_module_def.clone()); if let Some(mut mod_path) = mod_path { mod_path.segments.pop(); - let use_path = format!("{}::{}", mod_path.to_string(), path_segment.to_string()); - insert_use_statement_with_string_path( - path.syntax(), - &use_path, - ctx, - builder.text_edit_builder(), - ); + mod_path.segments.push(variant_hir_name.clone()); + insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder()); } Some(()) } @@ -175,7 +169,8 @@ fn update_reference( edit: &mut AssistDirector, reference: Reference, source_file: &SourceFile, - module_def: &ModuleDef, + enum_module_def: &ModuleDef, + variant_hir_name: &Name, visited_modules_set: &mut FxHashSet, ) -> Option<()> { let path_expr: ast::PathExpr = find_node_at_offset::( @@ -185,7 +180,6 @@ fn update_reference( let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let list = call.arg_list()?; let segment = path_expr.path()?.segment()?; - let segment_name = segment.name_ref()?; let module = ctx.sema.scope(&path_expr.syntax()).module()?; let list_range = list.syntax().text_range(); let inside_list_range = TextRange::new( @@ -194,7 +188,8 @@ fn update_reference( ); edit.perform(reference.file_range.file_id, |builder| { if !visited_modules_set.contains(&module) { - if insert_import(ctx, builder, &path_expr, &module, module_def, segment_name).is_some() + if insert_import(ctx, builder, &path_expr, &module, enum_module_def, variant_hir_name) + .is_some() { visited_modules_set.insert(module); } diff --git a/crates/ra_assists/src/utils/insert_use.rs b/crates/ra_assists/src/utils/insert_use.rs index 114f5949a..0ee43482f 100644 --- a/crates/ra_assists/src/utils/insert_use.rs +++ b/crates/ra_assists/src/utils/insert_use.rs @@ -23,16 +23,7 @@ pub(crate) fn insert_use_statement( ctx: &AssistContext, builder: &mut TextEditBuilder, ) { - insert_use_statement_with_string_path(position, &path_to_import.to_string(), ctx, builder); -} - -pub(crate) fn insert_use_statement_with_string_path( - position: &SyntaxNode, - path_to_import: &str, - ctx: &AssistContext, - builder: &mut TextEditBuilder, -) { - let target = path_to_import.split("::").map(SmolStr::new).collect::>(); + let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::>(); let container = ctx.sema.ancestors_with_macros(position.clone()).find_map(|n| { if let Some(module) = ast::Module::cast(n.clone()) { return module.item_list().map(|it| it.syntax().clone()); -- cgit v1.2.3 From a4a4a1854ebb53e1cdd7a5e3b308112bbbf3c676 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 29 May 2020 19:14:04 +0200 Subject: Fix type parameter defaults They should not be applied in expression or pattern contexts, unless there are other explicitly given type args. --- crates/ra_assists/src/handlers/add_explicit_type.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs index ab20c6649..90b06a625 100644 --- a/crates/ra_assists/src/handlers/add_explicit_type.rs +++ b/crates/ra_assists/src/handlers/add_explicit_type.rs @@ -195,7 +195,7 @@ struct Test { } fn main() { - let test<|> = Test { t: 23, k: 33 }; + let test<|> = Test { t: 23u8, k: 33 }; }"#, r#" struct Test { @@ -204,7 +204,7 @@ struct Test { } fn main() { - let test: Test = Test { t: 23, k: 33 }; + let test: Test = Test { t: 23u8, k: 33 }; }"#, ); } -- cgit v1.2.3 From 38fa4d17fb9622044ee0f0bc50d6c71d5aa46dd1 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Jun 2020 00:01:40 +0200 Subject: Simplify API --- crates/ra_assists/src/assist_context.rs | 99 ++++++---------------- crates/ra_assists/src/handlers/add_function.rs | 2 +- .../handlers/extract_struct_from_enum_variant.rs | 58 ++++++------- crates/ra_assists/src/handlers/fix_visibility.rs | 4 +- 4 files changed, 56 insertions(+), 107 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index 1925db8b2..edd8255f4 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -1,5 +1,7 @@ //! See `AssistContext` +use std::mem; + use algo::find_covering_element; use hir::Semantics; use ra_db::{FileId, FileRange}; @@ -19,7 +21,6 @@ use crate::{ assist_config::{AssistConfig, SnippetCap}, Assist, AssistId, GroupLabel, ResolvedAssist, }; -use rustc_hash::FxHashMap; /// `AssistContext` allows to apply an assist or check if it could be applied. /// @@ -139,16 +140,6 @@ impl Assists { let label = Assist::new(id, label.into(), None, target); self.add_impl(label, f) } - pub(crate) fn add_in_multiple_files( - &mut self, - id: AssistId, - label: impl Into, - target: TextRange, - f: impl FnOnce(&mut AssistDirector), - ) -> Option<()> { - let label = Assist::new(id, label.into(), None, target); - self.add_impl_multiple_files(label, f) - } pub(crate) fn add_group( &mut self, group: &GroupLabel, @@ -173,31 +164,6 @@ impl Assists { Some(()) } - fn add_impl_multiple_files( - &mut self, - label: Assist, - f: impl FnOnce(&mut AssistDirector), - ) -> Option<()> { - if !self.resolve { - self.buf.push((label, None)); - return None; - } - let mut director = AssistDirector::default(); - f(&mut director); - let changes = director.finish(); - let file_edits: Vec = - changes.into_iter().map(|mut change| change.source_file_edits.pop().unwrap()).collect(); - - let source_change = SourceChange { - source_file_edits: file_edits, - file_system_edits: vec![], - is_snippet: false, - }; - - self.buf.push((label, Some(source_change))); - Some(()) - } - fn finish(mut self) -> Vec<(Assist, Option)> { self.buf.sort_by_key(|(label, _edit)| label.target.len()); self.buf @@ -206,13 +172,32 @@ impl Assists { pub(crate) struct AssistBuilder { edit: TextEditBuilder, - file: FileId, + file_id: FileId, is_snippet: bool, + edits: Vec, } impl AssistBuilder { - pub(crate) fn new(file: FileId) -> AssistBuilder { - AssistBuilder { edit: TextEditBuilder::default(), file, is_snippet: false } + pub(crate) fn new(file_id: FileId) -> AssistBuilder { + AssistBuilder { + edit: TextEditBuilder::default(), + file_id, + is_snippet: false, + edits: Vec::new(), + } + } + + pub(crate) fn edit_file(&mut self, file_id: FileId) { + self.file_id = file_id; + } + + fn commit(&mut self) { + let edit = mem::take(&mut self.edit).finish(); + if !edit.is_empty() { + let new_edit = SourceFileEdit { file_id: self.file_id, edit }; + assert!(!self.edits.iter().any(|it| it.file_id == new_edit.file_id)); + self.edits.push(new_edit); + } } /// Remove specified `range` of text. @@ -270,48 +255,18 @@ impl AssistBuilder { algo::diff(&node, &new).into_text_edit(&mut self.edit) } - // FIXME: better API - pub(crate) fn set_file(&mut self, assist_file: FileId) { - self.file = assist_file; - } - // FIXME: kill this API /// Get access to the raw `TextEditBuilder`. pub(crate) fn text_edit_builder(&mut self) -> &mut TextEditBuilder { &mut self.edit } - fn finish(self) -> SourceChange { - let edit = self.edit.finish(); - let source_file_edit = SourceFileEdit { file_id: self.file, edit }; - let mut res: SourceChange = source_file_edit.into(); + fn finish(mut self) -> SourceChange { + self.commit(); + let mut res: SourceChange = mem::take(&mut self.edits).into(); if self.is_snippet { res.is_snippet = true; } res } } - -pub(crate) struct AssistDirector { - builders: FxHashMap, -} - -impl AssistDirector { - pub(crate) fn perform(&mut self, file_id: FileId, f: impl FnOnce(&mut AssistBuilder)) { - let mut builder = self.builders.entry(file_id).or_insert(AssistBuilder::new(file_id)); - f(&mut builder); - } - - fn finish(self) -> Vec { - self.builders - .into_iter() - .map(|(_, builder)| builder.finish()) - .collect::>() - } -} - -impl Default for AssistDirector { - fn default() -> Self { - AssistDirector { builders: FxHashMap::default() } - } -} diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs index 24f931a85..1cfbd75aa 100644 --- a/crates/ra_assists/src/handlers/add_function.rs +++ b/crates/ra_assists/src/handlers/add_function.rs @@ -64,7 +64,7 @@ pub(crate) fn add_function(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let target = call.syntax().text_range(); acc.add(AssistId("add_function"), "Add function", target, |builder| { let function_template = function_builder.render(); - builder.set_file(function_template.file); + builder.edit_file(function_template.file); let new_fn = function_template.to_string(ctx.config.snippet_cap); match ctx.config.snippet_cap { Some(cap) => builder.insert_snippet(cap, function_template.insert_offset, new_fn), diff --git a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs index 2c455a1fd..44db7917a 100644 --- a/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ra_assists/src/handlers/extract_struct_from_enum_variant.rs @@ -1,20 +1,17 @@ +use hir::{EnumVariant, Module, ModuleDef, Name}; +use ra_db::FileId; +use ra_fmt::leading_indent; use ra_ide_db::{defs::Definition, search::Reference, RootDatabase}; use ra_syntax::{ algo::find_node_at_offset, - ast::{self, AstNode, NameOwner}, + ast::{self, ArgListOwner, AstNode, NameOwner, VisibilityOwner}, SourceFile, SyntaxNode, TextRange, TextSize, }; +use rustc_hash::FxHashSet; use crate::{ - assist_context::{AssistBuilder, AssistDirector}, - utils::insert_use_statement, - AssistContext, AssistId, Assists, + assist_context::AssistBuilder, utils::insert_use_statement, AssistContext, AssistId, Assists, }; -use ast::{ArgListOwner, VisibilityOwner}; -use hir::{EnumVariant, Module, ModuleDef, Name}; -use ra_db::FileId; -use ra_fmt::leading_indent; -use rustc_hash::FxHashSet; // Assist: extract_struct_from_enum_variant // @@ -50,11 +47,11 @@ pub(crate) fn extract_struct_from_enum_variant( let enum_module_def = ModuleDef::from(enum_hir); let current_module = enum_hir.module(ctx.db); let target = variant.syntax().text_range(); - acc.add_in_multiple_files( + acc.add( AssistId("extract_struct_from_enum_variant"), "Extract struct from enum variant", target, - |edit| { + |builder| { let definition = Definition::ModuleDef(ModuleDef::EnumVariant(variant_hir)); let res = definition.find_usages(&ctx.db, None); let start_offset = variant.parent_enum().syntax().text_range().start(); @@ -64,7 +61,7 @@ pub(crate) fn extract_struct_from_enum_variant( let source_file = ctx.sema.parse(reference.file_range.file_id); update_reference( ctx, - edit, + builder, reference, &source_file, &enum_module_def, @@ -73,7 +70,7 @@ pub(crate) fn extract_struct_from_enum_variant( ); } extract_struct_def( - edit, + builder, enum_ast.syntax(), &variant_name, &field_list.to_string(), @@ -82,7 +79,7 @@ pub(crate) fn extract_struct_from_enum_variant( &visibility, ); let list_range = field_list.syntax().text_range(); - update_variant(edit, &variant_name, ctx.frange.file_id, list_range); + update_variant(builder, &variant_name, ctx.frange.file_id, list_range); }, ) } @@ -115,7 +112,7 @@ fn insert_import( } fn extract_struct_def( - edit: &mut AssistDirector, + builder: &mut AssistBuilder, enum_ast: &SyntaxNode, variant_name: &str, variant_list: &str, @@ -142,14 +139,13 @@ fn extract_struct_def( list_with_visibility(variant_list), indent ); - edit.perform(file_id, |builder| { - builder.insert(start_offset, struct_def); - }); + builder.edit_file(file_id); + builder.insert(start_offset, struct_def); Some(()) } fn update_variant( - edit: &mut AssistDirector, + builder: &mut AssistBuilder, variant_name: &str, file_id: FileId, list_range: TextRange, @@ -158,15 +154,14 @@ fn update_variant( list_range.start().checked_add(TextSize::from(1))?, list_range.end().checked_sub(TextSize::from(1))?, ); - edit.perform(file_id, |builder| { - builder.replace(inside_variant_range, variant_name); - }); + builder.edit_file(file_id); + builder.replace(inside_variant_range, variant_name); Some(()) } fn update_reference( ctx: &AssistContext, - edit: &mut AssistDirector, + builder: &mut AssistBuilder, reference: Reference, source_file: &SourceFile, enum_module_def: &ModuleDef, @@ -186,16 +181,15 @@ fn update_reference( list_range.start().checked_add(TextSize::from(1))?, list_range.end().checked_sub(TextSize::from(1))?, ); - edit.perform(reference.file_range.file_id, |builder| { - if !visited_modules_set.contains(&module) { - if insert_import(ctx, builder, &path_expr, &module, enum_module_def, variant_hir_name) - .is_some() - { - visited_modules_set.insert(module); - } + builder.edit_file(reference.file_range.file_id); + if !visited_modules_set.contains(&module) { + if insert_import(ctx, builder, &path_expr, &module, enum_module_def, variant_hir_name) + .is_some() + { + visited_modules_set.insert(module); } - builder.replace(inside_list_range, format!("{}{}", segment, list)); - }); + } + builder.replace(inside_list_range, format!("{}{}", segment, list)); Some(()) } diff --git a/crates/ra_assists/src/handlers/fix_visibility.rs b/crates/ra_assists/src/handlers/fix_visibility.rs index 9ec42f568..531b3560f 100644 --- a/crates/ra_assists/src/handlers/fix_visibility.rs +++ b/crates/ra_assists/src/handlers/fix_visibility.rs @@ -63,7 +63,7 @@ fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext) -> O }; acc.add(AssistId("fix_visibility"), assist_label, target, |builder| { - builder.set_file(target_file); + builder.edit_file(target_file); match ctx.config.snippet_cap { Some(cap) => builder.insert_snippet(cap, offset, format!("$0{} ", missing_visibility)), None => builder.insert(offset, format!("{} ", missing_visibility)), @@ -106,7 +106,7 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) -> format!("Change visibility of {}.{} to {}", parent_name, target_name, missing_visibility); acc.add(AssistId("fix_visibility"), assist_label, target, |builder| { - builder.set_file(target_file); + builder.edit_file(target_file); match ctx.config.snippet_cap { Some(cap) => builder.insert_snippet(cap, offset, format!("$0{} ", missing_visibility)), None => builder.insert(offset, format!("{} ", missing_visibility)), -- cgit v1.2.3 From 5233766ce51d7593bb02d041bd63fa3aad44f666 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Jun 2020 11:33:28 +0200 Subject: Simplify unwrapping of blocks --- crates/ra_assists/src/handlers/unwrap_block.rs | 128 ++++++++++--------------- 1 file changed, 49 insertions(+), 79 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 8440c7d0f..c48ecaae8 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -1,8 +1,5 @@ use ra_fmt::unwrap_trivial_block; -use ra_syntax::{ - ast::{self, ElseBranch, Expr, LoopBodyOwner}, - match_ast, AstNode, TextRange, T, -}; +use ra_syntax::{ast, AstNode, TextRange, T}; use crate::{AssistContext, AssistId, Assists}; @@ -29,89 +26,62 @@ pub(crate) fn unwrap_block(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let parent = block.syntax().parent()?; let assist_id = AssistId("unwrap_block"); let assist_label = "Unwrap block"; - - let (expr, expr_to_unwrap) = match_ast! { - match parent { - ast::ForExpr(for_expr) => { - let block_expr = for_expr.loop_body()?; - let expr_to_unwrap = extract_expr(ctx.frange.range, block_expr)?; - (ast::Expr::ForExpr(for_expr), expr_to_unwrap) - }, - ast::WhileExpr(while_expr) => { - let block_expr = while_expr.loop_body()?; - let expr_to_unwrap = extract_expr(ctx.frange.range, block_expr)?; - (ast::Expr::WhileExpr(while_expr), expr_to_unwrap) - }, - ast::LoopExpr(loop_expr) => { - let block_expr = loop_expr.loop_body()?; - let expr_to_unwrap = extract_expr(ctx.frange.range, block_expr)?; - (ast::Expr::LoopExpr(loop_expr), expr_to_unwrap) - }, - ast::IfExpr(if_expr) => { - let mut resp = None; - - let then_branch = if_expr.then_branch()?; - if then_branch.l_curly_token()?.text_range().contains_range(ctx.frange.range) { - if let Some(ancestor) = if_expr.syntax().parent().and_then(ast::IfExpr::cast) { - // For `else if` blocks - let ancestor_then_branch = ancestor.then_branch()?; - let l_curly_token = then_branch.l_curly_token()?; - - let target = then_branch.syntax().text_range(); - return acc.add(assist_id, assist_label, target, |edit| { - let range_to_del_else_if = TextRange::new(ancestor_then_branch.syntax().text_range().end(), l_curly_token.text_range().start()); - let range_to_del_rest = TextRange::new(then_branch.syntax().text_range().end(), if_expr.syntax().text_range().end()); - - edit.delete(range_to_del_rest); - edit.delete(range_to_del_else_if); - edit.replace(target, update_expr_string(then_branch.to_string(), &[' ', '{'])); - }); - } else { - resp = Some((ast::Expr::IfExpr(if_expr.clone()), Expr::BlockExpr(then_branch))); - } - } else if let Some(else_branch) = if_expr.else_branch() { - match else_branch { - ElseBranch::Block(else_block) => { - let l_curly_token = else_block.l_curly_token()?; - if l_curly_token.text_range().contains_range(ctx.frange.range) { - let target = else_block.syntax().text_range(); - return acc.add(assist_id, assist_label, target, |edit| { - let range_to_del = TextRange::new(then_branch.syntax().text_range().end(), l_curly_token.text_range().start()); - - edit.delete(range_to_del); - edit.replace(target, update_expr_string(else_block.to_string(), &[' ', '{'])); - }); - } - }, - ElseBranch::IfExpr(_) => {}, - } + let parent = ast::Expr::cast(parent)?; + + match parent.clone() { + ast::Expr::ForExpr(_) | ast::Expr::WhileExpr(_) | ast::Expr::LoopExpr(_) => (), + ast::Expr::IfExpr(if_expr) => { + let then_branch = if_expr.then_branch()?; + if then_branch == block { + if let Some(ancestor) = if_expr.syntax().parent().and_then(ast::IfExpr::cast) { + // For `else if` blocks + let ancestor_then_branch = ancestor.then_branch()?; + + let target = then_branch.syntax().text_range(); + return acc.add(assist_id, assist_label, target, |edit| { + let range_to_del_else_if = TextRange::new( + ancestor_then_branch.syntax().text_range().end(), + l_curly_token.text_range().start(), + ); + let range_to_del_rest = TextRange::new( + then_branch.syntax().text_range().end(), + if_expr.syntax().text_range().end(), + ); + + edit.delete(range_to_del_rest); + edit.delete(range_to_del_else_if); + edit.replace( + target, + update_expr_string(then_branch.to_string(), &[' ', '{']), + ); + }); } - - resp? - }, - _ => return None, + } else { + let target = block.syntax().text_range(); + return acc.add(assist_id, assist_label, target, |edit| { + let range_to_del = TextRange::new( + then_branch.syntax().text_range().end(), + l_curly_token.text_range().start(), + ); + + edit.delete(range_to_del); + edit.replace(target, update_expr_string(block.to_string(), &[' ', '{'])); + }); + } } + _ => return None, }; - let target = expr_to_unwrap.syntax().text_range(); - acc.add(assist_id, assist_label, target, |edit| { - edit.replace( - expr.syntax().text_range(), - update_expr_string(expr_to_unwrap.to_string(), &[' ', '{', '\n']), + let unwrapped = unwrap_trivial_block(block); + let target = unwrapped.syntax().text_range(); + acc.add(assist_id, assist_label, target, |builder| { + builder.replace( + parent.syntax().text_range(), + update_expr_string(unwrapped.to_string(), &[' ', '{', '\n']), ); }) } -fn extract_expr(cursor_range: TextRange, block: ast::BlockExpr) -> Option { - let cursor_in_range = block.l_curly_token()?.text_range().contains_range(cursor_range); - - if cursor_in_range { - Some(unwrap_trivial_block(block)) - } else { - None - } -} - fn update_expr_string(expr_str: String, trim_start_pat: &[char]) -> String { let expr_string = expr_str.trim_start_matches(trim_start_pat); let mut expr_string_lines: Vec<&str> = expr_string.lines().collect(); -- cgit v1.2.3 From 53cc2c16e7a53660fb7322d96284ef199699e250 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Jun 2020 11:52:45 +0200 Subject: Unwrap block works with match arms --- crates/ra_assists/src/handlers/unwrap_block.rs | 44 +++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index c48ecaae8..1fb13f481 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -1,5 +1,11 @@ use ra_fmt::unwrap_trivial_block; -use ra_syntax::{ast, AstNode, TextRange, T}; +use ra_syntax::{ + ast::{ + self, + edit::{AstNodeEdit, IndentLevel}, + }, + AstNode, TextRange, T, +}; use crate::{AssistContext, AssistId, Assists}; @@ -21,15 +27,21 @@ use crate::{AssistContext, AssistId, Assists}; // } // ``` pub(crate) fn unwrap_block(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let l_curly_token = ctx.find_token_at_offset(T!['{'])?; - let block = ast::BlockExpr::cast(l_curly_token.parent())?; - let parent = block.syntax().parent()?; let assist_id = AssistId("unwrap_block"); let assist_label = "Unwrap block"; + + let l_curly_token = ctx.find_token_at_offset(T!['{'])?; + let mut block = ast::BlockExpr::cast(l_curly_token.parent())?; + let mut parent = block.syntax().parent()?; + if ast::MatchArm::can_cast(parent.kind()) { + parent = parent.ancestors().find(|it| ast::MatchExpr::can_cast(it.kind()))? + } + let parent = ast::Expr::cast(parent)?; match parent.clone() { ast::Expr::ForExpr(_) | ast::Expr::WhileExpr(_) | ast::Expr::LoopExpr(_) => (), + ast::Expr::MatchExpr(_) => block = block.dedent(IndentLevel(1)), ast::Expr::IfExpr(if_expr) => { let then_branch = if_expr.then_branch()?; if then_branch == block { @@ -459,6 +471,30 @@ mod tests { ); } + #[test] + fn unwrap_match_arm() { + check_assist( + unwrap_block, + r#" +fn main() { + match rel_path { + Ok(rel_path) => {<|> + let rel_path = RelativePathBuf::from_path(rel_path).ok()?; + Some((*id, rel_path)) + } + Err(_) => None, + } +} +"#, + r#" +fn main() { + let rel_path = RelativePathBuf::from_path(rel_path).ok()?; + Some((*id, rel_path)) +} +"#, + ); + } + #[test] fn simple_if_in_while_bad_cursor_position() { check_assist_not_applicable( -- cgit v1.2.3 From 8cad7d1a2b2546986aa8f42207875b1c8eed948a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Jun 2020 12:38:47 +0200 Subject: Use correct indent when replacing with match --- crates/ra_assists/src/handlers/early_return.rs | 2 +- .../src/handlers/replace_if_let_with_match.rs | 37 ++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/early_return.rs b/crates/ra_assists/src/handlers/early_return.rs index 4cc75a7ce..dfade7432 100644 --- a/crates/ra_assists/src/handlers/early_return.rs +++ b/crates/ra_assists/src/handlers/early_return.rs @@ -154,7 +154,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) parent_block: &ast::BlockExpr, if_expr: &ast::IfExpr, ) -> SyntaxNode { - let then_block_items = then_block.dedent(IndentLevel::from(1)); + let then_block_items = then_block.dedent(IndentLevel(1)); let end_of_then = then_block_items.syntax().last_child_or_token().unwrap(); let end_of_then = if end_of_then.prev_sibling_or_token().map(|n| n.kind()) == Some(WHITESPACE) { diff --git a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs index e016f51c3..dfcd787de 100644 --- a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs @@ -51,6 +51,7 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) acc.add(AssistId("replace_if_let_with_match"), "Replace with match", target, move |edit| { let match_expr = { let then_arm = { + let then_block = then_block.reset_indent().indent(IndentLevel(1)); let then_expr = unwrap_trivial_block(then_block); make::match_arm(vec![pat.clone()], then_expr) }; @@ -64,8 +65,8 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext) let else_expr = unwrap_trivial_block(else_block); make::match_arm(vec![pattern], else_expr) }; - make::expr_match(expr, make::match_arm_list(vec![then_arm, else_arm])) - .indent(IndentLevel::from_node(if_expr.syntax())) + let match_expr = make::expr_match(expr, make::match_arm_list(vec![then_arm, else_arm])); + match_expr.indent(IndentLevel::from_node(if_expr.syntax())) }; edit.replace_ast::(if_expr.into(), match_expr); @@ -213,4 +214,36 @@ fn foo(x: Result) { "#, ); } + + #[test] + fn nested_indent() { + check_assist( + replace_if_let_with_match, + r#" +fn main() { + if true { + <|>if let Ok(rel_path) = path.strip_prefix(root_path) { + let rel_path = RelativePathBuf::from_path(rel_path).ok()?; + Some((*id, rel_path)) + } else { + None + } + } +} +"#, + r#" +fn main() { + if true { + match path.strip_prefix(root_path) { + Ok(rel_path) => { + let rel_path = RelativePathBuf::from_path(rel_path).ok()?; + Some((*id, rel_path)) + } + _ => None, + } + } +} +"#, + ) + } } -- cgit v1.2.3 From a70a0ca73ceac339de4e1df6d561894e485ba5ee Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 9 Jun 2020 18:48:44 +0200 Subject: ImportsLocator: use ImportMap for non-local crates --- crates/ra_assists/src/handlers/auto_import.rs | 47 ++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs index edf96d50e..1f4142747 100644 --- a/crates/ra_assists/src/handlers/auto_import.rs +++ b/crates/ra_assists/src/handlers/auto_import.rs @@ -130,7 +130,7 @@ impl AutoImportAssets { fn search_for_imports(&self, db: &RootDatabase) -> BTreeSet { let _p = profile("auto_import::search_for_imports"); let current_crate = self.module_with_name_to_import.krate(); - ImportsLocator::new(db) + ImportsLocator::new(db, current_crate) .find_imports(&self.get_search_query()) .into_iter() .filter_map(|candidate| match &self.import_candidate { @@ -841,4 +841,49 @@ fn main() { ", ) } + + #[test] + fn dep_import() { + check_assist( + auto_import, + r" + //- /lib.rs crate:dep + pub struct Struct; + + //- /main.rs crate:main deps:dep + fn main() { + Struct<|> + }", + r"use dep::Struct; + +fn main() { + Struct +} +", + ); + } + + #[test] + fn whole_segment() { + check_assist( + auto_import, + 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"use dep::fmt; + +struct S; +impl fmt::Display for S {} +", + ); + } } -- cgit v1.2.3 From 781b514e5886cbaff88483cdeddc504effef299c Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 10 Jun 2020 11:39:06 +0200 Subject: Add test for macro generated items --- crates/ra_assists/src/handlers/auto_import.rs | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs index 1f4142747..86a173ff5 100644 --- a/crates/ra_assists/src/handlers/auto_import.rs +++ b/crates/ra_assists/src/handlers/auto_import.rs @@ -865,6 +865,7 @@ fn main() { #[test] fn whole_segment() { + // Tests that only imports whose last segment matches the identifier get suggested. check_assist( auto_import, r" @@ -883,6 +884,36 @@ fn main() { struct S; impl fmt::Display for S {} +", + ); + } + + #[test] + fn macro_generated() { + // Tests that macro-generated items are suggested from external crates. + check_assist( + auto_import, + r" + //- /lib.rs crate:dep + + macro_rules! mac { + () => { + pub struct Cheese; + }; + } + + mac!(); + + //- /main.rs crate:main deps:dep + + fn main() { + Cheese<|>; + }", + r"use dep::Cheese; + +fn main() { + Cheese; +} ", ); } -- cgit v1.2.3 From 7e83ed99a887f959bd4cf97357faf373a09f9269 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 10 Jun 2020 16:04:55 +0200 Subject: Respect casing when searching for imports --- crates/ra_assists/src/handlers/auto_import.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs index 86a173ff5..5092bf336 100644 --- a/crates/ra_assists/src/handlers/auto_import.rs +++ b/crates/ra_assists/src/handlers/auto_import.rs @@ -914,6 +914,31 @@ impl fmt::Display for S {} fn main() { Cheese; } +", + ); + } + + #[test] + fn casing() { + // Tests that differently cased names don't interfere and we only suggest the matching one. + check_assist( + auto_import, + r" + //- /lib.rs crate:dep + + pub struct FMT; + pub struct fmt; + + //- /main.rs crate:main deps:dep + + fn main() { + FMT<|>; + }", + r"use dep::FMT; + +fn main() { + FMT; +} ", ); } -- cgit v1.2.3