Unnamed repository; edit this file 'description' to name the repository.
Merge #10645
10645: fix: make `rename` multi-token mapping aware r=Veykril a=spookyvision
Co-authored-by: Anatol Ulrich <[email protected]>
Co-authored-by: Lukas Wirth <[email protected]>
Co-authored-by: Anatol Ulrich <[email protected]>
| -rw-r--r-- | crates/ide/src/rename.rs | 211 | ||||
| -rw-r--r-- | crates/ide_db/src/source_change.rs | 13 | ||||
| -rw-r--r-- | crates/text_edit/src/lib.rs | 30 |
3 files changed, 179 insertions, 75 deletions
diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index d6f381497b..3afcccd64d 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -3,6 +3,7 @@ //! This is mostly front-end for [`ide_db::rename`], but it also includes the //! tests. This module also implements a couple of magic tricks, like renaming //! `self` and to `self` (to switch between associated function and method). + use hir::{AsAssocItem, InFile, Semantics}; use ide_db::{ base_db::FileId, @@ -10,6 +11,7 @@ use ide_db::{ rename::{bail, format_err, source_edit_from_references, IdentifierKind}, RootDatabase, }; +use itertools::Itertools; use stdx::{always, never}; use syntax::{ast, AstNode, SyntaxNode}; @@ -31,14 +33,33 @@ pub(crate) fn prepare_rename( let source_file = sema.parse(position.file_id); let syntax = source_file.syntax(); - let (name_like, def) = find_definition(&sema, syntax, position)?; - if def.range_for_rename(&sema).is_none() { - bail!("No references found at position") - } + let res = find_definitions(&sema, syntax, position)? + .map(|(name_like, def)| { + // ensure all ranges are valid - let frange = sema.original_range(name_like.syntax()); - always!(frange.range.contains_inclusive(position.offset) && frange.file_id == position.file_id); - Ok(RangeInfo::new(frange.range, ())) + if def.range_for_rename(&sema).is_none() { + bail!("No references found at position") + } + let frange = sema.original_range(name_like.syntax()); + + always!( + frange.range.contains_inclusive(position.offset) + && frange.file_id == position.file_id + ); + Ok(frange.range) + }) + .reduce(|acc, cur| match (acc, cur) { + // ensure all ranges are the same + (Ok(acc_inner), Ok(cur_inner)) if acc_inner == cur_inner => Ok(acc_inner), + (Err(e), _) => Err(e), + _ => bail!("inconsistent text range"), + }); + + match res { + // ensure at least one definition was found + Some(res) => res.map(|range| RangeInfo::new(range, ())), + None => bail!("No references found at position"), + } } // Feature: Rename @@ -61,20 +82,27 @@ pub(crate) fn rename( let source_file = sema.parse(position.file_id); let syntax = source_file.syntax(); - let (_name_like, def) = find_definition(&sema, syntax, position)?; + let defs = find_definitions(&sema, syntax, position)?; - if let Definition::Local(local) = def { - if let Some(self_param) = local.as_self_param(sema.db) { - cov_mark::hit!(rename_self_to_param); - return rename_self_to_param(&sema, local, self_param, new_name); - } - if new_name == "self" { - cov_mark::hit!(rename_to_self); - return rename_to_self(&sema, local); - } - } + let ops: RenameResult<Vec<SourceChange>> = defs + .map(|(_namelike, def)| { + if let Definition::Local(local) = def { + if let Some(self_param) = local.as_self_param(sema.db) { + cov_mark::hit!(rename_self_to_param); + return rename_self_to_param(&sema, local, self_param, new_name); + } + if new_name == "self" { + cov_mark::hit!(rename_to_self); + return rename_to_self(&sema, local); + } + } + def.rename(&sema, new_name) + }) + .collect(); - def.rename(&sema, new_name) + ops?.into_iter() + .reduce(|acc, elem| acc.merge(elem)) + .ok_or_else(|| format_err!("No references found at position")) } /// Called by the client when it is about to rename a file. @@ -91,59 +119,86 @@ pub(crate) fn will_rename_file( Some(change) } -fn find_definition( +fn find_definitions( sema: &Semantics<RootDatabase>, syntax: &SyntaxNode, position: FilePosition, -) -> RenameResult<(ast::NameLike, Definition)> { - let name_like = sema - .find_node_at_offset_with_descend::<ast::NameLike>(syntax, position.offset) - .ok_or_else(|| format_err!("No references found at position"))?; - - let def = match &name_like { - // renaming aliases would rename the item being aliased as the HIR doesn't track aliases yet - ast::NameLike::Name(name) - if name.syntax().parent().map_or(false, |it| ast::Rename::can_cast(it.kind())) => - { - bail!("Renaming aliases is currently unsupported") - } - ast::NameLike::Name(name) => NameClass::classify(sema, name).map(|class| match class { - NameClass::Definition(it) | NameClass::ConstReference(it) => it, - NameClass::PatFieldShorthand { local_def, field_ref: _ } => { - Definition::Local(local_def) - } - }), - ast::NameLike::NameRef(name_ref) => { - if let Some(def) = NameRefClass::classify(sema, name_ref).map(|class| match class { - NameRefClass::Definition(def) => def, - NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { - Definition::Local(local_ref) +) -> RenameResult<impl Iterator<Item = (ast::NameLike, Definition)>> { + let symbols = sema + .find_nodes_at_offset_with_descend::<ast::NameLike>(syntax, position.offset) + .map(|name_like| { + let res = match &name_like { + // renaming aliases would rename the item being aliased as the HIR doesn't track aliases yet + ast::NameLike::Name(name) + if name + .syntax() + .parent() + .map_or(false, |it| ast::Rename::can_cast(it.kind())) => + { + bail!("Renaming aliases is currently unsupported") + } + ast::NameLike::Name(name) => NameClass::classify(sema, name) + .map(|class| match class { + NameClass::Definition(it) | NameClass::ConstReference(it) => it, + NameClass::PatFieldShorthand { local_def, field_ref: _ } => { + Definition::Local(local_def) + } + }) + .map(|def| (name_like.clone(), def)) + .ok_or_else(|| format_err!("No references found at position")), + ast::NameLike::NameRef(name_ref) => { + NameRefClass::classify(sema, name_ref) + .map(|class| match class { + NameRefClass::Definition(def) => def, + NameRefClass::FieldShorthand { local_ref, field_ref: _ } => { + Definition::Local(local_ref) + } + }) + .ok_or_else(|| format_err!("No references found at position")) + .and_then(|def| { + // if the name differs from the definitions name it has to be an alias + if def + .name(sema.db) + .map_or(false, |it| it.to_string() != name_ref.text()) + { + Err(format_err!("Renaming aliases is currently unsupported")) + } else { + Ok((name_like.clone(), def)) + } + }) } - }) { - // if the name differs from the definitions name it has to be an alias - if def.name(sema.db).map_or(false, |it| it.to_string() != name_ref.text()) { - bail!("Renaming aliases is currently unsupported"); + ast::NameLike::Lifetime(lifetime) => { + NameRefClass::classify_lifetime(sema, lifetime) + .and_then(|class| match class { + NameRefClass::Definition(def) => Some(def), + _ => None, + }) + .or_else(|| { + NameClass::classify_lifetime(sema, lifetime).and_then(|it| match it { + NameClass::Definition(it) => Some(it), + _ => None, + }) + }) + .map(|def| (name_like, def)) + .ok_or_else(|| format_err!("No references found at position")) } - Some(def) + }; + res + }); + + let res: RenameResult<Vec<_>> = symbols.collect(); + match res { + Ok(v) => { + if v.is_empty() { + // FIXME: some semantic duplication between "empty vec" and "Err()" + Err(format_err!("No references found at position")) } else { - None + // remove duplicates, comparing `Definition`s + Ok(v.into_iter().unique_by(|t| t.1)) } } - ast::NameLike::Lifetime(lifetime) => NameRefClass::classify_lifetime(sema, lifetime) - .and_then(|class| match class { - NameRefClass::Definition(def) => Some(def), - _ => None, - }) - .or_else(|| { - NameClass::classify_lifetime(sema, lifetime).and_then(|it| match it { - NameClass::Definition(it) => Some(it), - _ => None, - }) - }), + Err(e) => Err(e), } - .ok_or_else(|| format_err!("No references found at position"))?; - - Ok((name_like, def)) } fn rename_to_self(sema: &Semantics<RootDatabase>, local: hir::Local) -> RenameResult<SourceChange> { @@ -516,6 +571,36 @@ fn main() { } #[test] + fn test_rename_macro_multiple_occurrences() { + check( + "Baaah", + r#"macro_rules! foo { + ($ident:ident) => { + const $ident: () = (); + struct $ident {} + }; +} + +foo!($0Foo); +const _: () = Foo; +const _: Foo = Foo {}; + "#, + r#" +macro_rules! foo { + ($ident:ident) => { + const $ident: () = (); + struct $ident {} + }; +} + +foo!(Baaah); +const _: () = Baaah; +const _: Baaah = Baaah {}; + "#, + ) + } + + #[test] fn test_rename_for_macro_args() { check( "b", diff --git a/crates/ide_db/src/source_change.rs b/crates/ide_db/src/source_change.rs index 4c2133e65b..aa910eb325 100644 --- a/crates/ide_db/src/source_change.rs +++ b/crates/ide_db/src/source_change.rs @@ -54,6 +54,13 @@ impl SourceChange { pub fn get_source_edit(&self, file_id: FileId) -> Option<&TextEdit> { self.source_file_edits.get(&file_id) } + + pub fn merge(mut self, other: SourceChange) -> SourceChange { + self.extend(other.source_file_edits); + self.extend(other.file_system_edits); + self.is_snippet |= other.is_snippet; + self + } } impl Extend<(FileId, TextEdit)> for SourceChange { @@ -62,6 +69,12 @@ impl Extend<(FileId, TextEdit)> for SourceChange { } } +impl Extend<FileSystemEdit> for SourceChange { + fn extend<T: IntoIterator<Item = FileSystemEdit>>(&mut self, iter: T) { + iter.into_iter().for_each(|edit| self.push_file_system_edit(edit)); + } +} + impl From<FxHashMap<FileId, TextEdit>> for SourceChange { fn from(source_file_edits: FxHashMap<FileId, TextEdit>) -> SourceChange { SourceChange { source_file_edits, file_system_edits: Vec::new(), is_snippet: false } diff --git a/crates/text_edit/src/lib.rs b/crates/text_edit/src/lib.rs index e9700c560a..21c622d3d6 100644 --- a/crates/text_edit/src/lib.rs +++ b/crates/text_edit/src/lib.rs @@ -3,12 +3,13 @@ //! `rust-analyzer` never mutates text itself and only sends diffs to clients, //! so `TextEdit` is the ultimate representation of the work done by //! rust-analyzer. + pub use text_size::{TextRange, TextSize}; /// `InsertDelete` -- a single "atomic" change to text /// /// Must not overlap with other `InDel`s -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Indel { pub insert: String, /// Refers to offsets in the original text @@ -116,11 +117,14 @@ impl TextEdit { pub fn union(&mut self, other: TextEdit) -> Result<(), TextEdit> { // FIXME: can be done without allocating intermediate vector let mut all = self.iter().chain(other.iter()).collect::<Vec<_>>(); - if !check_disjoint(&mut all) { + if !check_disjoint_and_sort(&mut all) { return Err(other); } + self.indels.extend(other.indels); - assert_disjoint(&mut self.indels); + check_disjoint_and_sort(&mut self.indels); + // Only dedup deletions and replacements, keep all insertions + self.indels.dedup_by(|a, b| a == b && !a.delete.is_empty()); Ok(()) } @@ -173,7 +177,7 @@ impl TextEditBuilder { } pub fn finish(self) -> TextEdit { let mut indels = self.indels; - assert_disjoint(&mut indels); + assert_disjoint_or_equal(&mut indels); TextEdit { indels } } pub fn invalidates_offset(&self, offset: TextSize) -> bool { @@ -182,18 +186,20 @@ impl TextEditBuilder { fn indel(&mut self, indel: Indel) { self.indels.push(indel); if self.indels.len() <= 16 { - assert_disjoint(&mut self.indels); + assert_disjoint_or_equal(&mut self.indels); } } } -fn assert_disjoint(indels: &mut [impl std::borrow::Borrow<Indel>]) { - assert!(check_disjoint(indels)); +fn assert_disjoint_or_equal(indels: &mut [Indel]) { + assert!(check_disjoint_and_sort(indels)); } -fn check_disjoint(indels: &mut [impl std::borrow::Borrow<Indel>]) -> bool { +// FIXME: Remove the impl Bound here, it shouldn't be needed +fn check_disjoint_and_sort(indels: &mut [impl std::borrow::Borrow<Indel>]) -> bool { indels.sort_by_key(|indel| (indel.borrow().delete.start(), indel.borrow().delete.end())); - indels - .iter() - .zip(indels.iter().skip(1)) - .all(|(l, r)| l.borrow().delete.end() <= r.borrow().delete.start()) + indels.iter().zip(indels.iter().skip(1)).all(|(l, r)| { + let l = l.borrow(); + let r = r.borrow(); + l.delete.end() <= r.delete.start() || l == r + }) } |