Merge #1511

1511: Send old-style responsed to goto definition unless the client explicitelly opts in r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

Author: bors[bot]

crates/ra_lsp_server/src/conv.rs

@@ -384,27 +384,53 @@ impl TryConvWith for &NavigationTarget {
     }
 }
 
-pub fn to_location_link(
-    target: &RangeInfo<NavigationTarget>,
-    world: &WorldSnapshot,
-    // line index for original range file
-    line_index: &LineIndex,
-) -> Result<LocationLink> {
-    let target_uri = target.info.file_id().try_conv_with(world)?;
-    let tgt_line_index = world.analysis().file_line_index(target.info.file_id());
-
-    let target_range = target.info.full_range().conv_with(&tgt_line_index);
-
-    let target_selection_range =
-        target.info.focus_range().map(|it| it.conv_with(&tgt_line_index)).unwrap_or(target_range);
+impl TryConvWith for (FileId, RangeInfo<NavigationTarget>) {
+    type Ctx = WorldSnapshot;
+    type Output = LocationLink;
+    fn try_conv_with(self, world: &WorldSnapshot) -> Result<LocationLink> {
+        let (src_file_id, target) = self;
+
+        let target_uri = target.info.file_id().try_conv_with(world)?;
+        let src_line_index = world.analysis().file_line_index(src_file_id);
+        let tgt_line_index = world.analysis().file_line_index(target.info.file_id());
+
+        let target_range = target.info.full_range().conv_with(&tgt_line_index);
+
+        let target_selection_range = target
+            .info
+            .focus_range()
+            .map(|it| it.conv_with(&tgt_line_index))
+            .unwrap_or(target_range);
+
+        let res = LocationLink {
+            origin_selection_range: Some(target.range.conv_with(&src_line_index)),
+            target_uri,
+            target_range,
+            target_selection_range,
+        };
+        Ok(res)
+    }
+}
 
-    let res = LocationLink {
-        origin_selection_range: Some(target.range.conv_with(line_index)),
-        target_uri,
-        target_range,
-        target_selection_range,
-    };
-    Ok(res)
+impl TryConvWith for (FileId, RangeInfo<Vec<NavigationTarget>>) {
+    type Ctx = WorldSnapshot;
+    type Output = req::GotoDefinitionResponse;
+    fn try_conv_with(self, world: &WorldSnapshot) -> Result<req::GotoTypeDefinitionResponse> {
+        let (file_id, RangeInfo { range, info: navs }) = self;
+        let links = navs
+            .into_iter()
+            .map(|nav| (file_id, RangeInfo::new(range, nav)))
+            .try_conv_with_to_vec(world)?;
+        if world.options.supports_location_link {
+            Ok(links.into())
+        } else {
+            let locations: Vec<Location> = links
+                .into_iter()
+                .map(|link| Location { uri: link.target_uri, range: link.target_selection_range })
+                .collect();
+            Ok(locations.into())
+        }
+    }
 }
 
 pub fn to_location(
@@ -452,3 +478,23 @@ where
         self.iter.next().map(|item| item.conv_with(self.ctx))
     }
 }
+
+pub trait TryConvWithToVec<'a>: Sized + 'a {
+    type Ctx;
+    type Output;
+
+    fn try_conv_with_to_vec(self, ctx: &'a Self::Ctx) -> Result<Vec<Self::Output>>;
+}
+
+impl<'a, I> TryConvWithToVec<'a> for I
+where
+    I: Iterator + 'a,
+    I::Item: TryConvWith,
+{
+    type Ctx = <I::Item as TryConvWith>::Ctx;
+    type Output = <I::Item as TryConvWith>::Output;
+
+    fn try_conv_with_to_vec(self, ctx: &'a Self::Ctx) -> Result<Vec<Self::Output>> {
+        self.map(|it| it.try_conv_with(ctx)).collect()
+    }
+}

crates/ra_lsp_server/src/main.rs

@@ -51,7 +51,7 @@ fn main_inner() -> Result<()> {
             .and_then(|v| InitializationOptions::deserialize(v).ok())
             .unwrap_or_default();
 
-        ra_lsp_server::main_loop(workspace_roots, opts, r, s)
+        ra_lsp_server::main_loop(workspace_roots, params.capabilities, opts, r, s)
     })?;
     log::info!("shutting down IO...");
     threads.join()?;

crates/ra_lsp_server/src/main_loop.rs

@@ -8,7 +8,7 @@ use crossbeam_channel::{select, unbounded, Receiver, RecvError, Sender};
 use gen_lsp_server::{
     handle_shutdown, ErrorCode, RawMessage, RawNotification, RawRequest, RawResponse,
 };
-use lsp_types::NumberOrString;
+use lsp_types::{ClientCapabilities, NumberOrString};
 use ra_ide_api::{Canceled, FileId, LibraryData};
 use ra_prof::profile;
 use ra_vfs::VfsTask;
@@ -22,7 +22,7 @@ use crate::{
     },
     project_model::workspace_loader,
     req,
-    world::{WorldSnapshot, WorldState},
+    world::{Options, WorldSnapshot, WorldState},
     InitializationOptions, Result,
 };
 
