Refactor handling of remote renames

crates/project/src/lsp_command.rs

@@ -1,14 +1,15 @@
 use crate::{Project, ProjectTransaction};
 use anyhow::{anyhow, Result};
-use client::proto;
+use client::{proto, PeerId};
 use futures::{future::LocalBoxFuture, FutureExt};
-use gpui::{AppContext, AsyncAppContext, ModelHandle};
+use gpui::{AppContext, AsyncAppContext, ModelContext, ModelHandle};
 use language::{
     proto::deserialize_anchor, range_from_lsp, Anchor, Bias, Buffer, PointUtf16, ToLspPosition,
+    ToPointUtf16,
 };
 use std::{ops::Range, path::Path};
 
-pub(crate) trait LspCommand: 'static {
+pub(crate) trait LspCommand: 'static + Sized {
     type Response: 'static + Default + Send;
     type LspRequest: 'static + Send + lsp::request::Request;
     type ProtoRequest: 'static + Send + proto::RequestMessage;
@@ -18,29 +19,50 @@ pub(crate) trait LspCommand: 'static {
         path: &Path,
         cx: &AppContext,
     ) -> <Self::LspRequest as lsp::request::Request>::Params;
-    fn to_proto(&self, project_id: u64, cx: &AppContext) -> Self::ProtoRequest;
     fn response_from_lsp(
         self,
         message: <Self::LspRequest as lsp::request::Request>::Result,
         project: ModelHandle<Project>,
+        buffer: ModelHandle<Buffer>,
         cx: AsyncAppContext,
     ) -> LocalBoxFuture<'static, Result<Self::Response>>;
+
+    fn to_proto(
+        &self,
+        project_id: u64,
+        buffer: &ModelHandle<Buffer>,
+        cx: &AppContext,
+    ) -> Self::ProtoRequest;
+    fn from_proto(
+        message: Self::ProtoRequest,
+        project: &mut Project,
+        buffer: &ModelHandle<Buffer>,
+        cx: &mut ModelContext<Project>,
+    ) -> Result<Self>;
+    fn buffer_id_from_proto(message: &Self::ProtoRequest) -> u64;
+
+    fn response_to_proto(
+        response: Self::Response,
+        project: &mut Project,
+        peer_id: PeerId,
+        buffer_version: &clock::Global,
+        cx: &mut ModelContext<Project>,
+    ) -> <Self::ProtoRequest as proto::RequestMessage>::Response;
     fn response_from_proto(
         self,
         message: <Self::ProtoRequest as proto::RequestMessage>::Response,
         project: ModelHandle<Project>,
+        buffer: ModelHandle<Buffer>,
         cx: AsyncAppContext,
     ) -> LocalBoxFuture<'static, Result<Self::Response>>;
 }
 
 pub(crate) struct PrepareRename {
-    pub buffer: ModelHandle<Buffer>,
     pub position: PointUtf16,
 }
 
 #[derive(Debug)]
 pub(crate) struct PerformRename {
-    pub buffer: ModelHandle<Buffer>,
     pub position: PointUtf16,
     pub new_name: String,
     pub push_to_history: bool,
@@ -60,29 +82,18 @@ impl LspCommand for PrepareRename {
         }
     }
 
-    fn to_proto(&self, project_id: u64, cx: &AppContext) -> proto::PrepareRename {
-        let buffer = &self.buffer.read(cx);
-        let buffer_id = buffer.remote_id();
-        proto::PrepareRename {
-            project_id,
-            buffer_id,
-            position: Some(language::proto::serialize_anchor(
-                &buffer.anchor_before(self.position),
-            )),
-        }
-    }
-
     fn response_from_lsp(
         self,
         message: Option<lsp::PrepareRenameResponse>,
         _: ModelHandle<Project>,
+        buffer: ModelHandle<Buffer>,
         cx: AsyncAppContext,
     ) -> LocalBoxFuture<'static, Result<Option<Range<Anchor>>>> {
         async move {
             Ok(message.and_then(|result| match result {
                 lsp::PrepareRenameResponse::Range(range)
-                | lsp::PrepareRenameResponse::RangeWithPlaceholder { range, .. } => {
+                | lsp::PrepareRenameResponse::RangeWithPlaceholder { range, .. } => buffer
-                    self.buffer.read_with(&cx, |buffer, _| {
+                    .read_with(&cx, |buffer, _| {
                         let range = range_from_lsp(range);
                         if buffer.clip_point_utf16(range.start, Bias::Left) == range.start
                             && buffer.clip_point_utf16(range.end, Bias::Left) == range.end
@@ -91,23 +102,80 @@ impl LspCommand for PrepareRename {
                         } else {
                             None
                         }
-                    })
+                    }),
-                }
                 _ => None,
             }))
         }
         .boxed_local()
     }
 
