From c447a795abecbf9a4138778bab44197250b2dc4a Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 16 Apr 2021 17:31:47 +0200 Subject: Prevent being able to rename items that are not part of the workspace --- crates/ide/src/lib.rs | 6 ++ crates/rust-analyzer/src/config.rs | 11 +++ .../diagnostics/test_data/clippy_pass_by_ref.txt | 1 + .../test_data/rustc_unused_variable.txt | 1 + .../test_data/rustc_unused_variable_as_hint.txt | 1 + .../test_data/rustc_unused_variable_as_info.txt | 1 + .../diagnostics/test_data/snap_multi_line_fix.txt | 1 + crates/rust-analyzer/src/diagnostics/to_proto.rs | 1 + crates/rust-analyzer/src/lsp_ext.rs | 6 ++ crates/rust-analyzer/src/to_proto.rs | 78 +++++++++++++++++----- 10 files changed, 92 insertions(+), 15 deletions(-) (limited to 'crates') 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 { + 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> { 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>>, #[serde(skip_serializing_if = "Option::is_none")] pub document_changes: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub change_annotations: + Option>, } #[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, + /// The annotation id if this is an annotated + #[serde(skip_serializing_if = "Option::is_none")] + pub annotation_id: Option, } pub enum HoverRequest {} diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 1a1f65f3b..13de11df1 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -1,5 +1,6 @@ //! Conversion of rust-analyzer specific types to lsp_types equivalents. use std::{ + collections::HashMap, path::{self, Path}, sync::atomic::{AtomicU32, Ordering}, }; @@ -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(snap: &GlobalStateSnapshot) -> Option { + snap.config.change_annotation_support().then(|| String::from("OutsideWorkspace")) +} + pub(crate) fn snippet_text_document_edit( snap: &GlobalStateSnapshot, is_snippet: bool, @@ -696,7 +702,19 @@ pub(crate) fn snippet_text_document_edit( ) -> Result { 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 outside_workspace_annotation = snap + .analysis + .is_library_file(file_id)? + .then(|| outside_workspace_annotation(snap)) + .flatten(); + let edits = edit + .into_iter() + .map(|it| { + let mut edit = snippet_text_edit(&line_index, is_snippet, it); + edit.annotation_id = outside_workspace_annotation.clone(); + edit + }) + .collect(); Ok(lsp_ext::SnippetTextDocumentEdit { text_document, edits }) } @@ -721,6 +739,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] }; @@ -734,7 +753,12 @@ pub(crate) fn snippet_text_document_ops( old_uri, new_uri, options: None, - annotation_id: None, + annotation_id: snap + .analysis + .is_library_file(src) + .unwrap() + .then(|| outside_workspace_annotation(snap)) + .flatten(), }); ops.push(lsp_ext::SnippetDocumentChangeOperation::Op(rename_file)) } @@ -747,6 +771,7 @@ pub(crate) fn snippet_workspace_edit( source_change: SourceChange, ) -> Result { let mut document_changes: Vec = Vec::new(); + for op in source_change.file_system_edits { let ops = snippet_text_document_ops(snap, op); document_changes.extend_from_slice(&ops); @@ -755,8 +780,24 @@ pub(crate) fn snippet_workspace_edit( 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 change_annotations = outside_workspace_annotation(snap).map(|annotation| { + use std::iter::FromIterator; + HashMap::from_iter(Some(( + annotation, + 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", + )), + }, + ))) + }); + let workspace_edit = lsp_ext::SnippetWorkspaceEdit { + changes: None, + document_changes: Some(document_changes), + change_annotations, + }; Ok(workspace_edit) } @@ -784,16 +825,7 @@ impl From 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 +833,23 @@ impl From for lsp_types::WorkspaceEdit { .collect(), ) }), - change_annotations: None, + change_annotations: snippet_workspace_edit.change_annotations, + } + } +} + +impl From + for lsp_types::OneOf +{ + 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 }), } } } -- cgit v1.2.3 From 493aaa140325f3b8fa40de3de58b34e4b96c5d13 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 19 Apr 2021 00:14:21 +0200 Subject: Better visualise control flow for change_annotation_support" --- crates/rust-analyzer/src/to_proto.rs | 97 +++++++++++++++++------------------- 1 file changed, 46 insertions(+), 51 deletions(-) (limited to 'crates') diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 13de11df1..fe4d0733d 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -1,16 +1,16 @@ //! Conversion of rust-analyzer specific types to lsp_types equivalents. use std::{ - collections::HashMap, + 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; @@ -690,8 +690,8 @@ pub(crate) fn goto_definition_response( } } -fn outside_workspace_annotation(snap: &GlobalStateSnapshot) -> Option { - snap.config.change_annotation_support().then(|| String::from("OutsideWorkspace")) +fn outside_workspace_annotation_id() -> String { + String::from("OutsideWorkspace") } pub(crate) fn snippet_text_document_edit( @@ -702,26 +702,21 @@ pub(crate) fn snippet_text_document_edit( ) -> Result { let text_document = optional_versioned_text_document_identifier(snap, file_id); let line_index = snap.file_line_index(file_id)?; - let outside_workspace_annotation = snap - .analysis - .is_library_file(file_id)? - .then(|| outside_workspace_annotation(snap)) - .flatten(); - let edits = edit - .into_iter() - .map(|it| { - let mut edit = snippet_text_edit(&line_index, is_snippet, it); - edit.annotation_id = outside_workspace_annotation.clone(); - edit - }) - .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 { +) -> Cancelable> { let mut ops = Vec::new(); match file_system_edit { FileSystemEdit::CreateFile { dst, initial_contents } => { @@ -749,21 +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: snap - .analysis - .is_library_file(src) - .unwrap() - .then(|| outside_workspace_annotation(snap)) - .flatten(), - }); - 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( @@ -773,31 +766,33 @@ pub(crate) fn snippet_workspace_edit( let mut document_changes: Vec = 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 change_annotations = outside_workspace_annotation(snap).map(|annotation| { - use std::iter::FromIterator; - HashMap::from_iter(Some(( - annotation, - 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", - )), - }, - ))) - }); - let workspace_edit = lsp_ext::SnippetWorkspaceEdit { + let mut workspace_edit = lsp_ext::SnippetWorkspaceEdit { changes: None, document_changes: Some(document_changes), - change_annotations, + 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) } -- cgit v1.2.3