Merge #1504

1504: Simplify LSP handlers r=matklad a=kjeremy

Takes advantage of protocol inheritance via composition and simplifies some responses via the `From`/`Into` traits.

Co-authored-by: Jeremy Kolb <[email protected]>

4 files changed, 30 insertions, 38 deletions

diff --git a/crates/gen_lsp_server/Cargo.toml b/crates/gen_lsp_server/Cargo.toml
index bf57df81f..36a29265c 100644
--- a/crates/gen_lsp_server/Cargo.toml
+++ b/crates/gen_lsp_server/Cargo.toml
@@ -8,7 +8,7 @@ license = "MIT OR Apache-2.0"
 description = "Generic LSP server scaffold."
 
 [dependencies]
-lsp-types = "0.58.0"
+lsp-types = "0.59.0"
 log = "0.4.3"
 serde_json = "1.0.34"
 serde = { version = "1.0.83", features = ["derive"] }
diff --git a/crates/ra_lsp_server/Cargo.toml b/crates/ra_lsp_server/Cargo.toml
index 2c4b3789e..50bc8111b 100644
--- a/crates/ra_lsp_server/Cargo.toml
+++ b/crates/ra_lsp_server/Cargo.toml
@@ -13,7 +13,7 @@ crossbeam-channel = "0.3.5"
 flexi_logger = "0.13.0"
 log = "0.4.3"
 url_serde = "0.2.0"
