Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #21979 from ChayimFriedman2/rename-ctor-field
feat: When renaming a field, rename variables in constructors as well
| -rw-r--r-- | crates/hir/src/lib.rs | 8 | ||||
| -rw-r--r-- | crates/hir/src/semantics.rs | 38 | ||||
| -rw-r--r-- | crates/ide-db/src/rename.rs | 107 | ||||
| -rw-r--r-- | crates/ide-db/src/text_edit.rs | 4 | ||||
| -rw-r--r-- | crates/ide/src/rename.rs | 67 |
5 files changed, 219 insertions, 5 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 91b48bbfb6..ecd11fb5d7 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1984,6 +1984,14 @@ impl Variant { Variant::EnumVariant(e) => (*e).name(db), } } + + pub fn adt(&self, db: &dyn HirDatabase) -> Adt { + match *self { + Variant::Struct(it) => it.into(), + Variant::Union(it) => it.into(), + Variant::EnumVariant(it) => it.parent_enum(db).into(), + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index b7cc780ae4..38656e1870 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -1635,6 +1635,44 @@ impl<'db> SemanticsImpl<'db> { .kmerge_by(|node1, node2| node1.text_range().len() < node2.text_range().len()) } + /// Returns the `return` expressions in this function's body, + /// excluding those inside closures or async blocks. + pub fn fn_return_points(&self, func: Function) -> Vec<InFile<ast::ReturnExpr>> { + let func_id = match func.id { + AnyFunctionId::FunctionId(id) => id, + _ => return vec![], + }; + let (body, source_map) = Body::with_source_map(self.db, func_id.into()); + + fn collect_returns( + sema: &SemanticsImpl<'_>, + body: &Body, + source_map: &hir_def::expr_store::ExpressionStoreSourceMap, + expr_id: ExprId, + acc: &mut Vec<InFile<ast::ReturnExpr>>, + ) { + match &body[expr_id] { + Expr::Closure { .. } | Expr::Const(_) => return, + Expr::Return { .. } => { + if let Ok(source) = source_map.expr_syntax(expr_id) + && let Some(ret_expr) = source.value.cast::<ast::ReturnExpr>() + { + let root = sema.parse_or_expand(source.file_id); + acc.push(InFile::new(source.file_id, ret_expr.to_node(&root))); + } + } + _ => {} + } + body.walk_child_exprs(expr_id, |child| { + collect_returns(sema, body, source_map, child, acc); + }); + } + + let mut returns = vec![]; + collect_returns(self, body, source_map, body.root_expr(), &mut returns); + returns + } + pub fn resolve_lifetime_param(&self, lifetime: &ast::Lifetime) -> Option<LifetimeParam> { let text = lifetime.text(); let lifetime_param = lifetime.syntax().ancestors().find_map(|syn| { diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index b18ed69d80..ac0b099713 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -28,7 +28,9 @@ use crate::{ }; use base_db::AnchoredPathBuf; use either::Either; -use hir::{FieldSource, FileRange, InFile, ModuleSource, Name, Semantics, sym}; +use hir::{FieldSource, FileRange, HasCrate, InFile, ModuleSource, Name, Semantics, sym}; +use itertools::Itertools; +use rustc_hash::FxHashSet; use span::{Edition, FileId, SyntaxContext}; use stdx::{TupleExt, never}; use syntax::{ @@ -405,6 +407,11 @@ fn rename_reference( source_edit_from_references(sema.db, references, def, &new_name, edition), ) })); + + if let Definition::Field(field) = def { + rename_field_constructors(sema, field, &new_name, &mut source_change, config); + } + if rename_definition == RenameDefinition::Yes { // This needs to come after the references edits, because we change the annotation of existing edits // if a conflict is detected. @@ -415,6 +422,104 @@ fn rename_reference( Ok(source_change) } +fn rename_field_constructors( + sema: &Semantics<'_, RootDatabase>, + field: hir::Field, + new_name: &Name, + source_change: &mut SourceChange, + config: &RenameConfig, +) { + let db = sema.db; + let old_name = field.name(db); + let adt = field.parent_def(db).adt(db); + adt.ty(db).iterate_assoc_items(db, |assoc_item| { + let ctor = assoc_item.as_function()?; + if ctor.has_self_param(db) { + return None; + } + if ctor.ret_type(db).as_adt() != Some(adt) { + return None; + } + + let source = sema.source(ctor); + let return_values = sema + .fn_return_points(ctor) + .into_iter() + .filter_map(|ret| ret.value.expr()) + .chain(source.and_then(|source| source.value.body()?.tail_expr())); + // FIXME: We could maybe skip ifs etc.. + + let get_renamed_field = |mut expr| { + while let ast::Expr::ParenExpr(e) = &expr { + expr = e.expr()?; + } + let ast::Expr::RecordExpr(expr) = expr else { return None }; + if sema.type_of_expr(&expr.clone().into())?.original.as_adt()? != adt { + return None; + }; + expr.record_expr_field_list()?.fields().find_map(|record_field| { + if record_field.name_ref().is_none() + && Name::new_root(&record_field.field_name()?.text()) == old_name + && let ast::Expr::PathExpr(field_name) = record_field.expr()? + { + field_name.path() + } else { + None + } + }) + }; + let renamed_fields = return_values + .map(get_renamed_field) + .map(|renamed_field| { + let renamed_field = renamed_field?; + let hir::PathResolution::Local(local) = sema.resolve_path(&renamed_field)? else { + return None; + }; + let range = sema.original_range_opt(renamed_field.syntax())?.range; + Some((range, local)) + }) + .collect::<Option<Vec<_>>>()?; + + let edition = ctor.krate(db).edition(db); + let locals = renamed_fields.iter().map(|&(_, local)| local).collect::<FxHashSet<_>>(); + let mut all_locals_source_change = SourceChange::default(); + for local in locals { + let mut local_source_change = Definition::Local(local) + .rename(sema, new_name.as_str(), RenameDefinition::Yes, config) + .ok()?; + + let (edit, _snippet) = + local_source_change.source_file_edits.values_mut().exactly_one().ok()?; + + // The struct literal will have an edit `old_name -> old_name: new_name`, and we need to remove + // that, as we want an overlapping edit `old_name -> new_name`. + for &(field_range, _) in &renamed_fields { + edit.cancel_edits_touching(field_range); + } + + all_locals_source_change = + std::mem::take(&mut all_locals_source_change).merge(local_source_change); + } + let (edit, _snippet) = + all_locals_source_change.source_file_edits.values_mut().exactly_one().ok()?; + for &(field_range, _) in &renamed_fields { + edit.union(TextEdit::replace(field_range, new_name.display(db, edition).to_string())) + .unwrap(); + } + + let file_id = *all_locals_source_change.source_file_edits.keys().exactly_one().ok()?; + if let Some((edit, _snippet)) = source_change.source_file_edits.get_mut(&file_id) { + for &(field_range, _) in &renamed_fields { + edit.cancel_edits_touching(field_range); + } + } + + *source_change = std::mem::take(source_change).merge(all_locals_source_change); + + None::<std::convert::Infallible> + }); +} + pub fn source_edit_from_references( db: &RootDatabase, references: &[FileReference], diff --git a/crates/ide-db/src/text_edit.rs b/crates/ide-db/src/text_edit.rs index d2a73710d5..f93b2cc74e 100644 --- a/crates/ide-db/src/text_edit.rs +++ b/crates/ide-db/src/text_edit.rs @@ -151,6 +151,10 @@ impl TextEdit { pub fn change_annotation(&self) -> Option<ChangeAnnotationId> { self.annotation } + + pub fn cancel_edits_touching(&mut self, touching: TextRange) { + self.indels.retain(|indel| indel.delete.intersect(touching).is_none()); + } } impl IntoIterator for TextEdit { diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 900a885a64..2c6116f745 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -1326,8 +1326,8 @@ impl Foo { struct Foo { foo$0: i32 } impl Foo { - fn new(foo: i32) -> Self { - Self { foo } + fn foo(foo: i32) { + Self { foo }; } } "#, @@ -1335,8 +1335,8 @@ impl Foo { struct Foo { field: i32 } impl Foo { - fn new(foo: i32) -> Self { - Self { field: foo } + fn foo(foo: i32) { + Self { field: foo }; } } "#, @@ -3930,4 +3930,63 @@ fn bar() { "#, ); } + + #[test] + fn rename_constructor_locals() { + check( + "field", + r#" +struct Struct { + struct_field$0: String, +} + +impl Struct { + fn new(struct_field: String) -> Self { + if false { + return Self { struct_field }; + } + Self { struct_field } + } +} + +mod foo { + macro_rules! m { + ($it:expr) => { return $it }; + } + + impl crate::Struct { + fn with_foo(struct_field: String) -> crate::Struct { + m!(crate::Struct { struct_field }); + } + } +} + "#, + r#" +struct Struct { + field: String, +} + +impl Struct { + fn new(field: String) -> Self { + if false { + return Self { field }; + } + Self { field } + } +} + +mod foo { + macro_rules! m { + ($it:expr) => { return $it }; + } + + impl crate::Struct { + fn with_foo(field: String) -> crate::Struct { + m!(crate::Struct { field }); + } + } +} + "#, + ); + } } |