Add support for rename with language servers that lack prepareRename (#23000)

Michael Sloan created

This adds support for LSPs that use the old rename flow which does not
first ask the LSP for the rename range and check that it is a valid
range to rename.

Closes #16663

Release Notes:

* Fixed rename symbols action when the language server does not have the
capability to prepare renames - such as `luau-lsp`.

Change summary

crates/editor/src/editor.rs         |  25 ++++
crates/editor/src/editor_tests.rs   | 101 +++++++++++++++++-
crates/project/src/lsp_command.rs   | 162 +++++++++++++++++++++++-------
crates/project/src/lsp_store.rs     |  28 ++--
crates/project/src/project.rs       |  12 +
crates/project/src/project_tests.rs |   5 
crates/proto/proto/zed.proto        |   1 
7 files changed, 268 insertions(+), 66 deletions(-)

Detailed changes

crates/editor/src/editor.rs ๐Ÿ”—

@@ -132,7 +132,7 @@ use project::{
     lsp_store::{FormatTrigger, LspFormatTarget, OpenLspBufferHandle},
     project_settings::{GitGutterSetting, ProjectSettings},
     CodeAction, Completion, CompletionIntent, DocumentHighlight, InlayHint, Location, LocationLink,
-    LspStore, Project, ProjectItem, ProjectTransaction, TaskSourceKind,
+    LspStore, PrepareRenameResponse, Project, ProjectItem, ProjectTransaction, TaskSourceKind,
 };
 use rand::prelude::*;
 use rpc::{proto::*, ErrorExt};
@@ -13941,7 +13941,28 @@ impl SemanticsProvider for Model<Project> {
         cx: &mut AppContext,
     ) -> Option<Task<Result<Option<Range<text::Anchor>>>>> {
         Some(self.update(cx, |project, cx| {
-            project.prepare_rename(buffer.clone(), position, cx)
+            let buffer = buffer.clone();
+            let task = project.prepare_rename(buffer.clone(), position, cx);
+            cx.spawn(|_, mut cx| async move {
+                Ok(match task.await? {
+                    PrepareRenameResponse::Success(range) => Some(range),
+                    PrepareRenameResponse::InvalidPosition => None,
+                    PrepareRenameResponse::OnlyUnpreparedRenameSupported => {
+                        // Fallback on using TreeSitter info to determine identifier range
+                        buffer.update(&mut cx, |buffer, _| {
+                            let snapshot = buffer.snapshot();
+                            let (range, kind) = snapshot.surrounding_word(position);
+                            if kind != Some(CharKind::Word) {
+                                return None;
+                            }
+                            Some(
+                                snapshot.anchor_before(range.start)
+                                    ..snapshot.anchor_after(range.end),
+                            )
+                        })?
+                    }
+                })
+            })
         }))
     }
 

crates/editor/src/editor_tests.rs ๐Ÿ”—

@@ -14734,7 +14734,14 @@ fn test_inline_completion_text_with_deletions(cx: &mut TestAppContext) {
 #[gpui::test]
 async fn test_rename_with_duplicate_edits(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});
-    let mut cx = EditorLspTestContext::new_rust(lsp::ServerCapabilities::default(), cx).await;
+    let capabilities = lsp::ServerCapabilities {
+        rename_provider: Some(lsp::OneOf::Right(lsp::RenameOptions {
+            prepare_provider: Some(true),
+            work_done_progress_options: Default::default(),
+        })),
+        ..Default::default()
+    };
+    let mut cx = EditorLspTestContext::new_rust(capabilities, cx).await;
 
     cx.set_state(indoc! {"
         struct Fห‡oo {}
@@ -14750,10 +14757,25 @@ async fn test_rename_with_duplicate_edits(cx: &mut gpui::TestAppContext) {
         );
     });
 
-    cx.update_editor(|e, cx| e.rename(&Rename, cx))
-        .expect("Rename was not started")
-        .await
-        .expect("Rename failed");
+    let mut prepare_rename_handler =
+        cx.handle_request::<lsp::request::PrepareRenameRequest, _, _>(move |_, _, _| async move {
+            Ok(Some(lsp::PrepareRenameResponse::Range(lsp::Range {
+                start: lsp::Position {
+                    line: 0,
+                    character: 7,
+                },
+                end: lsp::Position {
+                    line: 0,
+                    character: 10,
+                },
+            })))
+        });
+    let prepare_rename_task = cx
+        .update_editor(|e, cx| e.rename(&Rename, cx))
+        .expect("Prepare rename was not started");
+    prepare_rename_handler.next().await.unwrap();
+    prepare_rename_task.await.expect("Prepare rename failed");
+
     let mut rename_handler =
         cx.handle_request::<lsp::request::Rename, _, _>(move |url, _, _| async move {
             let edit = lsp::TextEdit {
@@ -14774,11 +14796,11 @@ async fn test_rename_with_duplicate_edits(cx: &mut gpui::TestAppContext) {
                 std::collections::HashMap::from_iter(Some((url, vec![edit.clone(), edit]))),
             )))
         });
