Don't rename field record patterns directly

2 files changed, 73 insertions, 39 deletions

diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs
index bbd9426b6..08f16b54d 100644
--- a/crates/ide/src/references/rename.rs
+++ b/crates/ide/src/references/rename.rs
@@ -183,25 +183,41 @@ fn source_edit_from_references(
 ) -> (FileId, TextEdit) {
     let mut edit = TextEdit::builder();
     for reference in references {
-        match reference.name.as_name_ref() {
+        let (range, replacement) = match &reference.name {
             // if the ranges differ then the node is inside a macro call, we can't really attempt
             // to make special rewrites like shorthand syntax and such, so just rename the node in
             // the macro input
-            Some(name_ref) if name_ref.syntax().text_range() == reference.range => {
-                let (range, replacement) = source_edit_from_name_ref(name_ref, new_name, def);
-                edit.replace(range, replacement);
+            NameLike::NameRef(name_ref) if name_ref.syntax().text_range() == reference.range => {
+                source_edit_from_name_ref(name_ref, new_name, def)
             }
-            _ => edit.replace(reference.range, new_name.to_owned()),
-        };
+            NameLike::Name(name) if name.syntax().text_range() == reference.range => {
+                source_edit_from_name(name, new_name)
+            }
+            _ => None,
+        }
+        .unwrap_or_else(|| (reference.range, new_name.to_string()));
+        edit.replace(range, replacement);
     }
     (file_id, edit.finish())
 }
 
+fn source_edit_from_name(name: &ast::Name, new_name: &str) -> Option<(TextRange, String)> {
+    if let Some(_) = ast::RecordPatField::for_field_name(name) {
+        if let Some(ident_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) {
+            return Some((
+                TextRange::empty(ident_pat.syntax().text_range().start()),
+                format!("{}: ", new_name),
+            ));
+        }
+    }
+    None
+}
+
 fn source_edit_from_name_ref(
     name_ref: &ast::NameRef,
     new_name: &str,
     def: Definition,
-) -> (TextRange, String) {
+) -> Option<(TextRange, String)> {
     if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) {
         let rcf_name_ref = record_field.name_ref();
         let rcf_expr = record_field.expr();
@@ -215,7 +231,7 @@ fn source_edit_from_name_ref(
                         // we do not want to erase attributes hence this range start
                         let s = field_name.syntax().text_range().start();
                         let e = record_field.syntax().text_range().end();
-                        return (TextRange::new(s, e), new_name.to_owned());
+                        return Some((TextRange::new(s, e), new_name.to_owned()));
                     }
                 } else if init == *name_ref {
                     if field_name.text() == new_name {
@@ -224,32 +240,27 @@ fn source_edit_from_name_ref(
                         // we do not want to erase attributes hence this range start
                         let s = field_name.syntax().text_range().start();
                         let e = record_field.syntax().text_range().end();
-                        return (TextRange::new(s, e), new_name.to_owned());
+                        return Some((TextRange::new(s, e), new_name.to_owned()));
                     }
                 }
+                None
             }
             // init shorthand
-            (None, Some(_)) => {
-                // FIXME: instead of splitting the shorthand, recursively trigger a rename of the
-                // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547
-                match def {
-                    Definition::Field(_) => {
-                        mark::hit!(test_rename_field_in_field_shorthand);
-                        let s = name_ref.syntax().text_range().start();
-                        return (TextRange::empty(s), format!("{}: ", new_name));
-                    }
-                    Definition::Local(_) => {
-                        mark::hit!(test_rename_local_in_field_shorthand);
-                        let s = name_ref.syntax().text_range().end();
-                        return (TextRange::empty(s), format!(": {}", new_name));
-                    }
-                    _ => {}
-                }
+            // FIXME: instead of splitting the shorthand, recursively trigger a rename of the
+            // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547
+            (None, Some(_)) if matches!(def, Definition::Field(_)) => {
+                mark::hit!(test_rename_field_in_field_shorthand);
+                let s = name_ref.syntax().text_range().start();
+                Some((TextRange::empty(s), format!("{}: ", new_name)))
+            }
+            (None, Some(_)) if matches!(def, Definition::Local(_)) => {
+                mark::hit!(test_rename_local_in_field_shorthand);
+                let s = name_ref.syntax().text_range().end();
+                Some((TextRange::empty(s), format!(": {}", new_name)))
             }
-            _ => {}
+            _ => None,
         }
-    }
-    if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) {
+    } else if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) {
         let rcf_name_ref = record_field.name_ref();
         let rcf_pat = record_field.pat();
         match (rcf_name_ref, rcf_pat) {
@@ -262,13 +273,16 @@ fn source_edit_from_name_ref(
                     // we do not want to erase attributes hence this range start
                     let s = field_name.syntax().text_range().start();
                     let e = record_field.syntax().text_range().end();
-                    return (TextRange::new(s, e), new_name.to_owned());
+                    Some((TextRange::new(s, e), pat.to_string()))
+                } else {
+                    None
                 }
             }
-            _ => {}
+            _ => None,
         }
