From 2bf6b16a7f174ea3f581f26f642b4febff0b9ce8 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 18 May 2020 00:11:40 +0200 Subject: [PATCH] Server side of SnippetTextEdit --- crates/ra_assists/src/assist_context.rs | 10 -- crates/rust-analyzer/src/diagnostics.rs | 10 +- ..._to_proto__tests__snap_multi_line_fix.snap | 6 +- ...to__tests__snap_rustc_unused_variable.snap | 6 +- .../rust-analyzer/src/diagnostics/to_proto.rs | 20 +-- crates/rust-analyzer/src/lsp_ext.rs | 53 ++++++- crates/rust-analyzer/src/main_loop.rs | 2 +- .../rust-analyzer/src/main_loop/handlers.rs | 82 ++++------- crates/rust-analyzer/src/to_proto.rs | 130 ++++++++++++++---- 9 files changed, 200 insertions(+), 119 deletions(-) diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index b90bbf8b2e6..0dcd9df61fb 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -208,16 +208,6 @@ pub(crate) fn insert_snippet( pub(crate) fn replace(&mut self, range: TextRange, replace_with: impl Into) { self.edit.replace(range, replace_with.into()) } - /// Append specified `text` at the given `offset` - pub(crate) fn replace_snippet( - &mut self, - _cap: SnippetCap, - range: TextRange, - replace_with: impl Into, - ) { - self.is_snippet = true; - self.edit.replace(range, replace_with.into()) - } pub(crate) fn replace_ast(&mut self, old: N, new: N) { algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit) } diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs index 4bdd45a7deb..25856c5436b 100644 --- a/crates/rust-analyzer/src/diagnostics.rs +++ b/crates/rust-analyzer/src/diagnostics.rs @@ -3,9 +3,11 @@ use std::{collections::HashMap, sync::Arc}; -use lsp_types::{CodeActionOrCommand, Diagnostic, Range}; +use lsp_types::{Diagnostic, Range}; use ra_ide::FileId; +use crate::lsp_ext; + pub type CheckFixes = Arc>>; #[derive(Debug, Default, Clone)] @@ -18,13 +20,13 @@ pub struct DiagnosticCollection { #[derive(Debug, Clone)] pub struct Fix { pub range: Range, - pub action: CodeActionOrCommand, + pub action: lsp_ext::CodeAction, } #[derive(Debug)] pub enum DiagnosticTask { ClearCheck, - AddCheck(FileId, Diagnostic, Vec), + AddCheck(FileId, Diagnostic, Vec), SetNative(FileId, Vec), } @@ -38,7 +40,7 @@ pub fn add_check_diagnostic( &mut self, file_id: FileId, diagnostic: Diagnostic, - fixes: Vec, + fixes: Vec, ) { let diagnostics = self.check.entry(file_id).or_default(); for existing_diagnostic in diagnostics.iter() { diff --git a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap index 076b3cf2730..96466b5c90b 100644 --- a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap +++ b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap @@ -68,9 +68,9 @@ expression: diag kind: Some( "quickfix", ), - diagnostics: None, + command: None, edit: Some( - WorkspaceEdit { + SnippetWorkspaceEdit { changes: Some( { "file:///test/src/main.rs": [ @@ -106,8 +106,6 @@ expression: diag document_changes: None, }, ), - command: None, - is_preferred: None, }, ], }, diff --git a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap index 69138c15b27..8f962277f07 100644 --- a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap +++ b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap @@ -53,9 +53,9 @@ expression: diag kind: Some( "quickfix", ), - diagnostics: None, + command: None, edit: Some( - WorkspaceEdit { + SnippetWorkspaceEdit { changes: Some( { "file:///test/driver/subcommand/repl.rs": [ @@ -78,8 +78,6 @@ expression: diag document_changes: None, }, ), - command: None, - is_preferred: None, }, ], }, diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs index eabf4908ff1..afea5952546 100644 --- a/crates/rust-analyzer/src/diagnostics/to_proto.rs +++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs @@ -7,13 +7,13 @@ }; use lsp_types::{ - CodeAction, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, - Location, NumberOrString, Position, Range, TextEdit, Url, WorkspaceEdit, + Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, + NumberOrString, Position, Range, TextEdit, Url, }; use ra_flycheck::{Applicability, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion}; use stdx::format_to; -use crate::Result; +use crate::{lsp_ext, Result}; /// Converts a Rust level string to a LSP severity fn map_level_to_severity(val: DiagnosticLevel) -> Option { @@ -110,7 +110,7 @@ fn is_deprecated(rd: &ra_flycheck::Diagnostic) -> bool { enum MappedRustChildDiagnostic { Related(DiagnosticRelatedInformation), - SuggestedFix(CodeAction), + SuggestedFix(lsp_ext::CodeAction), MessageLine(String), } @@ -143,13 +143,15 @@ fn map_rust_child_diagnostic( message: rd.message.clone(), }) } else { - MappedRustChildDiagnostic::SuggestedFix(CodeAction { + MappedRustChildDiagnostic::SuggestedFix(lsp_ext::CodeAction { title: rd.message.clone(), kind: Some("quickfix".to_string()), - diagnostics: None, - edit: Some(WorkspaceEdit::new(edit_map)), + edit: Some(lsp_ext::SnippetWorkspaceEdit { + // FIXME: there's no good reason to use edit_map here.... + changes: Some(edit_map), + document_changes: None, + }), command: None, - is_preferred: None, }) } } @@ -158,7 +160,7 @@ fn map_rust_child_diagnostic( pub(crate) struct MappedRustDiagnostic { pub location: Location, pub diagnostic: Diagnostic, - pub fixes: Vec, + pub fixes: Vec, } /// Converts a Rust root diagnostic to LSP form diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 313a8c7697e..c7a3a691149 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -1,6 +1,6 @@ //! rust-analyzer extensions to the LSP. -use std::path::PathBuf; +use std::{collections::HashMap, path::PathBuf}; use lsp_types::request::Request; use lsp_types::{Location, Position, Range, TextDocumentIdentifier}; @@ -137,7 +137,7 @@ pub struct Runnable { #[serde(rename_all = "camelCase")] pub struct SourceChange { pub label: String, - pub workspace_edit: lsp_types::WorkspaceEdit, + pub workspace_edit: SnippetWorkspaceEdit, pub cursor_position: Option, } @@ -183,3 +183,52 @@ pub struct SsrParams { pub query: String, pub parse_only: bool, } + +pub enum CodeActionRequest {} + +impl Request for CodeActionRequest { + type Params = lsp_types::CodeActionParams; + type Result = Option>; + const METHOD: &'static str = "textDocument/codeAction"; +} + +#[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] +pub struct CodeAction { + pub title: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub kind: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub command: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub edit: Option, +} + +#[derive(Debug, Eq, PartialEq, Clone, Default, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SnippetWorkspaceEdit { + pub changes: Option>>, + pub document_changes: Option>, +} + +#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)] +#[serde(untagged, rename_all = "lowercase")] +pub enum SnippetDocumentChangeOperation { + Op(lsp_types::ResourceOp), + Edit(SnippetTextDocumentEdit), +} + +#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SnippetTextDocumentEdit { + pub text_document: lsp_types::VersionedTextDocumentIdentifier, + pub edits: Vec, +} + +#[derive(Debug, Eq, PartialEq, Clone, Default, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SnippetTextEdit { + pub range: Range, + pub new_text: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub insert_text_format: Option, +} diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 15e5bb35496..87795fffbd6 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -518,6 +518,7 @@ fn on_request( .on::(handlers::handle_parent_module)? .on::(handlers::handle_runnables)? .on::(handlers::handle_inlay_hints)? + .on::(handlers::handle_code_action)? .on::(handlers::handle_on_type_formatting)? .on::(handlers::handle_document_symbol)? .on::(handlers::handle_workspace_symbol)? @@ -525,7 +526,6 @@ fn on_request( .on::(handlers::handle_goto_implementation)? .on::(handlers::handle_goto_type_definition)? .on::(handlers::handle_completion)? - .on::(handlers::handle_code_action)? .on::(handlers::handle_code_lens)? .on::(handlers::handle_code_lens_resolve)? .on::(handlers::handle_folding_range)? diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 13ae061fa0f..4ff8fa69e92 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -11,12 +11,11 @@ use lsp_types::{ CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, - CodeAction, CodeActionResponse, CodeLens, Command, CompletionItem, Diagnostic, - DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange, FoldingRangeParams, - Hover, HoverContents, Location, MarkupContent, MarkupKind, Position, PrepareRenameResponse, - Range, RenameParams, SemanticTokensParams, SemanticTokensRangeParams, - SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation, TextDocumentIdentifier, - TextEdit, Url, WorkspaceEdit, + CodeLens, Command, CompletionItem, Diagnostic, DocumentFormattingParams, DocumentHighlight, + DocumentSymbol, FoldingRange, FoldingRangeParams, Hover, HoverContents, Location, + MarkupContent, MarkupKind, Position, PrepareRenameResponse, Range, RenameParams, + SemanticTokensParams, SemanticTokensRangeParams, SemanticTokensRangeResult, + SemanticTokensResult, SymbolInformation, TextDocumentIdentifier, TextEdit, Url, WorkspaceEdit, }; use ra_ide::{ Assist, FileId, FilePosition, FileRange, Query, RangeInfo, Runnable, RunnableKind, SearchScope, @@ -585,9 +584,8 @@ pub fn handle_rename(world: WorldSnapshot, params: RenameParams) -> Result return Ok(None), Some(it) => it.info, }; - - let source_change = to_proto::source_change(&world, source_change)?; - Ok(Some(source_change.workspace_edit)) + let workspace_edit = to_proto::workspace_edit(&world, source_change)?; + Ok(Some(workspace_edit)) } pub fn handle_references( @@ -696,14 +694,21 @@ pub fn handle_formatting( pub fn handle_code_action( world: WorldSnapshot, params: lsp_types::CodeActionParams, -) -> Result> { +) -> Result>> { let _p = profile("handle_code_action"); + // We intentionally don't support command-based actions, as those either + // requires custom client-code anyway, or requires server-initiated edits. + // Server initiated edits break causality, so we avoid those as well. + if !world.config.client_caps.code_action_literals { + return Ok(None); + } + let file_id = from_proto::file_id(&world, ¶ms.text_document.uri)?; let line_index = world.analysis().file_line_index(file_id)?; let range = from_proto::text_range(&line_index, params.range); let diagnostics = world.analysis().diagnostics(file_id)?; - let mut res = CodeActionResponse::default(); + let mut res: Vec = Vec::new(); let fixes_from_diagnostics = diagnostics .into_iter() @@ -713,22 +718,9 @@ pub fn handle_code_action( for source_edit in fixes_from_diagnostics { let title = source_edit.label.clone(); - let edit = to_proto::source_change(&world, source_edit)?; - - let command = Command { - title, - command: "rust-analyzer.applySourceChange".to_string(), - arguments: Some(vec![to_value(edit).unwrap()]), - }; - let action = CodeAction { - title: command.title.clone(), - kind: None, - diagnostics: None, - edit: None, - command: Some(command), - is_preferred: None, - }; - res.push(action.into()); + let edit = to_proto::snippet_workspace_edit(&world, source_edit)?; + let action = lsp_ext::CodeAction { title, kind: None, edit: Some(edit), command: None }; + res.push(action); } for fix in world.check_fixes.get(&file_id).into_iter().flatten() { @@ -748,8 +740,13 @@ pub fn handle_code_action( .entry(label.to_owned()) .or_insert_with(|| { let idx = res.len(); - let dummy = Command::new(String::new(), String::new(), None); - res.push(dummy.into()); + let dummy = lsp_ext::CodeAction { + title: String::new(), + kind: None, + command: None, + edit: None, + }; + res.push(dummy); (idx, Vec::new()) }) .1 @@ -777,35 +774,10 @@ pub fn handle_code_action( command: "rust-analyzer.selectAndApplySourceChange".to_string(), arguments: Some(vec![serde_json::Value::Array(arguments)]), }); - res[idx] = CodeAction { - title, - kind: None, - diagnostics: None, - edit: None, - command, - is_preferred: None, - } - .into(); + res[idx] = lsp_ext::CodeAction { title, kind: None, edit: None, command }; } } - // If the client only supports commands then filter the list - // and remove and actions that depend on edits. - if !world.config.client_caps.code_action_literals { - // FIXME: use drain_filter once it hits stable. - res = res - .into_iter() - .filter_map(|it| match it { - cmd @ lsp_types::CodeActionOrCommand::Command(_) => Some(cmd), - lsp_types::CodeActionOrCommand::CodeAction(action) => match action.command { - Some(cmd) if action.edit.is_none() => { - Some(lsp_types::CodeActionOrCommand::Command(cmd)) - } - _ => None, - }, - }) - .collect(); - } Ok(Some(res)) } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index a8e2e535f9a..2b1a3378f8a 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -112,6 +112,22 @@ pub(crate) fn text_edit( lsp_types::TextEdit { range, new_text } } +pub(crate) fn snippet_text_edit( + line_index: &LineIndex, + line_endings: LineEndings, + is_snippet: bool, + indel: Indel, +) -> lsp_ext::SnippetTextEdit { + let text_edit = text_edit(line_index, line_endings, indel); + let insert_text_format = + if is_snippet { Some(lsp_types::InsertTextFormat::Snippet) } else { None }; + lsp_ext::SnippetTextEdit { + range: text_edit.range, + new_text: text_edit.new_text, + insert_text_format, + } +} + pub(crate) fn text_edit_vec( line_index: &LineIndex, line_endings: LineEndings, @@ -441,10 +457,11 @@ pub(crate) fn goto_definition_response( } } -pub(crate) fn text_document_edit( +pub(crate) fn snippet_text_document_edit( world: &WorldSnapshot, + is_snippet: bool, source_file_edit: SourceFileEdit, -) -> Result { +) -> Result { let text_document = versioned_text_document_identifier(world, source_file_edit.file_id, None)?; let line_index = world.analysis().file_line_index(source_file_edit.file_id)?; let line_endings = world.file_line_endings(source_file_edit.file_id); @@ -452,9 +469,9 @@ pub(crate) fn text_document_edit( .edit .as_indels() .iter() - .map(|it| text_edit(&line_index, line_endings, it.clone())) + .map(|it| snippet_text_edit(&line_index, line_endings, is_snippet, it.clone())) .collect(); - Ok(lsp_types::TextDocumentEdit { text_document, edits }) + Ok(lsp_ext::SnippetTextDocumentEdit { text_document, edits }) } pub(crate) fn resource_op( @@ -500,20 +517,70 @@ pub(crate) fn source_change( }) } }; - let mut document_changes: Vec = Vec::new(); + let label = source_change.label.clone(); + let workspace_edit = self::snippet_workspace_edit(world, source_change)?; + Ok(lsp_ext::SourceChange { label, workspace_edit, cursor_position }) +} + +pub(crate) fn snippet_workspace_edit( + world: &WorldSnapshot, + source_change: SourceChange, +) -> Result { + let mut document_changes: Vec = Vec::new(); for op in source_change.file_system_edits { let op = resource_op(&world, op)?; - document_changes.push(lsp_types::DocumentChangeOperation::Op(op)); + document_changes.push(lsp_ext::SnippetDocumentChangeOperation::Op(op)); } for edit in source_change.source_file_edits { - let edit = text_document_edit(&world, edit)?; - document_changes.push(lsp_types::DocumentChangeOperation::Edit(edit)); + let edit = snippet_text_document_edit(&world, source_change.is_snippet, edit)?; + document_changes.push(lsp_ext::SnippetDocumentChangeOperation::Edit(edit)); + } + let workspace_edit = + lsp_ext::SnippetWorkspaceEdit { changes: None, document_changes: Some(document_changes) }; + Ok(workspace_edit) +} + +pub(crate) fn workspace_edit( + world: &WorldSnapshot, + source_change: SourceChange, +) -> Result { + assert!(!source_change.is_snippet); + snippet_workspace_edit(world, source_change).map(|it| it.into()) +} + +impl From for lsp_types::WorkspaceEdit { + fn from(snippet_workspace_edit: lsp_ext::SnippetWorkspaceEdit) -> lsp_types::WorkspaceEdit { + lsp_types::WorkspaceEdit { + changes: None, + document_changes: snippet_workspace_edit.document_changes.map(|changes| { + lsp_types::DocumentChanges::Operations( + changes + .into_iter() + .map(|change| match change { + lsp_ext::SnippetDocumentChangeOperation::Op(op) => { + lsp_types::DocumentChangeOperation::Op(op) + } + lsp_ext::SnippetDocumentChangeOperation::Edit(edit) => { + lsp_types::DocumentChangeOperation::Edit( + lsp_types::TextDocumentEdit { + text_document: edit.text_document, + edits: edit + .edits + .into_iter() + .map(|edit| lsp_types::TextEdit { + range: edit.range, + new_text: edit.new_text, + }) + .collect(), + }, + ) + } + }) + .collect(), + ) + }), + } } - let workspace_edit = lsp_types::WorkspaceEdit { - changes: None, - document_changes: Some(lsp_types::DocumentChanges::Operations(document_changes)), - }; - Ok(lsp_ext::SourceChange { label: source_change.label, workspace_edit, cursor_position }) } pub fn call_hierarchy_item( @@ -571,22 +638,25 @@ fn main() { } } -pub(crate) fn code_action(world: &WorldSnapshot, assist: Assist) -> Result { - let source_change = source_change(&world, assist.source_change)?; - let arg = serde_json::to_value(source_change)?; - let title = assist.label; - let command = lsp_types::Command { - title: title.clone(), - command: "rust-analyzer.applySourceChange".to_string(), - arguments: Some(vec![arg]), - }; +pub(crate) fn code_action(world: &WorldSnapshot, assist: Assist) -> Result { + let res = if assist.source_change.is_snippet { + lsp_ext::CodeAction { + title: assist.label, + kind: Some(String::new()), + edit: Some(snippet_workspace_edit(world, assist.source_change)?), + command: None, + } + } else { + let source_change = source_change(&world, assist.source_change)?; + let arg = serde_json::to_value(source_change)?; + let title = assist.label; + let command = lsp_types::Command { + title: title.clone(), + command: "rust-analyzer.applySourceChange".to_string(), + arguments: Some(vec![arg]), + }; - Ok(lsp_types::CodeAction { - title, - kind: Some(String::new()), - diagnostics: None, - edit: None, - command: Some(command), - is_preferred: None, - }) + lsp_ext::CodeAction { title, kind: Some(String::new()), edit: None, command: Some(command) } + }; + Ok(res) }