From e29c474ad72eab5729eed9b3342118986115b072 Mon Sep 17 00:00:00 2001 From: IFcoltransG <47414286+IFcoltransG@users.noreply.github.com> Date: Fri, 20 Jan 2023 20:22:00 +1300 Subject: [PATCH 1/3] Implement basic destructure_struct_binding (no usages) --- .../handlers/destructure_struct_binding.rs | 351 ++++++++++++++++++ crates/ide-assists/src/lib.rs | 2 + 2 files changed, 353 insertions(+) create mode 100644 crates/ide-assists/src/handlers/destructure_struct_binding.rs diff --git a/crates/ide-assists/src/handlers/destructure_struct_binding.rs b/crates/ide-assists/src/handlers/destructure_struct_binding.rs new file mode 100644 index 000000000000..9444c4d6a7e0 --- /dev/null +++ b/crates/ide-assists/src/handlers/destructure_struct_binding.rs @@ -0,0 +1,351 @@ +use crate::assist_context::{AssistContext, Assists, SourceChangeBuilder}; +use ide_db::{ + assists::{AssistId, AssistKind}, + helpers::mod_path_to_ast, +}; +use syntax::{ast, AstNode, TextRange}; + +// Assist: destructure_struct_binding +// +// Destructure a struct binding in place. +pub(crate) fn destructure_struct_binding(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let ident_pat = ctx.find_node_at_offset::()?; + let data = collect_data(ident_pat, ctx)?; + + acc.add( + AssistId("destructure_struct_binding", AssistKind::RefactorRewrite), + "Destructure struct binding", + data.range, + |builder| { + edit_struct_assignment(ctx, builder, &data); + // FIXME: add struct usages + }, + ); + Some(()) +} + +fn collect_data(ident_pat: ast::IdentPat, ctx: &AssistContext<'_>) -> Option { + // if ident_pat.at_token().is_some() { + // // Cannot destructure pattern with sub-pattern: + // // Only IdentPat can have sub-pattern (as in `ident @ subpat`), + // // but not TupleStructPat (`Foo(a,b)`) or RecordPat (`Foo { a: b }`). + // cov_mark::hit!(destructure_struct_subpattern); + // return None; + // } + + let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?.adjusted(); + + // let ref_type = if ty.is_mutable_reference() { + // Some(RefType::Mutable) + // } else if ty.is_reference() { + // Some(RefType::ReadOnly) + // } else { + // None + // }; + + // might be reference + let ty = ty.strip_references(); + + let hir::Adt::Struct(struct_) = ty.as_adt()? + else { return None }; + + let module = ctx.sema.scope(ident_pat.syntax())?.module(); + let struct_def = hir::ModuleDef::from(struct_); + let struct_kind = struct_.kind(ctx.db()); + let struct_def_path = module.find_use_path(ctx.db(), struct_def, ctx.config.prefer_no_std)?; + + let range = ident_pat.syntax().text_range(); + + let fields = struct_.fields(ctx.db()); + + Some(StructData { ident_pat, range, struct_kind, struct_def_path, fields }) +} + +struct StructData { + ident_pat: ast::IdentPat, + range: TextRange, + // ref_type: Option, + struct_kind: hir::StructKind, + struct_def_path: hir::ModPath, + fields: Vec, +} + +// enum RefType { +// ReadOnly, +// Mutable, +// } + +fn edit_struct_assignment( + ctx: &AssistContext<'_>, + builder: &mut SourceChangeBuilder, + data: &StructData, +) { + let add_cursor = |text: &str| { + // place cursor on first item + if let Some(first_field) = field_names(ctx, data.struct_kind, &data.fields).get(0) { + text.replacen(first_field, &format!("$0{first_field}"), 1) + } else { + let mut full_replacement = String::from("$0"); + full_replacement.push_str(text); + full_replacement + } + }; + + // let struct_pat = { + // let struct_path = mod_path_to_ast(&data.mod_path); + // let original = &data.ident_pat; + + // let is_ref = original.ref_token().is_some(); + // let is_mut = original.mut_token().is_some(); + // let fields = data.field_names.iter().map(|name| { + // ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, ast::make::name(name))) + // }); + // ast::make::tuple_struct_pat(struct_path, fields) + // }; + + let struct_path = mod_path_to_ast(&data.struct_def_path); + let is_ref = data.ident_pat.ref_token().is_some(); + let is_mut = data.ident_pat.mut_token().is_some(); + + let field_names = field_names(ctx, data.struct_kind, &data.fields); + let pats = field_names.iter().map(|field_name| { + let name = ast::make::name(field_name); + ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, name)) + }); + let struct_pat: ast::Pat = match data.struct_kind { + hir::StructKind::Tuple => { + ast::Pat::TupleStructPat(ast::make::tuple_struct_pat(struct_path, pats)) + } + hir::StructKind::Record => ast::Pat::RecordPat(ast::make::record_pat(struct_path, pats)), + // bare identifier pattern, which matches the unit struct that it names + hir::StructKind::Unit => ast::make::path_pat(struct_path), + }; + + let text = struct_pat.to_string(); + match ctx.config.snippet_cap { + Some(cap) => { + let snip = add_cursor(&text); + builder.replace_snippet(cap, data.range, snip); + } + None => builder.replace(data.range, text), + }; +} + +fn field_names( + ctx: &AssistContext<'_>, + struct_kind: hir::StructKind, + fields: &Vec, +) -> Vec { + match struct_kind { + hir::StructKind::Tuple => { + fields.iter().enumerate().map(|(index, _)| generate_tuple_field_name(index)).collect() + } + hir::StructKind::Record => { + fields.iter().map(|field| generate_record_field_name(field.name(ctx.db()))).collect() + } + hir::StructKind::Unit => vec![], + } +} + +fn generate_tuple_field_name(index: usize) -> String { + // FIXME: detect if name already used + format!("_{}", index) +} + +fn generate_record_field_name(field_name: hir::Name) -> String { + // FIXME: detect if name already used + format!("{}", field_name.to_smol_str()) +} + +#[cfg(test)] + +mod tests { + use super::*; + + use crate::tests::{check_assist, check_assist_not_applicable}; + #[test] + fn dont_trigger_on_number() { + check_assist_not_applicable( + destructure_struct_binding, + r#" +fn main() { +let $0v = 42; +} + "#, + ) + } + #[test] + fn dont_trigger_on_tuple() { + check_assist_not_applicable( + destructure_struct_binding, + r#" +fn main() { + let $0v = (1, 2); +} + "#, + ) + } + + #[test] + fn destructure_3_tuple_struct() { + check_assist( + destructure_struct_binding, + r#" +struct Foo(u8, u8, u8); + +fn main() { + let $0st = Foo(1,2,3); +} + "#, + r#" +struct Foo(u8, u8, u8); + +fn main() { + let Foo($0_0, _1, _2) = Foo(1,2,3); +} + "#, + ) + } + + #[test] + fn destructure_record_struct() { + check_assist( + destructure_struct_binding, + r#" +struct Point {x: i32, y: i32}; + +fn main() { + let $0pt = Point {x: 1, y: -1}; +} + "#, + r#" +struct Point {x: i32, y: i32}; + +fn main() { + let Point { $0x, y } = Point {x: 1, y: -1}; +} + "#, + ) + } + #[test] + fn destructure_unit_struct() { + check_assist( + destructure_struct_binding, + r#" +struct Marker; + +fn main() { + let $0mk = Marker; +} + "#, + r#" +struct Marker; + +fn main() { + let $0Marker = Marker; +} + "#, + ) + } + #[test] + fn destructure_record_struct_in_other_module() { + check_assist( + destructure_struct_binding, + r#" +mod other { + pub struct Wrapper {pub(super) inner: bool}; +} + +fn main() { + let $0wrapped = other::Wrapper{ inner: false }; +} + "#, + r#" +mod other { + pub struct Wrapper {pub(super) inner: bool}; +} + +fn main() { + let other::Wrapper { $0inner } = other::Wrapper{ inner: false }; +} + "#, + ) + } + #[test] + fn destructure_in_parameter() { + check_assist( + destructure_struct_binding, + r#" +struct Foo { bar: (), baz: () }; + +fn func($0foo: Foo) {} + "#, + r#" +struct Foo { bar: (), baz: () }; + +fn func(Foo { $0bar, baz }: Foo) {} + "#, + ) + } + #[test] + fn destructure_ref() { + check_assist( + destructure_struct_binding, + r#" +struct Foo { bar: () }; + +fn main() { + let &$0foo = &Foo { bar: () }; +} + "#, + r#" +struct Foo { bar: () }; + +fn main() { + let &Foo { $0bar } = &Foo { bar: () }; +} + "#, + ) + } + + #[test] + fn destructure_inner_ref() { + check_assist( + destructure_struct_binding, + r#" +struct Foo { bar: () }; + +fn main() { + let $0foo = &Foo { bar: () }; +} + "#, + r#" +struct Foo { bar: () }; + +fn main() { + let Foo { $0bar } = &Foo { bar: () }; +} + "#, + ) + } + #[test] + fn with_mut() { + check_assist( + destructure_struct_binding, + r#" +struct Nums(i16, u8); + +fn main() { + let mut $0t = Nums(1,2); +} + "#, + r#" +struct Nums(i16, u8); + +fn main() { + let Nums(mut $0_0, mut _1) = Nums(1,2); +} + "#, + ) + } +} diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 546ef96260f2..625381f47e60 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -128,6 +128,7 @@ mod handlers { mod convert_while_to_loop; mod desugar_doc_comment; mod destructure_tuple_binding; + mod destructure_struct_binding; mod expand_glob_import; mod extract_expressions_from_format_string; mod extract_function; @@ -234,6 +235,7 @@ mod handlers { convert_while_to_loop::convert_while_to_loop, desugar_doc_comment::desugar_doc_comment, destructure_tuple_binding::destructure_tuple_binding, + destructure_struct_binding::destructure_struct_binding, expand_glob_import::expand_glob_import, extract_expressions_from_format_string::extract_expressions_from_format_string, extract_struct_from_enum_variant::extract_struct_from_enum_variant, From 5e655f439dd4c91644d91133e36926d1b167c34d Mon Sep 17 00:00:00 2001 From: IFcoltransG <47414286+IFcoltransG@users.noreply.github.com> Date: Sat, 21 Jan 2023 17:57:37 +1300 Subject: [PATCH 2/3] Amend destructure_struct Assist to make new idents rather than shorthand --- .../handlers/destructure_struct_binding.rs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/crates/ide-assists/src/handlers/destructure_struct_binding.rs b/crates/ide-assists/src/handlers/destructure_struct_binding.rs index 9444c4d6a7e0..94a689b3e583 100644 --- a/crates/ide-assists/src/handlers/destructure_struct_binding.rs +++ b/crates/ide-assists/src/handlers/destructure_struct_binding.rs @@ -108,15 +108,22 @@ fn edit_struct_assignment( let is_mut = data.ident_pat.mut_token().is_some(); let field_names = field_names(ctx, data.struct_kind, &data.fields); - let pats = field_names.iter().map(|field_name| { + let ident_pats = field_names.iter().map(|field_name| { let name = ast::make::name(field_name); ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, name)) }); let struct_pat: ast::Pat = match data.struct_kind { hir::StructKind::Tuple => { - ast::Pat::TupleStructPat(ast::make::tuple_struct_pat(struct_path, pats)) + ast::Pat::TupleStructPat(ast::make::tuple_struct_pat(struct_path, ident_pats)) + } + hir::StructKind::Record => { + let fields = ident_pats.zip(&data.fields).map(|(pat, field)| { + let field_name = field.name(ctx.db()).to_smol_str(); + ast::make::record_pat_field(ast::make::name_ref(&field_name), pat) + }); + let field_list = ast::make::record_pat_field_list(fields); + ast::Pat::RecordPat(ast::make::record_pat_with_fields(struct_path, field_list)) } - hir::StructKind::Record => ast::Pat::RecordPat(ast::make::record_pat(struct_path, pats)), // bare identifier pattern, which matches the unit struct that it names hir::StructKind::Unit => ast::make::path_pat(struct_path), }; @@ -154,7 +161,7 @@ fn generate_tuple_field_name(index: usize) -> String { fn generate_record_field_name(field_name: hir::Name) -> String { // FIXME: detect if name already used - format!("{}", field_name.to_smol_str()) + format!("_{}", field_name.to_smol_str()) } #[cfg(test)] @@ -222,7 +229,7 @@ fn main() { struct Point {x: i32, y: i32}; fn main() { - let Point { $0x, y } = Point {x: 1, y: -1}; + let Point { x: $0_x, y: _y } = Point {x: 1, y: -1}; } "#, ) @@ -266,7 +273,7 @@ mod other { } fn main() { - let other::Wrapper { $0inner } = other::Wrapper{ inner: false }; + let other::Wrapper { inner: $0_inner } = other::Wrapper{ inner: false }; } "#, ) @@ -283,7 +290,7 @@ fn func($0foo: Foo) {} r#" struct Foo { bar: (), baz: () }; -fn func(Foo { $0bar, baz }: Foo) {} +fn func(Foo { bar: $0_bar, baz: _baz }: Foo) {} "#, ) } @@ -302,7 +309,7 @@ fn main() { struct Foo { bar: () }; fn main() { - let &Foo { $0bar } = &Foo { bar: () }; + let &Foo { bar: $0_bar } = &Foo { bar: () }; } "#, ) @@ -323,7 +330,7 @@ fn main() { struct Foo { bar: () }; fn main() { - let Foo { $0bar } = &Foo { bar: () }; + let Foo { bar: $0_bar } = &Foo { bar: () }; } "#, ) From c9227cc582794118e88c65a3508bd9561d8127bf Mon Sep 17 00:00:00 2001 From: IFcoltransG <47414286+IFcoltransG@users.noreply.github.com> Date: Sat, 21 Jan 2023 22:27:51 +1300 Subject: [PATCH 3/3] Fix style --- .../handlers/destructure_struct_binding.rs | 140 ++++++++---------- crates/ide-assists/src/tests/generated.rs | 21 +++ 2 files changed, 82 insertions(+), 79 deletions(-) diff --git a/crates/ide-assists/src/handlers/destructure_struct_binding.rs b/crates/ide-assists/src/handlers/destructure_struct_binding.rs index 94a689b3e583..7ec9625ef5e7 100644 --- a/crates/ide-assists/src/handlers/destructure_struct_binding.rs +++ b/crates/ide-assists/src/handlers/destructure_struct_binding.rs @@ -1,15 +1,32 @@ -use crate::assist_context::{AssistContext, Assists, SourceChangeBuilder}; use ide_db::{ assists::{AssistId, AssistKind}, helpers::mod_path_to_ast, }; use syntax::{ast, AstNode, TextRange}; +use crate::assist_context::{AssistContext, Assists, SourceChangeBuilder}; + // Assist: destructure_struct_binding // // Destructure a struct binding in place. +// +// ``` +// struct Struct { a: u8, b: u8 } +// +// fn main() { +// let $0x = Struct { a: 1, b: 2 }; +// } +// ``` +// -> +// ``` +// struct Struct { a: u8, b: u8 } +// +// fn main() { +// let Struct { a: $0_a, b: _b } = Struct { a: 1, b: 2 }; +// } +// ``` pub(crate) fn destructure_struct_binding(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { - let ident_pat = ctx.find_node_at_offset::()?; + let ident_pat: ast::IdentPat = ctx.find_node_at_offset()?; let data = collect_data(ident_pat, ctx)?; acc.add( @@ -18,96 +35,54 @@ pub(crate) fn destructure_struct_binding(acc: &mut Assists, ctx: &AssistContext< data.range, |builder| { edit_struct_assignment(ctx, builder, &data); - // FIXME: add struct usages + // FIXME: add struct usages. }, ); Some(()) } -fn collect_data(ident_pat: ast::IdentPat, ctx: &AssistContext<'_>) -> Option { - // if ident_pat.at_token().is_some() { - // // Cannot destructure pattern with sub-pattern: - // // Only IdentPat can have sub-pattern (as in `ident @ subpat`), - // // but not TupleStructPat (`Foo(a,b)`) or RecordPat (`Foo { a: b }`). - // cov_mark::hit!(destructure_struct_subpattern); - // return None; - // } +struct StructData { + ident_pat: ast::IdentPat, + range: TextRange, + struct_kind: hir::StructKind, + struct_def_path: hir::ModPath, + fields: Vec, +} +fn collect_data(ident_pat: ast::IdentPat, ctx: &AssistContext<'_>) -> Option { let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?.adjusted(); - // let ref_type = if ty.is_mutable_reference() { - // Some(RefType::Mutable) - // } else if ty.is_reference() { - // Some(RefType::ReadOnly) - // } else { - // None - // }; - - // might be reference + // The type might be a reference. let ty = ty.strip_references(); - let hir::Adt::Struct(struct_) = ty.as_adt()? - else { return None }; + // The type is required to be a struct. + let strukt = match ty.as_adt()? { + hir::Adt::Struct(strukt) => strukt, + _ => return None, + }; let module = ctx.sema.scope(ident_pat.syntax())?.module(); - let struct_def = hir::ModuleDef::from(struct_); - let struct_kind = struct_.kind(ctx.db()); + let struct_def = hir::ModuleDef::from(strukt); + let struct_kind = strukt.kind(ctx.db()); let struct_def_path = module.find_use_path(ctx.db(), struct_def, ctx.config.prefer_no_std)?; let range = ident_pat.syntax().text_range(); - let fields = struct_.fields(ctx.db()); + let fields = strukt.fields(ctx.db()); Some(StructData { ident_pat, range, struct_kind, struct_def_path, fields }) } -struct StructData { - ident_pat: ast::IdentPat, - range: TextRange, - // ref_type: Option, - struct_kind: hir::StructKind, - struct_def_path: hir::ModPath, - fields: Vec, -} - -// enum RefType { -// ReadOnly, -// Mutable, -// } - fn edit_struct_assignment( ctx: &AssistContext<'_>, builder: &mut SourceChangeBuilder, data: &StructData, ) { - let add_cursor = |text: &str| { - // place cursor on first item - if let Some(first_field) = field_names(ctx, data.struct_kind, &data.fields).get(0) { - text.replacen(first_field, &format!("$0{first_field}"), 1) - } else { - let mut full_replacement = String::from("$0"); - full_replacement.push_str(text); - full_replacement - } - }; - - // let struct_pat = { - // let struct_path = mod_path_to_ast(&data.mod_path); - // let original = &data.ident_pat; - - // let is_ref = original.ref_token().is_some(); - // let is_mut = original.mut_token().is_some(); - // let fields = data.field_names.iter().map(|name| { - // ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, ast::make::name(name))) - // }); - // ast::make::tuple_struct_pat(struct_path, fields) - // }; - let struct_path = mod_path_to_ast(&data.struct_def_path); let is_ref = data.ident_pat.ref_token().is_some(); let is_mut = data.ident_pat.mut_token().is_some(); - let field_names = field_names(ctx, data.struct_kind, &data.fields); + let field_names = names_of_fields(ctx, data.struct_kind, &data.fields); let ident_pats = field_names.iter().map(|field_name| { let name = ast::make::name(field_name); ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, name)) @@ -124,48 +99,55 @@ fn edit_struct_assignment( let field_list = ast::make::record_pat_field_list(fields); ast::Pat::RecordPat(ast::make::record_pat_with_fields(struct_path, field_list)) } - // bare identifier pattern, which matches the unit struct that it names + // Bare identifier pattern, which matches the unit struct that it names. hir::StructKind::Unit => ast::make::path_pat(struct_path), }; let text = struct_pat.to_string(); match ctx.config.snippet_cap { Some(cap) => { - let snip = add_cursor(&text); + let snip = { + // place cursor on first item + match names_of_fields(ctx, data.struct_kind, &data.fields).get(0) { + Some(first_field) => text.replacen(first_field, &format!("$0{first_field}"), 1), + None => format!("$0{text}"), + } + }; builder.replace_snippet(cap, data.range, snip); } None => builder.replace(data.range, text), }; } -fn field_names( +fn names_of_fields( ctx: &AssistContext<'_>, struct_kind: hir::StructKind, fields: &Vec, ) -> Vec { match struct_kind { hir::StructKind::Tuple => { - fields.iter().enumerate().map(|(index, _)| generate_tuple_field_name(index)).collect() + (0..fields.len()) + .map(|index| { + // FIXME: detect if generated name already used + format!("_{}", index) + }) + .collect() } hir::StructKind::Record => { - fields.iter().map(|field| generate_record_field_name(field.name(ctx.db()))).collect() + fields + .iter() + .map(|field| { + let field_name = field.name(ctx.db()).to_smol_str(); + // FIXME: detect if generated name already used + format!("_{}", field_name) + }) + .collect() } hir::StructKind::Unit => vec![], } } -fn generate_tuple_field_name(index: usize) -> String { - // FIXME: detect if name already used - format!("_{}", index) -} - -fn generate_record_field_name(field_name: hir::Name) -> String { - // FIXME: detect if name already used - format!("_{}", field_name.to_smol_str()) -} - #[cfg(test)] - mod tests { use super::*; diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 16a06b60de90..d39488a58e2e 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -578,6 +578,27 @@ fn main() { ) } +#[test] +fn doctest_destructure_struct_binding() { + check_doc_test( + "destructure_struct_binding", + r#####" +struct Struct { a: u8, b: u8 } + +fn main() { + let $0x = Struct { a: 1, b: 2 }; +} +"#####, + r#####" +struct Struct { a: u8, b: u8 } + +fn main() { + let Struct { a: $0_a, b: _b } = Struct { a: 1, b: 2 }; +} +"#####, + ) +} + #[test] fn doctest_destructure_tuple_binding() { check_doc_test(