@@ -51,6 +51,7 @@ impl Error for LspError {}
 
 pub fn main_loop(
     ws_roots: Vec<PathBuf>,
+    client_caps: ClientCapabilities,
     options: InitializationOptions,
     msg_receiver: &Receiver<RawMessage>,
     msg_sender: &Sender<RawMessage>,
@@ -77,15 +78,27 @@ pub fn main_loop(
         loaded_workspaces
     };
 
-    let mut state = WorldState::new(ws_roots, workspaces, options.lru_capacity);
+    let mut state = WorldState::new(
+        ws_roots,
+        workspaces,
+        options.lru_capacity,
+        Options {
+            publish_decorations: options.publish_decorations,
+            show_workspace_loaded: options.show_workspace_loaded,
+            supports_location_link: client_caps
+                .text_document
+                .and_then(|it| it.definition)
+                .and_then(|it| it.link_support)
+                .unwrap_or(false),
+        },
+    );
 
     let pool = ThreadPool::new(THREADPOOL_SIZE);
     let (task_sender, task_receiver) = unbounded::<Task>();
     let mut pending_requests = PendingRequests::default();
 
     log::info!("server initialized, serving requests");
     let main_res = main_loop_inner(
-        options,
         &pool,
         msg_sender,
         msg_receiver,
@@ -159,7 +172,6 @@ impl fmt::Debug for Event {
 }
 
 fn main_loop_inner(
-    options: InitializationOptions,
     pool: &ThreadPool,
     msg_sender: &Sender<RawMessage>,
     msg_receiver: &Receiver<RawMessage>,
@@ -258,7 +270,7 @@ fn main_loop_inner(
             && in_flight_libraries == 0
         {
             let n_packages: usize = state.workspaces.iter().map(|it| it.count()).sum();
-            if options.show_workspace_loaded {
+            if state.options.show_workspace_loaded {
                 let msg = format!("workspace loaded, {} rust packages", n_packages);
                 show_message(req::MessageType::Info, msg, msg_sender);
             }
@@ -270,7 +282,7 @@ fn main_loop_inner(
             update_file_notifications_on_threadpool(
                 pool,
                 state.snapshot(),
-                options.publish_decorations,
+                state.options.publish_decorations,
                 task_sender.clone(),
                 subs.subscriptions(),
             )

crates/ra_lsp_server/src/main_loop/handlers.rs

@@ -9,8 +9,7 @@ use lsp_types::{
     TextDocumentIdentifier, TextEdit, WorkspaceEdit,
 };
 use ra_ide_api::{
-    AssistId, Cancelable, FileId, FilePosition, FileRange, FoldKind, Query, RangeInfo,
-    RunnableKind, Severity,
+    AssistId, Cancelable, FileId, FilePosition, FileRange, FoldKind, Query, RunnableKind, Severity,
 };
 use ra_prof::profile;
 use ra_syntax::{AstNode, SyntaxKind, TextRange, TextUnit};
@@ -21,7 +20,7 @@ use url_serde::Ser;
 
 use crate::{
     cargo_target_spec::{runnable_args, CargoTargetSpec},
-    conv::{to_location, to_location_link, Conv, ConvWith, MapConvWith, TryConvWith},
+    conv::{to_location, Conv, ConvWith, MapConvWith, TryConvWith, TryConvWithToVec},
     req::{self, Decoration},
     world::WorldSnapshot,
     LspError, Result,
@@ -263,72 +262,46 @@ pub fn handle_goto_definition(
     params: req::TextDocumentPositionParams,
 ) -> Result<Option<req::GotoDefinitionResponse>> {
     let position = params.try_conv_with(&world)?;
-    let line_index = world.analysis().file_line_index(position.file_id);
     let nav_info = match world.analysis().goto_definition(position)? {
         None => return Ok(None),
         Some(it) => it,
     };
-    let nav_range = nav_info.range;
-    let res = nav_info
-        .info
-        .into_iter()
-        .map(|nav| RangeInfo::new(nav_range, nav))
-        .map(|nav| to_location_link(&nav, &world, &line_index))
-        .collect::<Result<Vec<_>>>()?;
-    Ok(Some(res.into()))
+    let res = (position.file_id, nav_info).try_conv_with(&world)?;
+    Ok(Some(res))
 }
 
 pub fn handle_goto_implementation(
     world: WorldSnapshot,
     params: req::TextDocumentPositionParams,
 ) -> Result<Option<req::GotoImplementationResponse>> {
     let position = params.try_conv_with(&world)?;
-    let line_index = world.analysis().file_line_index(position.file_id);
     let nav_info = match world.analysis().goto_implementation(position)? {
         None => return Ok(None),
         Some(it) => it,
     };
-    let nav_range = nav_info.range;
-    let res = nav_info
-        .info
-        .into_iter()
-        .map(|nav| RangeInfo::new(nav_range, nav))
-        .map(|nav| to_location_link(&nav, &world, &line_index))
-        .collect::<Result<Vec<_>>>()?;
-    Ok(Some(res.into()))
+    let res = (position.file_id, nav_info).try_conv_with(&world)?;
+    Ok(Some(res))
 }
 
 pub fn handle_goto_type_definition(
     world: WorldSnapshot,
     params: req::TextDocumentPositionParams,
 ) -> Result<Option<req::GotoTypeDefinitionResponse>> {
     let position = params.try_conv_with(&world)?;
-    let line_index = world.analysis().file_line_index(position.file_id);
     let nav_info = match world.analysis().goto_type_definition(position)? {
         None => return Ok(None),
         Some(it) => it,
     };
-    let nav_range = nav_info.range;
-    let res = nav_info
-        .info
-        .into_iter()
-        .map(|nav| RangeInfo::new(nav_range, nav))
-        .map(|nav| to_location_link(&nav, &world, &line_index))
-        .collect::<Result<Vec<_>>>()?;
-    Ok(Some(res.into()))
+    let res = (position.file_id, nav_info).try_conv_with(&world)?;
+    Ok(Some(res))
 }
 
 pub fn handle_parent_module(
     world: WorldSnapshot,
     params: req::TextDocumentPositionParams,
 ) -> Result<Vec<Location>> {
     let position = params.try_conv_with(&world)?;
-    world
-        .analysis()
-        .parent_module(position)?
-        .into_iter()
-        .map(|nav| nav.try_conv_with(&world))
-        .collect::<Result<Vec<_>>>()
+    world.analysis().parent_module(position)?.iter().try_conv_with_to_vec(&world)
 }
 
 pub fn handle_runnables(

crates/ra_lsp_server/src/world.rs

@@ -19,13 +19,21 @@ use crate::{
     LspError, Result,
 };
 
+#[derive(Debug, Clone)]
+pub struct Options {
+    pub publish_decorations: bool,
+    pub show_workspace_loaded: bool,
+    pub supports_location_link: bool,
+}
+
 /// `WorldState` is the primary mutable state of the language server
 ///
 /// The most interesting components are `vfs`, which stores a consistent
 /// snapshot of the file systems, and `analysis_host`, which stores our
 /// incremental salsa database.
 #[derive(Debug)]
 pub struct WorldState {
+    pub options: Options,
     pub roots_to_scan: usize,
     pub roots: Vec<PathBuf>,
     pub workspaces: Arc<Vec<ProjectWorkspace>>,
@@ -36,6 +44,7 @@ pub struct WorldState {
 
 /// An immutable snapshot of the world's state at a point in time.
 pub struct WorldSnapshot {
+    pub options: Options,
     pub workspaces: Arc<Vec<ProjectWorkspace>>,
     pub analysis: Analysis,
     pub vfs: Arc<RwLock<Vfs>>,
@@ -47,6 +56,7 @@ impl WorldState {
         folder_roots: Vec<PathBuf>,
         workspaces: Vec<ProjectWorkspace>,
         lru_capacity: Option<usize>,
+        options: Options,
     ) -> WorldState {
         let mut change = AnalysisChange::new();
 
@@ -78,6 +88,7 @@ impl WorldState {
         let mut analysis_host = AnalysisHost::new(lru_capacity);
         analysis_host.apply_change(change);
         WorldState {
+            options,
             roots_to_scan,
             roots: folder_roots,
             workspaces: Arc::new(workspaces),
@@ -140,6 +151,7 @@ impl WorldState {
 
     pub fn snapshot(&self) -> WorldSnapshot {
         WorldSnapshot {
+            options: self.options.clone(),
             workspaces: Arc::clone(&self.workspaces),
             analysis: self.analysis_host.analysis(),
             vfs: Arc::clone(&self.vfs),

crates/ra_lsp_server/tests/heavy_tests/support.rs

@@ -13,7 +13,8 @@ use lsp_types::{
     notification::DidOpenTextDocument,
     notification::{Notification, ShowMessage},
     request::{Request, Shutdown},
-    DidOpenTextDocumentParams, TextDocumentIdentifier, TextDocumentItem, Url,
+    ClientCapabilities, DidOpenTextDocumentParams, GotoCapability, TextDocumentClientCapabilities,
+    TextDocumentIdentifier, TextDocumentItem, Url,
 };
 use serde::Serialize;
 use serde_json::{to_string_pretty, Value};
@@ -92,8 +93,25 @@ impl Server {
             "test server",
             128,
             move |msg_receiver, msg_sender| {
-                main_loop(roots, InitializationOptions::default(), &msg_receiver, &msg_sender)
-                    .unwrap()
+                main_loop(
+                    roots,
+                    ClientCapabilities {
+                        workspace: None,
+                        text_document: Some(TextDocumentClientCapabilities {
+                            definition: Some(GotoCapability {
+                                dynamic_registration: None,
+                                link_support: Some(true),
+                            }),
+                            ..Default::default()
+                        }),
+                        window: None,
+                        experimental: None,
+                    },
+                    InitializationOptions::default(),
+                    &msg_receiver,
+                    &msg_sender,
+                )
+                .unwrap()
             },
         );
         let res = Server { req_id: Cell::new(1), dir, messages: Default::default(), worker };