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 <lukastw97@gmail.com>

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

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

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(

crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt

@@ -179,6 +179,7 @@
                             },
                         ),
                         document_changes: None,
+                        change_annotations: None,
                     },
                 ),
                 is_preferred: Some(

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(

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(

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(

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,

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

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,
+    Annotation, AnnotationKind, Assist, AssistKind, CallInfo, Cancelable, CompletionItem,
-    CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit, Fold, FoldKind,
+    CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit,
-    Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, InlayHint, InlayKind,
+    Fold, FoldKind, Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, InlayHint,
-    InsertTextFormat, Markup, NavigationTarget, ReferenceAccess, RenameError, Runnable, Severity,
+    InlayKind, InsertTextFormat, Markup, NavigationTarget, ReferenceAccess, RenameError, Runnable,
-    SourceChange, StructureNodeKind, SymbolKind, TextEdit, TextRange, TextSize,
+    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 {
+            let mut rename_file =
-                old_uri,
+                lsp_types::RenameFile { old_uri, new_uri, options: None, annotation_id: None };
-                new_uri,
+            if snap.analysis.is_library_file(src) == Ok(true)
-                options: None,
+                && snap.config.change_annotation_support()
-                annotation_id: None,
+            {
-            });
+                rename_file.annotation_id = Some(outside_workspace_annotation_id())
-            ops.push(lsp_ext::SnippetDocumentChangeOperation::Op(rename_file))
+            }
+            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 =
+    let mut workspace_edit = lsp_ext::SnippetWorkspaceEdit {
-        lsp_ext::SnippetWorkspaceEdit { changes: None, document_changes: Some(document_changes) };
+        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: edit.edits.into_iter().map(From::from).collect(),
-                                            .edits
-                                            .into_iter()
-                                            .map(|edit| {
-                                                lsp_types::OneOf::Left(lsp_types::TextEdit {
-                                                    range: edit.range,
-                                                    new_text: edit.new_text,
-                                                })
-                                            })
-                                            .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 }),
         }
     }
 }

docs/dev/lsp-extensions.md

@@ -1,5 +1,5 @@
 <!---
-lsp_ext.rs hash: b19ddc3ab8767af9
+lsp_ext.rs hash: 28a9d5a24b7ca396
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue:
@@ -46,6 +46,7 @@ If this capability is set, `WorkspaceEdit`s returned from `codeAction` requests
 ```typescript
 interface SnippetTextEdit extends TextEdit {
     insertTextFormat?: InsertTextFormat;
+    annotationId?: ChangeAnnotationIdentifier;
 }
 ```