Merge #2449

2449: Only allow renames to valid identifiers r=matklad a=detrumi

Implements #2121 

Co-authored-by: Wilco Kusee <wilcokusee@gmail.com>

crates/ra_ide/src/references/rename.rs

@@ -2,7 +2,7 @@
 
 use hir::ModuleSource;
 use ra_db::{RelativePath, RelativePathBuf, SourceDatabase, SourceDatabaseExt};
-use ra_syntax::{algo::find_node_at_offset, ast, AstNode, SyntaxNode};
+use ra_syntax::{algo::find_node_at_offset, ast, tokenize, AstNode, SyntaxKind, SyntaxNode};
 use ra_text_edit::TextEdit;
 
 use crate::{
@@ -17,6 +17,13 @@ pub(crate) fn rename(
     position: FilePosition,
     new_name: &str,
 ) -> Option<RangeInfo<SourceChange>> {
+    let tokens = tokenize(new_name);
+    if tokens.len() != 1
+        || (tokens[0].kind != SyntaxKind::IDENT && tokens[0].kind != SyntaxKind::UNDERSCORE)
+    {
+        return None;
+    }
+
     let parse = db.parse(position.file_id);
     if let Some((ast_name, ast_module)) =
         find_name_and_module_at_offset(parse.tree().syntax(), position)
@@ -123,6 +130,49 @@ mod tests {
         mock_analysis::analysis_and_position, mock_analysis::single_file_with_position, FileId,
     };
 
+    #[test]
+    fn test_rename_to_underscore() {
+        test_rename(
+            r#"
+    fn main() {
+        let i<|> = 1;
+    }"#,
+            "_",
+            r#"
+    fn main() {
+        let _ = 1;
+    }"#,
+        );
+    }
+
+    #[test]
+    fn test_rename_to_raw_identifier() {
+        test_rename(
+            r#"
+    fn main() {
+        let i<|> = 1;
+    }"#,
+            "r#fn",
+            r#"
+    fn main() {
+        let r#fn = 1;
+    }"#,
+        );
+    }
+
+    #[test]
+    fn test_rename_to_invalid_identifier() {
+        let (analysis, position) = single_file_with_position(
+            "
+    fn main() {
+        let i<|> = 1;
+    }",
+        );
+        let new_name = "invalid!";
+        let source_change = analysis.rename(position, new_name).unwrap();
+        assert!(source_change.is_none());
+    }
+
     #[test]
     fn test_rename_for_local() {
         test_rename(

crates/ra_lsp_server/src/main_loop/handlers.rs

@@ -480,8 +480,6 @@ pub fn handle_prepare_rename(
     let _p = profile("handle_prepare_rename");
     let position = params.try_conv_with(&world)?;
 
-    // We support renaming references like handle_rename does.
-    // In the future we may want to reject the renaming of things like keywords here too.
     let optional_change = world.analysis().rename(position, "dummy")?;
     let range = match optional_change {
         None => return Ok(None),