Refactor rename name checking

2 files changed, 104 insertions, 59 deletions

diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 8cb4a51d8..88d215129 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -175,12 +175,7 @@ fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option<Hov
         Definition::SelfType(it) => it.target_ty(db).as_adt(),
         _ => None,
     }?;
-    match adt {
-        Adt::Struct(it) => it.try_to_nav(db),
-        Adt::Union(it) => it.try_to_nav(db),
-        Adt::Enum(it) => it.try_to_nav(db),
-    }
-    .map(to_action)
+    adt.try_to_nav(db).map(to_action)
 }
 
 fn runnable_action(
diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs
index 53d79333c..dd322631b 100644
--- a/crates/ide/src/references/rename.rs
+++ b/crates/ide/src/references/rename.rs
@@ -20,10 +20,11 @@ use test_utils::mark;
 use text_edit::TextEdit;
 
 use crate::{
-    references::find_all_refs, FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind,
+    FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind, ReferenceSearchResult,
     SourceChange, SourceFileEdit, TextRange, TextSize,
 };
 
+type RenameResult<T> = Result<T, RenameError>;
 #[derive(Debug)]
 pub struct RenameError(pub(crate) String);
 
@@ -38,7 +39,7 @@ impl Error for RenameError {}
 pub(crate) fn prepare_rename(
     db: &RootDatabase,
     position: FilePosition,
-) -> Result<RangeInfo<()>, RenameError> {
+) -> RenameResult<RangeInfo<()>> {
     let sema = Semantics::new(db);
     let source_file = sema.parse(position.file_id);
     let syntax = source_file.syntax();
@@ -49,10 +50,7 @@ pub(crate) fn prepare_rename(
     {
         rename_self_to_param(&sema, position, self_token, "dummy")
     } else {
-        let range = match find_all_refs(&sema, position, None) {
-            Some(RangeInfo { range, .. }) => range,
-            None => return Err(RenameError("No references found at position".to_string())),
-        };
+        let RangeInfo { range, .. } = find_all_refs(&sema, position)?;
         Ok(RangeInfo::new(range, SourceChange::from(vec![])))
     }
     .map(|info| RangeInfo::new(info.range, ()))
@@ -62,7 +60,7 @@ pub(crate) fn rename(
     db: &RootDatabase,
     position: FilePosition,
     new_name: &str,
-) -> Result<RangeInfo<SourceChange>, RenameError> {
+) -> RenameResult<RangeInfo<SourceChange>> {
     let sema = Semantics::new(db);
     rename_with_semantics(&sema, position, new_name)
 }
@@ -71,42 +69,18 @@ pub(crate) fn rename_with_semantics(
     sema: &Semantics<RootDatabase>,
     position: FilePosition,
     new_name: &str,
-) -> Result<RangeInfo<SourceChange>, RenameError> {
-    let is_lifetime_name = match lex_single_syntax_kind(new_name) {
-        Some(res) => match res {
-            (SyntaxKind::IDENT, _) => false,
-            (SyntaxKind::UNDERSCORE, _) => false,
-            (SyntaxKind::SELF_KW, _) => return rename_to_self(&sema, position),
-            (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => true,
-            (SyntaxKind::LIFETIME_IDENT, _) => {
-                return Err(RenameError(format!(
-                    "Invalid name `{0}`: Cannot rename lifetime to {0}",
-                    new_name
-                )))
-            }
-            (_, Some(syntax_error)) => {
-                return Err(RenameError(format!("Invalid name `{}`: {}", new_name, syntax_error)))
-            }
-            (_, None) => {
-                return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name)))
-            }
-        },
-        None => return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))),
-    };
-
+) -> RenameResult<RangeInfo<SourceChange>> {
     let source_file = sema.parse(position.file_id);
     let syntax = source_file.syntax();
-    // this is here to prevent lifetime renames from happening on modules and self
-    if is_lifetime_name {
-        rename_reference(&sema, position, new_name, is_lifetime_name)
-    } else if let Some(module) = find_module_at_offset(&sema, position, syntax) {
+
+    if let Some(module) = find_module_at_offset(&sema, position, syntax) {
         rename_mod(&sema, position, module, new_name)
     } else if let Some(self_token) =
         syntax.token_at_offset(position.offset).find(|t| t.kind() == SyntaxKind::SELF_KW)
     {
         rename_self_to_param(&sema, position, self_token, new_name)
     } else {
-        rename_reference(&sema, position, new_name, is_lifetime_name)
+        rename_reference(&sema, position, new_name)
     }
 }
 
@@ -127,6 +101,36 @@ pub(crate) fn will_rename_file(
     Some(change)
 }
 
+#[derive(PartialEq)]
+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),
+            (SyntaxKind::UNDERSCORE, _) => Ok(IdentifierKind::Underscore),
+            (SyntaxKind::SELF_KW, _) => Ok(IdentifierKind::ToSelf),
+            (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => {
+                Ok(IdentifierKind::Lifetime)
+            }
+            (SyntaxKind::LIFETIME_IDENT, _) => {
+                Err(format!("Invalid name `{0}`: Cannot rename lifetime to {0}", new_name))
+            }
+            (_, Some(syntax_error)) => {
+                Err(format!("Invalid name `{}`: {}", new_name, syntax_error))
+            }
+            (_, None) => Err(format!("Invalid name `{}`: not an identifier", new_name)),
+        },
+        None => Err(format!("Invalid name `{}`: not an identifier", new_name)),
+    }
+    .map_err(RenameError)
+}
+
 fn find_module_at_offset(
     sema: &Semantics<RootDatabase>,
     position: FilePosition,
@@ -155,6 +159,14 @@ fn find_module_at_offset(
     Some(module)
 }
 
+fn find_all_refs(
+    sema: &Semantics<RootDatabase>,
+    position: FilePosition,
+) -> RenameResult<RangeInfo<ReferenceSearchResult>> {
+    crate::references::find_all_refs(sema, position, None)
+        .ok_or_else(|| RenameError("No references found at position".to_string()))
+}
+
 fn source_edit_from_reference(
     sema: &Semantics<RootDatabase>,
     reference: Reference,
@@ -223,7 +235,13 @@ fn rename_mod(
     position: FilePosition,
     module: Module,
     new_name: &str,
-) -> Result<RangeInfo<SourceChange>, RenameError> {
+) -> RenameResult<RangeInfo<SourceChange>> {
+    if IdentifierKind::Ident != check_identifier(new_name)? {
+        return Err(RenameError(format!(
+            "Invalid name `{0}`: cannot rename module to {0}",
+            new_name
+        )));
+    }
     let mut source_file_edits = Vec::new();
     let mut file_system_edits = Vec::new();
 
@@ -254,8 +272,7 @@ fn rename_mod(
         source_file_edits.push(edit);
     }
 
-    let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
-        .ok_or_else(|| RenameError("No references found at position".to_string()))?;
+    let RangeInfo { range, info: refs } = find_all_refs(sema, position)?;
     let ref_edits = refs
         .references
         .into_iter()
@@ -310,8 +327,7 @@ fn rename_to_self(
         return Err(RenameError("Parameter type differs from impl block type".to_string()));
     }
 
-    let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
-        .ok_or_else(|| RenameError("No reference found at position".to_string()))?;
+    let RangeInfo { range, info: refs } = find_all_refs(sema, position)?;
 
     let (param_ref, usages): (Vec<Reference>, Vec<Reference>) = refs
         .into_iter()
@@ -367,6 +383,17 @@ fn rename_self_to_param(
     self_token: SyntaxToken,
     new_name: &str,
 ) -> Result<RangeInfo<SourceChange>, RenameError> {
+    let ident_kind = check_identifier(new_name)?;
+    match ident_kind {
+        IdentifierKind::Lifetime => {
+            return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name)))
+        }
+        IdentifierKind::ToSelf => {
+            // no-op
+            return Ok(RangeInfo::new(self_token.text_range(), SourceChange::default()));
+        }
+        _ => (),
+    }
     let source_file = sema.parse(position.file_id);
     let syn = source_file.syntax();
 
@@ -395,6 +422,12 @@ fn rename_self_to_param(
         }
     }
 
