From 09fc5e1dd7f8635d111c12e745cf782fe3b58d95 Mon Sep 17 00:00:00 2001
From: Danny Zhu <dzhu@dzhu.us>
Date: Sun, 25 Apr 2021 13:28:38 -0700
Subject: [PATCH] Check more carefully for cases where a rename can't be done

Attempting to rename an element of a tuple field would previously
replace the type with the new name, which doesn't make sense; now it
fails instead.

The check is done in both `prepare_rename` and `rename` so that the case
is caught before the user is prompted for a new name. Some other
existing failure cases are also now additionally checked in
`prepare_rename`.
---
 crates/ide/src/display/navigation_target.rs |   8 +-
 crates/ide/src/references/rename.rs         | 103 +++++++++++++++++++-
 2 files changed, 105 insertions(+), 6 deletions(-)

diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs
index 364be260c66..2079c22a37e 100644
--- a/crates/ide/src/display/navigation_target.rs
+++ b/crates/ide/src/display/navigation_target.rs
@@ -20,7 +20,7 @@ use syntax::{
 
 use crate::FileSymbol;
 
-/// `NavigationTarget` represents and element in the editor's UI which you can
+/// `NavigationTarget` represents an element in the editor's UI which you can
 /// click on to navigate to a particular piece of code.
 ///
 /// Typically, a `NavigationTarget` corresponds to some element in the source
@@ -35,12 +35,10 @@ pub struct NavigationTarget {
     /// Clients should use this range to answer "is the cursor inside the
     /// element?" question.
     pub full_range: TextRange,
-    /// A "most interesting" range withing the `full_range`.
+    /// A "most interesting" range within the `full_range`.
     ///
     /// Typically, `full_range` is the whole syntax node, including doc
-    /// comments, and `focus_range` is the range of the identifier. "Most
-    /// interesting" range within the full range, typically the range of
-    /// identifier.
+    /// comments, and `focus_range` is the range of the identifier.
     ///
     /// Clients should place the cursor on this range when navigating to this target.
     pub focus_range: Option<TextRange>,
diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs
index 2408a018117..175e7a31d89 100644
--- a/crates/ide/src/references/rename.rs
+++ b/crates/ide/src/references/rename.rs
@@ -50,6 +50,17 @@ pub(crate) fn prepare_rename(
     let sema = Semantics::new(db);
     let source_file = sema.parse(position.file_id);
     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"),
+        _ => {}
+    };
+    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)
         .ok_or_else(|| format_err!("No references found at position"))?;
@@ -507,7 +518,8 @@ fn source_edit_from_def(
         def.try_to_nav(sema.db).ok_or_else(|| format_err!("No references found at position"))?;
 
     let mut replacement_text = String::new();
-    let mut repl_range = nav.focus_or_full_range();
+    let mut repl_range =
+        nav.focus_range.ok_or_else(|| format_err!("No identifier available to rename"))?;
     if let Definition::Local(local) = def {
         if let Either::Left(pat) = local.source(sema.db).value {
             if matches!(
@@ -625,6 +637,49 @@ foo!(Foo$0);",
         check_prepare(r"struct$0 Foo;", expect![[r#"No references found at position"#]]);
     }
 
+    #[test]
+    fn test_prepare_rename_tuple_field() {
+        check_prepare(
+            r#"
+struct Foo(i32);
+
+fn baz() {
+    let mut x = Foo(4);
+    x.0$0 = 5;
+}
+"#,
+            expect![[r#"No identifier available to rename"#]],
+        );
+    }
+
+    #[test]
+    fn test_prepare_rename_builtin() {
+        check_prepare(
+            r#"
+fn foo() {
+    let x: i32$0 = 0;
+}
+"#,
+            expect![[r#"Cannot rename builtin type"#]],
+        );
+    }
+
+    #[test]
+    fn test_prepare_rename_self() {
+        check_prepare(
+            r#"
+struct Foo {}
+
+impl Foo {
+    fn foo(self) -> Self$0 {
+        self
+    }
+}
+"#,
+            expect![[r#"Cannot rename `Self`"#]],
+        );
+    }
+
     #[test]
     fn test_rename_to_underscore() {
         check("_", r#"fn main() { let i$0 = 1; }"#, r#"fn main() { let _ = 1; }"#);
@@ -1787,4 +1842,50 @@ fn foo() {
 "#,
         )
     }
+
+    #[test]
+    fn test_rename_tuple_field() {
+        check(
+            "foo",
+            r#"
+struct Foo(i32);
+
+fn baz() {
+    let mut x = Foo(4);
+    x.0$0 = 5;
+}
+"#,
+            "error: No identifier available to rename",
+        );
+    }
+
+    #[test]
+    fn test_rename_builtin() {
+        check(
+            "foo",
+            r#"
+fn foo() {
+    let x: i32$0 = 0;
+}
+"#,
+            "error: Cannot rename builtin type",
+        );
+    }
+
+    #[test]
+    fn test_rename_self() {
+        check(
+            "foo",
+            r#"
+struct Foo {}
+
+impl Foo {
+    fn foo(self) -> Self$0 {
+        self
+    }
+}
+"#,
+            "error: Cannot rename `Self`",
+        );
+    }
 }