-lsp-types = { version = "0.58.0", features = ["proposed"] }
+lsp-types = { version = "0.59.0", features = ["proposed"] }
 rustc-hash = "1.0"
 parking_lot = "0.8.0"
 
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index a8d6f7c23..62c8cbf71 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -2,11 +2,11 @@ use std::{fmt::Write as _, io::Write as _};
 
 use gen_lsp_server::ErrorCode;
 use lsp_types::{
-    CodeAction, CodeActionResponse, CodeLens, Command, Diagnostic, DiagnosticSeverity,
-    DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange, FoldingRangeKind,
-    FoldingRangeParams, Hover, HoverContents, Location, MarkupContent, MarkupKind, Position,
-    PrepareRenameResponse, Range, RenameParams, SymbolInformation, TextDocumentIdentifier,
-    TextEdit, WorkspaceEdit,
+    CodeAction, CodeActionResponse, CodeLens, Command, CompletionItem, Diagnostic,
+    DiagnosticSeverity, DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange,
+    FoldingRangeKind, FoldingRangeParams, Hover, HoverContents, Location, MarkupContent,
+    MarkupKind, Position, PrepareRenameResponse, Range, RenameParams, SymbolInformation,
+    TextDocumentIdentifier, TextEdit, WorkspaceEdit,
 };
 use ra_ide_api::{
     AssistId, Cancelable, FileId, FilePosition, FileRange, FoldKind, Query, RangeInfo,
@@ -153,14 +153,12 @@ pub fn handle_on_type_formatting(
     params: req::DocumentOnTypeFormattingParams,
 ) -> Result<Option<Vec<TextEdit>>> {
     let _p = profile("handle_on_type_formatting");
-    let file_id = params.text_document.try_conv_with(&world)?;
-    let line_index = world.analysis().file_line_index(file_id);
-    let position = FilePosition {
-        file_id,
-        /// in `ra_ide_api`, the `on_type` invariant is that
-        /// `text.char_at(position) == typed_char`.
-        offset: params.position.conv_with(&line_index) - TextUnit::of_char('.'),
-    };
+    let mut position = params.text_document_position.try_conv_with(&world)?;
+    let line_index = world.analysis().file_line_index(position.file_id);
+
+    // in `ra_ide_api`, the `on_type` invariant is that
+    // `text.char_at(position) == typed_char`.
+    position.offset = position.offset - TextUnit::of_char('.');
 
     let edit = match params.ch.as_str() {
         "=" => world.analysis().on_eq_typed(position),
@@ -214,7 +212,7 @@ pub fn handle_document_symbol(
         }
     }
 
-    Ok(Some(req::DocumentSymbolResponse::Nested(res)))
+    Ok(Some(res.into()))
 }
 
 pub fn handle_workspace_symbol(
@@ -277,7 +275,7 @@ pub fn handle_goto_definition(
         .map(|nav| RangeInfo::new(nav_range, nav))
         .map(|nav| to_location_link(&nav, &world, &line_index))
         .collect::<Result<Vec<_>>>()?;
-    Ok(Some(req::GotoDefinitionResponse::Link(res)))
+    Ok(Some(res.into()))
 }
 
 pub fn handle_goto_implementation(
@@ -297,7 +295,7 @@ pub fn handle_goto_implementation(
         .map(|nav| RangeInfo::new(nav_range, nav))
         .map(|nav| to_location_link(&nav, &world, &line_index))
         .collect::<Result<Vec<_>>>()?;
-    Ok(Some(req::GotoDefinitionResponse::Link(res)))
+    Ok(Some(res.into()))
 }
 
 pub fn handle_goto_type_definition(
@@ -317,7 +315,7 @@ pub fn handle_goto_type_definition(
         .map(|nav| RangeInfo::new(nav_range, nav))
         .map(|nav| to_location_link(&nav, &world, &line_index))
         .collect::<Result<Vec<_>>>()?;
-    Ok(Some(req::GotoDefinitionResponse::Link(res)))
+    Ok(Some(res.into()))
 }
 
 pub fn handle_parent_module(
@@ -407,12 +405,7 @@ pub fn handle_completion(
     params: req::CompletionParams,
 ) -> Result<Option<req::CompletionResponse>> {
     let _p = profile("handle_completion");
-    let position = {
-        let file_id = params.text_document.try_conv_with(&world)?;
-        let line_index = world.analysis().file_line_index(file_id);
-        let offset = params.position.conv_with(&line_index);
-        FilePosition { file_id, offset }
-    };
+    let position = params.text_document_position.try_conv_with(&world)?;
     let completion_triggered_after_single_colon = {
         let mut res = false;
         if let Some(ctx) = params.context {
@@ -440,9 +433,10 @@ pub fn handle_completion(
         Some(items) => items,
     };
     let line_index = world.analysis().file_line_index(position.file_id);
-    let items = items.into_iter().map(|item| item.conv_with(&line_index)).collect();
+    let items: Vec<CompletionItem> =
+        items.into_iter().map(|item| item.conv_with(&line_index)).collect();
 
-    Ok(Some(req::CompletionResponse::Array(items)))
+    Ok(Some(items.into()))
 }
 
 pub fn handle_folding_range(
@@ -543,9 +537,7 @@ pub fn handle_prepare_rename(
 }
 
 pub fn handle_rename(world: WorldSnapshot, params: RenameParams) -> Result<Option<WorkspaceEdit>> {
-    let file_id = params.text_document.try_conv_with(&world)?;
-    let line_index = world.analysis().file_line_index(file_id);
-    let offset = params.position.conv_with(&line_index);
+    let position = params.text_document_position.try_conv_with(&world)?;
 
     if params.new_name.is_empty() {
         return Err(LspError::new(
@@ -555,8 +547,7 @@ pub fn handle_rename(world: WorldSnapshot, params: RenameParams) -> Result<Optio
         .into());
     }
 
-    let optional_change =
-        world.analysis().rename(FilePosition { file_id, offset }, &*params.new_name)?;
+    let optional_change = world.analysis().rename(position, &*params.new_name)?;
     let change = match optional_change {
         None => return Ok(None),
         Some(it) => it,
@@ -571,11 +562,10 @@ pub fn handle_references(
     world: WorldSnapshot,
     params: req::ReferenceParams,
 ) -> Result<Option<Vec<Location>>> {
-    let file_id = params.text_document.try_conv_with(&world)?;
-    let line_index = world.analysis().file_line_index(file_id);
-    let offset = params.position.conv_with(&line_index);
+    let position = params.text_document_position.try_conv_with(&world)?;
+    let line_index = world.analysis().file_line_index(position.file_id);
 
-    let refs = match world.analysis().find_all_refs(FilePosition { file_id, offset })? {
+    let refs = match world.analysis().find_all_refs(position)? {
         None => return Ok(None),
         Some(refs) => refs,
     };
diff --git a/crates/ra_lsp_server/tests/heavy_tests/main.rs b/crates/ra_lsp_server/tests/heavy_tests/main.rs
index d271b02fc..451be32a8 100644
--- a/crates/ra_lsp_server/tests/heavy_tests/main.rs
+++ b/crates/ra_lsp_server/tests/heavy_tests/main.rs
@@ -37,9 +37,11 @@ use std::collections::Spam;
     eprintln!("loading took    {:?}", project_start.elapsed());
     let completion_start = Instant::now();
     let res = server.send_request::<Completion>(CompletionParams {
-        text_document: server.doc_id("src/lib.rs"),
+        text_document_position: TextDocumentPositionParams::new(
+            server.doc_id("src/lib.rs"),
+            Position::new(0, 23),
+        ),
         context: None,
-        position: Position::new(0, 23),
     });
     assert!(format!("{}", res).contains("HashMap"));
     eprintln!("completion took {:?}", completion_start.elapsed());