-    cx.update_editor(|e, cx| e.confirm_rename(&ConfirmRename, cx))
-        .expect("Confirm rename was not started")
-        .await
-        .expect("Confirm rename failed");
+    let rename_task = cx
+        .update_editor(|e, cx| e.confirm_rename(&ConfirmRename, cx))
+        .expect("Confirm rename was not started");
     rename_handler.next().await.unwrap();
+    rename_task.await.expect("Confirm rename failed");
     cx.run_until_parked();
 
     // Despite two edits, only one is actually applied as those are identical
@@ -14787,6 +14809,67 @@ async fn test_rename_with_duplicate_edits(cx: &mut gpui::TestAppContext) {
     "});
 }
 
+#[gpui::test]
+async fn test_rename_without_prepare(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+    // These capabilities indicate that the server does not support prepare rename.
+    let capabilities = lsp::ServerCapabilities {
+        rename_provider: Some(lsp::OneOf::Left(true)),
+        ..Default::default()
+    };
+    let mut cx = EditorLspTestContext::new_rust(capabilities, cx).await;
+
+    cx.set_state(indoc! {"
+        struct Fห‡oo {}
+    "});
+
+    cx.update_editor(|editor, cx| {
+        let highlight_range = Point::new(0, 7)..Point::new(0, 10);
+        let highlight_range = highlight_range.to_anchors(&editor.buffer().read(cx).snapshot(cx));
+        editor.highlight_background::<DocumentHighlightRead>(
+            &[highlight_range],
+            |c| c.editor_document_highlight_read_background,
+            cx,
+        );
+    });
+
+    cx.update_editor(|e, cx| e.rename(&Rename, cx))
+        .expect("Prepare rename was not started")
+        .await
+        .expect("Prepare rename failed");
+
+    let mut rename_handler =
+        cx.handle_request::<lsp::request::Rename, _, _>(move |url, _, _| async move {
+            let edit = lsp::TextEdit {
+                range: lsp::Range {
+                    start: lsp::Position {
+                        line: 0,
+                        character: 7,
+                    },
+                    end: lsp::Position {
+                        line: 0,
+                        character: 10,
+                    },
+                },
+                new_text: "FooRenamed".to_string(),
+            };
+            Ok(Some(lsp::WorkspaceEdit::new(
+                std::collections::HashMap::from_iter(Some((url, vec![edit]))),
+            )))
+        });
+    let rename_task = cx
+        .update_editor(|e, cx| e.confirm_rename(&ConfirmRename, cx))
+        .expect("Confirm rename was not started");
+    rename_handler.next().await.unwrap();
+    rename_task.await.expect("Confirm rename failed");
+    cx.run_until_parked();
+
+    // Correct range is renamed, as `surrounding_word` is used to find it.
+    cx.assert_editor_state(indoc! {"
+        struct FooRenamedห‡ {}
+    "});
+}
+
 fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
     let point = DisplayPoint::new(DisplayRow(row as u32), column as u32);
     point..point

crates/project/src/lsp_command.rs ๐Ÿ”—

@@ -4,7 +4,7 @@ use crate::{
     lsp_store::{LocalLspStore, LspStore},
     CodeAction, CoreCompletion, DocumentHighlight, Hover, HoverBlock, HoverBlockKind, InlayHint,
     InlayHintLabel, InlayHintLabelPart, InlayHintLabelPartTooltip, InlayHintTooltip, Location,
-    LocationLink, MarkupContent, ProjectTransaction, ResolveState,
+    LocationLink, MarkupContent, PrepareRenameResponse, ProjectTransaction, ResolveState,
 };
 use anyhow::{anyhow, Context, Result};
 use async_trait::async_trait;
@@ -23,7 +23,7 @@ use language::{
 use lsp::{
     AdapterServerCapabilities, CodeActionKind, CodeActionOptions, CompletionContext,
     CompletionListItemDefaultsEditRange, CompletionTriggerKind, DocumentHighlightKind,
-    LanguageServer, LanguageServerId, LinkedEditingRangeServerCapabilities, OneOf,
+    LanguageServer, LanguageServerId, LinkedEditingRangeServerCapabilities, OneOf, RenameOptions,
     ServerCapabilities,
 };
 use signature_help::{lsp_to_proto_signature, proto_to_lsp_signature};
@@ -76,14 +76,36 @@ pub trait LspCommand: 'static + Sized + Send + std::fmt::Debug {
     type LspRequest: 'static + Send + lsp::request::Request;
     type ProtoRequest: 'static + Send + proto::RequestMessage;
 
-    fn check_capabilities(&self, _: AdapterServerCapabilities) -> bool {
-        true
-    }
-
     fn status(&self) -> Option<String> {
         None
     }
 
+    fn to_lsp_params_or_response(
+        &self,
+        path: &Path,
+        buffer: &Buffer,
+        language_server: &Arc<LanguageServer>,
+        cx: &AppContext,
+    ) -> Result<
+        LspParamsOrResponse<<Self::LspRequest as lsp::request::Request>::Params, Self::Response>,
+    > {
+        if self.check_capabilities(language_server.adapter_server_capabilities()) {
+            Ok(LspParamsOrResponse::Params(self.to_lsp(
+                path,
+                buffer,
+                language_server,
+                cx,
+            )?))
+        } else {
+            Ok(LspParamsOrResponse::Response(Default::default()))
+        }
+    }
+
+    /// When false, `to_lsp_params_or_response` default implementation will return the default response.
+    fn check_capabilities(&self, _: AdapterServerCapabilities) -> bool {
+        true
+    }
+
     fn to_lsp(
         &self,
         path: &Path,
@@ -129,6 +151,11 @@ pub trait LspCommand: 'static + Sized + Send + std::fmt::Debug {
     fn buffer_id_from_proto(message: &Self::ProtoRequest) -> Result<BufferId>;
 }
 
+pub enum LspParamsOrResponse<P, R> {
+    Params(P),
+    Response(R),
+}
+
 #[derive(Debug)]
 pub(crate) struct PrepareRename {
     pub position: PointUtf16,
@@ -160,6 +187,7 @@ pub(crate) struct GetTypeDefinition {
 pub(crate) struct GetImplementation {
     pub position: PointUtf16,
 }
+
 #[derive(Debug)]
 pub(crate) struct GetReferences {
     pub position: PointUtf16,
@@ -191,6 +219,7 @@ pub(crate) struct GetCodeActions {
     pub range: Range<Anchor>,
     pub kinds: Option<Vec<lsp::CodeActionKind>>,
 }
+
 #[derive(Debug)]
 pub(crate) struct OnTypeFormatting {
     pub position: PointUtf16,
@@ -198,10 +227,12 @@ pub(crate) struct OnTypeFormatting {
     pub options: lsp::FormattingOptions,
     pub push_to_history: bool,
 }
+
 #[derive(Debug)]
 pub(crate) struct InlayHints {
     pub range: Range<Anchor>,
 }
+
 #[derive(Debug)]
 pub(crate) struct LinkedEditingRange {
     pub position: Anchor,
@@ -209,15 +240,38 @@ pub(crate) struct LinkedEditingRange {
 
 #[async_trait(?Send)]
 impl LspCommand for PrepareRename {
-    type Response = Option<Range<Anchor>>;
+    type Response = PrepareRenameResponse;
     type LspRequest = lsp::request::PrepareRenameRequest;
     type ProtoRequest = proto::PrepareRename;
 
-    fn check_capabilities(&self, capabilities: AdapterServerCapabilities) -> bool {
-        if let Some(lsp::OneOf::Right(rename)) = &capabilities.server_capabilities.rename_provider {
-            rename.prepare_provider == Some(true)
-        } else {
-            false
+    fn to_lsp_params_or_response(
+        &self,
+        path: &Path,
+        buffer: &Buffer,
+        language_server: &Arc<LanguageServer>,
+        cx: &AppContext,
+    ) -> Result<LspParamsOrResponse<lsp::TextDocumentPositionParams, PrepareRenameResponse>> {
+        let rename_provider = language_server
+            .adapter_server_capabilities()
+            .server_capabilities
+            .rename_provider;
+        match rename_provider {
+            Some(lsp::OneOf::Right(RenameOptions {
+                prepare_provider: Some(true),
+                ..
+            })) => Ok(LspParamsOrResponse::Params(self.to_lsp(
+                path,
+                buffer,
+                language_server,
+                cx,
+            )?)),
+            Some(lsp::OneOf::Right(_)) => Ok(LspParamsOrResponse::Response(
+                PrepareRenameResponse::OnlyUnpreparedRenameSupported,
+            )),
+            Some(lsp::OneOf::Left(true)) => Ok(LspParamsOrResponse::Response(
+                PrepareRenameResponse::OnlyUnpreparedRenameSupported,
+            )),
+            _ => Err(anyhow!("Rename not supported")),
         }
     }
 
@@ -238,21 +292,29 @@ impl LspCommand for PrepareRename {
         buffer: Model<Buffer>,
         _: LanguageServerId,
         mut cx: AsyncAppContext,
-    ) -> Result<Option<Range<Anchor>>> {
+    ) -> Result<PrepareRenameResponse> {
         buffer.update(&mut cx, |buffer, _| {
-            if let Some(
-                lsp::PrepareRenameResponse::Range(range)
-                | lsp::PrepareRenameResponse::RangeWithPlaceholder { range, .. },
-            ) = message
-            {
-                let Range { start, end } = range_from_lsp(range);
-                if buffer.clip_point_utf16(start, Bias::Left) == start.0
-                    && buffer.clip_point_utf16(end, Bias::Left) == end.0
-                {
-                    return Ok(Some(buffer.anchor_after(start)..buffer.anchor_before(end)));
+            match message {
+                Some(lsp::PrepareRenameResponse::Range(range))
+                | Some(lsp::PrepareRenameResponse::RangeWithPlaceholder { range, .. }) => {
+                    let Range { start, end } = range_from_lsp(range);
+                    if buffer.clip_point_utf16(start, Bias::Left) == start.0
+                        && buffer.clip_point_utf16(end, Bias::Left) == end.0
+                    {
+                        Ok(PrepareRenameResponse::Success(
+                            buffer.anchor_after(start)..buffer.anchor_before(end),
+                        ))
+                    } else {
+                        Ok(PrepareRenameResponse::InvalidPosition)
+                    }
+                }
+                Some(lsp::PrepareRenameResponse::DefaultBehavior { .. }) => {
+                    Err(anyhow!("Invalid for language server to send a `defaultBehavior` response to `prepareRename`"))
+                }
+                None => {
+                    Ok(PrepareRenameResponse::InvalidPosition)
                 }
             }
-            Ok(None)
         })?
     }
 
@@ -289,21 +351,34 @@ impl LspCommand for PrepareRename {
     }
 
     fn response_to_proto(
-        range: Option<Range<Anchor>>,
+        response: PrepareRenameResponse,
         _: &mut LspStore,
         _: PeerId,
         buffer_version: &clock::Global,
         _: &mut AppContext,
     ) -> proto::PrepareRenameResponse {
-        proto::PrepareRenameResponse {
-            can_rename: range.is_some(),
-            start: range
-                .as_ref()
-                .map(|range| language::proto::serialize_anchor(&range.start)),
-            end: range
-                .as_ref()
-                .map(|range| language::proto::serialize_anchor(&range.end)),
-            version: serialize_version(buffer_version),
+        match response {
+            PrepareRenameResponse::Success(range) => proto::PrepareRenameResponse {
+                can_rename: true,
+                only_unprepared_rename_supported: false,
+                start: Some(language::proto::serialize_anchor(&range.start)),
+                end: Some(language::proto::serialize_anchor(&range.end)),
+                version: serialize_version(buffer_version),
+            },
+            PrepareRenameResponse::OnlyUnpreparedRenameSupported => proto::PrepareRenameResponse {
+                can_rename: false,
+                only_unprepared_rename_supported: true,
+                start: None,
+                end: None,
+                version: vec![],
+            },
+            PrepareRenameResponse::InvalidPosition => proto::PrepareRenameResponse {
+                can_rename: false,
+                only_unprepared_rename_supported: false,
+                start: None,
+                end: None,
+                version: vec![],
+            },
         }
     }
 
@@ -313,18 +388,27 @@ impl LspCommand for PrepareRename {
         _: Model<LspStore>,
         buffer: Model<Buffer>,
         mut cx: AsyncAppContext,
-    ) -> Result<Option<Range<Anchor>>> {
+    ) -> Result<PrepareRenameResponse> {
         if message.can_rename {
             buffer
                 .update(&mut cx, |buffer, _| {
                     buffer.wait_for_version(deserialize_version(&message.version))
                 })?
                 .await?;
-            let start = message.start.and_then(deserialize_anchor);
-            let end = message.end.and_then(deserialize_anchor);
-            Ok(start.zip(end).map(|(start, end)| start..end))
+            if let (Some(start), Some(end)) = (
+                message.start.and_then(deserialize_anchor),
+                message.end.and_then(deserialize_anchor),
+            ) {
+                Ok(PrepareRenameResponse::Success(start..end))
+            } else {
+                Err(anyhow!(
+                    "Missing start or end position in remote project PrepareRenameResponse"
+                ))
+            }
+        } else if message.only_unprepared_rename_supported {
+            Ok(PrepareRenameResponse::OnlyUnpreparedRenameSupported)
         } else {
-            Ok(None)
+            Ok(PrepareRenameResponse::InvalidPosition)
         }
     }
 

crates/project/src/lsp_store.rs ๐Ÿ”—

@@ -3500,24 +3500,26 @@ impl LspStore {
         };
         let file = File::from_dyn(buffer.file()).and_then(File::as_local);
         if let (Some(file), Some(language_server)) = (file, language_server) {
-            let lsp_params = match request.to_lsp(&file.abs_path(cx), buffer, &language_server, cx)
-            {
-                Ok(lsp_params) => lsp_params,
+            let lsp_params = match request.to_lsp_params_or_response(
+                &file.abs_path(cx),
+                buffer,
+                &language_server,
+                cx,
+            ) {
+                Ok(LspParamsOrResponse::Params(lsp_params)) => lsp_params,
+                Ok(LspParamsOrResponse::Response(response)) => return Task::ready(Ok(response)),
                 Err(err) => {
-                    log::error!(
-                        "Preparing LSP request to {} failed: {}",
-                        language_server.name(),
-                        err
-                    );
-                    return Task::ready(Err(err));
+                    let message =
+                        format!("LSP request to {} failed: {}", language_server.name(), err);
+                    log::error!("{}", message);
+                    return Task::ready(Err(anyhow!(message)));
                 }
             };
             let status = request.status();
+            if !request.check_capabilities(language_server.adapter_server_capabilities()) {
+                return Task::ready(Ok(Default::default()));
+            }
             return cx.spawn(move |this, cx| async move {
-                if !request.check_capabilities(language_server.adapter_server_capabilities()) {
-                    return Ok(Default::default());
-                }
-
                 let lsp_request = language_server.request::<R::LspRequest>(lsp_params);
 
                 let id = lsp_request.id();

crates/project/src/project.rs ๐Ÿ”—

@@ -307,6 +307,14 @@ impl ProjectPath {
     }
 }
 
+#[derive(Debug, Default)]
+pub enum PrepareRenameResponse {
+    Success(Range<Anchor>),
+    OnlyUnpreparedRenameSupported,
+    #[default]
+    InvalidPosition,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct InlayHint {
     pub position: language::Anchor,
@@ -2908,7 +2916,7 @@ impl Project {
         buffer: Model<Buffer>,
         position: PointUtf16,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<Option<Range<Anchor>>>> {
+    ) -> Task<Result<PrepareRenameResponse>> {
         self.request_lsp(
             buffer,
             LanguageServerToQuery::Primary,
@@ -2921,7 +2929,7 @@ impl Project {
         buffer: Model<Buffer>,
         position: T,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<Option<Range<Anchor>>>> {
+    ) -> Task<Result<PrepareRenameResponse>> {
         let position = position.to_point_utf16(buffer.read(cx));
         self.prepare_rename_impl(buffer, position, cx)
     }

crates/project/src/project_tests.rs ๐Ÿ”—

@@ -4135,7 +4135,10 @@ async fn test_rename(cx: &mut gpui::TestAppContext) {
         .next()
         .await
         .unwrap();
-    let range = response.await.unwrap().unwrap();
+    let response = response.await.unwrap();
+    let PrepareRenameResponse::Success(range) = response else {
+        panic!("{:?}", response);
+    };
     let range = buffer.update(cx, |buffer, _| range.to_offset(buffer));
     assert_eq!(range, 6..9);
 

crates/proto/proto/zed.proto ๐Ÿ”—

@@ -1033,6 +1033,7 @@ message PrepareRenameResponse {
     Anchor start = 2;
     Anchor end = 3;
     repeated VectorClockEntry version = 4;
+    bool only_unprepared_rename_supported = 5;
 }
 
 message PerformRename {