From bda0c67ece3a53aa41486a44b0b84530222af37b Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Sat, 11 Jan 2025 14:22:17 -0700 Subject: [PATCH] Add support for rename with language servers that lack prepareRename (#23000) 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`. --- 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(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 4334a34bf088f0757c5b5adc0770e7b9718587e2..22d9650beb0679616dfb97728ccb32beb6064499 100644 --- a/crates/editor/src/editor.rs +++ b/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 { cx: &mut AppContext, ) -> Option>>>> { 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), + ) + })? + } + }) + }) })) } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 446bce8d6e29c80507acc7f311841b69c85ecad2..e477013e68735dbff7f96a012a43d3d9717a7791 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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::(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::(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::( + &[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::(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 { let point = DisplayPoint::new(DisplayRow(row as u32), column as u32); point..point diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index abc2e0a21d9f04599bd28a1058f6897763cec137..4d998631ba1b35e85889cb68324ce5283f421728 100644 --- a/crates/project/src/lsp_command.rs +++ b/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 { None } + fn to_lsp_params_or_response( + &self, + path: &Path, + buffer: &Buffer, + language_server: &Arc, + cx: &AppContext, + ) -> Result< + LspParamsOrResponse<::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; } +pub enum LspParamsOrResponse { + 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, pub kinds: Option>, } + #[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, } + #[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>; + 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, + cx: &AppContext, + ) -> Result> { + 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, _: LanguageServerId, mut cx: AsyncAppContext, - ) -> Result>> { + ) -> Result { 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>, + 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, buffer: Model, mut cx: AsyncAppContext, - ) -> Result>> { + ) -> Result { 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) } } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index ae73e84bbcd9361ea7d0a22ab2b4a9ab6dba561c..01643cf1fe6269ee729e67b970deddd6396b7462 100644 --- a/crates/project/src/lsp_store.rs +++ b/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::(lsp_params); let id = lsp_request.id(); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index dcf0defdf8eab832dd8868654f072a70cd313c44..5a5022a86023b22dd7d6fe180a87cc8165d20ba2 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -307,6 +307,14 @@ impl ProjectPath { } } +#[derive(Debug, Default)] +pub enum PrepareRenameResponse { + Success(Range), + OnlyUnpreparedRenameSupported, + #[default] + InvalidPosition, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct InlayHint { pub position: language::Anchor, @@ -2908,7 +2916,7 @@ impl Project { buffer: Model, position: PointUtf16, cx: &mut ModelContext, - ) -> Task>>> { + ) -> Task> { self.request_lsp( buffer, LanguageServerToQuery::Primary, @@ -2921,7 +2929,7 @@ impl Project { buffer: Model, position: T, cx: &mut ModelContext, - ) -> Task>>> { + ) -> Task> { let position = position.to_point_utf16(buffer.read(cx)); self.prepare_rename_impl(buffer, position, cx) } diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 700bd8571425b41aee2aa76f240e2b5c2b68ab81..005cd34cba2b2668a5247f4664db1d0075704d4d 100644 --- a/crates/project/src/project_tests.rs +++ b/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); diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index 3f426f0214f19bc322455edb4c9f44c9ad51dda0..c035ac1a04d54cddd9feff64aa24a552972b69d0 100644 --- a/crates/proto/proto/zed.proto +++ b/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 {