2 files changed, 174 insertions, 83 deletions

diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs
index 50cc1f963..6b3d02bf4 100644
--- a/crates/ide/src/references/rename.rs
+++ b/crates/ide/src/references/rename.rs
@@ -5,9 +5,9 @@
 use std::fmt::{self, Display};
 
 use either::Either;
-use hir::{AsAssocItem, InFile, Module, ModuleDef, ModuleSource, Semantics};
+use hir::{AsAssocItem, FieldSource, HasSource, InFile, ModuleSource, Semantics};
 use ide_db::{
-    base_db::{AnchoredPathBuf, FileId},
+    base_db::{AnchoredPathBuf, FileId, FileRange},
     defs::{Definition, NameClass, NameRefClass},
     search::FileReference,
     RootDatabase,
@@ -20,7 +20,7 @@ use syntax::{
 
 use text_edit::TextEdit;
 
-use crate::{display::TryToNav, FilePosition, FileSystemEdit, RangeInfo, SourceChange, TextRange};
+use crate::{FilePosition, FileSystemEdit, RangeInfo, SourceChange, TextRange};
 
 type RenameResult<T> = Result<T, RenameError>;
 #[derive(Debug)]
@@ -52,26 +52,9 @@ pub(crate) fn prepare_rename(
     let syntax = source_file.syntax();
 
     let def = find_definition(&sema, syntax, position)?;
-    match def {
-        Definition::SelfType(_) => bail!("Cannot rename `Self`"),
-        Definition::ModuleDef(ModuleDef::BuiltinType(_)) => bail!("Cannot rename builtin type"),
-        Definition::ModuleDef(ModuleDef::Module(_)) => (),
-        _ => {
-            let nav = def
-                .try_to_nav(sema.db)
-                .ok_or_else(|| format_err!("No references found at position"))?;
-            nav.focus_range.ok_or_else(|| format_err!("No identifier available to rename"))?;
-        }
-    };
-    let name_like = sema
-        .find_node_at_offset_with_descend(syntax, position.offset)
+    let frange = def_name_range(&&sema, def)
         .ok_or_else(|| format_err!("No references found at position"))?;
-    let node = match &name_like {
-        ast::NameLike::Name(it) => it.syntax(),
-        ast::NameLike::NameRef(it) => it.syntax(),
-        ast::NameLike::Lifetime(it) => it.syntax(),
-    };
-    Ok(RangeInfo::new(sema.original_range(node).range, ()))
+    Ok(RangeInfo::new(frange.range, ()))
 }
 
 // Feature: Rename
@@ -103,10 +86,24 @@ pub(crate) fn rename_with_semantics(
     let syntax = source_file.syntax();
 
     let def = find_definition(sema, syntax, position)?;
+
+    if let Definition::Local(local) = def {
+        if let Some(self_param) = local.as_self_param(sema.db) {
+            cov_mark::hit!(rename_self_to_param);
+            return rename_self_to_param(sema, local, self_param, new_name);
+        }
+        if new_name == "self" {
+            cov_mark::hit!(rename_to_self);
+            return rename_to_self(sema, local);
+        }
+    }
+
     match def {
-        Definition::ModuleDef(ModuleDef::Module(module)) => rename_mod(sema, module, new_name),
+        Definition::ModuleDef(hir::ModuleDef::Module(module)) => rename_mod(sema, module, new_name),
         Definition::SelfType(_) => bail!("Cannot rename `Self`"),
-        Definition::ModuleDef(ModuleDef::BuiltinType(_)) => bail!("Cannot rename builtin type"),
+        Definition::ModuleDef(hir::ModuleDef::BuiltinType(_)) => {
+            bail!("Cannot rename builtin type")
+        }
         def => rename_reference(sema, def, new_name),
     }
 }
@@ -128,26 +125,26 @@ pub(crate) fn will_rename_file(
 enum IdentifierKind {
     Ident,
     Lifetime,
-    ToSelf,
     Underscore,
 }
 
-fn check_identifier(new_name: &str) -> RenameResult<IdentifierKind> {
-    match lex_single_syntax_kind(new_name) {
-        Some(res) => match res {
-            (SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident),
-            (T![_], _) => Ok(IdentifierKind::Underscore),
-            (T![self], _) => Ok(IdentifierKind::ToSelf),
-            (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => {
-                Ok(IdentifierKind::Lifetime)
-            }
-            (SyntaxKind::LIFETIME_IDENT, _) => {
-                bail!("Invalid name `{}`: not a lifetime identifier", new_name)
-            }
-            (_, Some(syntax_error)) => bail!("Invalid name `{}`: {}", new_name, syntax_error),
-            (_, None) => bail!("Invalid name `{}`: not an identifier", new_name),
-        },
-        None => bail!("Invalid name `{}`: not an identifier", new_name),
+impl IdentifierKind {
+    fn classify(new_name: &str) -> RenameResult<IdentifierKind> {
+        match lex_single_syntax_kind(new_name) {
+            Some(res) => match res {
+                (SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident),
+                (T![_], _) => Ok(IdentifierKind::Underscore),
+                (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => {
+                    Ok(IdentifierKind::Lifetime)
+                }
+                (SyntaxKind::LIFETIME_IDENT, _) => {
+                    bail!("Invalid name `{}`: not a lifetime identifier", new_name)
+                }
+                (_, Some(syntax_error)) => bail!("Invalid name `{}`: {}", new_name, syntax_error),
+                (_, None) => bail!("Invalid name `{}`: not an identifier", new_name),
+            },
+            None => bail!("Invalid name `{}`: not an identifier", new_name),
+        }
     }
 }
 
@@ -194,10 +191,10 @@ fn find_definition(
 
 fn rename_mod(
     sema: &Semantics<RootDatabase>,
-    module: Module,
+    module: hir::Module,
     new_name: &str,
 ) -> RenameResult<SourceChange> {
-    if IdentifierKind::Ident != check_identifier(new_name)? {
+    if IdentifierKind::classify(new_name)? != IdentifierKind::Ident {
         bail!("Invalid name `{0}`: cannot rename module to {0}", new_name);
     }
 
@@ -227,7 +224,7 @@ fn rename_mod(
             _ => never!("Module source node is missing a name"),
         }
     }
-    let def = Definition::ModuleDef(ModuleDef::Module(module));
+    let def = Definition::ModuleDef(hir::ModuleDef::Module(module));
     let usages = def.usages(sema).all();
     let ref_edits = usages.iter().map(|(&file_id, references)| {
         (file_id, source_edit_from_references(references, def, new_name))
@@ -242,14 +239,14 @@ fn rename_reference(
     mut def: Definition,
     new_name: &str,
 ) -> RenameResult<SourceChange> {
-    let ident_kind = check_identifier(new_name)?;
+    let ident_kind = IdentifierKind::classify(new_name)?;
 
     if matches!(
         def, // is target a lifetime?
         Definition::GenericParam(hir::GenericParam::LifetimeParam(_)) | Definition::Label(_)
     ) {
         match ident_kind {
-            IdentifierKind::Ident | IdentifierKind::ToSelf | IdentifierKind::Underscore => {
+            IdentifierKind::Ident | IdentifierKind::Underscore => {
                 cov_mark::hit!(rename_not_a_lifetime_ident_ref);
                 bail!("Invalid name `{}`: not a lifetime identifier", new_name);
             }
@@ -261,25 +258,6 @@ fn rename_reference(
                 cov_mark::hit!(rename_not_an_ident_ref);
                 bail!("Invalid name `{}`: not an identifier", new_name);
             }
-            (IdentifierKind::ToSelf, Definition::Local(local)) => {
-                if local.is_self(sema.db) {
-                    // no-op
-                    cov_mark::hit!(rename_self_to_self);
-                    return Ok(SourceChange::default());
-                } else {
-                    cov_mark::hit!(rename_to_self);
-                    return rename_to_self(sema, local);
-                }
-            }
-            (ident_kind, Definition::Local(local)) => {
-                if let Some(self_param) = local.as_self_param(sema.db) {
-                    cov_mark::hit!(rename_self_to_param);
-                    return rename_self_to_param(sema, local, self_param, new_name, ident_kind);
-                } else {
-                    cov_mark::hit!(rename_local);
-                }
-            }
-            (IdentifierKind::ToSelf, _) => bail!("Invalid name `{}`: not an identifier", new_name),
             (IdentifierKind::Ident, _) => cov_mark::hit!(rename_non_local),
             (IdentifierKind::Underscore, _) => (),
         }
@@ -293,21 +271,21 @@ fn rename_reference(
             .and_then(|it| it.containing_trait_impl(sema.db))
             .and_then(|it| {
                 it.items(sema.db).into_iter().find_map(|it| match (it, mod_def) {
-                    (hir::AssocItem::Function(trait_func), ModuleDef::Function(func))
+                    (hir::AssocItem::Function(trait_func), hir::ModuleDef::Function(func))
                         if trait_func.name(sema.db) == func.name(sema.db) =>
                     {
-                        Some(Definition::ModuleDef(ModuleDef::Function(trait_func)))
+                        Some(Definition::ModuleDef(hir::ModuleDef::Function(trait_func)))
                     }
-                    (hir::AssocItem::Const(trait_konst), ModuleDef::Const(konst))
+                    (hir::AssocItem::Const(trait_konst), hir::ModuleDef::Const(konst))
                         if trait_konst.name(sema.db) == konst.name(sema.db) =>
                     {
-                        Some(Definition::ModuleDef(ModuleDef::Const(trait_konst)))
+                        Some(Definition::ModuleDef(hir::ModuleDef::Const(trait_konst)))
                     }
                     (
                         hir::AssocItem::TypeAlias(trait_type_alias),
-                        ModuleDef::TypeAlias(type_alias),
+                        hir::ModuleDef::TypeAlias(type_alias),
                     ) if trait_type_alias.name(sema.db) == type_alias.name(sema.db) => {
-                        Some(Definition::ModuleDef(ModuleDef::TypeAlias(trait_type_alias)))
+                        Some(Definition::ModuleDef(hir::ModuleDef::TypeAlias(trait_type_alias)))
                     }
                     _ => None,
                 })
@@ -398,8 +376,15 @@ fn rename_self_to_param(
     local: hir::Local,
     self_param: hir::SelfParam,
     new_name: &str,
-    identifier_kind: IdentifierKind,
 ) -> RenameResult<SourceChange> {
+    if new_name == "self" {
+        // Let's do nothing rather than complain.
+        cov_mark::hit!(rename_self_to_self);
+        return Ok(SourceChange::default());
+    }
+
+    let identifier_kind = IdentifierKind::classify(new_name)?;
+
     let InFile { file_id, value: self_param } =
         self_param.source(sema.db).ok_or_else(|| format_err!("cannot find function source"))?;
 
@@ -557,12 +542,11 @@ fn source_edit_from_def(
     def: Definition,
     new_name: &str,
 ) -> RenameResult<(FileId, TextEdit)> {
-    let nav =
-        def.try_to_nav(sema.db).ok_or_else(|| format_err!("No references found at position"))?;
+    let frange: FileRange = def_name_range(sema, def)
+        .ok_or_else(|| format_err!("No identifier available to rename"))?;
 
     let mut replacement_text = String::new();
-    let mut repl_range =
-        nav.focus_range.ok_or_else(|| format_err!("No identifier available to rename"))?;
+    let mut repl_range = frange.range;
     if let Definition::Local(local) = def {
         if let Either::Left(pat) = local.source(sema.db).value {
             if matches!(
@@ -582,7 +566,101 @@ fn source_edit_from_def(
         replacement_text.push_str(new_name);
     }
     let edit = TextEdit::replace(repl_range, replacement_text);
-    Ok((nav.file_id, edit))
+    Ok((frange.file_id, edit))
+}
+
+fn def_name_range(sema: &Semantics<RootDatabase>, def: Definition) -> Option<FileRange> {
+    // FIXME: the `original_file_range` calls here are wrong -- they never fail,
+    // and _fall back_ to the entirety of the macro call. Such fall back is
+    // incorrect for renames. The safe behavior would be to return an error for
+    // such cases. The correct behavior would be to return an auxiliary list of
+    // "can't rename these occurrences in macros" items, and then show some kind
+    // of a dialog to the user.
+
+    let res = match def {
+        Definition::Macro(mac) => {
+            let src = mac.source(sema.db)?;
+            let name = match &src.value {
+                Either::Left(it) => it.name()?,
+                Either::Right(it) => it.name()?,
+            };
+            src.with_value(name.syntax()).original_file_range(sema.db)
+        }
+        Definition::Field(field) => {
+            let src = field.source(sema.db)?;
+
+            match &src.value {
+                FieldSource::Named(record_field) => {
+                    let name = record_field.name()?;
+                    src.with_value(name.syntax()).original_file_range(sema.db)
+                }
+                FieldSource::Pos(_) => {
+                    return None;
+                }
+            }
+        }
+        Definition::ModuleDef(module_def) => match module_def {
+            hir::ModuleDef::Module(module) => {
+                let src = module.declaration_source(sema.db)?;
+                let name = src.value.name()?;
+                src.with_value(name.syntax()).original_file_range(sema.db)
+            }
+            hir::ModuleDef::Function(it) => name_range(it, sema)?,
+            hir::ModuleDef::Adt(adt) => match adt {
+                hir::Adt::Struct(it) => name_range(it, sema)?,
+                hir::Adt::Union(it) => name_range(it, sema)?,
+                hir::Adt::Enum(it) => name_range(it, sema)?,
+            },
+            hir::ModuleDef::Variant(it) => name_range(it, sema)?,
+            hir::ModuleDef::Const(it) => name_range(it, sema)?,
+            hir::ModuleDef::Static(it) => name_range(it, sema)?,
+            hir::ModuleDef::Trait(it) => name_range(it, sema)?,
+            hir::ModuleDef::TypeAlias(it) => name_range(it, sema)?,
+            hir::ModuleDef::BuiltinType(_) => return None,
+        },
+        Definition::SelfType(_) => return None,
+        Definition::Local(local) => {
+            let src = local.source(sema.db);
+            let name = match &src.value {
+                Either::Left(bind_pat) => bind_pat.name()?,
+                Either::Right(_) => return None,
+            };
+            src.with_value(name.syntax()).original_file_range(sema.db)
+        }
+        Definition::GenericParam(generic_param) => match generic_param {
+            hir::GenericParam::TypeParam(type_param) => {
+                let src = type_param.source(sema.db)?;
+                let name = match &src.value {
+                    Either::Left(_) => return None,
+                    Either::Right(type_param) => type_param.name()?,
+                };
+                src.with_value(name.syntax()).original_file_range(sema.db)
+            }
+            hir::GenericParam::LifetimeParam(lifetime_param) => {
+                let src = lifetime_param.source(sema.db)?;
+                let lifetime = src.value.lifetime()?;
+                src.with_value(lifetime.syntax()).original_file_range(sema.db)
+            }
+            hir::GenericParam::ConstParam(it) => name_range(it, sema)?,
+        },
+        Definition::Label(label) => {
+            let src = label.source(sema.db);
+            let lifetime = src.value.lifetime()?;
+            src.with_value(lifetime.syntax()).original_file_range(sema.db)
+        }
+    };
+    return Some(res);
+
+    fn name_range<D>(def: D, sema: &Semantics<RootDatabase>) -> Option<FileRange>
+    where
+        D: HasSource,
+        D::Ast: ast::NameOwner,
+    {
+        let src = def.source(sema.db)?;
+        let name = src.value.name()?;
+        let res = src.with_value(name.syntax()).original_file_range(sema.db);
+        Some(res)
+    }
 }
 
 #[cfg(test)]
@@ -659,7 +737,7 @@ mod tests {
     fn test_prepare_rename_namelikes() {
         check_prepare(r"fn name$0<'lifetime>() {}", expect![[r#"3..7: name"#]]);
         check_prepare(r"fn name<'lifetime$0>() {}", expect![[r#"8..17: 'lifetime"#]]);
-        check_prepare(r"fn name<'lifetime>() { name$0(); }", expect![[r#"23..27: name"#]]);
+        check_prepare(r"fn name<'lifetime>() { name$0(); }", expect![[r#"3..7: name"#]]);
     }
 
     #[test]
@@ -691,7 +769,7 @@ fn baz() {
     x.0$0 = 5;
 }
 "#,
-            expect![[r#"No identifier available to rename"#]],
+            expect![[r#"No references found at position"#]],
         );
     }
 
@@ -703,7 +781,7 @@ fn foo() {
     let x: i32$0 = 0;
 }
 "#,
-            expect![[r#"Cannot rename builtin type"#]],
+            expect![[r#"No references found at position"#]],
         );
     }
 
@@ -719,7 +797,7 @@ impl Foo {
     }
 }
 "#,
-            expect![[r#"Cannot rename `Self`"#]],
+            expect![[r#"No references found at position"#]],
         );
     }
 
@@ -801,7 +879,6 @@ impl Foo {
 
     #[test]
     fn test_rename_for_local() {
-        cov_mark::check!(rename_local);
         check(
             "k",
             r#"
diff --git a/crates/ide_assists/src/handlers/unmerge_use.rs b/crates/ide_assists/src/handlers/unmerge_use.rs
index 8d271e056..14e862cd0 100644
--- a/crates/ide_assists/src/handlers/unmerge_use.rs
+++ b/crates/ide_assists/src/handlers/unmerge_use.rs
@@ -73,7 +73,11 @@ fn resolve_full_path(tree: &ast::UseTree) -> Option<ast::Path> {
     for path in paths {
         final_path = ast::make::path_concat(path, final_path)
     }
-    Some(final_path)
+    if final_path.segment().map_or(false, |it| it.self_token().is_some()) {
+        final_path.qualifier()
+    } else {
+        Some(final_path)
+    }
 }
 
 #[cfg(test)]
@@ -223,4 +227,14 @@ pub use std::fmt::Display;
 ",
         );
     }
+
+    #[test]
+    fn unmerge_use_item_on_self() {
+        check_assist(
+            unmerge_use,
+            r"use std::process::{Command, self$0};",
+            r"use std::process::{Command};
+use std::process;",
+        );
+    }
 }