Merge pull request #21979 from ChayimFriedman2/rename-ctor-field

feat: When renaming a field, rename variables in constructors as well

Author: ChayimFriedman2

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)]

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| {

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],

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 {

crates/ide/src/rename.rs

@@ -1326,17 +1326,17 @@ impl Foo {
 struct Foo { foo$0: i32 }
 
 impl Foo {
-    fn new(foo: i32) -> Self {
-        Self { foo }
+    fn foo(foo: i32) {
+        Self { foo };
     }
 }
 "#,
             r#"
 struct Foo { field: i32 }
 
 impl Foo {
-    fn new(foo: i32) -> Self {
-        Self { field: foo }
+    fn foo(foo: i32) {
+        Self { field: foo };
     }
 }
 "#,
@@ -3926,6 +3926,65 @@ impl Foo {
 
 fn bar() {
     Foo::foo(&Foo, 1);
+}
+        "#,
+        );
+    }
+
+    #[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 });
+        }
+    }
 }
         "#,
         );