+    if edits.len() > 1 && ident_kind == IdentifierKind::Underscore {
+        return Err(RenameError(format!(
+            "Cannot rename reference to `_` as it is being referenced multiple times",
+        )));
+    }
+
     let range = ast::SelfParam::cast(self_token.parent())
         .map_or(self_token.text_range(), |p| p.syntax().text_range());
 
@@ -405,24 +438,36 @@ fn rename_reference(
     sema: &Semantics<RootDatabase>,
     position: FilePosition,
     new_name: &str,
-    is_lifetime_name: bool,
 ) -> Result<RangeInfo<SourceChange>, RenameError> {
-    let RangeInfo { range, info: refs } = match find_all_refs(sema, position, None) {
-        Some(range_info) => range_info,
-        None => return Err(RenameError("No references found at position".to_string())),
-    };
+    let ident_kind = check_identifier(new_name)?;
+    let RangeInfo { range, info: refs } = find_all_refs(sema, position)?;
 
-    match (refs.declaration.kind == ReferenceKind::Lifetime, is_lifetime_name) {
-        (true, false) => {
+    match (ident_kind, &refs.declaration.kind) {
+        (IdentifierKind::ToSelf, ReferenceKind::Lifetime)
+        | (IdentifierKind::Underscore, ReferenceKind::Lifetime)
+        | (IdentifierKind::Ident, ReferenceKind::Lifetime) => {
             return Err(RenameError(format!(
                 "Invalid name `{}`: not a lifetime identifier",
                 new_name
             )))
         }
-        (false, true) => {
+        (IdentifierKind::Lifetime, ReferenceKind::Lifetime) => (),
+        (IdentifierKind::Lifetime, _) => {
             return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name)))
         }
-        _ => (),
+        (IdentifierKind::ToSelf, ReferenceKind::SelfKw) => {
+            //no-op
+            return Ok(RangeInfo::new(range, SourceChange::default()));
+        }
+        (IdentifierKind::ToSelf, _) => {
+            return rename_to_self(sema, position);
+        }
+        (IdentifierKind::Underscore, _) if !refs.references.is_empty() => {
+            return Err(RenameError(format!(
+                "Cannot rename reference to `_` as it is being referenced multiple times",
+            )))
+        }
+        (IdentifierKind::Ident, _) | (IdentifierKind::Underscore, _) => (),
     }
 
     let edit = refs
@@ -430,10 +475,6 @@ fn rename_reference(
         .map(|reference| source_edit_from_reference(sema, reference, new_name))
         .collect::<Vec<_>>();
 
-    if edit.is_empty() {
-        return Err(RenameError("No references found at position".to_string()));
-    }
-
     Ok(RangeInfo::new(range, SourceChange::from(edit)))
 }
 
@@ -547,6 +588,15 @@ mod tests {
     }
 
     #[test]
+    fn test_rename_to_underscore_invalid() {
+        check(
+            "_",
+            r#"fn main(foo$0: ()) {foo;}"#,
+            "error: Cannot rename reference to `_` as it is being referenced multiple times",
+        );
+    }
+
+    #[test]
     fn test_rename_for_local() {
         check(
             "k",