+    } else {
+        None
     }
-    (name_ref.syntax().text_range(), new_name.to_owned())
 }
 
 fn rename_mod(
@@ -1491,7 +1505,7 @@ fn foo(i: i32) -> Foo {
     }
 
     #[test]
-    fn test_struct_field_destructure_into_shorthand() {
+    fn test_struct_field_pat_into_shorthand() {
         mark::check!(test_rename_field_put_init_shorthand_pat);
         check(
             "baz",
@@ -1499,16 +1513,16 @@ fn foo(i: i32) -> Foo {
 struct Foo { i$0: i32 }
 
 fn foo(foo: Foo) {
-    let Foo { i: baz } = foo;
-    let _ = baz;
+    let Foo { i: ref baz @ qux } = foo;
+    let _ = qux;
 }
 "#,
             r#"
 struct Foo { baz: i32 }
 
 fn foo(foo: Foo) {
-    let Foo { baz } = foo;
-    let _ = baz;
+    let Foo { ref baz @ qux } = foo;
+    let _ = qux;
 }
 "#,
         );
@@ -1582,6 +1596,27 @@ fn foo(Foo { i: bar }: foo) -> i32 {
     }
 
     #[test]
+    fn test_struct_field_complex_ident_pat() {
+        check(
+            "baz",
+            r#"
+struct Foo { i$0: i32 }
+
+fn foo(foo: Foo) {
+    let Foo { ref i } = foo;
+}
+"#,
+            r#"
+struct Foo { baz: i32 }
+
+fn foo(foo: Foo) {
+    let Foo { baz: ref i } = foo;
+}
+"#,
+        );
+    }
+
+    #[test]
     fn test_rename_lifetimes() {
         mark::check!(rename_lifetime);
         check(
diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs
index b105cb0e0..307e150e9 100644
--- a/crates/syntax/src/ast/node_ext.rs
+++ b/crates/syntax/src/ast/node_ext.rs
@@ -3,12 +3,11 @@
 
 use std::fmt;
 
-use ast::AttrsOwner;
 use itertools::Itertools;
 use parser::SyntaxKind;
 
 use crate::{
-    ast::{self, support, AstNode, AstToken, NameOwner, SyntaxNode},
+    ast::{self, support, AstNode, AstToken, AttrsOwner, NameOwner, SyntaxNode},
     SmolStr, SyntaxElement, SyntaxToken, T,
 };
 
@@ -324,7 +323,7 @@ impl ast::RecordPatField {
 
     pub fn for_field_name(field_name: &ast::Name) -> Option<ast::RecordPatField> {
         let candidate =
-            field_name.syntax().ancestors().nth(3).and_then(ast::RecordPatField::cast)?;
+            field_name.syntax().ancestors().nth(2).and_then(ast::RecordPatField::cast)?;
         match candidate.field_name()? {
             NameOrNameRef::Name(name) if name == *field_name => Some(candidate),
             _ => None,