Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/hir-def/src/path.rs | 16 | ||||
| -rw-r--r-- | crates/hir-def/src/resolver.rs | 166 | ||||
| -rw-r--r-- | crates/hir/src/semantics.rs | 95 | ||||
| -rw-r--r-- | crates/ide-assists/src/tests.rs | 30 | ||||
| -rw-r--r-- | crates/ide-completion/src/render.rs | 2 | ||||
| -rw-r--r-- | crates/ide-db/src/rename.rs | 36 | ||||
| -rw-r--r-- | crates/ide-db/src/source_change.rs | 45 | ||||
| -rw-r--r-- | crates/ide-db/src/text_edit.rs | 15 | ||||
| -rw-r--r-- | crates/ide/src/rename.rs | 81 | ||||
| -rw-r--r-- | crates/ide/src/ssr.rs | 7 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/handlers/request.rs | 21 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/to_proto.rs | 54 |
12 files changed, 509 insertions, 59 deletions
diff --git a/crates/hir-def/src/path.rs b/crates/hir-def/src/path.rs index e6c2504d07..713e738973 100644 --- a/crates/hir-def/src/path.rs +++ b/crates/hir-def/src/path.rs @@ -57,7 +57,7 @@ pub enum Path { /// or type anchor, it is `Path::Normal` with the generics filled with `None` even if there are none (practically /// this is not a problem since many more paths have generics than a type anchor). BarePath(Interned<ModPath>), - /// `Path::Normal` may have empty generics and type anchor (but generic args will be filled with `None`). + /// `Path::Normal` will always have either generics or type anchor. Normal(NormalPath), /// A link to a lang item. It is used in desugaring of things like `it?`. We can show these /// links via a normal path since they might be private and not accessible in the usage place. @@ -208,11 +208,15 @@ impl Path { mod_path.segments()[..mod_path.segments().len() - 1].iter().cloned(), )); let qualifier_generic_args = &generic_args[..generic_args.len() - 1]; - Some(Path::Normal(NormalPath::new( - type_anchor, - qualifier_mod_path, - qualifier_generic_args.iter().cloned(), - ))) + if type_anchor.is_none() && qualifier_generic_args.iter().all(|it| it.is_none()) { + Some(Path::BarePath(qualifier_mod_path)) + } else { + Some(Path::Normal(NormalPath::new( + type_anchor, + qualifier_mod_path, + qualifier_generic_args.iter().cloned(), + ))) + } } Path::LangItem(..) => None, } diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index e5774b4804..a2e6e4cc04 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -3,10 +3,11 @@ use std::{fmt, iter, mem}; use base_db::CrateId; use hir_expand::{name::Name, MacroDefId}; -use intern::sym; +use intern::{sym, Symbol}; use itertools::Itertools as _; use rustc_hash::FxHashSet; use smallvec::{smallvec, SmallVec}; +use span::SyntaxContextId; use triomphe::Arc; use crate::{ @@ -343,15 +344,7 @@ impl Resolver { } if n_segments <= 1 { - let mut hygiene_info = if !hygiene_id.is_root() { - let ctx = hygiene_id.lookup(db); - ctx.outer_expn.map(|expansion| { - let expansion = db.lookup_intern_macro_call(expansion); - (ctx.parent, expansion.def) - }) - } else { - None - }; + let mut hygiene_info = hygiene_info(db, hygiene_id); for scope in self.scopes() { match scope { Scope::ExprScope(scope) => { @@ -371,19 +364,7 @@ impl Resolver { } } Scope::MacroDefScope(macro_id) => { - if let Some((parent_ctx, label_macro_id)) = hygiene_info { - if label_macro_id == **macro_id { - // A macro is allowed to refer to variables from before its declaration. - // Therefore, if we got to the rib of its declaration, give up its hygiene - // and use its parent expansion. - let parent_ctx = db.lookup_intern_syntax_context(parent_ctx); - hygiene_id = HygieneId::new(parent_ctx.opaque_and_semitransparent); - hygiene_info = parent_ctx.outer_expn.map(|expansion| { - let expansion = db.lookup_intern_macro_call(expansion); - (parent_ctx.parent, expansion.def) - }); - } - } + handle_macro_def_scope(db, &mut hygiene_id, &mut hygiene_info, macro_id) } Scope::GenericParams { params, def } => { if let Some(id) = params.find_const_by_name(first_name, *def) { @@ -730,6 +711,107 @@ impl Resolver { }) } + /// Checks if we rename `renamed` (currently named `current_name`) to `new_name`, will the meaning of this reference + /// (that contains `current_name` path) change from `renamed` to some another variable (returned as `Some`). + pub fn rename_will_conflict_with_another_variable( + &self, + db: &dyn DefDatabase, + current_name: &Name, + current_name_as_path: &ModPath, + mut hygiene_id: HygieneId, + new_name: &Symbol, + to_be_renamed: BindingId, + ) -> Option<BindingId> { + let mut hygiene_info = hygiene_info(db, hygiene_id); + let mut will_be_resolved_to = None; + for scope in self.scopes() { + match scope { + Scope::ExprScope(scope) => { + for entry in scope.expr_scopes.entries(scope.scope_id) { + if entry.hygiene() == hygiene_id { + if entry.binding() == to_be_renamed { + // This currently resolves to our renamed variable, now `will_be_resolved_to` + // contains `Some` if the meaning will change or `None` if not. + return will_be_resolved_to; + } else if entry.name().symbol() == new_name { + will_be_resolved_to = Some(entry.binding()); + } + } + } + } + Scope::MacroDefScope(macro_id) => { + handle_macro_def_scope(db, &mut hygiene_id, &mut hygiene_info, macro_id) + } + Scope::GenericParams { params, def } => { + if params.find_const_by_name(current_name, *def).is_some() { + // It does not resolve to our renamed variable. + return None; + } + } + Scope::AdtScope(_) | Scope::ImplDefScope(_) => continue, + Scope::BlockScope(m) => { + if m.resolve_path_in_value_ns(db, current_name_as_path).is_some() { + // It does not resolve to our renamed variable. + return None; + } + } + } + } + // It does not resolve to our renamed variable. + None + } + + /// Checks if we rename `renamed` to `name`, will the meaning of this reference (that contains `name` path) change + /// from some other variable (returned as `Some`) to `renamed`. + pub fn rename_will_conflict_with_renamed( + &self, + db: &dyn DefDatabase, + name: &Name, + name_as_path: &ModPath, + mut hygiene_id: HygieneId, + to_be_renamed: BindingId, + ) -> Option<BindingId> { + let mut hygiene_info = hygiene_info(db, hygiene_id); + let mut will_resolve_to_renamed = false; + for scope in self.scopes() { + match scope { + Scope::ExprScope(scope) => { + for entry in scope.expr_scopes.entries(scope.scope_id) { + if entry.binding() == to_be_renamed { + will_resolve_to_renamed = true; + } else if entry.hygiene() == hygiene_id && entry.name() == name { + if will_resolve_to_renamed { + // This will resolve to the renamed variable before it resolves to the original variable. + return Some(entry.binding()); + } else { + // This will resolve to the original variable. + return None; + } + } + } + } + Scope::MacroDefScope(macro_id) => { + handle_macro_def_scope(db, &mut hygiene_id, &mut hygiene_info, macro_id) + } + Scope::GenericParams { params, def } => { + if params.find_const_by_name(name, *def).is_some() { + // Here and below, it might actually resolve to our renamed variable - in which case it'll + // hide the generic parameter or some other thing (not a variable). We don't check for that + // because due to naming conventions, it is rare that variable will shadow a non-variable. + return None; + } + } + Scope::AdtScope(_) | Scope::ImplDefScope(_) => continue, + Scope::BlockScope(m) => { + if m.resolve_path_in_value_ns(db, name_as_path).is_some() { + return None; + } + } + } + } + None + } + /// `expr_id` is required to be an expression id that comes after the top level expression scope in the given resolver #[must_use] pub fn update_to_inner_scope( @@ -795,6 +877,44 @@ impl Resolver { } } +#[inline] +fn handle_macro_def_scope( + db: &dyn DefDatabase, + hygiene_id: &mut HygieneId, + hygiene_info: &mut Option<(SyntaxContextId, MacroDefId)>, + macro_id: &MacroDefId, +) { + if let Some((parent_ctx, label_macro_id)) = hygiene_info { + if label_macro_id == macro_id { + // A macro is allowed to refer to variables from before its declaration. + // Therefore, if we got to the rib of its declaration, give up its hygiene + // and use its parent expansion. + let parent_ctx = db.lookup_intern_syntax_context(*parent_ctx); + *hygiene_id = HygieneId::new(parent_ctx.opaque_and_semitransparent); + *hygiene_info = parent_ctx.outer_expn.map(|expansion| { + let expansion = db.lookup_intern_macro_call(expansion); + (parent_ctx.parent, expansion.def) + }); + } + } +} + +#[inline] +fn hygiene_info( + db: &dyn DefDatabase, + hygiene_id: HygieneId, +) -> Option<(SyntaxContextId, MacroDefId)> { + if !hygiene_id.is_root() { + let ctx = hygiene_id.lookup(db); + ctx.outer_expn.map(|expansion| { + let expansion = db.lookup_intern_macro_call(expansion); + (ctx.parent, expansion.def) + }) + } else { + None + } +} + pub struct UpdateGuard(usize); impl Resolver { diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 1b8531209c..b0c2feaac1 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -12,8 +12,8 @@ use std::{ use either::Either; use hir_def::{ - expr_store::ExprOrPatSource, - hir::{Expr, ExprOrPatId}, + expr_store::{Body, ExprOrPatSource}, + hir::{BindingId, Expr, ExprId, ExprOrPatId, Pat}, lower::LowerCtx, nameres::{MacroSubNs, ModuleOrigin}, path::ModPath, @@ -629,6 +629,31 @@ impl<'db> SemanticsImpl<'db> { ) } + /// Checks if renaming `renamed` to `new_name` may introduce conflicts with other locals, + /// and returns the conflicting locals. + pub fn rename_conflicts(&self, to_be_renamed: &Local, new_name: &str) -> Vec<Local> { + let body = self.db.body(to_be_renamed.parent); + let resolver = to_be_renamed.parent.resolver(self.db.upcast()); + let starting_expr = + body.binding_owners.get(&to_be_renamed.binding_id).copied().unwrap_or(body.body_expr); + let mut visitor = RenameConflictsVisitor { + body: &body, + conflicts: FxHashSet::default(), + db: self.db, + new_name: Symbol::intern(new_name), + old_name: to_be_renamed.name(self.db).symbol().clone(), + owner: to_be_renamed.parent, + to_be_renamed: to_be_renamed.binding_id, + resolver, + }; + visitor.rename_conflicts(starting_expr); + visitor + .conflicts + .into_iter() + .map(|binding_id| Local { parent: to_be_renamed.parent, binding_id }) + .collect() + } + /// Retrieves all the formatting parts of the format_args! (or `asm!`) template string. pub fn as_format_args_parts( &self, @@ -2094,3 +2119,69 @@ impl ops::Deref for VisibleTraits { &self.0 } } + +struct RenameConflictsVisitor<'a> { + db: &'a dyn HirDatabase, + owner: DefWithBodyId, + resolver: Resolver, + body: &'a Body, + to_be_renamed: BindingId, + new_name: Symbol, + old_name: Symbol, + conflicts: FxHashSet<BindingId>, +} + +impl RenameConflictsVisitor<'_> { + fn resolve_path(&mut self, node: ExprOrPatId, path: &Path) { + if let Path::BarePath(path) = path { + if let Some(name) = path.as_ident() { + if *name.symbol() == self.new_name { + if let Some(conflicting) = self.resolver.rename_will_conflict_with_renamed( + self.db.upcast(), + name, + path, + self.body.expr_or_pat_path_hygiene(node), + self.to_be_renamed, + ) { + self.conflicts.insert(conflicting); + } + } else if *name.symbol() == self.old_name { + if let Some(conflicting) = + self.resolver.rename_will_conflict_with_another_variable( + self.db.upcast(), + name, + path, + self.body.expr_or_pat_path_hygiene(node), + &self.new_name, + self.to_be_renamed, + ) + { + self.conflicts.insert(conflicting); + } + } + } + } + } + + fn rename_conflicts(&mut self, expr: ExprId) { + match &self.body[expr] { + Expr::Path(path) => { + let guard = self.resolver.update_to_inner_scope(self.db.upcast(), self.owner, expr); + self.resolve_path(expr.into(), path); + self.resolver.reset_to_guard(guard); + } + &Expr::Assignment { target, .. } => { + let guard = self.resolver.update_to_inner_scope(self.db.upcast(), self.owner, expr); + self.body.walk_pats(target, &mut |pat| { + if let Pat::Path(path) = &self.body[pat] { + self.resolve_path(pat.into(), path); + } + }); + self.resolver.reset_to_guard(guard); + } + _ => {} + } + + self.body.walk_child_exprs(expr, |expr| self.rename_conflicts(expr)); + } +} diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 11aeb21c77..7d7012c462 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -710,18 +710,22 @@ pub fn test_some_range(a: int) -> bool { Indel { insert: "let", delete: 45..47, + annotation: None, }, Indel { insert: "var_name", delete: 48..60, + annotation: None, }, Indel { insert: "=", delete: 61..81, + annotation: None, }, Indel { insert: "5;\n if let 2..6 = var_name {\n true\n } else {\n false\n }", delete: 82..108, + annotation: None, }, ], }, @@ -739,6 +743,8 @@ pub fn test_some_range(a: int) -> bool { }, file_system_edits: [], is_snippet: true, + annotations: {}, + next_annotation_id: 0, }, ), command: Some( @@ -839,18 +845,22 @@ pub fn test_some_range(a: int) -> bool { Indel { insert: "let", delete: 45..47, + annotation: None, }, Indel { insert: "var_name", delete: 48..60, + annotation: None, }, Indel { insert: "=", delete: 61..81, + annotation: None, }, Indel { insert: "5;\n if let 2..6 = var_name {\n true\n } else {\n false\n }", delete: 82..108, + annotation: None, }, ], }, @@ -868,6 +878,8 @@ pub fn test_some_range(a: int) -> bool { }, file_system_edits: [], is_snippet: true, + annotations: {}, + next_annotation_id: 0, }, ), command: Some( @@ -902,22 +914,27 @@ pub fn test_some_range(a: int) -> bool { Indel { insert: "const", delete: 45..47, + annotation: None, }, Indel { insert: "VAR_NAME:", delete: 48..60, + annotation: None, }, Indel { insert: "i32", delete: 61..81, + annotation: None, }, Indel { insert: "=", delete: 82..86, + annotation: None, }, Indel { insert: "5;\n if let 2..6 = VAR_NAME {\n true\n } else {\n false\n }", delete: 87..108, + annotation: None, }, ], }, @@ -935,6 +952,8 @@ pub fn test_some_range(a: int) -> bool { }, file_system_edits: [], is_snippet: true, + annotations: {}, + next_annotation_id: 0, }, ), command: Some( @@ -969,22 +988,27 @@ pub fn test_some_range(a: int) -> bool { Indel { insert: "static", delete: 45..47, + annotation: None, }, Indel { insert: "VAR_NAME:", delete: 48..60, + annotation: None, }, Indel { insert: "i32", delete: 61..81, + annotation: None, }, Indel { insert: "=", delete: 82..86, + annotation: None, }, Indel { insert: "5;\n if let 2..6 = VAR_NAME {\n true\n } else {\n false\n }", delete: 87..108, + annotation: None, }, ], }, @@ -1002,6 +1026,8 @@ pub fn test_some_range(a: int) -> bool { }, file_system_edits: [], is_snippet: true, + annotations: {}, + next_annotation_id: 0, }, ), command: Some( @@ -1036,10 +1062,12 @@ pub fn test_some_range(a: int) -> bool { Indel { insert: "fun_name()", delete: 59..60, + annotation: None, }, Indel { insert: "\n\nfn fun_name() -> i32 {\n 5\n}", delete: 110..110, + annotation: None, }, ], }, @@ -1057,6 +1085,8 @@ pub fn test_some_range(a: int) -> bool { }, file_system_edits: [], is_snippet: true, + annotations: {}, + next_annotation_id: 0, }, ), command: None, diff --git a/crates/ide-completion/src/render.rs b/crates/ide-completion/src/render.rs index a61389ac55..4d0a4a4782 100644 --- a/crates/ide-completion/src/render.rs +++ b/crates/ide-completion/src/render.rs @@ -2770,10 +2770,12 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 } Indel { insert: "(", delete: 107..107, + annotation: None, }, Indel { insert: "qux)()", delete: 109..110, + annotation: None, }, ], }, diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index 59914bedde..1633065f65 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -22,7 +22,10 @@ //! Our current behavior is ¯\_(ツ)_/¯. use std::fmt; -use crate::text_edit::{TextEdit, TextEditBuilder}; +use crate::{ + source_change::ChangeAnnotation, + text_edit::{TextEdit, TextEditBuilder}, +}; use base_db::AnchoredPathBuf; use either::Either; use hir::{FieldSource, FileRange, HirFileIdExt, InFile, ModuleSource, Semantics}; @@ -365,10 +368,12 @@ fn rename_reference( })); let mut insert_def_edit = |def| { - let (file_id, edit) = source_edit_from_def(sema, def, new_name)?; + let (file_id, edit) = source_edit_from_def(sema, def, new_name, &mut source_change)?; source_change.insert_source_edit(file_id, edit); Ok(()) }; + // This needs to come after the references edits, because we change the annotation of existing edits + // if a conflict is detected. insert_def_edit(def)?; Ok(source_change) } @@ -537,6 +542,7 @@ fn source_edit_from_def( sema: &Semantics<'_, RootDatabase>, def: Definition, new_name: &str, + source_change: &mut SourceChange, ) -> Result<(FileId, TextEdit)> { let new_name_edition_aware = |new_name: &str, file_id: EditionedFileId| { if is_raw_identifier(new_name, file_id.edition()) { @@ -548,6 +554,23 @@ fn source_edit_from_def( let mut edit = TextEdit::builder(); if let Definition::Local(local) = def { let mut file_id = None; + + let conflict_annotation = if !sema.rename_conflicts(&local, new_name).is_empty() { + Some( + source_change.insert_annotation(ChangeAnnotation { + label: "This rename will change the program's meaning".to_owned(), + needs_confirmation: true, + description: Some( + "Some variable(s) will shadow the renamed variable \ + or be shadowed by it if the rename is performed" + .to_owned(), + ), + }), + ) + } else { + None + }; + for source in local.sources(sema.db) { let source = match source.source.clone().original_ast_node_rooted(sema.db) { Some(source) => source, @@ -611,8 +634,15 @@ fn source_edit_from_def( } } } + let mut edit = edit.finish(); + + for (edit, _) in source_change.source_file_edits.values_mut() { + edit.set_annotation(conflict_annotation); + } + edit.set_annotation(conflict_annotation); + let Some(file_id) = file_id else { bail!("No file available to rename") }; - return Ok((EditionedFileId::file_id(file_id), edit.finish())); + return Ok((EditionedFileId::file_id(file_id), edit)); } let FileRange { file_id, range } = def .range_for_rename(sema) diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index 34642d7eaf..b4d0b0dc9f 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -3,7 +3,7 @@ //! //! It can be viewed as a dual for `Change`. -use std::{collections::hash_map::Entry, iter, mem}; +use std::{collections::hash_map::Entry, fmt, iter, mem}; use crate::text_edit::{TextEdit, TextEditBuilder}; use crate::{assists::Command, syntax_helpers::tree_diff::diff, SnippetCap}; @@ -18,23 +18,33 @@ use syntax::{ AstNode, SyntaxElement, SyntaxNode, SyntaxNodePtr, SyntaxToken, TextRange, TextSize, }; +/// An annotation ID associated with an indel, to describe changes. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct ChangeAnnotationId(u32); + +impl fmt::Display for ChangeAnnotationId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + +#[derive(Debug, Clone)] +pub struct ChangeAnnotation { + pub label: String, + pub needs_confirmation: bool, + pub description: Option<String>, +} + #[derive(Default, Debug, Clone)] pub struct SourceChange { pub source_file_edits: IntMap<FileId, (TextEdit, Option<SnippetEdit>)>, pub file_system_edits: Vec<FileSystemEdit>, pub is_snippet: bool, + pub annotations: FxHashMap<ChangeAnnotationId, ChangeAnnotation>, + next_annotation_id: u32, } impl SourceChange { - /// Creates a new SourceChange with the given label - /// from the edits. - pub fn from_edits( - source_file_edits: IntMap<FileId, (TextEdit, Option<SnippetEdit>)>, - file_system_edits: Vec<FileSystemEdit>, - ) -> Self { - SourceChange { source_file_edits, file_system_edits, is_snippet: false } - } - pub fn from_text_edit(file_id: impl Into<FileId>, edit: TextEdit) -> Self { SourceChange { source_file_edits: iter::once((file_id.into(), (edit, None))).collect(), @@ -42,6 +52,13 @@ impl SourceChange { } } + pub fn insert_annotation(&mut self, annotation: ChangeAnnotation) -> ChangeAnnotationId { + let id = ChangeAnnotationId(self.next_annotation_id); + self.next_annotation_id += 1; + self.annotations.insert(id, annotation); + id + } + /// Inserts a [`TextEdit`] for the given [`FileId`]. This properly handles merging existing /// edits for a file if some already exist. pub fn insert_source_edit(&mut self, file_id: impl Into<FileId>, edit: TextEdit) { @@ -120,7 +137,12 @@ impl From<IntMap<FileId, TextEdit>> for SourceChange { fn from(source_file_edits: IntMap<FileId, TextEdit>) -> SourceChange { let source_file_edits = source_file_edits.into_iter().map(|(file_id, edit)| (file_id, (edit, None))).collect(); - SourceChange { source_file_edits, file_system_edits: Vec::new(), is_snippet: false } + SourceChange { + source_file_edits, + file_system_edits: Vec::new(), + is_snippet: false, + ..SourceChange::default() + } } } @@ -482,6 +504,7 @@ impl From<FileSystemEdit> for SourceChange { source_file_edits: Default::default(), file_system_edits: vec![edit], is_snippet: false, + ..SourceChange::default() } } } diff --git a/crates/ide-db/src/text_edit.rs b/crates/ide-db/src/text_edit.rs index 0c675f0619..b59010f2f8 100644 --- a/crates/ide-db/src/text_edit.rs +++ b/crates/ide-db/src/text_edit.rs @@ -8,6 +8,8 @@ use itertools::Itertools; pub use span::{TextRange, TextSize}; use std::cmp::max; +use crate::source_change::ChangeAnnotationId; + /// `InsertDelete` -- a single "atomic" change to text /// /// Must not overlap with other `InDel`s @@ -16,6 +18,7 @@ pub struct Indel { pub insert: String, /// Refers to offsets in the original text pub delete: TextRange, + pub annotation: Option<ChangeAnnotationId>, } #[derive(Default, Debug, Clone)] @@ -37,7 +40,7 @@ impl Indel { Indel::replace(range, String::new()) } pub fn replace(range: TextRange, replace_with: String) -> Indel { - Indel { delete: range, insert: replace_with } + Indel { delete: range, insert: replace_with, annotation: None } } pub fn apply(&self, text: &mut String) { @@ -138,6 +141,14 @@ impl TextEdit { } Some(res) } + + pub fn set_annotation(&mut self, annotation: Option<ChangeAnnotationId>) { + if annotation.is_some() { + for indel in &mut self.indels { + indel.annotation = annotation; + } + } + } } impl IntoIterator for TextEdit { @@ -180,7 +191,7 @@ impl TextEditBuilder { pub fn invalidates_offset(&self, offset: TextSize) -> bool { self.indels.iter().any(|indel| indel.delete.contains_inclusive(offset)) } - fn indel(&mut self, indel: Indel) { + pub fn indel(&mut self, indel: Indel) { self.indels.push(indel); if self.indels.len() <= 16 { assert_disjoint_or_equal(&mut self.indels); diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 3e8295e3f0..d0e1c2097a 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -446,6 +446,7 @@ mod tests { use expect_test::{expect, Expect}; use ide_db::source_change::SourceChange; use ide_db::text_edit::TextEdit; + use itertools::Itertools; use stdx::trim_indent; use test_utils::assert_eq_text; @@ -496,6 +497,31 @@ mod tests { }; } + #[track_caller] + fn check_conflicts(new_name: &str, #[rust_analyzer::rust_fixture] ra_fixture: &str) { + let (analysis, position, conflicts) = fixture::annotations(ra_fixture); + let source_change = analysis.rename(position, new_name).unwrap().unwrap(); + let expected_conflicts = conflicts + .into_iter() + .map(|(file_range, _)| (file_range.file_id, file_range.range)) + .sorted_unstable_by_key(|(file_id, range)| (*file_id, range.start())) + .collect_vec(); + let found_conflicts = source_change + .source_file_edits + .iter() + .flat_map(|(file_id, (edit, _))| { + edit.into_iter() + .filter(|edit| edit.annotation.is_some()) + .map(move |edit| (*file_id, edit.delete)) + }) + .sorted_unstable_by_key(|(file_id, range)| (*file_id, range.start())) + .collect_vec(); + assert_eq!( + expected_conflicts, found_conflicts, + "rename conflicts mismatch: {source_change:#?}" + ); + } + fn check_expect( new_name: &str, #[rust_analyzer::rust_fixture] ra_fixture: &str, @@ -548,6 +574,37 @@ mod tests { } #[test] + fn rename_will_shadow() { + check_conflicts( + "new_name", + r#" +fn foo() { + let mut new_name = 123; + let old_name$0 = 456; + // ^^^^^^^^ + new_name = 789 + new_name; +} + "#, + ); + } + + #[test] + fn rename_will_be_shadowed() { + check_conflicts( + "new_name", + r#" +fn foo() { + let mut old_name$0 = 456; + // ^^^^^^^^ + let new_name = 123; + old_name = 789 + old_name; + // ^^^^^^^^ ^^^^^^^^ +} + "#, + ); + } + + #[test] fn test_prepare_rename_namelikes() { check_prepare(r"fn name$0<'lifetime>() {}", expect![[r#"3..7: name"#]]); check_prepare(r"fn name<'lifetime$0>() {}", expect![[r#"9..17: lifetime"#]]); @@ -1024,6 +1081,7 @@ mod foo$0; Indel { insert: "foo2", delete: 4..7, + annotation: None, }, ], ), @@ -1071,6 +1129,7 @@ use crate::foo$0::FooContent; Indel { insert: "quux", delete: 8..11, + annotation: None, }, ], ), @@ -1082,6 +1141,7 @@ use crate::foo$0::FooContent; Indel { insert: "quux", delete: 11..14, + annotation: None, }, ], ), @@ -1123,6 +1183,7 @@ mod fo$0o; Indel { insert: "foo2", delete: 4..7, + annotation: None, }, ], ), @@ -1171,6 +1232,7 @@ mod outer { mod fo$0o; } Indel { insert: "bar", delete: 16..19, + annotation: None, }, ], ), @@ -1242,6 +1304,7 @@ pub mod foo$0; Indel { insert: "foo2", delete: 27..30, + annotation: None, }, ], ), @@ -1253,6 +1316,7 @@ pub mod foo$0; Indel { insert: "foo2", delete: 8..11, + annotation: None, }, ], ), @@ -1308,6 +1372,7 @@ mod quux; Indel { insert: "foo2", delete: 4..7, + annotation: None, }, ], ), @@ -1441,10 +1506,12 @@ pub fn baz() {} Indel { insert: "r#fn", delete: 4..7, + annotation: None, }, Indel { insert: "r#fn", delete: 22..25, + annotation: None, }, ], ), @@ -1509,10 +1576,12 @@ pub fn baz() {} Indel { insert: "foo", delete: 4..8, + annotation: None, }, Indel { insert: "foo", delete: 23..27, + annotation: None, }, ], ), @@ -1574,6 +1643,7 @@ fn bar() { Indel { insert: "dyn", delete: 7..10, + annotation: None, }, ], ), @@ -1585,6 +1655,7 @@ fn bar() { Indel { insert: "r#dyn", delete: 18..21, + annotation: None, }, ], ), @@ -1614,6 +1685,7 @@ fn bar() { Indel { insert: "r#dyn", delete: 7..10, + annotation: None, }, ], ), @@ -1625,6 +1697,7 @@ fn bar() { Indel { insert: "dyn", delete: 18..21, + annotation: None, }, ], ), @@ -1654,6 +1727,7 @@ fn bar() { Indel { insert: "r#dyn", delete: 7..10, + annotation: None, }, ], ), @@ -1665,6 +1739,7 @@ fn bar() { Indel { insert: "dyn", delete: 18..21, + annotation: None, }, ], ), @@ -1701,10 +1776,12 @@ fn bar() { Indel { insert: "abc", delete: 7..10, + annotation: None, }, Indel { insert: "abc", delete: 32..35, + annotation: None, }, ], ), @@ -1716,6 +1793,7 @@ fn bar() { Indel { insert: "abc", delete: 18..23, + annotation: None, }, ], ), @@ -1749,10 +1827,12 @@ fn bar() { Indel { insert: "abc", delete: 7..12, + annotation: None, }, Indel { insert: "abc", delete: 34..39, + annotation: None, }, ], ), @@ -1764,6 +1844,7 @@ fn bar() { Indel { insert: "abc", delete: 18..21, + annotation: None, }, ], ), diff --git a/crates/ide/src/ssr.rs b/crates/ide/src/ssr.rs index 77a011cac1..90e350949b 100644 --- a/crates/ide/src/ssr.rs +++ b/crates/ide/src/ssr.rs @@ -139,6 +139,7 @@ mod tests { Indel { insert: "3", delete: 33..34, + annotation: None, }, ], }, @@ -147,6 +148,8 @@ mod tests { }, file_system_edits: [], is_snippet: false, + annotations: {}, + next_annotation_id: 0, }, ), command: None, @@ -179,6 +182,7 @@ mod tests { Indel { insert: "3", delete: 33..34, + annotation: None, }, ], }, @@ -192,6 +196,7 @@ mod tests { Indel { insert: "3", delete: 11..12, + annotation: None, }, ], }, @@ -200,6 +205,8 @@ mod tests { }, file_system_edits: [], is_snippet: false, + annotations: {}, + next_annotation_id: 0, }, ), command: None, diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 4ab96e9e2d..68b2d6b696 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -427,7 +427,12 @@ pub(crate) fn handle_on_enter( Some(it) => it, }; let line_index = snap.file_line_index(position.file_id)?; - let edit = to_proto::snippet_text_edit_vec(&line_index, true, edit); + let edit = to_proto::snippet_text_edit_vec( + &line_index, + true, + edit, + snap.config.change_annotation_support(), + ); Ok(Some(edit)) } @@ -464,7 +469,12 @@ pub(crate) fn handle_on_type_formatting( let (_, (text_edit, snippet_edit)) = edit.source_file_edits.into_iter().next().unwrap(); stdx::always!(snippet_edit.is_none(), "on type formatting shouldn't use structured snippets"); - let change = to_proto::snippet_text_edit_vec(&line_index, edit.is_snippet, text_edit); + let change = to_proto::snippet_text_edit_vec( + &line_index, + edit.is_snippet, + text_edit, + snap.config.change_annotation_support(), + ); Ok(Some(change)) } @@ -2025,7 +2035,12 @@ pub(crate) fn handle_move_item( match snap.analysis.move_item(range, direction)? { Some(text_edit) => { let line_index = snap.file_line_index(file_id)?; - Ok(to_proto::snippet_text_edit_vec(&line_index, true, text_edit)) + Ok(to_proto::snippet_text_edit_vec( + &line_index, + true, + text_edit, + snap.config.change_annotation_support(), + )) } None => Ok(vec![]), } diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index 3c206f47db..6db7bcb111 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -200,7 +200,10 @@ pub(crate) fn snippet_text_edit( line_index: &LineIndex, is_snippet: bool, indel: Indel, + client_supports_annotations: bool, ) -> lsp_ext::SnippetTextEdit { + let annotation_id = + indel.annotation.filter(|_| client_supports_annotations).map(|it| it.to_string()); let text_edit = text_edit(line_index, indel); let insert_text_format = if is_snippet { Some(lsp_types::InsertTextFormat::SNIPPET) } else { None }; @@ -208,7 +211,7 @@ pub(crate) fn snippet_text_edit( range: text_edit.range, new_text: text_edit.new_text, insert_text_format, - annotation_id: None, + annotation_id, } } @@ -223,10 +226,13 @@ pub(crate) fn snippet_text_edit_vec( line_index: &LineIndex, is_snippet: bool, text_edit: TextEdit, + clients_support_annotations: bool, ) -> Vec<lsp_ext::SnippetTextEdit> { text_edit .into_iter() - .map(|indel| self::snippet_text_edit(line_index, is_snippet, indel)) + .map(|indel| { + self::snippet_text_edit(line_index, is_snippet, indel, clients_support_annotations) + }) .collect() } @@ -1072,6 +1078,7 @@ fn merge_text_and_snippet_edits( line_index: &LineIndex, edit: TextEdit, snippet_edit: SnippetEdit, + client_supports_annotations: bool, ) -> Vec<SnippetTextEdit> { let mut edits: Vec<SnippetTextEdit> = vec![]; let mut snippets = snippet_edit.into_edit_ranges().into_iter().peekable(); @@ -1120,7 +1127,12 @@ fn merge_text_and_snippet_edits( edits.push(snippet_text_edit( line_index, true, - Indel { insert: format!("${snippet_index}"), delete: snippet_range }, + Indel { + insert: format!("${snippet_index}"), + delete: snippet_range, + annotation: None, + }, + client_supports_annotations, )) } @@ -1178,12 +1190,22 @@ fn merge_text_and_snippet_edits( edits.push(snippet_text_edit( line_index, true, - Indel { insert: new_text, delete: current_indel.delete }, + Indel { + insert: new_text, + delete: current_indel.delete, + annotation: current_indel.annotation, + }, + client_supports_annotations, )) } else { // snippet edit was beyond the current one // since it wasn't consumed, it's available for the next pass - edits.push(snippet_text_edit(line_index, false, current_indel)); + edits.push(snippet_text_edit( + line_index, + false, + current_indel, + client_supports_annotations, + )); } // update the final source -> initial source mapping offset @@ -1208,7 +1230,8 @@ fn merge_text_and_snippet_edits( snippet_text_edit( line_index, true, - Indel { insert: format!("${snippet_index}"), delete: snippet_range }, + Indel { insert: format!("${snippet_index}"), delete: snippet_range, annotation: None }, + client_supports_annotations, ) })); @@ -1224,10 +1247,13 @@ pub(crate) fn snippet_text_document_edit( ) -> Cancellable<lsp_ext::SnippetTextDocumentEdit> { let text_document = optional_versioned_text_document_identifier(snap, file_id); let line_index = snap.file_line_index(file_id)?; + let client_supports_annotations = snap.config.change_annotation_support(); let mut edits = if let Some(snippet_edit) = snippet_edit { - merge_text_and_snippet_edits(&line_index, edit, snippet_edit) + merge_text_and_snippet_edits(&line_index, edit, snippet_edit, client_supports_annotations) } else { - edit.into_iter().map(|it| snippet_text_edit(&line_index, is_snippet, it)).collect() + edit.into_iter() + .map(|it| snippet_text_edit(&line_index, is_snippet, it, client_supports_annotations)) + .collect() }; if snap.analysis.is_library_file(file_id)? && snap.config.change_annotation_support() { @@ -1348,6 +1374,16 @@ pub(crate) fn snippet_workspace_edit( )), }, )) + .chain(source_change.annotations.into_iter().map(|(id, annotation)| { + ( + id.to_string(), + lsp_types::ChangeAnnotation { + label: annotation.label, + description: annotation.description, + needs_confirmation: Some(annotation.needs_confirmation), + }, + ) + })) .collect(), ) } @@ -2023,7 +2059,7 @@ fn bar(_: usize) {} encoding: PositionEncoding::Utf8, }; - let res = merge_text_and_snippet_edits(&line_index, edit, snippets); + let res = merge_text_and_snippet_edits(&line_index, edit, snippets, true); // Ensure that none of the ranges overlap { |