Merge #8540

8540: Prevent being able to rename items that are not part of the workspace r=Veykril a=Veykril

This change causes renames that happen on items coming from crates outside the workspace to fail. I believe this should be the right approach, but usage of cargo's workspace might not be entirely correct for preventing these kinds of refactoring from touching things they shouldn't. I'm not entirely sure?

cc #6623, this is one of the bigger footguns when it comes to refactoring, especially in combination with import aliases people tend to rename items coming from a crates dependency which this prevents.

Co-authored-by: Lukas Wirth <[email protected]>

10 files changed, 101 insertions, 29 deletions

diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index b24c664ba..99e45633e 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -244,6 +244,12 @@ impl Analysis {
         self.with_db(|db| db.parse(file_id).tree())
     }
 
+    /// Returns true if this file belongs to an immutable library.
+    pub fn is_library_file(&self, file_id: FileId) -> Cancelable<bool> {
+        use ide_db::base_db::SourceDatabaseExt;
+        self.with_db(|db| db.source_root(db.file_source_root(file_id)).is_library)
+    }
+
     /// Gets the file's `LineIndex`: data structure to convert between absolute
     /// offsets and line/column representation.
     pub fn file_line_index(&self, file_id: FileId) -> Cancelable<Arc<LineIndex>> {
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 1edaa394a..7ddea22c8 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -400,6 +400,17 @@ impl Config {
     pub fn will_rename(&self) -> bool {
         try_or!(self.caps.workspace.as_ref()?.file_operations.as_ref()?.will_rename?, false)
     }
+    pub fn change_annotation_support(&self) -> bool {
+        try_!(self
+            .caps
+            .workspace
+            .as_ref()?
+            .workspace_edit
+            .as_ref()?
+            .change_annotation_support
+            .as_ref()?)
+        .is_some()
+    }
     pub fn code_action_resolve(&self) -> bool {
         try_or!(
             self.caps
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt b/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
index 23ec2efba..227d96d51 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
@@ -326,6 +326,7 @@
                             },
                         ),
                         document_changes: None,
+                        change_annotations: None,
                     },
                 ),
                 is_preferred: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
index b6acb5f42..f8adfad3b 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
@@ -179,6 +179,7 @@
                             },
                         ),
                         document_changes: None,
+                        change_annotations: None,
                     },
                 ),
                 is_preferred: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
index d765257c4..5a70d2ed7 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
@@ -179,6 +179,7 @@
                             },
                         ),
                         document_changes: None,
+                        change_annotations: None,
                     },
                 ),
                 is_preferred: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
index 6b0d94878..04ca0c9c2 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
@@ -179,6 +179,7 @@
                             },
                         ),
                         document_changes: None,
+                        change_annotations: None,
                     },
                 ),
                 is_preferred: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt b/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
index a0cfb8d33..57d2f1ae3 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
@@ -339,6 +339,7 @@
                             },
                         ),
                         document_changes: None,
+                        change_annotations: None,
                     },
                 ),
                 is_preferred: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs
index e2f319f6b..ca18997e4 100644
--- a/crates/rust-analyzer/src/diagnostics/to_proto.rs
+++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs
@@ -136,6 +136,7 @@ fn map_rust_child_diagnostic(
                     // FIXME: there's no good reason to use edit_map here....
                     changes: Some(edit_map),
                     document_changes: None,
+                    change_annotations: None,
                 }),
                 is_preferred: Some(true),
                 data: None,
diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs
index d648cda32..b8835a534 100644
--- a/crates/rust-analyzer/src/lsp_ext.rs
+++ b/crates/rust-analyzer/src/lsp_ext.rs
@@ -312,6 +312,9 @@ pub struct SnippetWorkspaceEdit {
     pub changes: Option<HashMap<lsp_types::Url, Vec<lsp_types::TextEdit>>>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub document_changes: Option<Vec<SnippetDocumentChangeOperation>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub change_annotations:
+        Option<HashMap<lsp_types::ChangeAnnotationIdentifier, lsp_types::ChangeAnnotation>>,
 }
 
 #[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
