itsscb/rust-analyzer
1
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer.git synced 2025-09-28 11:20:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Warn the user when a rename will change the meaning of the program

Browse Source 
Specifically, when a rename of a local will change some code that refers it to refer another local, or some code that refer another local to refer to it.

We do it by introducing a dummy edit with an annotation. I'm not a fond of this approach, but I don't think LSP has a better way.
This commit is contained in:
Chayim Refael Friedman 2025-02-02 01:49:04 +02:00
parent bd0289e0e9
commit 62e7d2851b
12 changed files with 509 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
crates/hir-def/src/path.rs
View File

@ -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,
}

166
crates/hir-def/src/resolver.rs
View File

@ -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 {

95
crates/hir/src/semantics.rs
View File

@ -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));
}
}

30
crates/ide-assists/src/tests.rs
View File

@ -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,

2
crates/ide-completion/src/render.rs
View File

@ -2686,10 +2686,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,
},
],
},

36
crates/ide-db/src/rename.rs
View File

@ -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)

45
crates/ide-db/src/source_change.rs
View File

@ -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()
}
}
}

15
crates/ide-db/src/text_edit.rs
View File

@ -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);

81
crates/ide/src/rename.rs
View File

@ -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,
@ -547,6 +573,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"#]]);
@ -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,
},
],
),

7
crates/ide/src/ssr.rs
View File

@ -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,

21
crates/rust-analyzer/src/handlers/request.rs
View File

@ -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![]),
}

54
crates/rust-analyzer/src/lsp/to_proto.rs
View File

@ -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
{