From ac38a16b9ca24216debcb3a2cf98c375a38a7ec7 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Tue, 29 Jul 2025 21:22:29 -0400 Subject: [PATCH 01/17] intial work extract-struct-from-function-signature --- .../extract_struct_from_function_signature.rs | 255 ++++++++++++++++++ crates/ide-assists/src/lib.rs | 2 + 2 files changed, 257 insertions(+) create mode 100644 crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs new file mode 100644 index 000000000000..88e92f8c7319 --- /dev/null +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -0,0 +1,255 @@ +use hir::{Function, ModuleDef, Visibility}; +use ide_db::{RootDatabase, assists::AssistId, path_transform::PathTransform}; +use itertools::Itertools; +use syntax::{ + ast::{ + self, edit::{AstNodeEdit, IndentLevel}, make, HasAttrs, HasGenericParams, HasName, HasVisibility + }, match_ast, ted, AstNode, SyntaxElement, SyntaxKind, SyntaxNode, T +}; + +use crate::{AssistContext, Assists}; +// Assist: extract_struct_from_function_signature +// +// Extracts a struct (part) of the signature of a function. +// +// ``` +// fn foo(bar: u32, baz: u32) { ... } +// ``` +// -> +// ``` +// struct FooStruct { +// bar: u32, +// baz: u32, +// } +// +// fn foo(FooStruct) { ... } +// ``` + +pub(crate) fn extract_struct_from_function_signature( + acc: &mut Assists, + ctx: &AssistContext<'_>, +) -> Option<()> { + // TODO: get more specific than param list + // how to get function name and param list/part of param list the is selected seperatly + // or maybe just auto generate random name not based on function name? + let fn_ast = ctx.find_node_at_offset::()?; + let fn_name = fn_ast.name()?; + + let fn_hir = ctx.sema.to_def(&fn_ast)?; + if existing_definition(ctx.db(), &fn_name, &fn_hir) { + cov_mark::hit!(test_extract_function_signature_not_applicable_if_struct_exists); + return None; + } + + // TODO: does this capture parenthesis + let target = fn_ast.param_list()?.syntax().text_range(); + // TODO: special handiling for self? + // TODO: special handling for destrutered types (or maybe just don't suppurt code action on + // destructed types yet + let field_list = make::record_field_list( + fn_ast + .param_list()? + .params() + .map(|param| Some(make::record_field(None, make::name("todo"), param.ty()?))) + .collect::>>()? + .into_iter(), + ); + acc.add( + AssistId::refactor_rewrite("extract_struct_from_function_signature"), + "Extract struct from signature of a function", + target, + |builder| { + builder.edit_file(ctx.vfs_file_id()); + + let generic_params = fn_ast + .generic_param_list() + .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); + let generics = generic_params.as_ref().map(|generics| generics.clone_for_update()); + + // resolve GenericArg in field_list to actual type + let field_list = if let Some((target_scope, source_scope)) = + ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(field_list.syntax())) + { + let field_list = field_list.reset_indent(); + let field_list = + PathTransform::generic_transformation(&target_scope, &source_scope) + .apply(field_list.syntax()); + match_ast! { + match field_list { + ast::RecordFieldList(field_list) => field_list, + _ => unreachable!(), + } + } + } else { + field_list.clone_for_update() + }; + + let def = + create_struct_def(fn_name.clone(), &fn_ast, &field_list, generics); + + let indent = fn_ast.indent_level(); + let def = def.indent(indent); + + ted::insert_all( + ted::Position::before(fn_ast.syntax()), + vec![ + def.syntax().clone().into(), + make::tokens::whitespace(&format!("\n\n{indent}")).into(), + ], + ); + }, + ) +} +fn create_struct_def( + name: ast::Name, + fn_ast: &ast::Fn, + field_list: &ast::RecordFieldList, + generics: Option, +) -> ast::Struct { + let enum_vis = fn_ast.visibility(); + + let insert_vis = |node: &'_ SyntaxNode, vis: &'_ SyntaxNode| { + let vis = vis.clone_for_update(); + ted::insert(ted::Position::before(node), vis); + }; + + // for fields without any existing visibility, use visibility of enum + let field_list: ast::FieldList = { + if let Some(vis) = &enum_vis { + field_list + .fields() + .filter(|field| field.visibility().is_none()) + .filter_map(|field| field.name()) + .for_each(|it| insert_vis(it.syntax(), vis.syntax())); + } + + field_list.clone().into() + }; + let field_list = field_list.indent(IndentLevel::single()); + + let strukt = make::struct_(enum_vis, name, generics, field_list).clone_for_update(); + + // take comments from variant + ted::insert_all( + ted::Position::first_child_of(strukt.syntax()), + take_all_comments(fn_ast.syntax()), + ); + + // copy attributes from enum + ted::insert_all( + ted::Position::first_child_of(strukt.syntax()), + fn_ast + .attrs() + .flat_map(|it| { + vec![it.syntax().clone_for_update().into(), make::tokens::single_newline().into()] + }) + .collect(), + ); + + strukt +} +// Note: this also detaches whitespace after comments, +// since `SyntaxNode::splice_children` (and by extension `ted::insert_all_raw`) +// detaches nodes. If we only took the comments, we'd leave behind the old whitespace. +fn take_all_comments(node: &SyntaxNode) -> Vec { + let mut remove_next_ws = false; + node.children_with_tokens() + .filter_map(move |child| match child.kind() { + SyntaxKind::COMMENT => { + remove_next_ws = true; + child.detach(); + Some(child) + } + SyntaxKind::WHITESPACE if remove_next_ws => { + remove_next_ws = false; + child.detach(); + Some(make::tokens::single_newline().into()) + } + _ => { + remove_next_ws = false; + None + } + }) + .collect() +} +fn extract_generic_params( + known_generics: &ast::GenericParamList, + field_list: &ast::RecordFieldList, +) -> Option { + let mut generics = known_generics.generic_params().map(|param| (param, false)).collect_vec(); + + let tagged_one = field_list + .fields() + .filter_map(|f| f.ty()) + .fold(false, |tagged, ty| tag_generics_in_function_signature(&ty, &mut generics) || tagged); + + let generics = generics.into_iter().filter_map(|(param, tag)| tag.then_some(param)); + tagged_one.then(|| make::generic_param_list(generics)) +} +fn tag_generics_in_function_signature( + ty: &ast::Type, + generics: &mut [(ast::GenericParam, bool)], +) -> bool { + let mut tagged_one = false; + + for token in ty.syntax().descendants_with_tokens().filter_map(SyntaxElement::into_token) { + for (param, tag) in generics.iter_mut().filter(|(_, tag)| !tag) { + match param { + ast::GenericParam::LifetimeParam(lt) + if matches!(token.kind(), T![lifetime_ident]) => + { + if let Some(lt) = lt.lifetime() { + if lt.text().as_str() == token.text() { + *tag = true; + tagged_one = true; + break; + } + } + } + param if matches!(token.kind(), T![ident]) => { + if match param { + ast::GenericParam::ConstParam(konst) => konst + .name() + .map(|name| name.text().as_str() == token.text()) + .unwrap_or_default(), + ast::GenericParam::TypeParam(ty) => ty + .name() + .map(|name| name.text().as_str() == token.text()) + .unwrap_or_default(), + ast::GenericParam::LifetimeParam(lt) => lt + .lifetime() + .map(|lt| lt.text().as_str() == token.text()) + .unwrap_or_default(), + } { + *tag = true; + tagged_one = true; + break; + } + } + _ => (), + } + } + } + + tagged_one +} +fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Function) -> bool { + variant + .module(db) + .scope(db, None) + .into_iter() + .filter(|(_, def)| match def { + // only check type-namespace + hir::ScopeDef::ModuleDef(def) => matches!( + def, + ModuleDef::Module(_) + | ModuleDef::Adt(_) + | ModuleDef::Variant(_) + | ModuleDef::Trait(_) + | ModuleDef::TypeAlias(_) + | ModuleDef::BuiltinType(_) + ), + _ => false, + }) + .any(|(name, _)| name.as_str() == variant_name.text().trim_start_matches("r#")) +} diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 4682c0473238..fe581e14f409 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -233,6 +233,7 @@ mod handlers { mod unwrap_type_to_generic_arg; mod wrap_return_type; mod wrap_unwrap_cfg_attr; + mod extract_struct_from_function_signature; pub(crate) fn all() -> &'static [Handler] { &[ @@ -280,6 +281,7 @@ mod handlers { expand_rest_pattern::expand_rest_pattern, extract_expressions_from_format_string::extract_expressions_from_format_string, extract_struct_from_enum_variant::extract_struct_from_enum_variant, + extract_struct_from_function_signature::extract_struct_from_function_signature, extract_type_alias::extract_type_alias, fix_visibility::fix_visibility, flip_binexpr::flip_binexpr, From 239c1dd492838171c5febd915209456730445075 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Wed, 30 Jul 2025 10:01:30 -0400 Subject: [PATCH 02/17] feat: extract_struct_from_function_signature now actually uses the parameter name for field. and generates correct struct name. --- .../extract_struct_from_function_signature.rs | 39 +++++++++++++------ crates/ide-assists/src/lib.rs | 2 +- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 88e92f8c7319..516dae4c7032 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -1,10 +1,15 @@ -use hir::{Function, ModuleDef, Visibility}; +use hir::{Function, ModuleDef}; use ide_db::{RootDatabase, assists::AssistId, path_transform::PathTransform}; use itertools::Itertools; +use stdx::to_camel_case; use syntax::{ + AstNode, SyntaxElement, SyntaxKind, SyntaxNode, T, ast::{ - self, edit::{AstNodeEdit, IndentLevel}, make, HasAttrs, HasGenericParams, HasName, HasVisibility - }, match_ast, ted, AstNode, SyntaxElement, SyntaxKind, SyntaxNode, T + self, HasAttrs, HasGenericParams, HasName, HasVisibility, + edit::{AstNodeEdit, IndentLevel}, + make, + }, + match_ast, ted, }; use crate::{AssistContext, Assists}; @@ -50,10 +55,16 @@ pub(crate) fn extract_struct_from_function_signature( fn_ast .param_list()? .params() - .map(|param| Some(make::record_field(None, make::name("todo"), param.ty()?))) - .collect::>>()? - .into_iter(), + .map(|param| { + Some(make::record_field( + fn_ast.visibility(), + param.pat().and_then(pat_to_name)?, + param.ty()?, + )) + }) + .collect::>>()?, ); + let name = make::name(&format!("{}Struct", to_camel_case(fn_name.text_non_mutable()))); acc.add( AssistId::refactor_rewrite("extract_struct_from_function_signature"), "Extract struct from signature of a function", @@ -84,8 +95,7 @@ pub(crate) fn extract_struct_from_function_signature( field_list.clone_for_update() }; - let def = - create_struct_def(fn_name.clone(), &fn_ast, &field_list, generics); + let def = create_struct_def(name.clone(), &fn_ast, &field_list, generics); let indent = fn_ast.indent_level(); let def = def.indent(indent); @@ -100,13 +110,20 @@ pub(crate) fn extract_struct_from_function_signature( }, ) } + +fn pat_to_name(pat: ast::Pat) -> Option { + match pat { + ast::Pat::IdentPat(ident_pat) => ident_pat.name(), + _ => None, + } +} fn create_struct_def( name: ast::Name, fn_ast: &ast::Fn, field_list: &ast::RecordFieldList, generics: Option, ) -> ast::Struct { - let enum_vis = fn_ast.visibility(); + let fn_vis = fn_ast.visibility(); let insert_vis = |node: &'_ SyntaxNode, vis: &'_ SyntaxNode| { let vis = vis.clone_for_update(); @@ -115,7 +132,7 @@ fn create_struct_def( // for fields without any existing visibility, use visibility of enum let field_list: ast::FieldList = { - if let Some(vis) = &enum_vis { + if let Some(vis) = &fn_vis { field_list .fields() .filter(|field| field.visibility().is_none()) @@ -127,7 +144,7 @@ fn create_struct_def( }; let field_list = field_list.indent(IndentLevel::single()); - let strukt = make::struct_(enum_vis, name, generics, field_list).clone_for_update(); + let strukt = make::struct_(fn_vis, name, generics, field_list).clone_for_update(); // take comments from variant ted::insert_all( diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index fe581e14f409..3e1c214afd5c 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -146,6 +146,7 @@ mod handlers { mod extract_function; mod extract_module; mod extract_struct_from_enum_variant; + mod extract_struct_from_function_signature; mod extract_type_alias; mod extract_variable; mod fix_visibility; @@ -233,7 +234,6 @@ mod handlers { mod unwrap_type_to_generic_arg; mod wrap_return_type; mod wrap_unwrap_cfg_attr; - mod extract_struct_from_function_signature; pub(crate) fn all() -> &'static [Handler] { &[ From d70fc2acb68a677172a9b512064b9d0225883c7b Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Wed, 30 Jul 2025 13:43:30 -0400 Subject: [PATCH 03/17] feat: extract_struct_from_function_signature only capture comments and attributes around the parameters added some info logging trying to debug salsa query error --- .../extract_struct_from_function_signature.rs | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 516dae4c7032..fff869d972b0 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -51,6 +51,8 @@ pub(crate) fn extract_struct_from_function_signature( // TODO: special handiling for self? // TODO: special handling for destrutered types (or maybe just don't suppurt code action on // destructed types yet + let params = fn_ast.param_list()?; + let field_list = make::record_field_list( fn_ast .param_list()? @@ -70,14 +72,21 @@ pub(crate) fn extract_struct_from_function_signature( "Extract struct from signature of a function", target, |builder| { + let fn_ast = builder.make_mut(fn_ast); + tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); + tracing::info!("extract_struct_from_function_signature: editing main file"); let generic_params = fn_ast .generic_param_list() - .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); + .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); + tracing::info!("extract_struct_from_function_signature: collecting generics"); let generics = generic_params.as_ref().map(|generics| generics.clone_for_update()); // resolve GenericArg in field_list to actual type + // we are getting a query error from salsa, I think it is because the field list is + // constructed in new generation, so maybe do the resolving while its still param list + // and then convert it into record list after let field_list = if let Some((target_scope, source_scope)) = ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(field_list.syntax())) { @@ -94,8 +103,9 @@ pub(crate) fn extract_struct_from_function_signature( } else { field_list.clone_for_update() }; - - let def = create_struct_def(name.clone(), &fn_ast, &field_list, generics); + tracing::info!("extract_struct_from_function_signature: collecting fields"); + let def = create_struct_def(name.clone(), &fn_ast, ¶ms, &field_list, generics); + tracing::info!("extract_struct_from_function_signature: creating struct"); let indent = fn_ast.indent_level(); let def = def.indent(indent); @@ -107,6 +117,7 @@ pub(crate) fn extract_struct_from_function_signature( make::tokens::whitespace(&format!("\n\n{indent}")).into(), ], ); + tracing::info!("extract_struct_from_function_signature: inserting struct {def}"); }, ) } @@ -120,6 +131,7 @@ fn pat_to_name(pat: ast::Pat) -> Option { fn create_struct_def( name: ast::Name, fn_ast: &ast::Fn, + param_ast: &ast::ParamList, field_list: &ast::RecordFieldList, generics: Option, ) -> ast::Struct { @@ -146,17 +158,19 @@ fn create_struct_def( let strukt = make::struct_(fn_vis, name, generics, field_list).clone_for_update(); - // take comments from variant + // take comments from only inside signature ted::insert_all( ted::Position::first_child_of(strukt.syntax()), - take_all_comments(fn_ast.syntax()), + take_all_comments(param_ast.syntax()), ); - // copy attributes from enum + // TODO: this may not be correct as we shouldn't put all the attributes at the top + // copy attributes from each parameter ted::insert_all( ted::Position::first_child_of(strukt.syntax()), - fn_ast - .attrs() + param_ast + .params() + .flat_map(|p| p.attrs()) .flat_map(|it| { vec![it.syntax().clone_for_update().into(), make::tokens::single_newline().into()] }) From 1f599a24c2e20eee9ddc6cffeb982c6e43fb81f9 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Wed, 30 Jul 2025 15:10:54 -0400 Subject: [PATCH 04/17] feat: extract_struct_from_function_signature fixed issue with resolving generics (I think) started writing the code to update the original function --- .../extract_struct_from_function_signature.rs | 49 +++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index fff869d972b0..31438872705e 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -61,6 +61,7 @@ pub(crate) fn extract_struct_from_function_signature( Some(make::record_field( fn_ast.visibility(), param.pat().and_then(pat_to_name)?, + // TODO: how are we going to handle references without explicit lifetimes param.ty()?, )) }) @@ -72,23 +73,27 @@ pub(crate) fn extract_struct_from_function_signature( "Extract struct from signature of a function", target, |builder| { + // TODO: update calls to the function let fn_ast = builder.make_mut(fn_ast); + let params = builder.make_mut(params); tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); tracing::info!("extract_struct_from_function_signature: editing main file"); let generic_params = fn_ast .generic_param_list() - .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); + .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); tracing::info!("extract_struct_from_function_signature: collecting generics"); let generics = generic_params.as_ref().map(|generics| generics.clone_for_update()); // resolve GenericArg in field_list to actual type - // we are getting a query error from salsa, I think it is because the field list is - // constructed in new generation, so maybe do the resolving while its still param list - // and then convert it into record list after + // we would get a query error from salsa, if we would use the field_list + // I think it is because the field list is + // constructed in new generation. + // So I do the resolving while its still param list + // and then apply it into record list after let field_list = if let Some((target_scope, source_scope)) = - ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(field_list.syntax())) + ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(params.syntax())) { let field_list = field_list.reset_indent(); let field_list = @@ -118,10 +123,44 @@ pub(crate) fn extract_struct_from_function_signature( ], ); tracing::info!("extract_struct_from_function_signature: inserting struct {def}"); + update_function(name, &fn_ast, generic_params.map(|g| g.clone_for_update())); }, ) } +fn update_function( + name: ast::Name, + fn_ast: &ast::Fn, + generics: Option, +) -> Option<()> { + let generic_args = generics + .filter(|generics| generics.generic_params().count() > 0) + .map(|generics| generics.to_generic_args()); + // FIXME: replace with a `ast::make` constructor + let ty = match generic_args { + Some(generic_args) => make::ty(&format!("{name}{generic_args}")), + None => make::ty(&name.text()), + }; + + let param = make::param( + // TODO: do we want to destructure the struct + ast::Pat::IdentPat(make::ident_pat( + false, + fn_ast.param_list()?.params().any(|p| { + p.pat() + .is_some_and(|p| matches!(p, ast::Pat::IdentPat(p) if p.mut_token().is_some())) + }), + name, + )), + ty, + ); + // TODO: will eventually need to handle self to + let param_list = make::param_list(None, std::iter::once(param)).clone_for_update(); + ted::replace(fn_ast.param_list()?.syntax(), param_list.syntax()); + // TODO: update uses of parameters in function, if we do not destructure + Some(()) +} + fn pat_to_name(pat: ast::Pat) -> Option { match pat { ast::Pat::IdentPat(ident_pat) => ident_pat.name(), From 87e28877447d9d735ba4bbd2274039555df57abc Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Wed, 30 Jul 2025 20:21:58 -0400 Subject: [PATCH 05/17] feat: extract_struct_from_function_signature code action only avaliable in signature part of a function for some reason still does not change anything in the file --- .../extract_struct_from_function_signature.rs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 31438872705e..e46349a48397 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -38,6 +38,9 @@ pub(crate) fn extract_struct_from_function_signature( // how to get function name and param list/part of param list the is selected seperatly // or maybe just auto generate random name not based on function name? let fn_ast = ctx.find_node_at_offset::()?; + // we independently get the param list without going through fn (fn_ast.param_list()), because for some reason when we + // go through the fn, the text_range is the whole function. + let params_list = ctx.find_node_at_offset::()?; let fn_name = fn_ast.name()?; let fn_hir = ctx.sema.to_def(&fn_ast)?; @@ -47,11 +50,10 @@ pub(crate) fn extract_struct_from_function_signature( } // TODO: does this capture parenthesis - let target = fn_ast.param_list()?.syntax().text_range(); + let target = params_list.syntax().text_range(); // TODO: special handiling for self? // TODO: special handling for destrutered types (or maybe just don't suppurt code action on // destructed types yet - let params = fn_ast.param_list()?; let field_list = make::record_field_list( fn_ast @@ -75,7 +77,6 @@ pub(crate) fn extract_struct_from_function_signature( |builder| { // TODO: update calls to the function let fn_ast = builder.make_mut(fn_ast); - let params = builder.make_mut(params); tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); tracing::info!("extract_struct_from_function_signature: editing main file"); @@ -93,7 +94,7 @@ pub(crate) fn extract_struct_from_function_signature( // So I do the resolving while its still param list // and then apply it into record list after let field_list = if let Some((target_scope, source_scope)) = - ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(params.syntax())) + ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(params_list.syntax())) { let field_list = field_list.reset_indent(); let field_list = @@ -109,7 +110,7 @@ pub(crate) fn extract_struct_from_function_signature( field_list.clone_for_update() }; tracing::info!("extract_struct_from_function_signature: collecting fields"); - let def = create_struct_def(name.clone(), &fn_ast, ¶ms, &field_list, generics); + let def = create_struct_def(name.clone(), &fn_ast, ¶ms_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); let indent = fn_ast.indent_level(); @@ -123,7 +124,8 @@ pub(crate) fn extract_struct_from_function_signature( ], ); tracing::info!("extract_struct_from_function_signature: inserting struct {def}"); - update_function(name, &fn_ast, generic_params.map(|g| g.clone_for_update())); + update_function(name, &fn_ast, generic_params.map(|g| g.clone_for_update())).unwrap(); + tracing::info!("extract_struct_from_function_signature: updating function signature and parameter uses"); }, ) } @@ -144,6 +146,8 @@ fn update_function( let param = make::param( // TODO: do we want to destructure the struct + // would make it easier in that we would not have to update all the uses of the variables in + // the function ast::Pat::IdentPat(make::ident_pat( false, fn_ast.param_list()?.params().any(|p| { @@ -154,9 +158,9 @@ fn update_function( )), ty, ); - // TODO: will eventually need to handle self to - let param_list = make::param_list(None, std::iter::once(param)).clone_for_update(); - ted::replace(fn_ast.param_list()?.syntax(), param_list.syntax()); + // TODO: will eventually need to handle self too + let params_list = make::param_list(None, std::iter::once(param)).clone_for_update(); + ted::replace(fn_ast.param_list()?.syntax(), params_list.syntax()); // TODO: update uses of parameters in function, if we do not destructure Some(()) } From 37c231b2a458f497ebafd6fc947ade0467551587 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Wed, 30 Jul 2025 21:30:14 -0400 Subject: [PATCH 06/17] feat: extract_struct_from_function_signature made the edit actually have a effect by putting the edit_file first made the parameter in original function lower snake case made the checking if pre existing struct with new name use the new name as opposed to the original function name fixed autogenerated test --- .../extract_struct_from_function_signature.rs | 39 +++++++++++++------ crates/ide-assists/src/tests/generated.rs | 15 +++++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index e46349a48397..4a1fe152a5dd 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -1,7 +1,7 @@ use hir::{Function, ModuleDef}; use ide_db::{RootDatabase, assists::AssistId, path_transform::PathTransform}; use itertools::Itertools; -use stdx::to_camel_case; +use stdx::{to_camel_case, to_lower_snake_case}; use syntax::{ AstNode, SyntaxElement, SyntaxKind, SyntaxNode, T, ast::{ @@ -18,16 +18,13 @@ use crate::{AssistContext, Assists}; // Extracts a struct (part) of the signature of a function. // // ``` -// fn foo(bar: u32, baz: u32) { ... } +// fn foo($0bar: u32, baz: u32) { ... } // ``` // -> // ``` -// struct FooStruct { -// bar: u32, -// baz: u32, -// } +// struct FooStruct{ bar: u32, baz: u32 } // -// fn foo(FooStruct) { ... } +// fn foo(foo_struct: FooStruct) { ... } // ``` pub(crate) fn extract_struct_from_function_signature( @@ -42,9 +39,10 @@ pub(crate) fn extract_struct_from_function_signature( // go through the fn, the text_range is the whole function. let params_list = ctx.find_node_at_offset::()?; let fn_name = fn_ast.name()?; + let name = make::name(&format!("{}Struct", to_camel_case(fn_name.text_non_mutable()))); let fn_hir = ctx.sema.to_def(&fn_ast)?; - if existing_definition(ctx.db(), &fn_name, &fn_hir) { + if existing_definition(ctx.db(), &name, &fn_hir) { cov_mark::hit!(test_extract_function_signature_not_applicable_if_struct_exists); return None; } @@ -69,16 +67,16 @@ pub(crate) fn extract_struct_from_function_signature( }) .collect::>>()?, ); - let name = make::name(&format!("{}Struct", to_camel_case(fn_name.text_non_mutable()))); acc.add( AssistId::refactor_rewrite("extract_struct_from_function_signature"), "Extract struct from signature of a function", target, |builder| { // TODO: update calls to the function - let fn_ast = builder.make_mut(fn_ast); tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); + // this has to be after the edit_file (order matters) + let fn_ast = builder.make_mut(fn_ast); tracing::info!("extract_struct_from_function_signature: editing main file"); let generic_params = fn_ast @@ -154,7 +152,8 @@ fn update_function( p.pat() .is_some_and(|p| matches!(p, ast::Pat::IdentPat(p) if p.mut_token().is_some())) }), - name, + // TODO: maybe make a method that maps over a name's text + make::name(&to_lower_snake_case(name.text_non_mutable())), )), ty, ); @@ -327,3 +326,21 @@ fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Fu }) .any(|(name, _)| name.as_str() == variant_name.text().trim_start_matches("r#")) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::check_assist_not_applicable; + + #[test] + fn test_extract_function_signature_not_applicable_if_struct_exists() { + cov_mark::check!(test_extract_function_signature_not_applicable_if_struct_exists); + check_assist_not_applicable( + extract_struct_from_function_signature, + r#" +struct OneStruct; +fn one($0x: u8, y: u32) {} +"#, + ); + } +} diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 91348be97eb7..e6ce4873f8a1 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1187,6 +1187,21 @@ enum A { One(One) } ) } +#[test] +fn doctest_extract_struct_from_function_signature() { + check_doc_test( + "extract_struct_from_function_signature", + r#####" +fn foo($0bar: u32, baz: u32) { ... } +"#####, + r#####" +struct FooStruct{ bar: u32, baz: u32 } + +fn foo(foo_struct: FooStruct) { ... } +"#####, + ) +} + #[test] fn doctest_extract_type_alias() { check_doc_test( From c795f0db449dcf84bdf737f5eee67f299e0ef9a0 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Wed, 30 Jul 2025 22:16:10 -0400 Subject: [PATCH 07/17] feat: extract_struct_from_function_signature fixed typos --- .../src/handlers/extract_struct_from_function_signature.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 4a1fe152a5dd..fa9652668246 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -32,7 +32,7 @@ pub(crate) fn extract_struct_from_function_signature( ctx: &AssistContext<'_>, ) -> Option<()> { // TODO: get more specific than param list - // how to get function name and param list/part of param list the is selected seperatly + // how to get function name and param list/part of param list the is selected separately // or maybe just auto generate random name not based on function name? let fn_ast = ctx.find_node_at_offset::()?; // we independently get the param list without going through fn (fn_ast.param_list()), because for some reason when we @@ -50,7 +50,7 @@ pub(crate) fn extract_struct_from_function_signature( // TODO: does this capture parenthesis let target = params_list.syntax().text_range(); // TODO: special handiling for self? - // TODO: special handling for destrutered types (or maybe just don't suppurt code action on + // TODO: special handling for destrutered types (or maybe just don't support code action on // destructed types yet let field_list = make::record_field_list( From b370c03e689cddc2fa562be13016daa23de00a46 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Thu, 31 Jul 2025 20:58:12 -0400 Subject: [PATCH 08/17] feat: extract-struct-from-function-signature now destructures struct in function signature and only add paramereters that are selected to the struct added another test --- .../extract_struct_from_function_signature.rs | 113 ++++++++++++------ crates/ide-assists/src/tests/generated.rs | 2 +- 2 files changed, 75 insertions(+), 40 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index fa9652668246..af73eb9a641d 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -1,7 +1,6 @@ use hir::{Function, ModuleDef}; use ide_db::{RootDatabase, assists::AssistId, path_transform::PathTransform}; use itertools::Itertools; -use stdx::{to_camel_case, to_lower_snake_case}; use syntax::{ AstNode, SyntaxElement, SyntaxKind, SyntaxNode, T, ast::{ @@ -9,7 +8,8 @@ use syntax::{ edit::{AstNodeEdit, IndentLevel}, make, }, - match_ast, ted, + match_ast, + ted::{self, Element}, }; use crate::{AssistContext, Assists}; @@ -18,7 +18,7 @@ use crate::{AssistContext, Assists}; // Extracts a struct (part) of the signature of a function. // // ``` -// fn foo($0bar: u32, baz: u32) { ... } +// fn foo($0bar: u32, baz: u32$0) { ... } // ``` // -> // ``` @@ -31,15 +31,18 @@ pub(crate) fn extract_struct_from_function_signature( acc: &mut Assists, ctx: &AssistContext<'_>, ) -> Option<()> { - // TODO: get more specific than param list - // how to get function name and param list/part of param list the is selected separately - // or maybe just auto generate random name not based on function name? let fn_ast = ctx.find_node_at_offset::()?; - // we independently get the param list without going through fn (fn_ast.param_list()), because for some reason when we - // go through the fn, the text_range is the whole function. - let params_list = ctx.find_node_at_offset::()?; + let param_list = fn_ast.param_list()?; + let used_param_list = param_list + .params() + // filter to only parameters in selection + .filter(|p| p.syntax().text_range().intersect(ctx.selection_trimmed()).is_some()) + .collect_vec(); + // TODO: make sure at least one thing there + let target = + used_param_list.iter().map(|p| p.syntax().text_range()).reduce(|t, t2| t.cover(t2))?; let fn_name = fn_ast.name()?; - let name = make::name(&format!("{}Struct", to_camel_case(fn_name.text_non_mutable()))); + let name = make::name(&format!("{}Struct", stdx::to_camel_case(fn_name.text_non_mutable()))); let fn_hir = ctx.sema.to_def(&fn_ast)?; if existing_definition(ctx.db(), &name, &fn_hir) { @@ -47,19 +50,17 @@ pub(crate) fn extract_struct_from_function_signature( return None; } - // TODO: does this capture parenthesis - let target = params_list.syntax().text_range(); // TODO: special handiling for self? // TODO: special handling for destrutered types (or maybe just don't support code action on // destructed types yet let field_list = make::record_field_list( - fn_ast - .param_list()? - .params() + used_param_list + .iter() .map(|param| { Some(make::record_field( fn_ast.visibility(), + // only works if its an ident pattern param.pat().and_then(pat_to_name)?, // TODO: how are we going to handle references without explicit lifetimes param.ty()?, @@ -76,7 +77,10 @@ pub(crate) fn extract_struct_from_function_signature( tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); // this has to be after the edit_file (order matters) + // fn_ast and param_list must be "mut" for the effect to work on used_param_lsit let fn_ast = builder.make_mut(fn_ast); + let param_list = builder.make_mut(param_list); + let used_param_list = used_param_list.into_iter().map(|p| builder.make_mut(p)).collect_vec(); tracing::info!("extract_struct_from_function_signature: editing main file"); let generic_params = fn_ast @@ -92,7 +96,7 @@ pub(crate) fn extract_struct_from_function_signature( // So I do the resolving while its still param list // and then apply it into record list after let field_list = if let Some((target_scope, source_scope)) = - ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(params_list.syntax())) + ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(param_list.syntax())) { let field_list = field_list.reset_indent(); let field_list = @@ -108,12 +112,13 @@ pub(crate) fn extract_struct_from_function_signature( field_list.clone_for_update() }; tracing::info!("extract_struct_from_function_signature: collecting fields"); - let def = create_struct_def(name.clone(), &fn_ast, ¶ms_list, &field_list, generics); + let def = create_struct_def(name.clone(), &fn_ast, &used_param_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); let indent = fn_ast.indent_level(); let def = def.indent(indent); + ted::insert_all( ted::Position::before(fn_ast.syntax()), vec![ @@ -122,7 +127,7 @@ pub(crate) fn extract_struct_from_function_signature( ], ); tracing::info!("extract_struct_from_function_signature: inserting struct {def}"); - update_function(name, &fn_ast, generic_params.map(|g| g.clone_for_update())).unwrap(); + update_function(name, generic_params.map(|g| g.clone_for_update()), &used_param_list).unwrap(); tracing::info!("extract_struct_from_function_signature: updating function signature and parameter uses"); }, ) @@ -130,8 +135,8 @@ pub(crate) fn extract_struct_from_function_signature( fn update_function( name: ast::Name, - fn_ast: &ast::Fn, generics: Option, + used_param_list: &[ast::Param], ) -> Option<()> { let generic_args = generics .filter(|generics| generics.generic_params().count() > 0) @@ -143,24 +148,26 @@ fn update_function( }; let param = make::param( - // TODO: do we want to destructure the struct - // would make it easier in that we would not have to update all the uses of the variables in + // do we want to destructure the struct + // makes it easier in that we would not have to update all the uses of the variables in // the function - ast::Pat::IdentPat(make::ident_pat( - false, - fn_ast.param_list()?.params().any(|p| { - p.pat() - .is_some_and(|p| matches!(p, ast::Pat::IdentPat(p) if p.mut_token().is_some())) - }), - // TODO: maybe make a method that maps over a name's text - make::name(&to_lower_snake_case(name.text_non_mutable())), + ast::Pat::RecordPat(make::record_pat( + make::path_from_text(name.text_non_mutable()), + used_param_list + .iter() + .map(|p| p.pat()) + .chain(std::iter::once(Some(ast::Pat::RestPat(make::rest_pat())))) + .collect::>>()?, )), ty, - ); + ) + .clone_for_update(); // TODO: will eventually need to handle self too - let params_list = make::param_list(None, std::iter::once(param)).clone_for_update(); - ted::replace(fn_ast.param_list()?.syntax(), params_list.syntax()); - // TODO: update uses of parameters in function, if we do not destructure + + let range = used_param_list.first()?.syntax().syntax_element() + ..=used_param_list.last()?.syntax().syntax_element(); + ted::replace_all(range, vec![param.syntax().syntax_element()]); + // no need update uses of parameters in function, because we destructure the struct Some(()) } @@ -173,7 +180,7 @@ fn pat_to_name(pat: ast::Pat) -> Option { fn create_struct_def( name: ast::Name, fn_ast: &ast::Fn, - param_ast: &ast::ParamList, + param_ast: &[ast::Param], field_list: &ast::RecordFieldList, generics: Option, ) -> ast::Struct { @@ -203,7 +210,7 @@ fn create_struct_def( // take comments from only inside signature ted::insert_all( ted::Position::first_child_of(strukt.syntax()), - take_all_comments(param_ast.syntax()), + take_all_comments(param_ast.iter()), ); // TODO: this may not be correct as we shouldn't put all the attributes at the top @@ -211,7 +218,7 @@ fn create_struct_def( ted::insert_all( ted::Position::first_child_of(strukt.syntax()), param_ast - .params() + .iter() .flat_map(|p| p.attrs()) .flat_map(|it| { vec![it.syntax().clone_for_update().into(), make::tokens::single_newline().into()] @@ -224,9 +231,9 @@ fn create_struct_def( // Note: this also detaches whitespace after comments, // since `SyntaxNode::splice_children` (and by extension `ted::insert_all_raw`) // detaches nodes. If we only took the comments, we'd leave behind the old whitespace. -fn take_all_comments(node: &SyntaxNode) -> Vec { +fn take_all_comments<'a>(node: impl Iterator) -> Vec { let mut remove_next_ws = false; - node.children_with_tokens() + node.flat_map(|p| p.syntax().children_with_tokens()) .filter_map(move |child| match child.kind() { SyntaxKind::COMMENT => { remove_next_ws = true; @@ -330,7 +337,7 @@ fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Fu #[cfg(test)] mod tests { use super::*; - use crate::tests::check_assist_not_applicable; + use crate::tests::{check_assist, check_assist_not_applicable}; #[test] fn test_extract_function_signature_not_applicable_if_struct_exists() { @@ -340,6 +347,34 @@ mod tests { r#" struct OneStruct; fn one($0x: u8, y: u32) {} +"#, + ); + } + #[test] + fn test_extract_function_signature_single_parameters() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo($0bar: i32$0, baz: i32) {} +"#, + r#" +struct FooStruct{ bar: i32 } + +fn foo(FooStruct { bar, .. }: FooStruct, baz: i32) {} +"#, + ); + } + #[test] + fn test_extract_function_signature_all_parameters() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo($0bar: i32, baz: i32$0) {} +"#, + r#" +struct FooStruct{ bar: i32, baz: i32 } + +fn foo(FooStruct { bar, baz, .. }: FooStruct) {} "#, ); } diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index e6ce4873f8a1..9ee99ee25327 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1192,7 +1192,7 @@ fn doctest_extract_struct_from_function_signature() { check_doc_test( "extract_struct_from_function_signature", r#####" -fn foo($0bar: u32, baz: u32) { ... } +fn foo($0bar: u32, baz: u32$0) { ... } "#####, r#####" struct FooStruct{ bar: u32, baz: u32 } From 4edccf8c69cbf79935f3dff96f05cdf07a9437d8 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Fri, 1 Aug 2025 01:45:28 -0400 Subject: [PATCH 09/17] feat: extract_struct_from_function_signature working on making updating references. also fixed typos fixed and added more tests --- .../extract_struct_from_function_signature.rs | 276 ++++++++++++++++-- crates/ide-assists/src/tests/generated.rs | 2 +- 2 files changed, 260 insertions(+), 18 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index af73eb9a641d..632bd76e1a34 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -1,10 +1,22 @@ -use hir::{Function, ModuleDef}; -use ide_db::{RootDatabase, assists::AssistId, path_transform::PathTransform}; +use std::ops::Range; + +use hir::{Function, HasCrate, Module, ModuleDef}; +use ide_db::{ + FxHashSet, RootDatabase, + assists::AssistId, + defs::Definition, + helpers::mod_path_to_ast, + imports::insert_use::{ImportScope, InsertUseConfig, insert_use}, + path_transform::PathTransform, + search::FileReference, + source_change::SourceChangeBuilder, +}; use itertools::Itertools; use syntax::{ - AstNode, SyntaxElement, SyntaxKind, SyntaxNode, T, + AstNode, Edition, SyntaxElement, SyntaxKind, SyntaxNode, T, ast::{ - self, HasAttrs, HasGenericParams, HasName, HasVisibility, + self, CallExpr, HasArgList, HasAttrs, HasGenericParams, HasName, HasVisibility, + RecordExprField, edit::{AstNodeEdit, IndentLevel}, make, }, @@ -24,7 +36,7 @@ use crate::{AssistContext, Assists}; // ``` // struct FooStruct{ bar: u32, baz: u32 } // -// fn foo(foo_struct: FooStruct) { ... } +// fn foo(FooStruct { bar, baz, .. }: FooStruct) { ... } // ``` pub(crate) fn extract_struct_from_function_signature( @@ -50,7 +62,7 @@ pub(crate) fn extract_struct_from_function_signature( return None; } - // TODO: special handiling for self? + // TODO: special handling for self? // TODO: special handling for destrutered types (or maybe just don't support code action on // destructed types yet @@ -68,20 +80,68 @@ pub(crate) fn extract_struct_from_function_signature( }) .collect::>>()?, ); + + let start_index = used_param_list.first()?.syntax().index(); + let end_index = used_param_list.last()?.syntax().index(); + let used_params_range = start_index..end_index + 1; acc.add( AssistId::refactor_rewrite("extract_struct_from_function_signature"), "Extract struct from signature of a function", target, |builder| { + let edition = fn_hir.krate(ctx.db()).edition(ctx.db()); + let enum_module_def = ModuleDef::from(fn_hir); + + let usages = Definition::Function(fn_hir).usages(&ctx.sema).all(); + let mut visited_modules_set = FxHashSet::default(); + let current_module = fn_hir.module(ctx.db()); + visited_modules_set.insert(current_module); + // record file references of the file the def resides in, we only want to swap to the edited file in the builder once + + let mut def_file_references = None; + + for (file_id, references) in usages { + if file_id == ctx.file_id() { + def_file_references = Some(references); + continue; + } + builder.edit_file(file_id.file_id(ctx.db())); + let processed = process_references( + ctx, + builder, + &mut visited_modules_set, + &enum_module_def, + references, + name.clone() + ); + processed.into_iter().for_each(|(path, node, import)| { + apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list); + }); + } + // TODO: update calls to the function tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); - // this has to be after the edit_file (order matters) - // fn_ast and param_list must be "mut" for the effect to work on used_param_lsit - let fn_ast = builder.make_mut(fn_ast); - let param_list = builder.make_mut(param_list); + let fn_ast_mut = builder.make_mut(fn_ast.clone()); + builder.make_mut(param_list.clone()); let used_param_list = used_param_list.into_iter().map(|p| builder.make_mut(p)).collect_vec(); tracing::info!("extract_struct_from_function_signature: editing main file"); + // this has to be after the edit_file (order matters) + // fn_ast and param_list must be "mut" for the effect to work on used_param_list + if let Some(references) = def_file_references { + let processed = process_references( + ctx, + builder, + &mut visited_modules_set, + &enum_module_def, + references, + name.clone() + ); + processed.into_iter().for_each(|(path, node, import)| { + apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list); + }); + } + let generic_params = fn_ast .generic_param_list() @@ -112,15 +172,15 @@ pub(crate) fn extract_struct_from_function_signature( field_list.clone_for_update() }; tracing::info!("extract_struct_from_function_signature: collecting fields"); - let def = create_struct_def(name.clone(), &fn_ast, &used_param_list, &field_list, generics); + let def = create_struct_def(name.clone(), &fn_ast_mut, &used_param_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); - let indent = fn_ast.indent_level(); + let indent = fn_ast_mut.indent_level(); let def = def.indent(indent); ted::insert_all( - ted::Position::before(fn_ast.syntax()), + ted::Position::before(fn_ast_mut.syntax()), vec![ def.syntax().clone().into(), make::tokens::whitespace(&format!("\n\n{indent}")).into(), @@ -164,9 +224,11 @@ fn update_function( .clone_for_update(); // TODO: will eventually need to handle self too - let range = used_param_list.first()?.syntax().syntax_element() - ..=used_param_list.last()?.syntax().syntax_element(); - ted::replace_all(range, vec![param.syntax().syntax_element()]); + let start_index = used_param_list.first().unwrap().syntax().index(); + let end_index = used_param_list.last().unwrap().syntax().index(); + let used_params_range = start_index..end_index + 1; + let new = vec![param.syntax().syntax_element()]; + used_param_list.first()?.syntax().parent()?.splice_children(used_params_range, new); // no need update uses of parameters in function, because we destructure the struct Some(()) } @@ -334,6 +396,104 @@ fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Fu .any(|(name, _)| name.as_str() == variant_name.text().trim_start_matches("r#")) } +fn process_references( + ctx: &AssistContext<'_>, + builder: &mut SourceChangeBuilder, + visited_modules: &mut FxHashSet, + function_module_def: &ModuleDef, + refs: Vec, + name: ast::Name, +) -> Vec<(ast::PathSegment, SyntaxNode, Option<(ImportScope, hir::ModPath)>)> { + // we have to recollect here eagerly as we are about to edit the tree we need to calculate the changes + // and corresponding nodes up front + let name = make::name_ref(name.text_non_mutable()); + refs.into_iter() + .flat_map(|reference| { + let (segment, scope_node, module) = + reference_to_node(&ctx.sema, reference, name.clone())?; + let scope_node = builder.make_syntax_mut(scope_node); + if !visited_modules.contains(&module) { + let mod_path = module.find_use_path( + ctx.sema.db, + *function_module_def, + ctx.config.insert_use.prefix_kind, + ctx.config.import_path_config(), + ); + if let Some(mut mod_path) = mod_path { + mod_path.pop_segment(); + mod_path.push_segment(hir::Name::new_root(name.text_non_mutable()).clone()); + let scope = ImportScope::find_insert_use_container(&scope_node, &ctx.sema)?; + visited_modules.insert(module); + return Some((segment, scope_node, Some((scope, mod_path)))); + } + } + Some((segment, scope_node, None)) + }) + .collect() +} +fn reference_to_node( + sema: &hir::Semantics<'_, RootDatabase>, + reference: FileReference, + name: ast::NameRef, +) -> Option<(ast::PathSegment, SyntaxNode, hir::Module)> { + // filter out the reference in macro + let segment = + reference.name.as_name_ref()?.syntax().parent().and_then(ast::PathSegment::cast)?; + + let segment_range = segment.syntax().text_range(); + if segment_range != reference.range { + return None; + } + + let parent = segment.parent_path().syntax().parent()?; + let expr_or_pat = match_ast! { + match parent { + ast::PathExpr(_it) => parent.parent()?, + ast::RecordExpr(_it) => parent, + ast::TupleStructPat(_it) => parent, + ast::RecordPat(_it) => parent, + _ => return None, + } + }; + let module = sema.scope(&expr_or_pat)?.module(); + let segment = make::path_segment(name); + Some((segment, expr_or_pat, module)) +} + +fn apply_references( + insert_use_cfg: InsertUseConfig, + segment: ast::PathSegment, + node: SyntaxNode, + import: Option<(ImportScope, hir::ModPath)>, + edition: Edition, + used_params_range: Range, + field_list: &ast::RecordFieldList, +) -> Option<()> { + if let Some((scope, path)) = import { + insert_use(&scope, mod_path_to_ast(&path, edition), &insert_use_cfg); + } + // deep clone to prevent cycle + let path = make::path_from_segments(std::iter::once(segment.clone_subtree()), false); + let call = CallExpr::cast(node)?; + let fields = make::record_expr_field_list( + call.arg_list()? + .args() + .skip(used_params_range.start - 1) + .take(used_params_range.end - used_params_range.start) + .zip(field_list.fields()) + .map(|e| { + e.1.name().map(|name| -> RecordExprField { + make::record_expr_field(make::name_ref(name.text_non_mutable()), Some(e.0)) + }) + }) + .collect::>>()?, + ); + let record_expr = make::record_expr(path, fields).clone_for_update(); + call.arg_list()? + .syntax() + .splice_children(used_params_range, vec![record_expr.syntax().syntax_element()]); + Some(()) +} #[cfg(test)] mod tests { use super::*; @@ -351,7 +511,7 @@ fn one($0x: u8, y: u32) {} ); } #[test] - fn test_extract_function_signature_single_parameters() { + fn test_extract_function_signature_single_parameter() { check_assist( extract_struct_from_function_signature, r#" @@ -378,4 +538,86 @@ fn foo(FooStruct { bar, baz, .. }: FooStruct) {} "#, ); } + #[test] + fn test_extract_function_signature_all_parameters_with_reference() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo($0bar: i32, baz: i32$0) {} + +fn main() { + foo(1, 2) +} +"#, + r#" +struct FooStruct{ bar: i32, baz: i32 } + +fn foo(FooStruct { bar, baz, .. }: FooStruct) {} + +fn main() { + foo(FooStruct { bar: 1, baz: 2 }) +} +"#, + ); + } + #[test] + fn test_extract_function_signature_single_parameter_with_reference_seperate_and_inself() { + check_assist( + extract_struct_from_function_signature, + r#" +mod a { + pub fn foo($0bar: i32$0, baz: i32) { + foo(1, 2) + } +} + +mod b { + use crate::a::foo; + + fn main() { + foo(1, 2) + } +} +"#, + r#" +mod a { + pub struct FooStruct{ pub bar: i32 } + + pub fn foo(FooStruct { bar, .. }: FooStruct, baz: i32) { + foo(FooStruct { bar: 1 }, 2) + } +} + +mod b { + use crate::a::{foo, FooStruct}; + + fn main() { + foo(FooStruct { bar: 1 }, 2) + } +} +"#, + ); + } + #[test] + fn test_extract_function_signature_single_parameter_with_reference() { + check_assist( + extract_struct_from_function_signature, + r#" + fn foo($0bar: i32$0, baz: i32) {} + + fn main() { + foo(1, 2) + } + "#, + r#" + struct FooStruct{ bar: i32 } + + fn foo(FooStruct { bar, .. }: FooStruct, baz: i32) {} + + fn main() { + foo(FooStruct { bar: 1 }, 2) + } + "#, + ); + } } diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 9ee99ee25327..8acf2efc079e 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1197,7 +1197,7 @@ fn foo($0bar: u32, baz: u32$0) { ... } r#####" struct FooStruct{ bar: u32, baz: u32 } -fn foo(foo_struct: FooStruct) { ... } +fn foo(FooStruct { bar, baz, .. }: FooStruct) { ... } "#####, ) } From e8a8d0680a7d95409ec7961c6edf859986fcddda Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Fri, 1 Aug 2025 16:52:42 -0400 Subject: [PATCH 10/17] feat: extract_struct_from_function_signature starting to work on lifetime part --- .../extract_struct_from_function_signature.rs | 179 ++++++++++++++++-- crates/syntax/src/ast/edit_in_place.rs | 33 ++++ 2 files changed, 199 insertions(+), 13 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 632bd76e1a34..b811a202e85f 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -89,6 +89,7 @@ pub(crate) fn extract_struct_from_function_signature( "Extract struct from signature of a function", target, |builder| { + let new_lifetime_count = field_list.fields().filter_map(|f|f.ty()).map(|t|new_life_time_count(&t)).sum(); let edition = fn_hir.krate(ctx.db()).edition(ctx.db()); let enum_module_def = ModuleDef::from(fn_hir); @@ -115,7 +116,10 @@ pub(crate) fn extract_struct_from_function_signature( name.clone() ); processed.into_iter().for_each(|(path, node, import)| { - apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list); + apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list, + name.clone(), + new_lifetime_count + ); }); } @@ -138,7 +142,10 @@ pub(crate) fn extract_struct_from_function_signature( name.clone() ); processed.into_iter().for_each(|(path, node, import)| { - apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list); + apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list, + name.clone(), + new_lifetime_count + ); }); } @@ -147,7 +154,7 @@ pub(crate) fn extract_struct_from_function_signature( .generic_param_list() .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); tracing::info!("extract_struct_from_function_signature: collecting generics"); - let generics = generic_params.as_ref().map(|generics| generics.clone_for_update()); + let mut generics = generic_params.as_ref().map(|generics| generics.clone_for_update()); // resolve GenericArg in field_list to actual type // we would get a query error from salsa, if we would use the field_list @@ -171,6 +178,7 @@ pub(crate) fn extract_struct_from_function_signature( } else { field_list.clone_for_update() }; + field_list.fields().filter_map(|f|f.ty()).try_for_each(|t|generate_new_lifetimes(&t, &mut generics)); tracing::info!("extract_struct_from_function_signature: collecting fields"); let def = create_struct_def(name.clone(), &fn_ast_mut, &used_param_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); @@ -187,7 +195,7 @@ pub(crate) fn extract_struct_from_function_signature( ], ); tracing::info!("extract_struct_from_function_signature: inserting struct {def}"); - update_function(name, generic_params.map(|g| g.clone_for_update()), &used_param_list).unwrap(); + update_function(name, generic_params.map(|g| g.clone_for_update()), &used_param_list, new_lifetime_count).unwrap(); tracing::info!("extract_struct_from_function_signature: updating function signature and parameter uses"); }, ) @@ -197,10 +205,19 @@ fn update_function( name: ast::Name, generics: Option, used_param_list: &[ast::Param], + new_lifetime_count: usize, ) -> Option<()> { - let generic_args = generics - .filter(|generics| generics.generic_params().count() > 0) - .map(|generics| generics.to_generic_args()); + // TODO: add new generics if needed + let generic_args = + generics.filter(|generics| generics.generic_params().count() > 0).map(|generics| { + let args = generics.to_generic_args().clone_for_update(); + (0..new_lifetime_count).for_each(|_| { + args.add_generic_arg( + make::lifetime_arg(make::lifetime("'_")).clone_for_update().into(), + ) + }); + args + }); // FIXME: replace with a `ast::make` constructor let ty = match generic_args { Some(generic_args) => make::ty(&format!("{name}{generic_args}")), @@ -212,6 +229,7 @@ fn update_function( // makes it easier in that we would not have to update all the uses of the variables in // the function ast::Pat::RecordPat(make::record_pat( + // TODO: need to have no turbofish kept lifetimes/generics if make::path_from_text(name.text_non_mutable()), used_param_list .iter() @@ -328,6 +346,44 @@ fn extract_generic_params( let generics = generics.into_iter().filter_map(|(param, tag)| tag.then_some(param)); tagged_one.then(|| make::generic_param_list(generics)) } +fn generate_unique_lifetime_param_name( + existing_type_param_list: &Option, +) -> Option { + match existing_type_param_list { + Some(type_params) => { + let used_lifetime_params: FxHashSet<_> = + type_params.lifetime_params().map(|p| p.syntax().text().to_string()).collect(); + ('a'..='z').map(|it| format!("'{it}")).find(|it| !used_lifetime_params.contains(it)) + } + None => Some("'a".to_owned()), + } + .map(|it| make::lifetime(&it)) +} +fn new_life_time_count(ty: &ast::Type) -> usize { + ty.syntax() + .descendants() + .filter_map(ast::Lifetime::cast) + .filter(|lifetime| lifetime.text() == "'_") + .count() +} +fn generate_new_lifetimes( + ty: &ast::Type, + existing_type_param_list: &mut Option, +) -> Option<()> { + for token in ty.syntax().descendants() { + if let Some(lt) = ast::Lifetime::cast(token.clone()) + && lt.text() == "'_" + { + let new_lt = generate_unique_lifetime_param_name(existing_type_param_list)?; + existing_type_param_list + .get_or_insert(make::generic_param_list(std::iter::empty()).clone_for_update()) + .add_generic_param(make::lifetime_param(new_lt.clone()).clone_for_update().into()); + + ted::replace(lt.syntax(), new_lt.clone_for_update().syntax()); + } + } + Some(()) +} fn tag_generics_in_function_signature( ty: &ast::Type, generics: &mut [(ast::GenericParam, bool)], @@ -409,8 +465,7 @@ fn process_references( let name = make::name_ref(name.text_non_mutable()); refs.into_iter() .flat_map(|reference| { - let (segment, scope_node, module) = - reference_to_node(&ctx.sema, reference, name.clone())?; + let (segment, scope_node, module) = reference_to_node(&ctx.sema, reference)?; let scope_node = builder.make_syntax_mut(scope_node); if !visited_modules.contains(&module) { let mod_path = module.find_use_path( @@ -434,7 +489,6 @@ fn process_references( fn reference_to_node( sema: &hir::Semantics<'_, RootDatabase>, reference: FileReference, - name: ast::NameRef, ) -> Option<(ast::PathSegment, SyntaxNode, hir::Module)> { // filter out the reference in macro let segment = @@ -456,8 +510,8 @@ fn reference_to_node( } }; let module = sema.scope(&expr_or_pat)?.module(); - let segment = make::path_segment(name); - Some((segment, expr_or_pat, module)) + + Some((segment.clone_for_update(), expr_or_pat, module)) } fn apply_references( @@ -468,12 +522,28 @@ fn apply_references( edition: Edition, used_params_range: Range, field_list: &ast::RecordFieldList, + name: ast::Name, + new_lifetime_count: usize, ) -> Option<()> { if let Some((scope, path)) = import { insert_use(&scope, mod_path_to_ast(&path, edition), &insert_use_cfg); } + // TODO: figure out lifetimes in referecnecs + // becauuse we have to convert from segment being non turbofish, also only need + // generics/lifetimes that are used in struct possibly not all the no ones for the original call + // if no specified lifetimes/generics we just give empty one + // if new_lifetime_count > 0 { + // (0..new_lifetime_count).for_each(|_| { + // segment + // .get_or_create_generic_arg_list() + // .add_generic_arg(make::lifetime_arg(make::lifetime("'_")).clone_for_update().into()) + // }); + // } + + ted::replace(segment.name_ref()?.syntax(), name.clone_for_update().syntax()); // deep clone to prevent cycle let path = make::path_from_segments(std::iter::once(segment.clone_subtree()), false); + // TODO: do I need to to method call to let call = CallExpr::cast(node)?; let fields = make::record_expr_field_list( call.arg_list()? @@ -489,6 +559,7 @@ fn apply_references( .collect::>>()?, ); let record_expr = make::record_expr(path, fields).clone_for_update(); + call.arg_list()? .syntax() .splice_children(used_params_range, vec![record_expr.syntax().syntax_element()]); @@ -561,7 +632,7 @@ fn main() { ); } #[test] - fn test_extract_function_signature_single_parameter_with_reference_seperate_and_inself() { + fn test_extract_function_signature_single_parameter_with_reference_separate_and_in_self() { check_assist( extract_struct_from_function_signature, r#" @@ -620,4 +691,86 @@ mod b { "#, ); } + + #[test] + fn test_extract_function_signature_single_parameter_generic() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo<'a, A>($0bar: &'a A$0, baz: i32) {} +"#, + r#" +struct FooStruct<'a, A>{ bar: &'a A } + +fn foo<'a, A>(FooStruct { bar, .. }: FooStruct<'a, A>, baz: i32) {} +"#, + ); + } + #[test] + fn test_extract_function_signature_single_parameter_generic_with_reference_in_self() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo<'a, A>($0bar: &'a A$0, baz: i32) { + foo(1, 2) +} +"#, + r#" +struct FooStruct<'a, A>{ bar: &'a A } + +fn foo<'a, A>(FooStruct { bar, .. }: FooStruct<'a, A>, baz: i32) { + foo(FooStruct { bar: 1 }, 2) +} +"#, + ); + } + + #[test] + fn test_extract_function_signature_single_parameter_anonymous_lifetime() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo($0bar: &'_ i32$0, baz: i32) {} +"#, + r#" +struct FooStruct<'a>{ bar: &'a i32 } + +fn foo(FooStruct { bar, .. }: FooStruct, baz: i32) {} +"#, + ); + } + #[test] + fn test_extract_function_signature_single_parameter_anonymous_and_normal_lifetime() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo<'a>($0bar: &'_ &'a i32$0, baz: i32) {} +"#, + r#" +struct FooStruct<'a, 'b>{ bar: &'b &'a i32 } + +fn foo<'a>(FooStruct { bar, .. }: FooStruct<'a, '_>, baz: i32) {} +"#, + ); + } + + #[test] + fn test_extract_function_signature_single_parameter_anonymous_and_normal_lifetime_with_reference_in_self() + { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo<'a>($0bar: &'_ &'a i32$0, baz: i32) { + foo(bar, baz) +} +"#, + r#" +struct FooStruct<'a, 'b>{ bar: &'b &'a i32 } + +fn foo<'a>(FooStruct { bar, .. }: FooStruct<'a, '_>, baz: i32) { + foo(FooStruct { bar: bar }, baz) +} +"#, + ); + } } diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index b50ce6442432..264ce2a35c05 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -380,7 +380,40 @@ impl ast::GenericParamList { make::generic_arg_list(args) } } +impl ast::GenericArgList { + pub fn add_generic_arg(&self, generic_arg: ast::GenericArg) { + match self.generic_args().last() { + Some(last_param) => { + let position = Position::after(last_param.syntax()); + let elements = vec![ + make::token(T![,]).into(), + make::tokens::single_space().into(), + generic_arg.syntax().clone().into(), + ]; + ted::insert_all(position, elements); + } + None => { + let after_l_angle = Position::after(self.l_angle_token().unwrap()); + ted::insert(after_l_angle, generic_arg.syntax()); + } + } + } + /// Removes the existing generic param + pub fn remove_generic_arg(&self, generic_arg: ast::GenericArg) { + if let Some(previous) = generic_arg.syntax().prev_sibling() { + if let Some(next_token) = previous.next_sibling_or_token() { + ted::remove_all(next_token..=generic_arg.syntax().clone().into()); + } + } else if let Some(next) = generic_arg.syntax().next_sibling() { + if let Some(next_token) = next.prev_sibling_or_token() { + ted::remove_all(generic_arg.syntax().clone().into()..=next_token); + } + } else { + ted::remove(generic_arg.syntax()); + } + } +} impl ast::WhereClause { pub fn add_predicate(&self, predicate: ast::WherePred) { if let Some(pred) = self.predicates().last() From d076444120c178f8071a65530812dade58cf6164 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Fri, 1 Aug 2025 19:28:05 -0400 Subject: [PATCH 11/17] feat: extract_struct_from_function_signature the signature of the function now uses the struct with all the generic args while uses don't specifiy any generics --- .../extract_struct_from_function_signature.rs | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index b811a202e85f..01b621591fad 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -15,8 +15,8 @@ use itertools::Itertools; use syntax::{ AstNode, Edition, SyntaxElement, SyntaxKind, SyntaxNode, T, ast::{ - self, CallExpr, HasArgList, HasAttrs, HasGenericParams, HasName, HasVisibility, - RecordExprField, + self, CallExpr, HasArgList, HasAttrs, HasGenericArgs, HasGenericParams, HasName, + HasVisibility, RecordExprField, edit::{AstNodeEdit, IndentLevel}, make, }, @@ -66,6 +66,7 @@ pub(crate) fn extract_struct_from_function_signature( // TODO: special handling for destrutered types (or maybe just don't support code action on // destructed types yet + // TODO: don't allow if there is impl traits let field_list = make::record_field_list( used_param_list .iter() @@ -118,12 +119,11 @@ pub(crate) fn extract_struct_from_function_signature( processed.into_iter().for_each(|(path, node, import)| { apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list, name.clone(), - new_lifetime_count + // new_lifetime_count ); }); } - // TODO: update calls to the function tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); let fn_ast_mut = builder.make_mut(fn_ast.clone()); @@ -144,7 +144,7 @@ pub(crate) fn extract_struct_from_function_signature( processed.into_iter().for_each(|(path, node, import)| { apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list, name.clone(), - new_lifetime_count + // new_lifetime_count ); }); } @@ -207,9 +207,10 @@ fn update_function( used_param_list: &[ast::Param], new_lifetime_count: usize, ) -> Option<()> { - // TODO: add new generics if needed - let generic_args = - generics.filter(|generics| generics.generic_params().count() > 0).map(|generics| { + let generic_args = generics + .filter(|generics| generics.generic_params().count() > 0) + .or((new_lifetime_count > 0).then_some(make::generic_param_list(std::iter::empty()))) + .map(|generics| { let args = generics.to_generic_args().clone_for_update(); (0..new_lifetime_count).for_each(|_| { args.add_generic_arg( @@ -490,14 +491,14 @@ fn reference_to_node( sema: &hir::Semantics<'_, RootDatabase>, reference: FileReference, ) -> Option<(ast::PathSegment, SyntaxNode, hir::Module)> { - // filter out the reference in macro + // filter out the reference in macro (seems to be probalamtic with lifetimes/generics arguments) let segment = reference.name.as_name_ref()?.syntax().parent().and_then(ast::PathSegment::cast)?; - let segment_range = segment.syntax().text_range(); - if segment_range != reference.range { - return None; - } + // let segment_range = segment.syntax().text_range(); + // if segment_range != reference.range { + // return None; + // } let parent = segment.parent_path().syntax().parent()?; let expr_or_pat = match_ast! { @@ -523,7 +524,7 @@ fn apply_references( used_params_range: Range, field_list: &ast::RecordFieldList, name: ast::Name, - new_lifetime_count: usize, + // new_lifetime_count: usize, ) -> Option<()> { if let Some((scope, path)) = import { insert_use(&scope, mod_path_to_ast(&path, edition), &insert_use_cfg); @@ -540,6 +541,10 @@ fn apply_references( // }); // } + // current idea: the lifetimes can be inferred from the call + if let Some(generics) = segment.generic_arg_list() { + ted::remove(generics.syntax()); + } ted::replace(segment.name_ref()?.syntax(), name.clone_for_update().syntax()); // deep clone to prevent cycle let path = make::path_from_segments(std::iter::once(segment.clone_subtree()), false); @@ -735,7 +740,7 @@ fn foo($0bar: &'_ i32$0, baz: i32) {} r#" struct FooStruct<'a>{ bar: &'a i32 } -fn foo(FooStruct { bar, .. }: FooStruct, baz: i32) {} +fn foo(FooStruct { bar, .. }: FooStruct<'_>, baz: i32) {} "#, ); } From 9036099636e2065dd6a377ca20b0423242046d6e Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Sat, 2 Aug 2025 23:28:13 -0400 Subject: [PATCH 12/17] feat: extract_struct_from_function_signature do not support if there is impl trait also ideas for when handling self parameter cleaned up comments --- .../extract_struct_from_function_signature.rs | 72 ++++++++++++------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 01b621591fad..b696a2538ac9 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -43,14 +43,14 @@ pub(crate) fn extract_struct_from_function_signature( acc: &mut Assists, ctx: &AssistContext<'_>, ) -> Option<()> { - let fn_ast = ctx.find_node_at_offset::()?; + let find_node_at_offset = ctx.find_node_at_offset::()?; + let fn_ast = find_node_at_offset; let param_list = fn_ast.param_list()?; let used_param_list = param_list .params() // filter to only parameters in selection .filter(|p| p.syntax().text_range().intersect(ctx.selection_trimmed()).is_some()) .collect_vec(); - // TODO: make sure at least one thing there let target = used_param_list.iter().map(|p| p.syntax().text_range()).reduce(|t, t2| t.cover(t2))?; let fn_name = fn_ast.name()?; @@ -62,25 +62,20 @@ pub(crate) fn extract_struct_from_function_signature( return None; } - // TODO: special handling for self? - // TODO: special handling for destrutered types (or maybe just don't support code action on + // TODO: (future)special handling for self + // since it puts struct above function it invalid needs to go outside the the impl block + // if uses self parameter and that is selected: + // do we still keep in it in the impl block/does it matter what type of impl block it is (if its + // a trait then probably not) + // what should the name for self parameter be in the struct + // also you would need to grab out any generics from that impl block itself and any where + // clauses + // we also need special handling for method calls + + // TODO: (future)special handling for destrutered types (or maybe just don't support code action on // destructed types yet - // TODO: don't allow if there is impl traits - let field_list = make::record_field_list( - used_param_list - .iter() - .map(|param| { - Some(make::record_field( - fn_ast.visibility(), - // only works if its an ident pattern - param.pat().and_then(pat_to_name)?, - // TODO: how are we going to handle references without explicit lifetimes - param.ty()?, - )) - }) - .collect::>>()?, - ); + let field_list = extract_field_list(&fn_ast, &used_param_list)?; let start_index = used_param_list.first()?.syntax().index(); let end_index = used_param_list.last()?.syntax().index(); @@ -201,6 +196,26 @@ pub(crate) fn extract_struct_from_function_signature( ) } +fn extract_field_list( + fn_ast: &ast::Fn, + used_param_list: &[ast::Param], +) -> Option { + let field_list = make::record_field_list( + used_param_list + .iter() + .map(|param| { + Some(make::record_field( + fn_ast.visibility(), + // only works if its an ident pattern + param.pat().and_then(pat_to_name)?, + param.ty().filter(|ty| !contains_impl_trait(ty))?, + )) + }) + .collect::>>()?, + ); + Some(field_list) +} + fn update_function( name: ast::Name, generics: Option, @@ -226,11 +241,10 @@ fn update_function( }; let param = make::param( - // do we want to destructure the struct + // we destructure the struct // makes it easier in that we would not have to update all the uses of the variables in // the function ast::Pat::RecordPat(make::record_pat( - // TODO: need to have no turbofish kept lifetimes/generics if make::path_from_text(name.text_non_mutable()), used_param_list .iter() @@ -241,17 +255,17 @@ fn update_function( ty, ) .clone_for_update(); - // TODO: will eventually need to handle self too + // it is fine to unwrap() to because there is at least one parameter (if there is no parameters + // the code action will not show) let start_index = used_param_list.first().unwrap().syntax().index(); let end_index = used_param_list.last().unwrap().syntax().index(); let used_params_range = start_index..end_index + 1; let new = vec![param.syntax().syntax_element()]; - used_param_list.first()?.syntax().parent()?.splice_children(used_params_range, new); + used_param_list.first().unwrap().syntax().parent()?.splice_children(used_params_range, new); // no need update uses of parameters in function, because we destructure the struct Some(()) } - fn pat_to_name(pat: ast::Pat) -> Option { match pat { ast::Pat::IdentPat(ident_pat) => ident_pat.name(), @@ -367,6 +381,9 @@ fn new_life_time_count(ty: &ast::Type) -> usize { .filter(|lifetime| lifetime.text() == "'_") .count() } +fn contains_impl_trait(ty: &ast::Type) -> bool { + ty.syntax().descendants().any(|ty| ty.kind() == ast::ImplTraitType::kind()) +} fn generate_new_lifetimes( ty: &ast::Type, existing_type_param_list: &mut Option, @@ -778,4 +795,11 @@ fn foo<'a>(FooStruct { bar, .. }: FooStruct<'a, '_>, baz: i32) { "#, ); } + #[test] + fn test_extract_function_signature_not_applicable_with_impl_trait() { + check_assist_not_applicable( + extract_struct_from_function_signature, + r"fn foo($0i: impl ToString) { }", + ); + } } From 26d52e4c7ed10bc9ec018e64a786d571bbf2ae61 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Sun, 3 Aug 2025 13:15:14 -0400 Subject: [PATCH 13/17] feat: extract_struct_from_function_signature made it work for normal parameters in methods --- .../extract_struct_from_function_signature.rs | 165 ++++++++++++------ 1 file changed, 108 insertions(+), 57 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index b696a2538ac9..3c3532463b59 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -14,9 +14,10 @@ use ide_db::{ use itertools::Itertools; use syntax::{ AstNode, Edition, SyntaxElement, SyntaxKind, SyntaxNode, T, + algo::find_node_at_range, ast::{ - self, CallExpr, HasArgList, HasAttrs, HasGenericArgs, HasGenericParams, HasName, - HasVisibility, RecordExprField, + self, HasArgList, HasAttrs, HasGenericParams, HasName, HasVisibility, MethodCallExpr, + RecordExprField, edit::{AstNodeEdit, IndentLevel}, make, }, @@ -111,10 +112,9 @@ pub(crate) fn extract_struct_from_function_signature( references, name.clone() ); - processed.into_iter().for_each(|(path, node, import)| { - apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list, + processed.into_iter().for_each(|(path, import)| { + apply_references(ctx.config.insert_use, path ,import, edition, used_params_range.clone(), &field_list, name.clone(), - // new_lifetime_count ); }); } @@ -136,10 +136,9 @@ pub(crate) fn extract_struct_from_function_signature( references, name.clone() ); - processed.into_iter().for_each(|(path, node, import)| { - apply_references(ctx.config.insert_use, path, node, import, edition, used_params_range.clone(), &field_list, + processed.into_iter().for_each(|(path, import)| { + apply_references(ctx.config.insert_use, path, import, edition, used_params_range.clone(), &field_list, name.clone(), - // new_lifetime_count ); }); } @@ -178,12 +177,13 @@ pub(crate) fn extract_struct_from_function_signature( let def = create_struct_def(name.clone(), &fn_ast_mut, &used_param_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); - let indent = fn_ast_mut.indent_level(); + // if in impl block then put struct before the impl block + let (indent, syntax) = param_list.self_param().and_then(|_|ctx.find_node_at_range::() ).map(|impl_|builder.make_mut(impl_)).map(|impl_|( impl_.indent_level(), impl_.syntax().clone())).unwrap_or((fn_ast.indent_level(), fn_ast_mut.syntax().clone())); let def = def.indent(indent); ted::insert_all( - ted::Position::before(fn_ast_mut.syntax()), + ted::Position::before(syntax), vec![ def.syntax().clone().into(), make::tokens::whitespace(&format!("\n\n{indent}")).into(), @@ -477,14 +477,15 @@ fn process_references( function_module_def: &ModuleDef, refs: Vec, name: ast::Name, -) -> Vec<(ast::PathSegment, SyntaxNode, Option<(ImportScope, hir::ModPath)>)> { +) -> Vec<(CallExpr, Option<(ImportScope, hir::ModPath)>)> { // we have to recollect here eagerly as we are about to edit the tree we need to calculate the changes // and corresponding nodes up front let name = make::name_ref(name.text_non_mutable()); refs.into_iter() .flat_map(|reference| { - let (segment, scope_node, module) = reference_to_node(&ctx.sema, reference)?; + let (call, scope_node, module) = reference_to_node(&ctx.sema, reference)?; let scope_node = builder.make_syntax_mut(scope_node); + let call = builder.make_mut(call); if !visited_modules.contains(&module) { let mod_path = module.find_use_path( ctx.sema.db, @@ -497,81 +498,57 @@ fn process_references( mod_path.push_segment(hir::Name::new_root(name.text_non_mutable()).clone()); let scope = ImportScope::find_insert_use_container(&scope_node, &ctx.sema)?; visited_modules.insert(module); - return Some((segment, scope_node, Some((scope, mod_path)))); + return Some((call, Some((scope, mod_path)))); } } - Some((segment, scope_node, None)) + Some((call, None)) }) .collect() } fn reference_to_node( sema: &hir::Semantics<'_, RootDatabase>, reference: FileReference, -) -> Option<(ast::PathSegment, SyntaxNode, hir::Module)> { - // filter out the reference in macro (seems to be probalamtic with lifetimes/generics arguments) - let segment = - reference.name.as_name_ref()?.syntax().parent().and_then(ast::PathSegment::cast)?; +) -> Option<(CallExpr, SyntaxNode, hir::Module)> { + // find neareat method call/call to the reference because different amount of parents between + // name and full call depending on if its method call or normal call + let node = + find_node_at_range::(reference.name.as_name_ref()?.syntax(), reference.range)?; // let segment_range = segment.syntax().text_range(); // if segment_range != reference.range { // return None; // } - let parent = segment.parent_path().syntax().parent()?; - let expr_or_pat = match_ast! { - match parent { - ast::PathExpr(_it) => parent.parent()?, - ast::RecordExpr(_it) => parent, - ast::TupleStructPat(_it) => parent, - ast::RecordPat(_it) => parent, - _ => return None, - } - }; - let module = sema.scope(&expr_or_pat)?.module(); + let module = sema.scope(&node.syntax())?.module(); - Some((segment.clone_for_update(), expr_or_pat, module)) + Some((node.clone(), node.syntax().clone(), module)) } fn apply_references( insert_use_cfg: InsertUseConfig, - segment: ast::PathSegment, - node: SyntaxNode, + call: CallExpr, import: Option<(ImportScope, hir::ModPath)>, edition: Edition, used_params_range: Range, field_list: &ast::RecordFieldList, name: ast::Name, - // new_lifetime_count: usize, ) -> Option<()> { if let Some((scope, path)) = import { insert_use(&scope, mod_path_to_ast(&path, edition), &insert_use_cfg); } - // TODO: figure out lifetimes in referecnecs - // becauuse we have to convert from segment being non turbofish, also only need - // generics/lifetimes that are used in struct possibly not all the no ones for the original call - // if no specified lifetimes/generics we just give empty one - // if new_lifetime_count > 0 { - // (0..new_lifetime_count).for_each(|_| { - // segment - // .get_or_create_generic_arg_list() - // .add_generic_arg(make::lifetime_arg(make::lifetime("'_")).clone_for_update().into()) - // }); - // } // current idea: the lifetimes can be inferred from the call - if let Some(generics) = segment.generic_arg_list() { - ted::remove(generics.syntax()); - } - ted::replace(segment.name_ref()?.syntax(), name.clone_for_update().syntax()); - // deep clone to prevent cycle - let path = make::path_from_segments(std::iter::once(segment.clone_subtree()), false); - // TODO: do I need to to method call to - let call = CallExpr::cast(node)?; + let path = make::path_from_text(name.text_non_mutable()); let fields = make::record_expr_field_list( call.arg_list()? .args() - .skip(used_params_range.start - 1) - .take(used_params_range.end - used_params_range.start) + .skip(match call { + // for some reason the indices for parameters of method go in increments of 3s (but + // start at 4 to accommodate the self parameter) + CallExpr::Method(_) => used_params_range.start / 3 - 1, + CallExpr::Normal(_) => used_params_range.start - 1, + }) + // the zip implicitly makes that it will only take the amount of parameters required .zip(field_list.fields()) .map(|e| { e.1.name().map(|name| -> RecordExprField { @@ -582,11 +559,57 @@ fn apply_references( ); let record_expr = make::record_expr(path, fields).clone_for_update(); - call.arg_list()? - .syntax() - .splice_children(used_params_range, vec![record_expr.syntax().syntax_element()]); + // range for method definition used parames seems to be off + call.arg_list()?.syntax().splice_children( + match call { + // but at call sites methods don't include the self argument as part of the "arg list" so + // we have to decduct one parameters (for some reason length 3) from range + CallExpr::Method(_) => (used_params_range.start - 3)..(used_params_range.end - 3), + CallExpr::Normal(_) => used_params_range, + }, + vec![record_expr.syntax().syntax_element()], + ); Some(()) } + +#[derive(Debug, Clone)] +enum CallExpr { + Normal(ast::CallExpr), + Method(ast::MethodCallExpr), +} +impl AstNode for CallExpr { + fn can_cast(kind: SyntaxKind) -> bool + where + Self: Sized, + { + kind == ast::MethodCallExpr::kind() && kind == ast::CallExpr::kind() + } + + fn cast(syntax: SyntaxNode) -> Option + where + Self: Sized, + { + ast::CallExpr::cast(syntax.clone()) + .map(CallExpr::Normal) + .or(MethodCallExpr::cast(syntax).map(CallExpr::Method)) + } + + fn syntax(&self) -> &SyntaxNode { + match self { + CallExpr::Normal(call_expr) => call_expr.syntax(), + CallExpr::Method(method_call_expr) => method_call_expr.syntax(), + } + } +} +impl HasArgList for CallExpr { + fn arg_list(&self) -> Option { + match self { + CallExpr::Normal(call_expr) => call_expr.arg_list(), + CallExpr::Method(method_call_expr) => method_call_expr.arg_list(), + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -802,4 +825,32 @@ fn foo<'a>(FooStruct { bar, .. }: FooStruct<'a, '_>, baz: i32) { r"fn foo($0i: impl ToString) { }", ); } + #[test] + fn test_extract_function_signature_in_method() { + check_assist( + extract_struct_from_function_signature, + r#" +struct Foo +impl Foo { + fn foo(&self, $0j: i32, i: i32$0, z:i32) { } +} + +fn bar() { + Foo.foo(1, 2, 3) +} +"#, + r#" +struct Foo +struct FooStruct{ j: i32, i: i32 } + +impl Foo { + fn foo(&self, FooStruct { j, i, .. }: FooStruct, z:i32) { } +} + +fn bar() { + Foo.foo(FooStruct { j: 1, i: 2 }, 3) +} +"#, + ); + } } From 9c5390ab98e4a39f691344aab00268e65a5cc4f0 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Sun, 3 Aug 2025 17:38:41 -0400 Subject: [PATCH 14/17] feat: extract_struct_from_function_signature cleanup and follow stye guide a bit --- .../extract_struct_from_function_signature.rs | 69 ++++++++++--------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 3c3532463b59..4e6dfba61f08 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -1,6 +1,6 @@ use std::ops::Range; -use hir::{Function, HasCrate, Module, ModuleDef}; +use hir::{HasCrate, Module, ModuleDef}; use ide_db::{ FxHashSet, RootDatabase, assists::AssistId, @@ -16,8 +16,7 @@ use syntax::{ AstNode, Edition, SyntaxElement, SyntaxKind, SyntaxNode, T, algo::find_node_at_range, ast::{ - self, HasArgList, HasAttrs, HasGenericParams, HasName, HasVisibility, MethodCallExpr, - RecordExprField, + self, HasArgList, HasAttrs, HasGenericParams, HasName, HasVisibility, edit::{AstNodeEdit, IndentLevel}, make, }, @@ -45,8 +44,8 @@ pub(crate) fn extract_struct_from_function_signature( ctx: &AssistContext<'_>, ) -> Option<()> { let find_node_at_offset = ctx.find_node_at_offset::()?; - let fn_ast = find_node_at_offset; - let param_list = fn_ast.param_list()?; + let func = find_node_at_offset; + let param_list = func.param_list()?; let used_param_list = param_list .params() // filter to only parameters in selection @@ -54,10 +53,10 @@ pub(crate) fn extract_struct_from_function_signature( .collect_vec(); let target = used_param_list.iter().map(|p| p.syntax().text_range()).reduce(|t, t2| t.cover(t2))?; - let fn_name = fn_ast.name()?; + let fn_name = func.name()?; let name = make::name(&format!("{}Struct", stdx::to_camel_case(fn_name.text_non_mutable()))); - let fn_hir = ctx.sema.to_def(&fn_ast)?; + let fn_hir = ctx.sema.to_def(&func)?; if existing_definition(ctx.db(), &name, &fn_hir) { cov_mark::hit!(test_extract_function_signature_not_applicable_if_struct_exists); return None; @@ -76,17 +75,17 @@ pub(crate) fn extract_struct_from_function_signature( // TODO: (future)special handling for destrutered types (or maybe just don't support code action on // destructed types yet - let field_list = extract_field_list(&fn_ast, &used_param_list)?; + let field_list = extract_field_list(&func, &used_param_list)?; - let start_index = used_param_list.first()?.syntax().index(); - let end_index = used_param_list.last()?.syntax().index(); - let used_params_range = start_index..end_index + 1; + let start_idx = used_param_list.first()?.syntax().index(); + let end_idx = used_param_list.last()?.syntax().index(); + let used_params_range = start_idx..end_idx + 1; acc.add( AssistId::refactor_rewrite("extract_struct_from_function_signature"), "Extract struct from signature of a function", target, |builder| { - let new_lifetime_count = field_list.fields().filter_map(|f|f.ty()).map(|t|new_life_time_count(&t)).sum(); + let n_new_lifetimes = field_list.fields().filter_map(|f|f.ty()).map(|t|new_life_time_count(&t)).sum(); let edition = fn_hir.krate(ctx.db()).edition(ctx.db()); let enum_module_def = ModuleDef::from(fn_hir); @@ -121,12 +120,12 @@ pub(crate) fn extract_struct_from_function_signature( tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); - let fn_ast_mut = builder.make_mut(fn_ast.clone()); + let func_mut = builder.make_mut(func.clone()); builder.make_mut(param_list.clone()); let used_param_list = used_param_list.into_iter().map(|p| builder.make_mut(p)).collect_vec(); tracing::info!("extract_struct_from_function_signature: editing main file"); // this has to be after the edit_file (order matters) - // fn_ast and param_list must be "mut" for the effect to work on used_param_list + // func and param_list must be "mut" for the effect to work on used_param_list if let Some(references) = def_file_references { let processed = process_references( ctx, @@ -144,7 +143,7 @@ pub(crate) fn extract_struct_from_function_signature( } - let generic_params = fn_ast + let generic_params = func .generic_param_list() .and_then(|known_generics| extract_generic_params(&known_generics, &field_list)); tracing::info!("extract_struct_from_function_signature: collecting generics"); @@ -157,7 +156,7 @@ pub(crate) fn extract_struct_from_function_signature( // So I do the resolving while its still param list // and then apply it into record list after let field_list = if let Some((target_scope, source_scope)) = - ctx.sema.scope(fn_ast.syntax()).zip(ctx.sema.scope(param_list.syntax())) + ctx.sema.scope(func.syntax()).zip(ctx.sema.scope(param_list.syntax())) { let field_list = field_list.reset_indent(); let field_list = @@ -174,11 +173,11 @@ pub(crate) fn extract_struct_from_function_signature( }; field_list.fields().filter_map(|f|f.ty()).try_for_each(|t|generate_new_lifetimes(&t, &mut generics)); tracing::info!("extract_struct_from_function_signature: collecting fields"); - let def = create_struct_def(name.clone(), &fn_ast_mut, &used_param_list, &field_list, generics); + let def = create_struct_def(name.clone(), &func_mut, &used_param_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); // if in impl block then put struct before the impl block - let (indent, syntax) = param_list.self_param().and_then(|_|ctx.find_node_at_range::() ).map(|impl_|builder.make_mut(impl_)).map(|impl_|( impl_.indent_level(), impl_.syntax().clone())).unwrap_or((fn_ast.indent_level(), fn_ast_mut.syntax().clone())); + let (indent, syntax) = param_list.self_param().and_then(|_|ctx.find_node_at_range::() ).map(|imp|builder.make_mut(imp)).map(|imp|( imp.indent_level(), imp.syntax().clone())).unwrap_or((func.indent_level(), func_mut.syntax().clone())); let def = def.indent(indent); @@ -190,14 +189,14 @@ pub(crate) fn extract_struct_from_function_signature( ], ); tracing::info!("extract_struct_from_function_signature: inserting struct {def}"); - update_function(name, generic_params.map(|g| g.clone_for_update()), &used_param_list, new_lifetime_count).unwrap(); + update_function(name, generic_params.map(|g| g.clone_for_update()), &used_param_list, n_new_lifetimes).unwrap(); tracing::info!("extract_struct_from_function_signature: updating function signature and parameter uses"); }, ) } fn extract_field_list( - fn_ast: &ast::Fn, + func: &ast::Fn, used_param_list: &[ast::Param], ) -> Option { let field_list = make::record_field_list( @@ -205,7 +204,7 @@ fn extract_field_list( .iter() .map(|param| { Some(make::record_field( - fn_ast.visibility(), + func.visibility(), // only works if its an ident pattern param.pat().and_then(pat_to_name)?, param.ty().filter(|ty| !contains_impl_trait(ty))?, @@ -220,14 +219,14 @@ fn update_function( name: ast::Name, generics: Option, used_param_list: &[ast::Param], - new_lifetime_count: usize, + n_new_lifetimes: usize, ) -> Option<()> { let generic_args = generics .filter(|generics| generics.generic_params().count() > 0) - .or((new_lifetime_count > 0).then_some(make::generic_param_list(std::iter::empty()))) + .or((n_new_lifetimes > 0).then_some(make::generic_param_list(std::iter::empty()))) .map(|generics| { let args = generics.to_generic_args().clone_for_update(); - (0..new_lifetime_count).for_each(|_| { + (0..n_new_lifetimes).for_each(|_| { args.add_generic_arg( make::lifetime_arg(make::lifetime("'_")).clone_for_update().into(), ) @@ -258,9 +257,9 @@ fn update_function( // it is fine to unwrap() to because there is at least one parameter (if there is no parameters // the code action will not show) - let start_index = used_param_list.first().unwrap().syntax().index(); - let end_index = used_param_list.last().unwrap().syntax().index(); - let used_params_range = start_index..end_index + 1; + let start_idx = used_param_list.first().unwrap().syntax().index(); + let end_idx = used_param_list.last().unwrap().syntax().index(); + let used_params_range = start_idx..end_idx + 1; let new = vec![param.syntax().syntax_element()]; used_param_list.first().unwrap().syntax().parent()?.splice_children(used_params_range, new); // no need update uses of parameters in function, because we destructure the struct @@ -274,12 +273,12 @@ fn pat_to_name(pat: ast::Pat) -> Option { } fn create_struct_def( name: ast::Name, - fn_ast: &ast::Fn, + func: &ast::Fn, param_ast: &[ast::Param], field_list: &ast::RecordFieldList, generics: Option, ) -> ast::Struct { - let fn_vis = fn_ast.visibility(); + let fn_vis = func.visibility(); let insert_vis = |node: &'_ SyntaxNode, vis: &'_ SyntaxNode| { let vis = vis.clone_for_update(); @@ -449,7 +448,11 @@ fn tag_generics_in_function_signature( tagged_one } -fn existing_definition(db: &RootDatabase, variant_name: &ast::Name, variant: &Function) -> bool { +fn existing_definition( + db: &RootDatabase, + variant_name: &ast::Name, + variant: &hir::Function, +) -> bool { variant .module(db) .scope(db, None) @@ -519,7 +522,7 @@ fn reference_to_node( // return None; // } - let module = sema.scope(&node.syntax())?.module(); + let module = sema.scope(node.syntax())?.module(); Some((node.clone(), node.syntax().clone(), module)) } @@ -551,7 +554,7 @@ fn apply_references( // the zip implicitly makes that it will only take the amount of parameters required .zip(field_list.fields()) .map(|e| { - e.1.name().map(|name| -> RecordExprField { + e.1.name().map(|name| -> ast::RecordExprField { make::record_expr_field(make::name_ref(name.text_non_mutable()), Some(e.0)) }) }) @@ -591,7 +594,7 @@ impl AstNode for CallExpr { { ast::CallExpr::cast(syntax.clone()) .map(CallExpr::Normal) - .or(MethodCallExpr::cast(syntax).map(CallExpr::Method)) + .or(ast::MethodCallExpr::cast(syntax).map(CallExpr::Method)) } fn syntax(&self) -> &SyntaxNode { From a6b57c2e003e5161faa9164960125bdb34079c89 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Sun, 3 Aug 2025 20:01:17 -0400 Subject: [PATCH 15/17] feat: extract_struct_from_function_signature fixed bug where if extracting from function/method in impl and was referenced somewhere else in impl it would break down b/c making the impl mut would happen after the reference was updated --- .../extract_struct_from_function_signature.rs | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 4e6dfba61f08..6215b8f77440 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -120,7 +120,11 @@ pub(crate) fn extract_struct_from_function_signature( tracing::info!("extract_struct_from_function_signature: starting edit"); builder.edit_file(ctx.vfs_file_id()); + // atl the make muts should generally before any edits happen let func_mut = builder.make_mut(func.clone()); + // if in impl block then put struct before the impl block + let (indent, syntax) = param_list.self_param().and_then(|_|ctx.find_node_at_range::() ) + .map(|imp|( imp.indent_level(), builder.make_syntax_mut(imp.syntax().clone()))).unwrap_or((func.indent_level(), func_mut.syntax().clone())); builder.make_mut(param_list.clone()); let used_param_list = used_param_list.into_iter().map(|p| builder.make_mut(p)).collect_vec(); tracing::info!("extract_struct_from_function_signature: editing main file"); @@ -175,12 +179,7 @@ pub(crate) fn extract_struct_from_function_signature( tracing::info!("extract_struct_from_function_signature: collecting fields"); let def = create_struct_def(name.clone(), &func_mut, &used_param_list, &field_list, generics); tracing::info!("extract_struct_from_function_signature: creating struct"); - - // if in impl block then put struct before the impl block - let (indent, syntax) = param_list.self_param().and_then(|_|ctx.find_node_at_range::() ).map(|imp|builder.make_mut(imp)).map(|imp|( imp.indent_level(), imp.syntax().clone())).unwrap_or((func.indent_level(), func_mut.syntax().clone())); let def = def.indent(indent); - - ted::insert_all( ted::Position::before(syntax), vec![ @@ -850,6 +849,40 @@ impl Foo { fn foo(&self, FooStruct { j, i, .. }: FooStruct, z:i32) { } } +fn bar() { + Foo.foo(FooStruct { j: 1, i: 2 }, 3) +} +"#, + ); + } + #[test] + fn test_extract_function_signature_in_method_with_reference_in_impl() { + check_assist( + extract_struct_from_function_signature, + r#" +struct Foo +impl Foo { + fn foo(&self, $0j: i32, i: i32$0, z:i32) { } + fn baz(&self) { + self.foo(4, 5, 6) + } +} + +fn bar() { + Foo.foo(1, 2, 3) +} +"#, + r#" +struct Foo +struct FooStruct{ j: i32, i: i32 } + +impl Foo { + fn foo(&self, FooStruct { j, i, .. }: FooStruct, z:i32) { } + fn baz(&self) { + self.foo(FooStruct { j: 4, i: 5 }, 6) + } +} + fn bar() { Foo.foo(FooStruct { j: 1, i: 2 }, 3) } From 26ba1b960ebb2bfbb801cb698d7ffe29f88f1d29 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Mon, 4 Aug 2025 16:41:46 -0400 Subject: [PATCH 16/17] feat: extract_struct_from_function_signature mostly copies attributes and comments properly (edge case where comment in between to parameters is lost) reference types with no lifetimes now get an autogenerated lifetime (still need to figure out how to do it for types with only lifetimes that do not have explicit lifetimes) --- .../extract_struct_from_function_signature.rs | 104 +++++++++++++----- 1 file changed, 77 insertions(+), 27 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index 6215b8f77440..c69aa19aa37b 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -72,7 +72,7 @@ pub(crate) fn extract_struct_from_function_signature( // clauses // we also need special handling for method calls - // TODO: (future)special handling for destrutered types (or maybe just don't support code action on + // TODO: (future)special handling for destrutered types (right now we don't support code action on // destructed types yet let field_list = extract_field_list(&func, &used_param_list)?; @@ -285,7 +285,7 @@ fn create_struct_def( }; // for fields without any existing visibility, use visibility of enum - let field_list: ast::FieldList = { + let field_list = { if let Some(vis) = &fn_vis { field_list .fields() @@ -294,39 +294,34 @@ fn create_struct_def( .for_each(|it| insert_vis(it.syntax(), vis.syntax())); } - field_list.clone().into() + field_list }; + // if we do not expleictly copy over comments/attribures they just get lost + // TODO: what about comments/attributes in between parameters + param_ast.iter().zip(field_list.fields()).for_each(|(param, field)| { + let elements = take_all_comments(param.clone()); + ted::insert_all(ted::Position::first_child_of(field.syntax()), elements); + ted::insert_all( + ted::Position::first_child_of(field.syntax()), + param + .attrs() + .flat_map(|it| [it.syntax().clone().into(), make::tokens::single_newline().into()]) + .collect(), + ); + }); let field_list = field_list.indent(IndentLevel::single()); - let strukt = make::struct_(fn_vis, name, generics, field_list).clone_for_update(); - // take comments from only inside signature - ted::insert_all( - ted::Position::first_child_of(strukt.syntax()), - take_all_comments(param_ast.iter()), - ); - // TODO: this may not be correct as we shouldn't put all the attributes at the top - // copy attributes from each parameter - ted::insert_all( - ted::Position::first_child_of(strukt.syntax()), - param_ast - .iter() - .flat_map(|p| p.attrs()) - .flat_map(|it| { - vec![it.syntax().clone_for_update().into(), make::tokens::single_newline().into()] - }) - .collect(), - ); - - strukt + make::struct_(fn_vis, name, generics, field_list.into()).clone_for_update() } // Note: this also detaches whitespace after comments, // since `SyntaxNode::splice_children` (and by extension `ted::insert_all_raw`) // detaches nodes. If we only took the comments, we'd leave behind the old whitespace. -fn take_all_comments<'a>(node: impl Iterator) -> Vec { +fn take_all_comments(node: impl ast::AstNode) -> Vec { let mut remove_next_ws = false; - node.flat_map(|p| p.syntax().children_with_tokens()) + node.syntax() + .children_with_tokens() .filter_map(move |child| match child.kind() { SyntaxKind::COMMENT => { remove_next_ws = true; @@ -375,8 +370,13 @@ fn generate_unique_lifetime_param_name( fn new_life_time_count(ty: &ast::Type) -> usize { ty.syntax() .descendants() - .filter_map(ast::Lifetime::cast) - .filter(|lifetime| lifetime.text() == "'_") + .filter(|t| { + match_ast! { match t { + ast::Lifetime(lt) => lt.text() == "'_", + ast::RefType(r) => r.lifetime().is_none(), + _ => false + }} + }) .count() } fn contains_impl_trait(ty: &ast::Type) -> bool { @@ -387,6 +387,8 @@ fn generate_new_lifetimes( existing_type_param_list: &mut Option, ) -> Option<()> { for token in ty.syntax().descendants() { + // we do not have to worry about for<'a> because we are only looking at '_ or &Type + // if you have an unbound lifetime thats on you if let Some(lt) = ast::Lifetime::cast(token.clone()) && lt.text() == "'_" { @@ -396,7 +398,18 @@ fn generate_new_lifetimes( .add_generic_param(make::lifetime_param(new_lt.clone()).clone_for_update().into()); ted::replace(lt.syntax(), new_lt.clone_for_update().syntax()); + } else if let Some(r) = ast::RefType::cast(token.clone()) + && r.lifetime().is_none() + { + let new_lt = generate_unique_lifetime_param_name(existing_type_param_list)?; + existing_type_param_list + .get_or_insert(make::generic_param_list(std::iter::empty()).clone_for_update()) + .add_generic_param(make::lifetime_param(new_lt.clone()).clone_for_update().into()); + ted::insert(ted::Position::after(r.amp_token()?), new_lt.clone_for_update().syntax()); } + // TODO: nominal types that have only lifetimes + // struct Bar<'a, 'b> { f: &'a &'b i32 } + // fn foo(f: Bar) {} } Some(()) } @@ -782,6 +795,21 @@ fn foo($0bar: &'_ i32$0, baz: i32) {} r#" struct FooStruct<'a>{ bar: &'a i32 } +fn foo(FooStruct { bar, .. }: FooStruct<'_>, baz: i32) {} +"#, + ); + } + + #[test] + fn test_extract_function_signature_single_parameter_with_plain_reference_type() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo($0bar: &i32$0, baz: i32) {} +"#, + r#" +struct FooStruct<'a>{ bar: &'a i32 } + fn foo(FooStruct { bar, .. }: FooStruct<'_>, baz: i32) {} "#, ); @@ -889,4 +917,26 @@ fn bar() { "#, ); } + #[test] + fn test_extract_function_signature_in_method_comments_and_attributes() { + check_assist( + extract_struct_from_function_signature, + r#" +fn foo( + #[foo] + // gag + $0f: i32, +) { } +"#, + r#" +struct FooStruct{ #[foo] +// gag +f: i32 } + +fn foo( + FooStruct { f, .. }: FooStruct, +) { } +"#, + ) + } } From a1bb8394eba0e7906b4f3dfb3eb6c8ab01142830 Mon Sep 17 00:00:00 2001 From: mendelsshop Date: Mon, 4 Aug 2025 16:47:40 -0400 Subject: [PATCH 17/17] feat: extract_struct_from_function_signature fixed formatting --- .../src/handlers/extract_struct_from_function_signature.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs index c69aa19aa37b..c0c28dfa5915 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_function_signature.rs @@ -311,8 +311,6 @@ fn create_struct_def( }); let field_list = field_list.indent(IndentLevel::single()); - - make::struct_(fn_vis, name, generics, field_list.into()).clone_for_update() } // Note: this also detaches whitespace after comments,