From d3a112d68c74cbd02630f6c909071c94872c193f Mon Sep 17 00:00:00 2001 From: unexge Date: Sun, 4 Apr 2021 00:01:49 +0300 Subject: [PATCH 01/10] Allow including `Self` kw references to `FindUsages` --- crates/ide_db/src/search.rs | 43 +++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index b55e3851eb..02f5e514b5 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -4,10 +4,13 @@ //! get a super-set of matches. Then, we we confirm each match using precise //! name resolution. -use std::{convert::TryInto, mem}; +use std::{convert::TryInto, iter, mem}; use base_db::{FileId, FileRange, SourceDatabase, SourceDatabaseExt}; -use hir::{DefWithBody, HasAttrs, HasSource, InFile, ModuleSource, Semantics, Visibility}; +use either::Either; +use hir::{ + DefWithBody, HasAttrs, HasSource, InFile, ModuleDef, ModuleSource, Semantics, Visibility, +}; use once_cell::unsync::Lazy; use rustc_hash::FxHashMap; use syntax::{ast, match_ast, AstNode, TextRange, TextSize}; @@ -295,7 +298,7 @@ impl Definition { } pub fn usages<'a>(&'a self, sema: &'a Semantics) -> FindUsages<'a> { - FindUsages { def: self, sema, scope: None } + FindUsages { def: self, sema, scope: None, include_self_kw_refs: false } } } @@ -303,9 +306,15 @@ pub struct FindUsages<'a> { def: &'a Definition, sema: &'a Semantics<'a, RootDatabase>, scope: Option, + include_self_kw_refs: bool, } impl<'a> FindUsages<'a> { + pub fn include_self_kw_refs(mut self, include: bool) -> FindUsages<'a> { + self.include_self_kw_refs = include; + self + } + pub fn in_scope(self, scope: SearchScope) -> FindUsages<'a> { self.set_scope(Some(scope)) } @@ -352,6 +361,8 @@ impl<'a> FindUsages<'a> { }; let pat = name.as_str(); + let search_for_self = self.include_self_kw_refs; + for (file_id, search_range) in search_scope { let text = sema.db.file_text(file_id); let search_range = @@ -359,7 +370,13 @@ impl<'a> FindUsages<'a> { let tree = Lazy::new(|| sema.parse(file_id).syntax().clone()); - for (idx, _) in text.match_indices(pat) { + let matches = text.match_indices(pat).chain(if search_for_self { + Either::Left(text.match_indices("Self")) + } else { + Either::Right(iter::empty()) + }); + + for (idx, _) in matches { let offset: TextSize = idx.try_into().unwrap(); if !search_range.contains_inclusive(offset) { continue; @@ -413,6 +430,24 @@ impl<'a> FindUsages<'a> { sink: &mut dyn FnMut(FileId, FileReference) -> bool, ) -> bool { match NameRefClass::classify(self.sema, &name_ref) { + Some(NameRefClass::Definition(Definition::SelfType(impl_))) => { + let ty = impl_.self_ty(self.sema.db); + + if let Some(adt) = ty.as_adt() { + if &Definition::ModuleDef(ModuleDef::Adt(adt)) == self.def { + let FileRange { file_id, range } = + self.sema.original_range(name_ref.syntax()); + let reference = FileReference { + range, + name: ast::NameLike::NameRef(name_ref.clone()), + access: None, + }; + return sink(file_id, reference); + } + } + + false + } Some(NameRefClass::Definition(def)) if &def == self.def => { let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax()); let reference = FileReference { From 8d4be829e09dae4af7b4a82b379fdb8700b0929f Mon Sep 17 00:00:00 2001 From: unexge Date: Sun, 4 Apr 2021 00:04:31 +0300 Subject: [PATCH 02/10] Add convert tuple struct to named struct assist --- .../convert_tuple_struct_to_named_struct.rs | 345 ++++++++++++++++++ crates/ide_assists/src/lib.rs | 2 + crates/ide_assists/src/tests/generated.rs | 15 + crates/ide_db/src/search.rs | 18 +- crates/syntax/src/ast/make.rs | 26 ++ 5 files changed, 397 insertions(+), 9 deletions(-) create mode 100644 crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs new file mode 100644 index 0000000000..b2f7be011e --- /dev/null +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -0,0 +1,345 @@ +use hir::{Adt, ModuleDef}; +use ide_db::defs::Definition; +use syntax::{ + ast::{self, AstNode, GenericParamsOwner, VisibilityOwner}, + match_ast, +}; + +use crate::{assist_context::AssistBuilder, AssistContext, AssistId, AssistKind, Assists}; + +// Assist: convert_tuple_struct_to_named_struct +// +// Converts tuple struct to struct with named fields. +// +// ``` +// struct Inner; +// struct A$0(Inner); +// ``` +// -> +// ``` +// struct Inner; +// struct A { field1: Inner } +// ``` +pub(crate) fn convert_tuple_struct_to_named_struct( + acc: &mut Assists, + ctx: &AssistContext, +) -> Option<()> { + let strukt = ctx.find_node_at_offset::()?; + let tuple_fields = match strukt.field_list()? { + ast::FieldList::TupleFieldList(it) => it, + ast::FieldList::RecordFieldList(_) => return None, + }; + + let target = strukt.syntax().text_range(); + acc.add( + AssistId("convert_tuple_struct_to_named_struct", AssistKind::RefactorRewrite), + "Convert to named struct", + target, + |edit| { + let names = generate_names(tuple_fields.fields()); + edit_field_references(ctx, edit, tuple_fields.fields(), &names); + edit_struct_references(ctx, edit, &strukt, &names); + edit_struct_def(ctx, edit, &strukt, tuple_fields, names); + }, + ) +} + +fn edit_struct_def( + ctx: &AssistContext, + edit: &mut AssistBuilder, + strukt: &ast::Struct, + tuple_fields: ast::TupleFieldList, + names: Vec, +) { + let record_fields = tuple_fields + .fields() + .zip(names) + .map(|(f, name)| ast::make::record_field(f.visibility(), name, f.ty().unwrap())); + let record_fields = ast::make::record_field_list(record_fields); + let tuple_fields_text_range = tuple_fields.syntax().text_range(); + + edit.edit_file(ctx.frange.file_id); + + if let Some(w) = strukt.where_clause() { + edit.delete(w.syntax().text_range()); + edit.insert(tuple_fields_text_range.start(), ast::make::tokens::single_newline().text()); + edit.insert(tuple_fields_text_range.start(), w.syntax().text()); + edit.insert(tuple_fields_text_range.start(), ","); + edit.insert(tuple_fields_text_range.start(), ast::make::tokens::single_newline().text()); + } else { + edit.insert(tuple_fields_text_range.start(), ast::make::tokens::single_space().text()); + } + + edit.replace(tuple_fields_text_range, record_fields.to_string()); + strukt.semicolon_token().map(|t| edit.delete(t.text_range())); +} + +fn edit_struct_references( + ctx: &AssistContext, + edit: &mut AssistBuilder, + strukt: &ast::Struct, + names: &[ast::Name], +) { + let strukt_def = ctx.sema.to_def(strukt).unwrap(); + let usages = Definition::ModuleDef(ModuleDef::Adt(Adt::Struct(strukt_def))) + .usages(&ctx.sema) + .include_self_kw_refs(true) + .all(); + + for (file_id, refs) in usages { + edit.edit_file(file_id); + for r in refs { + for node in r.name.syntax().ancestors() { + match_ast! { + match node { + ast::TupleStructPat(tuple_struct_pat) => { + edit.replace( + tuple_struct_pat.syntax().text_range(), + ast::make::record_pat_with_fields( + tuple_struct_pat.path().unwrap(), + ast::make::record_pat_field_list(tuple_struct_pat.fields().zip(names).map( + |(pat, name)| { + ast::make::record_pat_field( + ast::make::name_ref(&name.to_string()), + pat, + ) + }, + )), + ) + .to_string(), + ); + }, + // for tuple struct creations like: Foo(42) + ast::CallExpr(call_expr) => { + let path = call_expr.syntax().descendants().find_map(ast::PathExpr::cast).unwrap(); + let arg_list = + call_expr.syntax().descendants().find_map(ast::ArgList::cast).unwrap(); + + edit.replace( + call_expr.syntax().text_range(), + ast::make::record_expr( + path.path().unwrap(), + ast::make::record_expr_field_list(arg_list.args().zip(names).map( + |(expr, name)| { + ast::make::record_expr_field( + ast::make::name_ref(&name.to_string()), + Some(expr), + ) + }, + )), + ) + .to_string(), + ); + }, + _ => () + } + } + } + } + } +} + +fn edit_field_references( + ctx: &AssistContext, + edit: &mut AssistBuilder, + fields: impl Iterator, + names: &[ast::Name], +) { + for (field, name) in fields.zip(names) { + let field = match ctx.sema.to_def(&field) { + Some(it) => it, + None => continue, + }; + let def = Definition::Field(field); + let usages = def.usages(&ctx.sema).all(); + for (file_id, refs) in usages { + edit.edit_file(file_id); + for r in refs { + if let Some(name_ref) = r.name.as_name_ref() { + edit.replace(name_ref.syntax().text_range(), name.text()); + } + } + } + } +} + +fn generate_names(fields: impl Iterator) -> Vec { + fields.enumerate().map(|(i, _)| ast::make::name(&format!("field{}", i + 1))).collect() +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn not_applicable_other_than_tuple_struct() { + check_assist_not_applicable( + convert_tuple_struct_to_named_struct, + r#"struct Foo$0 { bar: u32 };"#, + ); + check_assist_not_applicable(convert_tuple_struct_to_named_struct, r#"struct Foo$0;"#); + } + + #[test] + fn convert_simple_struct() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct Inner; +struct A$0(Inner); + +impl A { + fn new() -> A { + A(Inner) + } + + fn into_inner(self) -> Inner { + self.0 + } +}"#, + r#" +struct Inner; +struct A { field1: Inner } + +impl A { + fn new() -> A { + A { field1: Inner } + } + + fn into_inner(self) -> Inner { + self.field1 + } +}"#, + ); + } + + #[test] + fn convert_struct_referenced_via_self_kw() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct Inner; +struct A$0(Inner); + +impl A { + fn new() -> Self { + Self(Inner) + } + + fn into_inner(self) -> Inner { + self.0 + } +}"#, + r#" +struct Inner; +struct A { field1: Inner } + +impl A { + fn new() -> Self { + Self { field1: Inner } + } + + fn into_inner(self) -> Inner { + self.field1 + } +}"#, + ); + } + + #[test] + fn convert_destructured_struct() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct Inner; +struct A$0(Inner); + +impl A { + fn into_inner(self) -> Inner { + let A(first) = self; + first + } + + fn into_inner_via_self(self) -> Inner { + let Self(first) = self; + first + } +}"#, + r#" +struct Inner; +struct A { field1: Inner } + +impl A { + fn into_inner(self) -> Inner { + let A { field1: first } = self; + first + } + + fn into_inner_via_self(self) -> Inner { + let Self { field1: first } = self; + first + } +}"#, + ); + } + + #[test] + fn convert_struct_with_visibility() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct A$0(pub u32, pub(crate) u64); + +impl A { + fn new() -> A { + A(42, 42) + } + + fn into_first(self) -> u32 { + self.0 + } + + fn into_second(self) -> u64 { + self.1 + } +}"#, + r#" +struct A { pub field1: u32, pub(crate) field2: u64 } + +impl A { + fn new() -> A { + A { field1: 42, field2: 42 } + } + + fn into_first(self) -> u32 { + self.field1 + } + + fn into_second(self) -> u64 { + self.field2 + } +}"#, + ); + } + + #[test] + fn convert_struct_with_where_clause() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct Wrap$0(T) +where + T: Display; +"#, + r#" +struct Wrap +where + T: Display, +{ field1: T } + +"#, + ); + } +} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 3e2c82dace..1c55b9fbf9 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -118,6 +118,7 @@ mod handlers { mod convert_comment_block; mod convert_iter_for_each_to_for; mod convert_into_to_from; + mod convert_tuple_struct_to_named_struct; mod early_return; mod expand_glob_import; mod extract_function; @@ -187,6 +188,7 @@ mod handlers { convert_comment_block::convert_comment_block, convert_iter_for_each_to_for::convert_iter_for_each_to_for, convert_into_to_from::convert_into_to_from, + convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct, early_return::convert_to_guarded_return, expand_glob_import::expand_glob_import, extract_struct_from_enum_variant::extract_struct_from_enum_variant, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 27a22ca10c..53f455adf9 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -291,6 +291,21 @@ fn main() { ) } +#[test] +fn doctest_convert_tuple_struct_to_named_struct() { + check_doc_test( + "convert_tuple_struct_to_named_struct", + r#####" +struct Inner; +struct A$0(Inner); +"#####, + r#####" +struct Inner; +struct A { field1: Inner } +"#####, + ) +} + #[test] fn doctest_expand_glob_import() { check_doc_test( diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 02f5e514b5..90e4e7b037 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -430,6 +430,15 @@ impl<'a> FindUsages<'a> { sink: &mut dyn FnMut(FileId, FileReference) -> bool, ) -> bool { match NameRefClass::classify(self.sema, &name_ref) { + Some(NameRefClass::Definition(def)) if &def == self.def => { + let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax()); + let reference = FileReference { + range, + name: ast::NameLike::NameRef(name_ref.clone()), + access: reference_access(&def, &name_ref), + }; + sink(file_id, reference) + } Some(NameRefClass::Definition(Definition::SelfType(impl_))) => { let ty = impl_.self_ty(self.sema.db); @@ -448,15 +457,6 @@ impl<'a> FindUsages<'a> { false } - Some(NameRefClass::Definition(def)) if &def == self.def => { - let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax()); - let reference = FileReference { - range, - name: ast::NameLike::NameRef(name_ref.clone()), - access: reference_access(&def, &name_ref), - }; - sink(file_id, reference) - } Some(NameRefClass::FieldShorthand { local_ref: local, field_ref: field }) => { let FileRange { file_id, range } = self.sema.original_range(name_ref.syntax()); let reference = match self.def { diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index c6a7b99b7c..3a588e5400 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -133,6 +133,17 @@ pub fn use_(visibility: Option, use_tree: ast::UseTree) -> ast: ast_from_text(&format!("{}use {};", visibility, use_tree)) } +pub fn record_expr(path: ast::Path, fields: ast::RecordExprFieldList) -> ast::RecordExpr { + ast_from_text(&format!("fn f() {{ {} {} }}", path, fields)) +} + +pub fn record_expr_field_list( + fields: impl IntoIterator, +) -> ast::RecordExprFieldList { + let fields = fields.into_iter().join(", "); + ast_from_text(&format!("fn f() {{ S {{ {} }} }}", fields)) +} + pub fn record_expr_field(name: ast::NameRef, expr: Option) -> ast::RecordExprField { return match expr { Some(expr) => from_text(&format!("{}: {}", name, expr)), @@ -325,6 +336,21 @@ pub fn record_pat(path: ast::Path, pats: impl IntoIterator) -> } } +pub fn record_pat_with_fields(path: ast::Path, fields: ast::RecordPatFieldList) -> ast::RecordPat { + ast_from_text(&format!("fn f({} {}: ()))", path, fields)) +} + +pub fn record_pat_field_list( + fields: impl IntoIterator, +) -> ast::RecordPatFieldList { + let fields = fields.into_iter().join(", "); + ast_from_text(&format!("fn f(S {{ {} }}: ()))", fields)) +} + +pub fn record_pat_field(name_ref: ast::NameRef, pat: ast::Pat) -> ast::RecordPatField { + ast_from_text(&format!("fn f(S {{ {}: {} }}: ()))", name_ref, pat)) +} + /// Returns a `BindPat` if the path has just one segment, a `PathPat` otherwise. pub fn path_pat(path: ast::Path) -> ast::Pat { return from_text(&path.to_string()); From 53599d11f6fec625bef69aeb699e657cfc37c310 Mon Sep 17 00:00:00 2001 From: unexge Date: Wed, 21 Apr 2021 10:27:26 +0300 Subject: [PATCH 03/10] Fix incorrectly replacing method calls in "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 58 +++++++++++++------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index b2f7be011e..b68b1d06f7 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -1,5 +1,5 @@ use hir::{Adt, ModuleDef}; -use ide_db::defs::Definition; +use ide_db::defs::{Definition, NameRefClass}; use syntax::{ ast::{self, AstNode, GenericParamsOwner, VisibilityOwner}, match_ast, @@ -80,11 +80,9 @@ fn edit_struct_references( strukt: &ast::Struct, names: &[ast::Name], ) { - let strukt_def = ctx.sema.to_def(strukt).unwrap(); - let usages = Definition::ModuleDef(ModuleDef::Adt(Adt::Struct(strukt_def))) - .usages(&ctx.sema) - .include_self_kw_refs(true) - .all(); + let strukt = ctx.sema.to_def(strukt).unwrap(); + let strukt_def = Definition::ModuleDef(ModuleDef::Adt(Adt::Struct(strukt))); + let usages = strukt_def.usages(&ctx.sema).include_self_kw_refs(true).all(); for (file_id, refs) in usages { edit.edit_file(file_id); @@ -109,16 +107,26 @@ fn edit_struct_references( .to_string(), ); }, - // for tuple struct creations like: Foo(42) + // for tuple struct creations like Foo(42) ast::CallExpr(call_expr) => { - let path = call_expr.syntax().descendants().find_map(ast::PathExpr::cast).unwrap(); + let path = call_expr.syntax().descendants().find_map(ast::PathExpr::cast).unwrap().path().unwrap(); + + // this also includes method calls like Foo::new(42), we should skip them + if let Some(Some(name_ref)) = path.segment().map(|s| s.name_ref()) { + match NameRefClass::classify(&ctx.sema, &name_ref) { + Some(NameRefClass::Definition(Definition::SelfType(_))) => {}, + Some(NameRefClass::Definition(def)) if def == strukt_def => {}, + _ => continue, + }; + } + let arg_list = call_expr.syntax().descendants().find_map(ast::ArgList::cast).unwrap(); edit.replace( call_expr.syntax().text_range(), ast::make::record_expr( - path.path().unwrap(), + path, ast::make::record_expr_field_list(arg_list.args().zip(names).map( |(expr, name)| { ast::make::record_expr_field( @@ -191,8 +199,12 @@ struct Inner; struct A$0(Inner); impl A { - fn new() -> A { - A(Inner) + fn new(inner: Inner) -> A { + A(inner) + } + + fn new_with_default() -> A { + A::new(Inner) } fn into_inner(self) -> Inner { @@ -204,8 +216,12 @@ struct Inner; struct A { field1: Inner } impl A { - fn new() -> A { - A { field1: Inner } + fn new(inner: Inner) -> A { + A { field1: inner } + } + + fn new_with_default() -> A { + A::new(Inner) } fn into_inner(self) -> Inner { @@ -224,8 +240,12 @@ struct Inner; struct A$0(Inner); impl A { - fn new() -> Self { - Self(Inner) + fn new(inner: Inner) -> Self { + Self(inner) + } + + fn new_with_default() -> Self { + Self::new(Inner) } fn into_inner(self) -> Inner { @@ -237,8 +257,12 @@ struct Inner; struct A { field1: Inner } impl A { - fn new() -> Self { - Self { field1: Inner } + fn new(inner: Inner) -> Self { + Self { field1: inner } + } + + fn new_with_default() -> Self { + Self::new(Inner) } fn into_inner(self) -> Inner { From e0a60e71d7aab59858f62a16fce52ba35aeafc28 Mon Sep 17 00:00:00 2001 From: unexge Date: Wed, 21 Apr 2021 10:57:36 +0300 Subject: [PATCH 04/10] Add larger example for "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 34 ++++++++++++++++--- crates/ide_assists/src/tests/generated.rs | 34 ++++++++++++++++--- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index b68b1d06f7..ee6ddfbc60 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -12,13 +12,39 @@ use crate::{assist_context::AssistBuilder, AssistContext, AssistId, AssistKind, // Converts tuple struct to struct with named fields. // // ``` -// struct Inner; -// struct A$0(Inner); +// struct Point$0(f32, f32); +// +// impl Point { +// pub fn new(x: f32, y: f32) -> Self { +// Point(x, y) +// } +// +// pub fn x(&self) -> f32 { +// self.0 +// } +// +// pub fn y(&self) -> f32 { +// self.1 +// } +// } // ``` // -> // ``` -// struct Inner; -// struct A { field1: Inner } +// struct Point { field1: f32, field2: f32 } +// +// impl Point { +// pub fn new(x: f32, y: f32) -> Self { +// Point { field1: x, field2: y } +// } +// +// pub fn x(&self) -> f32 { +// self.field1 +// } +// +// pub fn y(&self) -> f32 { +// self.field2 +// } +// } // ``` pub(crate) fn convert_tuple_struct_to_named_struct( acc: &mut Assists, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 53f455adf9..f4a4749c80 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -296,12 +296,38 @@ fn doctest_convert_tuple_struct_to_named_struct() { check_doc_test( "convert_tuple_struct_to_named_struct", r#####" -struct Inner; -struct A$0(Inner); +struct Point$0(f32, f32); + +impl Point { + pub fn new(x: f32, y: f32) -> Self { + Point(x, y) + } + + pub fn x(&self) -> f32 { + self.0 + } + + pub fn y(&self) -> f32 { + self.1 + } +} "#####, r#####" -struct Inner; -struct A { field1: Inner } +struct Point { field1: f32, field2: f32 } + +impl Point { + pub fn new(x: f32, y: f32) -> Self { + Point { field1: x, field2: y } + } + + pub fn x(&self) -> f32 { + self.field1 + } + + pub fn y(&self) -> f32 { + self.field2 + } +} "#####, ) } From 96d694062bfbb94592dfdef7517c500aedcb9185 Mon Sep 17 00:00:00 2001 From: unexge Date: Wed, 21 Apr 2021 16:01:13 +0300 Subject: [PATCH 05/10] Remove `unwrap`s in "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index ee6ddfbc60..086a444259 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -1,4 +1,4 @@ -use hir::{Adt, ModuleDef}; +use hir::{Adt, ModuleDef, Struct}; use ide_db::defs::{Definition, NameRefClass}; use syntax::{ ast::{self, AstNode, GenericParamsOwner, VisibilityOwner}, @@ -55,6 +55,7 @@ pub(crate) fn convert_tuple_struct_to_named_struct( ast::FieldList::TupleFieldList(it) => it, ast::FieldList::RecordFieldList(_) => return None, }; + let strukt_def = ctx.sema.to_def(&strukt)?; let target = strukt.syntax().text_range(); acc.add( @@ -64,7 +65,7 @@ pub(crate) fn convert_tuple_struct_to_named_struct( |edit| { let names = generate_names(tuple_fields.fields()); edit_field_references(ctx, edit, tuple_fields.fields(), &names); - edit_struct_references(ctx, edit, &strukt, &names); + edit_struct_references(ctx, edit, strukt_def, &names); edit_struct_def(ctx, edit, &strukt, tuple_fields, names); }, ) @@ -80,7 +81,7 @@ fn edit_struct_def( let record_fields = tuple_fields .fields() .zip(names) - .map(|(f, name)| ast::make::record_field(f.visibility(), name, f.ty().unwrap())); + .filter_map(|(f, name)| Some(ast::make::record_field(f.visibility(), name, f.ty()?))); let record_fields = ast::make::record_field_list(record_fields); let tuple_fields_text_range = tuple_fields.syntax().text_range(); @@ -103,10 +104,9 @@ fn edit_struct_def( fn edit_struct_references( ctx: &AssistContext, edit: &mut AssistBuilder, - strukt: &ast::Struct, + strukt: Struct, names: &[ast::Name], ) { - let strukt = ctx.sema.to_def(strukt).unwrap(); let strukt_def = Definition::ModuleDef(ModuleDef::Adt(Adt::Struct(strukt))); let usages = strukt_def.usages(&ctx.sema).include_self_kw_refs(true).all(); @@ -117,10 +117,15 @@ fn edit_struct_references( match_ast! { match node { ast::TupleStructPat(tuple_struct_pat) => { + let path = match tuple_struct_pat.path() { + Some(it) => it, + None => continue, + }; + edit.replace( tuple_struct_pat.syntax().text_range(), ast::make::record_pat_with_fields( - tuple_struct_pat.path().unwrap(), + path, ast::make::record_pat_field_list(tuple_struct_pat.fields().zip(names).map( |(pat, name)| { ast::make::record_pat_field( @@ -135,7 +140,10 @@ fn edit_struct_references( }, // for tuple struct creations like Foo(42) ast::CallExpr(call_expr) => { - let path = call_expr.syntax().descendants().find_map(ast::PathExpr::cast).unwrap().path().unwrap(); + let path = match call_expr.syntax().descendants().find_map(ast::PathExpr::cast).map(|expr| expr.path()) { + Some(Some(it)) => it, + _ => continue, + }; // this also includes method calls like Foo::new(42), we should skip them if let Some(Some(name_ref)) = path.segment().map(|s| s.name_ref()) { @@ -146,8 +154,10 @@ fn edit_struct_references( }; } - let arg_list = - call_expr.syntax().descendants().find_map(ast::ArgList::cast).unwrap(); + let arg_list = match call_expr.syntax().descendants().find_map(ast::ArgList::cast) { + Some(it) => it, + None => continue, + }; edit.replace( call_expr.syntax().text_range(), From 6630266ce17112bb42f2fa62f975d53512ace682 Mon Sep 17 00:00:00 2001 From: unexge Date: Wed, 21 Apr 2021 16:20:08 +0300 Subject: [PATCH 06/10] Add multi file test for "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index 086a444259..0df96be031 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -384,6 +384,39 @@ impl A { ); } + #[test] + fn convert_struct_with_multi_file_references() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +//- /main.rs +struct Inner; +struct A$0(Inner); + +mod foo; + +//- /foo.rs +use crate::{A, Inner}; +fn f() { + let a = A(Inner); +} +"#, + r#" +//- /main.rs +struct Inner; +struct A { field1: Inner } + +mod foo; + +//- /foo.rs +use crate::{A, Inner}; +fn f() { + let a = A { field1: Inner }; +} +"#, + ); + } + #[test] fn convert_struct_with_where_clause() { check_assist( From 322cd1fa7fb870f44860bc42ef3b894e5a42bb70 Mon Sep 17 00:00:00 2001 From: unexge Date: Wed, 21 Apr 2021 16:42:47 +0300 Subject: [PATCH 07/10] Use multiple loops instead of `Iterator::chain` in `FindUsages` --- crates/ide_db/src/search.rs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 90e4e7b037..8f899ea56b 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -4,10 +4,9 @@ //! get a super-set of matches. Then, we we confirm each match using precise //! name resolution. -use std::{convert::TryInto, iter, mem}; +use std::{convert::TryInto, mem}; use base_db::{FileId, FileRange, SourceDatabase, SourceDatabaseExt}; -use either::Either; use hir::{ DefWithBody, HasAttrs, HasSource, InFile, ModuleDef, ModuleSource, Semantics, Visibility, }; @@ -370,37 +369,47 @@ impl<'a> FindUsages<'a> { let tree = Lazy::new(|| sema.parse(file_id).syntax().clone()); - let matches = text.match_indices(pat).chain(if search_for_self { - Either::Left(text.match_indices("Self")) - } else { - Either::Right(iter::empty()) - }); - - for (idx, _) in matches { + let mut handle_match = |idx: usize| -> bool { let offset: TextSize = idx.try_into().unwrap(); if !search_range.contains_inclusive(offset) { - continue; + return false; } if let Some(name) = sema.find_node_at_offset_with_descend(&tree, offset) { match name { ast::NameLike::NameRef(name_ref) => { if self.found_name_ref(&name_ref, sink) { - return; + return true; } } ast::NameLike::Name(name) => { if self.found_name(&name, sink) { - return; + return true; } } ast::NameLike::Lifetime(lifetime) => { if self.found_lifetime(&lifetime, sink) { - return; + return true; } } } } + + return false; + }; + + for (idx, _) in text.match_indices(pat) { + if handle_match(idx) { + return; + } + } + + if search_for_self { + for (idx, _) in text.match_indices("Self") { + if handle_match(idx) { + return; + } + } } } } From affd8d351863a4f9f5729eb3487942fa7bfa5755 Mon Sep 17 00:00:00 2001 From: unexge Date: Thu, 22 Apr 2021 11:33:56 +0300 Subject: [PATCH 08/10] Move reference editing logic into own function to make error handling more ease in "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 129 +++++++++--------- 1 file changed, 61 insertions(+), 68 deletions(-) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index 0df96be031..994e1a01a0 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -1,8 +1,7 @@ -use hir::{Adt, ModuleDef, Struct}; use ide_db::defs::{Definition, NameRefClass}; use syntax::{ ast::{self, AstNode, GenericParamsOwner, VisibilityOwner}, - match_ast, + match_ast, SyntaxNode, }; use crate::{assist_context::AssistBuilder, AssistContext, AssistId, AssistKind, Assists}; @@ -104,80 +103,74 @@ fn edit_struct_def( fn edit_struct_references( ctx: &AssistContext, edit: &mut AssistBuilder, - strukt: Struct, + strukt: hir::Struct, names: &[ast::Name], ) { - let strukt_def = Definition::ModuleDef(ModuleDef::Adt(Adt::Struct(strukt))); + let strukt_def = Definition::ModuleDef(hir::ModuleDef::Adt(hir::Adt::Struct(strukt))); let usages = strukt_def.usages(&ctx.sema).include_self_kw_refs(true).all(); + let edit_node = |edit: &mut AssistBuilder, node: SyntaxNode| -> Option<()> { + match_ast! { + match node { + ast::TupleStructPat(tuple_struct_pat) => { + edit.replace( + tuple_struct_pat.syntax().text_range(), + ast::make::record_pat_with_fields( + tuple_struct_pat.path()?, + ast::make::record_pat_field_list(tuple_struct_pat.fields().zip(names).map( + |(pat, name)| { + ast::make::record_pat_field( + ast::make::name_ref(&name.to_string()), + pat, + ) + }, + )), + ) + .to_string(), + ); + }, + // for tuple struct creations like Foo(42) + ast::CallExpr(call_expr) => { + let path = call_expr.syntax().descendants().find_map(ast::PathExpr::cast).and_then(|expr| expr.path())?; + + // this also includes method calls like Foo::new(42), we should skip them + if let Some(name_ref) = path.segment().and_then(|s| s.name_ref()) { + match NameRefClass::classify(&ctx.sema, &name_ref) { + Some(NameRefClass::Definition(Definition::SelfType(_))) => {}, + Some(NameRefClass::Definition(def)) if def == strukt_def => {}, + _ => return None, + }; + } + + let arg_list = call_expr.syntax().descendants().find_map(ast::ArgList::cast)?; + + edit.replace( + call_expr.syntax().text_range(), + ast::make::record_expr( + path, + ast::make::record_expr_field_list(arg_list.args().zip(names).map( + |(expr, name)| { + ast::make::record_expr_field( + ast::make::name_ref(&name.to_string()), + Some(expr), + ) + }, + )), + ) + .to_string(), + ); + }, + _ => () + } + } + Some(()) + }; + for (file_id, refs) in usages { edit.edit_file(file_id); for r in refs { for node in r.name.syntax().ancestors() { - match_ast! { - match node { - ast::TupleStructPat(tuple_struct_pat) => { - let path = match tuple_struct_pat.path() { - Some(it) => it, - None => continue, - }; - - edit.replace( - tuple_struct_pat.syntax().text_range(), - ast::make::record_pat_with_fields( - path, - ast::make::record_pat_field_list(tuple_struct_pat.fields().zip(names).map( - |(pat, name)| { - ast::make::record_pat_field( - ast::make::name_ref(&name.to_string()), - pat, - ) - }, - )), - ) - .to_string(), - ); - }, - // for tuple struct creations like Foo(42) - ast::CallExpr(call_expr) => { - let path = match call_expr.syntax().descendants().find_map(ast::PathExpr::cast).map(|expr| expr.path()) { - Some(Some(it)) => it, - _ => continue, - }; - - // this also includes method calls like Foo::new(42), we should skip them - if let Some(Some(name_ref)) = path.segment().map(|s| s.name_ref()) { - match NameRefClass::classify(&ctx.sema, &name_ref) { - Some(NameRefClass::Definition(Definition::SelfType(_))) => {}, - Some(NameRefClass::Definition(def)) if def == strukt_def => {}, - _ => continue, - }; - } - - let arg_list = match call_expr.syntax().descendants().find_map(ast::ArgList::cast) { - Some(it) => it, - None => continue, - }; - - edit.replace( - call_expr.syntax().text_range(), - ast::make::record_expr( - path, - ast::make::record_expr_field_list(arg_list.args().zip(names).map( - |(expr, name)| { - ast::make::record_expr_field( - ast::make::name_ref(&name.to_string()), - Some(expr), - ) - }, - )), - ) - .to_string(), - ); - }, - _ => () - } - } + edit_node(edit, node); } } } From 97270dfb9162bfcefa78a7ed39551069ae267f48 Mon Sep 17 00:00:00 2001 From: unexge Date: Fri, 23 Apr 2021 13:08:07 +0300 Subject: [PATCH 09/10] Stop iterating reference after made an edit in "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index 994e1a01a0..cac4bec3d1 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -160,7 +160,7 @@ fn edit_struct_references( .to_string(), ); }, - _ => () + _ => return None, } } Some(()) @@ -170,7 +170,9 @@ fn edit_struct_references( edit.edit_file(file_id); for r in refs { for node in r.name.syntax().ancestors() { - edit_node(edit, node); + if edit_node(edit, node).is_some() { + break; + } } } } @@ -377,6 +379,49 @@ impl A { ); } + #[test] + fn convert_struct_with_wrapped_references() { + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct Inner$0(u32); +struct Outer(Inner); + +impl Outer { + fn new() -> Self { + Self(Inner(42)) + } + + fn into_inner(self) -> u32 { + (self.0).0 + } + + fn into_inner_destructed(self) -> u32 { + let Outer(Inner(x)) = self; + x + } +}"#, + r#" +struct Inner { field1: u32 } +struct Outer(Inner); + +impl Outer { + fn new() -> Self { + Self(Inner { field1: 42 }) + } + + fn into_inner(self) -> u32 { + (self.0).field1 + } + + fn into_inner_destructed(self) -> u32 { + let Outer(Inner { field1: x }) = self; + x + } +}"#, + ); + } + #[test] fn convert_struct_with_multi_file_references() { check_assist( From 5e765895cf6fea42649a846c0297d95a9aa7b162 Mon Sep 17 00:00:00 2001 From: unexge Date: Fri, 23 Apr 2021 16:18:10 +0300 Subject: [PATCH 10/10] Add missing test case for "Convert to named struct" assist --- .../convert_tuple_struct_to_named_struct.rs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs index cac4bec3d1..b5b5ada5e7 100644 --- a/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/crates/ide_assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -418,6 +418,46 @@ impl Outer { let Outer(Inner { field1: x }) = self; x } +}"#, + ); + + check_assist( + convert_tuple_struct_to_named_struct, + r#" +struct Inner(u32); +struct Outer$0(Inner); + +impl Outer { + fn new() -> Self { + Self(Inner(42)) + } + + fn into_inner(self) -> u32 { + (self.0).0 + } + + fn into_inner_destructed(self) -> u32 { + let Outer(Inner(x)) = self; + x + } +}"#, + r#" +struct Inner(u32); +struct Outer { field1: Inner } + +impl Outer { + fn new() -> Self { + Self { field1: Inner(42) } + } + + fn into_inner(self) -> u32 { + (self.field1).0 + } + + fn into_inner_destructed(self) -> u32 { + let Outer { field1: Inner(x) } = self; + x + } }"#, ); }