From 1901172841c3986900633b4d216926f200fa35e5 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 13 Feb 2021 20:49:04 +0100 Subject: Prevent aliases from being renamed for now --- crates/ide/src/references/rename.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index b04214291..0e79fbe47 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -75,8 +75,7 @@ pub(crate) fn rename_with_semantics( let source_file = sema.parse(position.file_id); let syntax = source_file.syntax(); - let def = find_definition(sema, syntax, position) - .ok_or_else(|| format_err!("No references found at position"))?; + let def = find_definition(sema, syntax, position)?; match def { Definition::ModuleDef(ModuleDef::Module(module)) => rename_mod(&sema, module, new_name), def => rename_reference(sema, def, new_name), @@ -149,18 +148,30 @@ fn find_definition( sema: &Semantics, syntax: &SyntaxNode, position: FilePosition, -) -> Option { - let def = match find_name_like(sema, syntax, position)? { - NameLike::Name(name) => NameClass::classify(sema, &name)?.referenced_or_defined(sema.db), - NameLike::NameRef(name_ref) => NameRefClass::classify(sema, &name_ref)?.referenced(sema.db), +) -> RenameResult { + match find_name_like(sema, syntax, position) + .ok_or_else(|| format_err!("No references found at position"))? + { + // renaming aliases would rename the item being aliased as the HIR doesn't track aliases yet + NameLike::Name(name) + if name.syntax().parent().map_or(false, |it| ast::Rename::can_cast(it.kind())) => + { + bail!("Renaming aliases is currently unsupported") + } + NameLike::Name(name) => { + NameClass::classify(sema, &name).map(|class| class.referenced_or_defined(sema.db)) + } + NameLike::NameRef(name_ref) => { + NameRefClass::classify(sema, &name_ref).map(|class| class.referenced(sema.db)) + } NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, &lifetime) .map(|class| NameRefClass::referenced(class, sema.db)) .or_else(|| { NameClass::classify_lifetime(sema, &lifetime) .map(|it| it.referenced_or_defined(sema.db)) - })?, - }; - Some(def) + }), + } + .ok_or_else(|| format_err!("No references found at position")) } fn source_edit_from_references( @@ -172,10 +183,10 @@ fn source_edit_from_references( ) -> (FileId, TextEdit) { let mut edit = TextEdit::builder(); for reference in references { - let (range, replacement) = match &reference.name { - NameLike::Name(_) => (None, format!("{}", new_name)), - NameLike::NameRef(name_ref) => source_edit_from_name_ref(name_ref, new_name, def), - NameLike::Lifetime(_) => (None, format!("{}", new_name)), + let (range, replacement) = if let Some(name_ref) = reference.name.as_name_ref() { + source_edit_from_name_ref(name_ref, new_name, def) + } else { + (None, new_name.to_owned()) }; // FIXME: Some(range) will be incorrect when we are inside macros edit.replace(range.unwrap_or(reference.range), replacement); -- cgit v1.2.3 From 9b045069241f3a610f5ee1ff448e6e8997b5f92f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 13 Feb 2021 21:41:04 +0100 Subject: Fallback to renaming input NameRef node for macros when inside macro --- crates/ide/src/references/rename.rs | 69 ++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 16 deletions(-) diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 0e79fbe47..bbd9426b6 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -183,13 +183,16 @@ fn source_edit_from_references( ) -> (FileId, TextEdit) { let mut edit = TextEdit::builder(); for reference in references { - let (range, replacement) = if let Some(name_ref) = reference.name.as_name_ref() { - source_edit_from_name_ref(name_ref, new_name, def) - } else { - (None, new_name.to_owned()) + match reference.name.as_name_ref() { + // if the ranges differ then the node is inside a macro call, we can't really attempt + // to make special rewrites like shorthand syntax and such, so just rename the node in + // the macro input + Some(name_ref) if name_ref.syntax().text_range() == reference.range => { + let (range, replacement) = source_edit_from_name_ref(name_ref, new_name, def); + edit.replace(range, replacement); + } + _ => edit.replace(reference.range, new_name.to_owned()), }; - // FIXME: Some(range) will be incorrect when we are inside macros - edit.replace(range.unwrap_or(reference.range), replacement); } (file_id, edit.finish()) } @@ -198,7 +201,7 @@ fn source_edit_from_name_ref( name_ref: &ast::NameRef, new_name: &str, def: Definition, -) -> (Option, String) { +) -> (TextRange, String) { if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) { let rcf_name_ref = record_field.name_ref(); let rcf_expr = record_field.expr(); @@ -208,20 +211,20 @@ fn source_edit_from_name_ref( if field_name == *name_ref { if init.text() == new_name { mark::hit!(test_rename_field_put_init_shorthand); - // same names, we can use a shorthand here instead + // same names, we can use a shorthand here instead. // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); let e = record_field.syntax().text_range().end(); - return (Some(TextRange::new(s, e)), format!("{}", new_name)); + return (TextRange::new(s, e), new_name.to_owned()); } } else if init == *name_ref { if field_name.text() == new_name { mark::hit!(test_rename_local_put_init_shorthand); - // same names, we can use a shorthand here instead + // same names, we can use a shorthand here instead. // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); let e = record_field.syntax().text_range().end(); - return (Some(TextRange::new(s, e)), format!("{}", new_name)); + return (TextRange::new(s, e), new_name.to_owned()); } } } @@ -233,12 +236,12 @@ fn source_edit_from_name_ref( Definition::Field(_) => { mark::hit!(test_rename_field_in_field_shorthand); let s = name_ref.syntax().text_range().start(); - return (Some(TextRange::empty(s)), format!("{}: ", new_name)); + return (TextRange::empty(s), format!("{}: ", new_name)); } Definition::Local(_) => { mark::hit!(test_rename_local_in_field_shorthand); let s = name_ref.syntax().text_range().end(); - return (Some(TextRange::empty(s)), format!(": {}", new_name)); + return (TextRange::empty(s), format!(": {}", new_name)); } _ => {} } @@ -255,17 +258,17 @@ fn source_edit_from_name_ref( // field name is being renamed if pat.name().map_or(false, |it| it.text() == new_name) { mark::hit!(test_rename_field_put_init_shorthand_pat); - // same names, we can use a shorthand here instead + // same names, we can use a shorthand here instead/ // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); let e = record_field.syntax().text_range().end(); - return (Some(TextRange::new(s, e)), format!("{}", new_name)); + return (TextRange::new(s, e), new_name.to_owned()); } } _ => {} } } - (None, format!("{}", new_name)) + (name_ref.syntax().text_range(), new_name.to_owned()) } fn rename_mod( @@ -1682,6 +1685,40 @@ struct Foo; impl Foo { fn foo(self) {} } +"#, + ) + } + + #[test] + fn test_rename_field_in_pat_in_macro_doesnt_shorthand() { + // ideally we would be able to make this emit a short hand, but I doubt this is easily possible + check( + "baz", + r#" +macro_rules! foo { + ($pattern:pat) => { + let $pattern = loop {}; + }; +} +struct Foo { + bar$0: u32, +} +fn foo() { + foo!(Foo { bar: baz }); +} +"#, + r#" +macro_rules! foo { + ($pattern:pat) => { + let $pattern = loop {}; + }; +} +struct Foo { + baz: u32, +} +fn foo() { + foo!(Foo { baz: baz }); +} "#, ) } -- cgit v1.2.3 From 7b64622780bfa33c593ba856bdb6cfc31b220265 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 13 Feb 2021 23:35:04 +0100 Subject: Don't rename field record patterns directly --- crates/ide/src/references/rename.rs | 107 ++++++++++++++++++++++++------------ crates/syntax/src/ast/node_ext.rs | 5 +- 2 files changed, 73 insertions(+), 39 deletions(-) diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index bbd9426b6..08f16b54d 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -183,25 +183,41 @@ fn source_edit_from_references( ) -> (FileId, TextEdit) { let mut edit = TextEdit::builder(); for reference in references { - match reference.name.as_name_ref() { + let (range, replacement) = match &reference.name { // if the ranges differ then the node is inside a macro call, we can't really attempt // to make special rewrites like shorthand syntax and such, so just rename the node in // the macro input - Some(name_ref) if name_ref.syntax().text_range() == reference.range => { - let (range, replacement) = source_edit_from_name_ref(name_ref, new_name, def); - edit.replace(range, replacement); + NameLike::NameRef(name_ref) if name_ref.syntax().text_range() == reference.range => { + source_edit_from_name_ref(name_ref, new_name, def) } - _ => edit.replace(reference.range, new_name.to_owned()), - }; + NameLike::Name(name) if name.syntax().text_range() == reference.range => { + source_edit_from_name(name, new_name) + } + _ => None, + } + .unwrap_or_else(|| (reference.range, new_name.to_string())); + edit.replace(range, replacement); } (file_id, edit.finish()) } +fn source_edit_from_name(name: &ast::Name, new_name: &str) -> Option<(TextRange, String)> { + if let Some(_) = ast::RecordPatField::for_field_name(name) { + if let Some(ident_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) { + return Some(( + TextRange::empty(ident_pat.syntax().text_range().start()), + format!("{}: ", new_name), + )); + } + } + None +} + fn source_edit_from_name_ref( name_ref: &ast::NameRef, new_name: &str, def: Definition, -) -> (TextRange, String) { +) -> Option<(TextRange, String)> { if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) { let rcf_name_ref = record_field.name_ref(); let rcf_expr = record_field.expr(); @@ -215,7 +231,7 @@ fn source_edit_from_name_ref( // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); let e = record_field.syntax().text_range().end(); - return (TextRange::new(s, e), new_name.to_owned()); + return Some((TextRange::new(s, e), new_name.to_owned())); } } else if init == *name_ref { if field_name.text() == new_name { @@ -224,32 +240,27 @@ fn source_edit_from_name_ref( // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); let e = record_field.syntax().text_range().end(); - return (TextRange::new(s, e), new_name.to_owned()); + return Some((TextRange::new(s, e), new_name.to_owned())); } } + None } // init shorthand - (None, Some(_)) => { - // FIXME: instead of splitting the shorthand, recursively trigger a rename of the - // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547 - match def { - Definition::Field(_) => { - mark::hit!(test_rename_field_in_field_shorthand); - let s = name_ref.syntax().text_range().start(); - return (TextRange::empty(s), format!("{}: ", new_name)); - } - Definition::Local(_) => { - mark::hit!(test_rename_local_in_field_shorthand); - let s = name_ref.syntax().text_range().end(); - return (TextRange::empty(s), format!(": {}", new_name)); - } - _ => {} - } + // FIXME: instead of splitting the shorthand, recursively trigger a rename of the + // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547 + (None, Some(_)) if matches!(def, Definition::Field(_)) => { + mark::hit!(test_rename_field_in_field_shorthand); + let s = name_ref.syntax().text_range().start(); + Some((TextRange::empty(s), format!("{}: ", new_name))) + } + (None, Some(_)) if matches!(def, Definition::Local(_)) => { + mark::hit!(test_rename_local_in_field_shorthand); + let s = name_ref.syntax().text_range().end(); + Some((TextRange::empty(s), format!(": {}", new_name))) } - _ => {} + _ => None, } - } - if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) { + } else if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) { let rcf_name_ref = record_field.name_ref(); let rcf_pat = record_field.pat(); match (rcf_name_ref, rcf_pat) { @@ -262,13 +273,16 @@ fn source_edit_from_name_ref( // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); let e = record_field.syntax().text_range().end(); - return (TextRange::new(s, e), new_name.to_owned()); + Some((TextRange::new(s, e), pat.to_string())) + } else { + None } } - _ => {} + _ => None, } + } else { + None } - (name_ref.syntax().text_range(), new_name.to_owned()) } fn rename_mod( @@ -1491,7 +1505,7 @@ fn foo(i: i32) -> Foo { } #[test] - fn test_struct_field_destructure_into_shorthand() { + fn test_struct_field_pat_into_shorthand() { mark::check!(test_rename_field_put_init_shorthand_pat); check( "baz", @@ -1499,16 +1513,16 @@ fn foo(i: i32) -> Foo { struct Foo { i$0: i32 } fn foo(foo: Foo) { - let Foo { i: baz } = foo; - let _ = baz; + let Foo { i: ref baz @ qux } = foo; + let _ = qux; } "#, r#" struct Foo { baz: i32 } fn foo(foo: Foo) { - let Foo { baz } = foo; - let _ = baz; + let Foo { ref baz @ qux } = foo; + let _ = qux; } "#, ); @@ -1581,6 +1595,27 @@ fn foo(Foo { i: bar }: foo) -> i32 { ) } + #[test] + fn test_struct_field_complex_ident_pat() { + check( + "baz", + r#" +struct Foo { i$0: i32 } + +fn foo(foo: Foo) { + let Foo { ref i } = foo; +} +"#, + r#" +struct Foo { baz: i32 } + +fn foo(foo: Foo) { + let Foo { baz: ref i } = foo; +} +"#, + ); + } + #[test] fn test_rename_lifetimes() { mark::check!(rename_lifetime); diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index b105cb0e0..307e150e9 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/crates/syntax/src/ast/node_ext.rs @@ -3,12 +3,11 @@ use std::fmt; -use ast::AttrsOwner; use itertools::Itertools; use parser::SyntaxKind; use crate::{ - ast::{self, support, AstNode, AstToken, NameOwner, SyntaxNode}, + ast::{self, support, AstNode, AstToken, AttrsOwner, NameOwner, SyntaxNode}, SmolStr, SyntaxElement, SyntaxToken, T, }; @@ -324,7 +323,7 @@ impl ast::RecordPatField { pub fn for_field_name(field_name: &ast::Name) -> Option { let candidate = - field_name.syntax().ancestors().nth(3).and_then(ast::RecordPatField::cast)?; + field_name.syntax().ancestors().nth(2).and_then(ast::RecordPatField::cast)?; match candidate.field_name()? { NameOrNameRef::Name(name) if name == *field_name => Some(candidate), _ => None, -- cgit v1.2.3