From 09fc5e1dd7f8635d111c12e745cf782fe3b58d95 Mon Sep 17 00:00:00 2001 From: Danny Zhu Date: Sun, 25 Apr 2021 13:28:38 -0700 Subject: 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 364be260c..2079c22a3 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, diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 2408a0181..175e7a31d 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`", + ); + } } -- cgit v1.2.3