@@ -335,6 +338,9 @@ pub struct SnippetTextEdit {
     pub new_text: String,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub insert_text_format: Option<lsp_types::InsertTextFormat>,
+    /// The annotation id if this is an annotated
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub annotation_id: Option<lsp_types::ChangeAnnotationIdentifier>,
 }
 
 pub enum HoverRequest {}
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 1a1f65f3b..fe4d0733d 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -1,15 +1,16 @@
 //! Conversion of rust-analyzer specific types to lsp_types equivalents.
 use std::{
+    iter::once,
     path::{self, Path},
     sync::atomic::{AtomicU32, Ordering},
 };
 
 use ide::{
-    Annotation, AnnotationKind, Assist, AssistKind, CallInfo, CompletionItem, CompletionItemKind,
-    CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit, Fold, FoldKind,
-    Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, InlayHint, InlayKind,
-    InsertTextFormat, Markup, NavigationTarget, ReferenceAccess, RenameError, Runnable, Severity,
-    SourceChange, StructureNodeKind, SymbolKind, TextEdit, TextRange, TextSize,
+    Annotation, AnnotationKind, Assist, AssistKind, CallInfo, Cancelable, CompletionItem,
+    CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit,
+    Fold, FoldKind, Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, InlayHint,
+    InlayKind, InsertTextFormat, Markup, NavigationTarget, ReferenceAccess, RenameError, Runnable,
+    Severity, SourceChange, StructureNodeKind, SymbolKind, TextEdit, TextRange, TextSize,
 };
 use itertools::Itertools;
 use serde_json::to_value;
@@ -174,6 +175,7 @@ pub(crate) fn snippet_text_edit(
         range: text_edit.range,
         new_text: text_edit.new_text,
         insert_text_format,
+        annotation_id: None,
     }
 }
 
@@ -688,6 +690,10 @@ pub(crate) fn goto_definition_response(
     }
 }
 
