Merge #6552

6552: Properly handle shorthands in destructure patterns when renaming r=SomeoneToIgnore a=Veykril

Fixes #6548 and #6551.

Co-authored-by: Lukas Wirth <[email protected]>

Author: bors[bot]

crates/ide/src/references.rs

@@ -110,14 +110,23 @@ pub(crate) fn find_all_refs(
         .filter(|r| search_kind == ReferenceKind::Other || search_kind == r.kind)
         .collect();
 
-    let decl_range = def.try_to_nav(sema.db)?.focus_or_full_range();
-
-    let declaration = Declaration {
-        nav: def.try_to_nav(sema.db)?,
-        kind: ReferenceKind::Other,
-        access: decl_access(&def, &syntax, decl_range),
+    let nav = def.try_to_nav(sema.db)?;
+    let decl_range = nav.focus_or_full_range();
+
+    let mut kind = ReferenceKind::Other;
+    if let Definition::Local(local) = def {
+        if let either::Either::Left(pat) = local.source(sema.db).value {
+            if matches!(
+                pat.syntax().parent().and_then(ast::RecordPatField::cast),
+                Some(pat_field) if pat_field.name_ref().is_none()
+            ) {
+                kind = ReferenceKind::FieldShorthandForLocal;
+            }
+        }
     };
 
+    let declaration = Declaration { nav, kind, access: decl_access(&def, &syntax, decl_range) };
+
     Some(RangeInfo::new(range, ReferenceSearchResult { declaration, references }))
 }
 
@@ -613,7 +622,7 @@ fn foo() {
             expect![[r#"
                 f RECORD_FIELD FileId(0) 15..21 15..16 Other
 
-                FileId(0) 55..56 Other Read
+                FileId(0) 55..56 RecordFieldExprOrPat Read
                 FileId(0) 68..69 Other Write
             "#]],
         );
@@ -748,7 +757,7 @@ fn f() -> m::En {
             expect![[r#"
                 field RECORD_FIELD FileId(0) 56..65 56..61 Other
 
-                FileId(0) 125..130 Other Read
+                FileId(0) 125..130 RecordFieldExprOrPat Read
             "#]],
         );
     }

crates/ide/src/references/rename.rs

@@ -1,7 +1,7 @@
 //! FIXME: write short doc here
 
 use hir::{Module, ModuleDef, ModuleSource, Semantics};
-use ide_db::base_db::SourceDatabaseExt;
+use ide_db::base_db::{FileRange, SourceDatabaseExt};
 use ide_db::{
     defs::{Definition, NameClass, NameRefClass},
     RootDatabase,
@@ -106,9 +106,12 @@ fn find_module_at_offset(
     Some(module)
 }
 
-fn source_edit_from_reference(reference: Reference, new_name: &str) -> SourceFileEdit {
+fn source_edit_from_reference(
+    sema: &Semantics<RootDatabase>,
+    reference: Reference,
+    new_name: &str,
+) -> SourceFileEdit {
     let mut replacement_text = String::new();
-    let file_id = reference.file_range.file_id;
     let range = match reference.kind {
         ReferenceKind::FieldShorthandForField => {
             mark::hit!(test_rename_struct_field_for_shorthand);
@@ -122,12 +125,48 @@ fn source_edit_from_reference(reference: Reference, new_name: &str) -> SourceFil
             replacement_text.push_str(new_name);
             TextRange::new(reference.file_range.range.end(), reference.file_range.range.end())
         }
+        ReferenceKind::RecordFieldExprOrPat => {
+            mark::hit!(test_rename_field_expr_pat);
+            replacement_text.push_str(new_name);
+            edit_text_range_for_record_field_expr_or_pat(sema, reference.file_range, new_name)
+        }
         _ => {
             replacement_text.push_str(new_name);
             reference.file_range.range
         }
     };
-    SourceFileEdit { file_id, edit: TextEdit::replace(range, replacement_text) }
+    SourceFileEdit {
+        file_id: reference.file_range.file_id,
+        edit: TextEdit::replace(range, replacement_text),
+    }
+}
+
+fn edit_text_range_for_record_field_expr_or_pat(
+    sema: &Semantics<RootDatabase>,
+    file_range: FileRange,
+    new_name: &str,
+) -> TextRange {
+    let source_file = sema.parse(file_range.file_id);
+    let file_syntax = source_file.syntax();
+    let original_range = file_range.range;
+
+    syntax::algo::find_node_at_range::<ast::RecordExprField>(file_syntax, original_range)
+        .and_then(|field_expr| match field_expr.expr().and_then(|e| e.name_ref()) {
+            Some(name) if &name.to_string() == new_name => Some(field_expr.syntax().text_range()),
+            _ => None,
+        })
+        .or_else(|| {
+            syntax::algo::find_node_at_range::<ast::RecordPatField>(file_syntax, original_range)
+                .and_then(|field_pat| match field_pat.pat() {
+                    Some(ast::Pat::IdentPat(pat))
+                        if pat.name().map(|n| n.to_string()).as_deref() == Some(new_name) =>
+                    {
+                        Some(field_pat.syntax().text_range())
+                    }
+                    _ => None,
+                })
+        })
+        .unwrap_or(original_range)
 }
 
 fn rename_mod(
@@ -170,7 +209,7 @@ fn rename_mod(
     let ref_edits = refs
         .references
         .into_iter()
-        .map(|reference| source_edit_from_reference(reference, new_name));
+        .map(|reference| source_edit_from_reference(sema, reference, new_name));
     source_file_edits.extend(ref_edits);
 
     Ok(RangeInfo::new(range, SourceChange::from_edits(source_file_edits, file_system_edits)))
@@ -211,7 +250,7 @@ fn rename_to_self(
 
     let mut edits = usages
         .into_iter()
-        .map(|reference| source_edit_from_reference(reference, "self"))
+        .map(|reference| source_edit_from_reference(sema, reference, "self"))
         .collect::<Vec<_>>();
 
     edits.push(SourceFileEdit {
@@ -300,7 +339,7 @@ fn rename_reference(
 
     let edit = refs
         .into_iter()
-        .map(|reference| source_edit_from_reference(reference, new_name))
+        .map(|reference| source_edit_from_reference(sema, reference, new_name))
         .collect::<Vec<_>>();
 
     if edit.is_empty() {
@@ -1097,4 +1136,116 @@ impl Foo {
 "#,
         );
     }
+
+    #[test]
+    fn test_initializer_use_field_init_shorthand() {
+        mark::check!(test_rename_field_expr_pat);
+        check(
+            "bar",
+            r#"
+struct Foo { i<|>: i32 }
+
+fn foo(bar: i32) -> Foo {
+    Foo { i: bar }
+}
+"#,
+            r#"
+struct Foo { bar: i32 }
+
+fn foo(bar: i32) -> Foo {
+    Foo { bar }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn test_struct_field_destructure_into_shorthand() {
+        check(
+            "baz",
+            r#"
+struct Foo { i<|>: i32 }
+
+fn foo(foo: Foo) {
+    let Foo { i: baz } = foo;
+    let _ = baz;
+}
+"#,
+            r#"
+struct Foo { baz: i32 }
+
+fn foo(foo: Foo) {
+    let Foo { baz } = foo;
+    let _ = baz;
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn test_rename_binding_in_destructure_pat() {
+        let expected_fixture = r#"
+struct Foo {
+    i: i32,
+}
+
+fn foo(foo: Foo) {
+    let Foo { i: bar } = foo;
+    let _ = bar;
+}
+"#;
+        check(
+            "bar",
+            r#"
+struct Foo {
+    i: i32,
+}
+
+fn foo(foo: Foo) {
+    let Foo { i: b } = foo;
+    let _ = b<|>;
+}
+"#,
+            expected_fixture,
+        );
+        check(
+            "bar",
+            r#"
+struct Foo {
+    i: i32,
+}
+
+fn foo(foo: Foo) {
+    let Foo { i } = foo;
+    let _ = i<|>;
+}
+"#,
+            expected_fixture,
+        );
+    }
+
+    #[test]
+    fn test_rename_binding_in_destructure_param_pat() {
+        check(
+            "bar",
+            r#"
+struct Foo {
+    i: i32
+}
+
+fn foo(Foo { i }: foo) -> i32 {
+    i<|>
+}
+"#,
+            r#"
+struct Foo {
+    i: i32
+}
+
+fn foo(Foo { i: bar }: foo) -> i32 {
+    bar
+}
+"#,
+        )
+    }
 }

crates/ide_db/src/search.rs

@@ -30,6 +30,7 @@ pub enum ReferenceKind {
     FieldShorthandForField,
     FieldShorthandForLocal,
     StructLiteral,
+    RecordFieldExprOrPat,
     Other,
 }
 
@@ -278,8 +279,9 @@ impl<'a> FindUsages<'a> {
     ) -> bool {
         match NameRefClass::classify(self.sema, &name_ref) {
             Some(NameRefClass::Definition(def)) if &def == self.def => {
-                let kind = if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref)
-                {
+                let kind = if is_record_field_expr_or_pat(&name_ref) {
+                    ReferenceKind::RecordFieldExprOrPat
+                } else if is_record_lit_name_ref(&name_ref) || is_call_expr_name_ref(&name_ref) {
                     ReferenceKind::StructLiteral
                 } else {
                     ReferenceKind::Other
@@ -385,3 +387,17 @@ fn is_record_lit_name_ref(name_ref: &ast::NameRef) -> bool {
         .map(|p| p.name_ref().as_ref() == Some(name_ref))
         .unwrap_or(false)
 }
+
+fn is_record_field_expr_or_pat(name_ref: &ast::NameRef) -> bool {
+    if let Some(parent) = name_ref.syntax().parent() {
+        match_ast! {
+            match parent {
+                ast::RecordExprField(it) => true,
+                ast::RecordPatField(_it) => true,
+                _ => false,
+            }
+        }
+    } else {
+        false
+    }
+}

crates/syntax/src/ast/expr_ext.rs

@@ -22,6 +22,18 @@ impl ast::Expr {
             _ => false,
         }
     }
+
+    pub fn name_ref(&self) -> Option<ast::NameRef> {
+        if let ast::Expr::PathExpr(expr) = self {
+            let path = expr.path()?;
+            let segment = path.segment()?;
+            let name_ref = segment.name_ref()?;
+            if path.qualifier().is_none() {
+                return Some(name_ref);
+            }
+        }
+        None
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/syntax/src/ast/node_ext.rs

@@ -203,15 +203,7 @@ impl ast::RecordExprField {
         if let Some(name_ref) = self.name_ref() {
             return Some(name_ref);
         }
-        if let Some(ast::Expr::PathExpr(expr)) = self.expr() {
-            let path = expr.path()?;
-            let segment = path.segment()?;
-            let name_ref = segment.name_ref()?;
-            if path.qualifier().is_none() {
-                return Some(name_ref);
-            }
-        }
-        None
+        self.expr()?.name_ref()
     }
 }