Merge #7668

7668: Finalize rename infra rewrite r=matklad a=Veykril

This should be the final PR in regards to rewriting rename stuff, #4290.

It addresses 3 things:
	- Currently renaming import aliases causes some undesired behavior(see #5198) which is why this PR causes us to just return an error if an attempt at renaming an alias is made for the time being. Though this only prevents it from happening when the alias import is renamed, so its not too helpful.
	- Fixes #6898
	- If we are inside a macro file simply rename the input name node as there isn't really a way to do any of the fancy shorthand renames and similar things as for that we would have to exactly know what the macro generates and what not.

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/ide/src/references/rename.rs

@@ -75,8 +75,7 @@ pub(crate) fn rename_with_semantics(
     let source_file = sema.parse(position.file_id);
     let syntax = source_file.syntax();
 
-    let def = find_definition(sema, syntax, position)
+    let def = find_definition(sema, syntax, position)?;
-        .ok_or_else(|| format_err!("No references found at position"))?;
     match def {
         Definition::ModuleDef(ModuleDef::Module(module)) => rename_mod(&sema, module, new_name),
         def => rename_reference(sema, def, new_name),
@@ -149,18 +148,30 @@ fn find_definition(
     sema: &Semantics<RootDatabase>,
     syntax: &SyntaxNode,
     position: FilePosition,
-) -> Option<Definition> {
+) -> RenameResult<Definition> {
-    let def = match find_name_like(sema, syntax, position)? {
+    match find_name_like(sema, syntax, position)
-        NameLike::Name(name) => NameClass::classify(sema, &name)?.referenced_or_defined(sema.db),
+        .ok_or_else(|| format_err!("No references found at position"))?
-        NameLike::NameRef(name_ref) => NameRefClass::classify(sema, &name_ref)?.referenced(sema.db),
+    {
+        // renaming aliases would rename the item being aliased as the HIR doesn't track aliases yet
+        NameLike::Name(name)
+            if name.syntax().parent().map_or(false, |it| ast::Rename::can_cast(it.kind())) =>
+        {
+            bail!("Renaming aliases is currently unsupported")
+        }
+        NameLike::Name(name) => {
+            NameClass::classify(sema, &name).map(|class| class.referenced_or_defined(sema.db))
+        }
+        NameLike::NameRef(name_ref) => {
+            NameRefClass::classify(sema, &name_ref).map(|class| class.referenced(sema.db))
+        }
         NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime)
             .map(|class| NameRefClass::referenced(class, sema.db))
             .or_else(|| {
                 NameClass::classify_lifetime(sema, &lifetime)
                     .map(|it| it.referenced_or_defined(sema.db))
-            })?,
+            }),
-    };
+    }
-    Some(def)
+    .ok_or_else(|| format_err!("No references found at position"))
 }
 
 fn source_edit_from_references(
@@ -173,21 +184,40 @@ fn source_edit_from_references(
     let mut edit = TextEdit::builder();
     for reference in references {
         let (range, replacement) = match &reference.name {
-            NameLike::Name(_) => (None, format!("{}", new_name)),
+            // if the ranges differ then the node is inside a macro call, we can't really attempt
-            NameLike::NameRef(name_ref) => source_edit_from_name_ref(name_ref, new_name, def),
+            // to make special rewrites like shorthand syntax and such, so just rename the node in
-            NameLike::Lifetime(_) => (None, format!("{}", new_name)),
+            // the macro input
-        };
+            NameLike::NameRef(name_ref) if name_ref.syntax().text_range() == reference.range => {
-        // FIXME: Some(range) will be incorrect when we are inside macros
+                source_edit_from_name_ref(name_ref, new_name, def)
-        edit.replace(range.unwrap_or(reference.range), replacement);
+            }
+            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,
-) -> (Option<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();
@@ -197,45 +227,40 @@ fn source_edit_from_name_ref(
                 if field_name == *name_ref {
                     if init.text() == new_name {
                         mark::hit!(test_rename_field_put_init_shorthand);
-                        // same names, we can use a shorthand here instead
+                        // same names, we can use a shorthand here instead.
                         // 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 (Some(TextRange::new(s, e)), format!("{}", new_name));
+                        return Some((TextRange::new(s, e), new_name.to_owned()));
                     }
                 } else if init == *name_ref {
                     if field_name.text() == new_name {
                         mark::hit!(test_rename_local_put_init_shorthand);
-                        // same names, we can use a shorthand here instead
+                        // same names, we can use a shorthand here instead.
                         // 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 (Some(TextRange::new(s, e)), format!("{}", new_name));
+                        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 {
+            (None, Some(_)) if matches!(def, Definition::Field(_)) => {
-                    Definition::Field(_) => {
                 mark::hit!(test_rename_field_in_field_shorthand);
                 let s = name_ref.syntax().text_range().start();
-                        return (Some(TextRange::empty(s)), format!("{}: ", new_name));
+                Some((TextRange::empty(s), format!("{}: ", new_name)))
             }
-                    Definition::Local(_) => {
+            (None, Some(_)) if matches!(def, Definition::Local(_)) => {
                 mark::hit!(test_rename_local_in_field_shorthand);
                 let s = name_ref.syntax().text_range().end();
-                        return (Some(TextRange::empty(s)), format!(": {}", new_name));
+                Some((TextRange::empty(s), format!(": {}", new_name)))
             }
-                    _ => {}
+            _ => None,
         }
-            }
+    } else if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) {
-            _ => {}
-        }
-    }
-    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) {
@@ -244,17 +269,20 @@ fn source_edit_from_name_ref(
                 // field name is being renamed
                 if pat.name().map_or(false, |it| it.text() == new_name) {
                     mark::hit!(test_rename_field_put_init_shorthand_pat);
-                    // same names, we can use a shorthand here instead
+                    // same names, we can use a shorthand here instead/
                     // 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 (Some(TextRange::new(s, e)), format!("{}", new_name));
+                    Some((TextRange::new(s, e), pat.to_string()))
+                } else {
+                    None
                 }
             }
-            _ => {}
+            _ => None,
         }
+    } else {
+        None
     }
-    (None, format!("{}", new_name))
 }
 
 fn rename_mod(
@@ -1477,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",
@@ -1485,16 +1513,16 @@ fn foo(i: i32) -> Foo {
 struct Foo { i$0: i32 }
 
 fn foo(foo: Foo) {
-    let Foo { i: baz } = foo;
+    let Foo { i: ref baz @ qux } = foo;
-    let _ = baz;
+    let _ = qux;
 }
 "#,
             r#"
 struct Foo { baz: i32 }
 
 fn foo(foo: Foo) {
-    let Foo { baz } = foo;
+    let Foo { ref baz @ qux } = foo;
-    let _ = baz;
+    let _ = qux;
 }
 "#,
         );
@@ -1567,6 +1595,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);
@@ -1671,6 +1720,40 @@ struct Foo;
 impl Foo {
     fn foo(self) {}
 }
+"#,
+        )
+    }
+
+    #[test]
+    fn test_rename_field_in_pat_in_macro_doesnt_shorthand() {
+        // ideally we would be able to make this emit a short hand, but I doubt this is easily possible
+        check(
+            "baz",
+            r#"
+macro_rules! foo {
+    ($pattern:pat) => {
+        let $pattern = loop {};
+    };
+}
+struct Foo {
+    bar$0: u32,
+}
+fn foo() {
+    foo!(Foo { bar: baz });
+}
+"#,
+            r#"
+macro_rules! foo {
+    ($pattern:pat) => {
+        let $pattern = loop {};
+    };
+}
+struct Foo {
+    baz: u32,
+}
+fn foo() {
+    foo!(Foo { baz: baz });
+}
 "#,
         )
     }

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,