+fn outside_workspace_annotation_id() -> String {
+    String::from("OutsideWorkspace")
+}
+
 pub(crate) fn snippet_text_document_edit(
     snap: &GlobalStateSnapshot,
     is_snippet: bool,
@@ -696,14 +702,21 @@ pub(crate) fn snippet_text_document_edit(
 ) -> Result<lsp_ext::SnippetTextDocumentEdit> {
     let text_document = optional_versioned_text_document_identifier(snap, file_id);
     let line_index = snap.file_line_index(file_id)?;
-    let edits = edit.into_iter().map(|it| snippet_text_edit(&line_index, is_snippet, it)).collect();
+    let mut edits: Vec<_> =
+        edit.into_iter().map(|it| snippet_text_edit(&line_index, is_snippet, it)).collect();
+
+    if snap.analysis.is_library_file(file_id)? && snap.config.change_annotation_support() {
+        for edit in &mut edits {
+            edit.annotation_id = Some(outside_workspace_annotation_id())
+        }
+    }
     Ok(lsp_ext::SnippetTextDocumentEdit { text_document, edits })
 }
 
 pub(crate) fn snippet_text_document_ops(
     snap: &GlobalStateSnapshot,
     file_system_edit: FileSystemEdit,
-) -> Vec<lsp_ext::SnippetDocumentChangeOperation> {
+) -> Cancelable<Vec<lsp_ext::SnippetDocumentChangeOperation>> {
     let mut ops = Vec::new();
     match file_system_edit {
         FileSystemEdit::CreateFile { dst, initial_contents } => {
@@ -721,6 +734,7 @@ pub(crate) fn snippet_text_document_ops(
                     range: lsp_types::Range::default(),
                     new_text: initial_contents,
                     insert_text_format: Some(lsp_types::InsertTextFormat::PlainText),
+                    annotation_id: None,
                 };
                 let edit_file =
                     lsp_ext::SnippetTextDocumentEdit { text_document, edits: vec![text_edit] };
@@ -730,16 +744,19 @@ pub(crate) fn snippet_text_document_ops(
         FileSystemEdit::MoveFile { src, dst } => {
             let old_uri = snap.file_id_to_url(src);
             let new_uri = snap.anchored_path(&dst);
-            let rename_file = lsp_types::ResourceOp::Rename(lsp_types::RenameFile {
-                old_uri,
-                new_uri,
-                options: None,
-                annotation_id: None,
-            });
-            ops.push(lsp_ext::SnippetDocumentChangeOperation::Op(rename_file))
+            let mut rename_file =
+                lsp_types::RenameFile { old_uri, new_uri, options: None, annotation_id: None };
+            if snap.analysis.is_library_file(src) == Ok(true)
+                && snap.config.change_annotation_support()
+            {
+                rename_file.annotation_id = Some(outside_workspace_annotation_id())
+            }
+            ops.push(lsp_ext::SnippetDocumentChangeOperation::Op(lsp_types::ResourceOp::Rename(
+                rename_file,
+            )))
         }
     }
-    ops
+    Ok(ops)
 }
 
 pub(crate) fn snippet_workspace_edit(
@@ -747,16 +764,35 @@ pub(crate) fn snippet_workspace_edit(
     source_change: SourceChange,
 ) -> Result<lsp_ext::SnippetWorkspaceEdit> {
     let mut document_changes: Vec<lsp_ext::SnippetDocumentChangeOperation> = Vec::new();
+
     for op in source_change.file_system_edits {
-        let ops = snippet_text_document_ops(snap, op);
+        let ops = snippet_text_document_ops(snap, op)?;
         document_changes.extend_from_slice(&ops);
     }
     for (file_id, edit) in source_change.source_file_edits {
         let edit = snippet_text_document_edit(&snap, source_change.is_snippet, file_id, edit)?;
         document_changes.push(lsp_ext::SnippetDocumentChangeOperation::Edit(edit));
     }
-    let workspace_edit =
-        lsp_ext::SnippetWorkspaceEdit { changes: None, document_changes: Some(document_changes) };
+    let mut workspace_edit = lsp_ext::SnippetWorkspaceEdit {
+        changes: None,
+        document_changes: Some(document_changes),
+        change_annotations: None,
+    };
+    if snap.config.change_annotation_support() {
+        workspace_edit.change_annotations = Some(
+            once((
+                outside_workspace_annotation_id(),
+                lsp_types::ChangeAnnotation {
+                    label: String::from("Edit outside of the workspace"),
+                    needs_confirmation: Some(true),
+                    description: Some(String::from(
+                        "This edit lies outside of the workspace and may affect dependencies",
+                    )),
+                },
+            ))
+            .collect(),
+        )
+    }
     Ok(workspace_edit)
 }
 
@@ -784,16 +820,7 @@ impl From<lsp_ext::SnippetWorkspaceEdit> for lsp_types::WorkspaceEdit {
                                 lsp_types::DocumentChangeOperation::Edit(
                                     lsp_types::TextDocumentEdit {
                                         text_document: edit.text_document,
-                                        edits: edit
-                                            .edits
-                                            .into_iter()
-                                            .map(|edit| {
-                                                lsp_types::OneOf::Left(lsp_types::TextEdit {
-                                                    range: edit.range,
-                                                    new_text: edit.new_text,
-                                                })
-                                            })
-                                            .collect(),
+                                        edits: edit.edits.into_iter().map(From::from).collect(),
                                     },
                                 )
                             }
@@ -801,7 +828,23 @@ impl From<lsp_ext::SnippetWorkspaceEdit> for lsp_types::WorkspaceEdit {
                         .collect(),
                 )
             }),
-            change_annotations: None,
+            change_annotations: snippet_workspace_edit.change_annotations,
+        }
+    }
+}
+
+impl From<lsp_ext::SnippetTextEdit>
+    for lsp_types::OneOf<lsp_types::TextEdit, lsp_types::AnnotatedTextEdit>
+{
+    fn from(
+        lsp_ext::SnippetTextEdit { annotation_id, insert_text_format:_, new_text, range }: lsp_ext::SnippetTextEdit,
+    ) -> Self {
+        match annotation_id {
+            Some(annotation_id) => lsp_types::OneOf::Right(lsp_types::AnnotatedTextEdit {
+                text_edit: lsp_types::TextEdit { range, new_text },
+                annotation_id,
+            }),
+            None => lsp_types::OneOf::Left(lsp_types::TextEdit { range, new_text }),
         }
     }
 }