+    fn to_proto(
+        &self,
+        project_id: u64,
+        buffer: &ModelHandle<Buffer>,
+        cx: &AppContext,
+    ) -> proto::PrepareRename {
+        proto::PrepareRename {
+            project_id,
+            buffer_id: buffer.read(cx).remote_id(),
+            position: Some(language::proto::serialize_anchor(
+                &buffer.read(cx).anchor_before(self.position),
+            )),
+        }
+    }
+
+    fn buffer_id_from_proto(message: &proto::PrepareRename) -> u64 {
+        message.buffer_id
+    }
+
+    fn from_proto(
+        message: proto::PrepareRename,
+        _: &mut Project,
+        buffer: &ModelHandle<Buffer>,
+        cx: &mut ModelContext<Project>,
+    ) -> Result<Self> {
+        let buffer = buffer.read(cx);
+        let position = message
+            .position
+            .and_then(deserialize_anchor)
+            .ok_or_else(|| anyhow!("invalid position"))?;
+        if !buffer.can_resolve(&position) {
+            Err(anyhow!("cannot resolve position"))?;
+        }
+        Ok(Self {
+            position: position.to_point_utf16(buffer),
+        })
+    }
+
+    fn response_to_proto(
+        range: Option<Range<Anchor>>,
+        _: &mut Project,
+        _: PeerId,
+        buffer_version: &clock::Global,
+        _: &mut ModelContext<Project>,
+    ) -> 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: buffer_version.into(),
+        }
+    }
+
     fn response_from_proto(
         self,
         message: proto::PrepareRenameResponse,
         _: ModelHandle<Project>,
+        buffer: ModelHandle<Buffer>,
         mut cx: AsyncAppContext,
     ) -> LocalBoxFuture<'static, Result<Option<Range<Anchor>>>> {
         async move {
             if message.can_rename {
-                self.buffer
+                buffer
                     .update(&mut cx, |buffer, _| {
                         buffer.wait_for_version(message.version.into())
                     })
@@ -141,29 +209,16 @@ impl LspCommand for PerformRename {
         }
     }
 
-    fn to_proto(&self, project_id: u64, cx: &AppContext) -> proto::PerformRename {
-        let buffer = &self.buffer.read(cx);
-        let buffer_id = buffer.remote_id();
-        proto::PerformRename {
-            project_id,
-            buffer_id,
-            position: Some(language::proto::serialize_anchor(
-                &buffer.anchor_before(self.position),
-            )),
-            new_name: self.new_name.clone(),
-        }
-    }
-
     fn response_from_lsp(
         self,
         message: Option<lsp::WorkspaceEdit>,
         project: ModelHandle<Project>,
+        buffer: ModelHandle<Buffer>,
         mut cx: AsyncAppContext,
     ) -> LocalBoxFuture<'static, Result<ProjectTransaction>> {
         async move {
             if let Some(edit) = message {
-                let (language_name, language_server) =
+                let (language_name, language_server) = buffer.read_with(&cx, |buffer, _| {
-                    self.buffer.read_with(&cx, |buffer, _| {
                     let language = buffer
                         .language()
                         .ok_or_else(|| anyhow!("buffer's language was removed"))?;
@@ -189,10 +244,67 @@ impl LspCommand for PerformRename {
         .boxed_local()
     }
 
+    fn to_proto(
+        &self,
+        project_id: u64,
+        buffer: &ModelHandle<Buffer>,
+        cx: &AppContext,
+    ) -> proto::PerformRename {
+        let buffer = buffer.read(cx);
+        let buffer_id = buffer.remote_id();
+        proto::PerformRename {
+            project_id,
+            buffer_id,
+            position: Some(language::proto::serialize_anchor(
+                &buffer.anchor_before(self.position),
+            )),
+            new_name: self.new_name.clone(),
+        }
+    }
+
+    fn buffer_id_from_proto(message: &proto::PerformRename) -> u64 {
+        message.buffer_id
+    }
+
+    fn from_proto(
+        message: proto::PerformRename,
+        _: &mut Project,
+        buffer: &ModelHandle<Buffer>,
+        cx: &mut ModelContext<Project>,
+    ) -> Result<Self> {
+        let position = message
+            .position
+            .and_then(deserialize_anchor)
+            .ok_or_else(|| anyhow!("invalid position"))?;
+        let buffer = buffer.read(cx);
+        if !buffer.can_resolve(&position) {
+            Err(anyhow!("cannot resolve position"))?;
+        }
+        Ok(Self {
+            position: position.to_point_utf16(buffer),
+            new_name: message.new_name,
+            push_to_history: false,
+        })
+    }
+
+    fn response_to_proto(
+        response: ProjectTransaction,
+        project: &mut Project,
+        peer_id: PeerId,
+        _: &clock::Global,
+        cx: &mut ModelContext<Project>,
+    ) -> proto::PerformRenameResponse {
+        let transaction = project.serialize_project_transaction_for_peer(response, peer_id, cx);
+        proto::PerformRenameResponse {
+            transaction: Some(transaction),
+        }
+    }
+
     fn response_from_proto(
         self,
         message: proto::PerformRenameResponse,
         project: ModelHandle<Project>,
+        _: ModelHandle<Buffer>,
         mut cx: AsyncAppContext,
     ) -> LocalBoxFuture<'static, Result<ProjectTransaction>> {
         async move {

crates/project/src/project.rs

@@ -184,8 +184,8 @@ impl Project {
         client.add_entity_request_handler(Self::handle_get_code_actions);
         client.add_entity_request_handler(Self::handle_get_completions);
         client.add_entity_request_handler(Self::handle_get_definition);
-        client.add_entity_request_handler(Self::handle_prepare_rename);
+        client.add_entity_request_handler(Self::handle_lsp_command::<lsp_command::PrepareRename>);
-        client.add_entity_request_handler(Self::handle_perform_rename);
+        client.add_entity_request_handler(Self::handle_lsp_command::<lsp_command::PerformRename>);
         client.add_entity_request_handler(Self::handle_open_buffer);
         client.add_entity_request_handler(Self::handle_save_buffer);
     }
@@ -1829,7 +1829,7 @@ impl Project {
         cx: &mut ModelContext<Self>,
     ) -> Task<Result<Option<Range<Anchor>>>> {
         let position = position.to_point_utf16(buffer.read(cx));
-        self.request_lsp(buffer.clone(), PrepareRename { buffer, position }, cx)
+        self.request_lsp(buffer, PrepareRename { position }, cx)
     }
 
     pub fn perform_rename<T: ToPointUtf16>(
@@ -1842,9 +1842,8 @@ impl Project {
     ) -> Task<Result<ProjectTransaction>> {
         let position = position.to_point_utf16(buffer.read(cx));
         self.request_lsp(
-            buffer.clone(),
-            PerformRename {
             buffer,
+            PerformRename {
                 position,
                 new_name,
                 push_to_history,
@@ -1862,8 +1861,8 @@ impl Project {
     where
         <R::LspRequest as lsp::request::Request>::Result: Send,
     {
-        let buffer = buffer_handle.read(cx);
         if self.is_local() {
+            let buffer = buffer_handle.read(cx);
             let file = File::from_dyn(buffer.file()).and_then(File::as_local);
             if let Some((file, language_server)) = file.zip(buffer.language_server().cloned()) {
                 let lsp_params = request.to_lsp(&file.abs_path(cx), cx);
@@ -1872,15 +1871,19 @@ impl Project {
                         .request::<R::LspRequest>(lsp_params)
                         .await
                         .context("lsp request failed")?;
-                    request.response_from_lsp(response, this, cx).await
+                    request
+                        .response_from_lsp(response, this, buffer_handle, cx)
+                        .await
                 });
             }
         } else if let Some(project_id) = self.remote_id() {
             let rpc = self.client.clone();
-            let message = request.to_proto(project_id, cx);
+            let message = request.to_proto(project_id, &buffer_handle, cx);
             return cx.spawn(|this, cx| async move {
                 let response = rpc.request(message).await?;
-                request.response_from_proto(response, this, cx).await
+                request
+                    .response_from_proto(response, this, buffer_handle, cx)
+                    .await
             });
         }
         Task::ready(Ok(Default::default()))
@@ -2619,84 +2622,36 @@ impl Project {
         })
     }
 
-    async fn handle_prepare_rename(
+    async fn handle_lsp_command<T: LspCommand>(
         this: ModelHandle<Self>,
-        envelope: TypedEnvelope<proto::PrepareRename>,
+        envelope: TypedEnvelope<T::ProtoRequest>,
         _: Arc<Client>,
         mut cx: AsyncAppContext,
-    ) -> Result<proto::PrepareRenameResponse> {
+    ) -> Result<<T::ProtoRequest as proto::RequestMessage>::Response>
+    where
+        <T::LspRequest as lsp::request::Request>::Result: Send,
+    {
         let sender_id = envelope.original_sender_id()?;
-        let position = envelope
+        let (request, buffer_version) = this.update(&mut cx, |this, cx| {
-            .payload
+            let buffer_id = T::buffer_id_from_proto(&envelope.payload);
-            .position
-            .and_then(deserialize_anchor)
-            .ok_or_else(|| anyhow!("invalid position"))?;
-        let (prepare_rename, version) = this.update(&mut cx, |this, cx| {
             let buffer_handle = this
                 .shared_buffers
                 .get(&sender_id)
-                .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned())
+                .and_then(|shared_buffers| shared_buffers.get(&buffer_id).cloned())
-                .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?;
+                .ok_or_else(|| anyhow!("unknown buffer id {}", buffer_id))?;
-            let buffer = buffer_handle.read(cx);
+            let buffer_version = buffer_handle.read(cx).version();
-            let version = buffer.version();
+            let request = T::from_proto(envelope.payload, this, &buffer_handle, cx)?;
-            if buffer.can_resolve(&position) {
+            Ok::<_, anyhow::Error>((this.request_lsp(buffer_handle, request, cx), buffer_version))
-                Ok((this.prepare_rename(buffer_handle, position, cx), version))
-            } else {
-                Err(anyhow!("cannot resolve position"))
-            }
         })?;
-
+        let response = request.await?;
-        let range = prepare_rename.await?;
+        this.update(&mut cx, |this, cx| {
-        Ok(proto::PrepareRenameResponse {
+            Ok(T::response_to_proto(
-            can_rename: range.is_some(),
+                response,
-            start: range
+                this,
-                .as_ref()
+                sender_id,
-                .map(|range| language::proto::serialize_anchor(&range.start)),
+                &buffer_version,
-            end: range
-                .as_ref()
-                .map(|range| language::proto::serialize_anchor(&range.end)),
-            version: (&version).into(),
-        })
-    }
-
-    async fn handle_perform_rename(
-        this: ModelHandle<Self>,
-        envelope: TypedEnvelope<proto::PerformRename>,
-        _: Arc<Client>,
-        mut cx: AsyncAppContext,
-    ) -> Result<proto::PerformRenameResponse> {
-        let sender_id = envelope.original_sender_id()?;
-        let position = envelope
-            .payload
-            .position
-            .and_then(deserialize_anchor)
-            .ok_or_else(|| anyhow!("invalid position"))?;
-        let perform_rename = this.update(&mut cx, |this, cx| {
-            let buffer_handle = this
-                .shared_buffers
-                .get(&sender_id)
-                .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned())
-                .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?;
-            let buffer = buffer_handle.read(cx);
-            if buffer.can_resolve(&position) {
-                Ok(this.perform_rename(
-                    buffer_handle,
-                    position,
-                    envelope.payload.new_name,
-                    false,
                 cx,
             ))
-            } else {
-                Err(anyhow!("cannot resolve position"))
-            }
-        })?;
-
-        let transaction = perform_rename.await?;
-        let transaction = this.update(&mut cx, |this, cx| {
-            this.serialize_project_transaction_for_peer(transaction, sender_id, cx)
-        });
-        Ok(proto::PerformRenameResponse {
-            transaction: Some(transaction),
         })
     }