Skip to content

Commit c58d702

Browse files
skagedalclaudecoder3101
authored
feat(rename): Allow RPCs, fields, reference sites to be renamed with rpc/request/response chains (#131)
I would like the "rename" action to just work everywhere for the symbol under the cursor. This is wired into my muscle-memory from using other editors and I tend to do it even when it's a bit silly, like in cases where the symbol just would never be referenced from anywhere else (such as a service name in Protobuf). I like to just not have to think about it. I filed an issue before (#129) about wanting to be able to rename a message from a referenced site. This PR includes that, but also some other rename improvements. If you would rather want me to split it into separate PRs, let me know. I find the rpc/Request/Response chain rename especially useful. I realize all protobuf users may not follow this convention, but it should (hopefully) not get in the way for people who don't. Let me know if you would rather like this to be configurable. ## Additions **Service and RPC symbol rename.** `can_rename` was gated on `message_name`/`enum_name` parents only, so invoking rename on a service or RPC method was a silent no-op. A new `is_renameable` predicate accepts `service_name` and `rpc_name`, and `rename_tree`'s empty-ancestor path returns a single in-file edit (services/RPCs have no cross-file references to propagate). **Rename from a type-reference site.** Previously you had to navigate to the declaration first. Now invoking rename on a `MyRequest` reference inside `rpc DoThing(MyRequest)` — or on the inner segment of a qualified type like `Outer.Inner` — works: the LSP resolves the partial qualified path under the cursor to the declaration via the existing definition machinery and runs the regular rename from there. The user's "rename the segment under cursor" intent is honored: cursor on `Outer` renames `Outer`, cursor on `Inner` renames `Inner`. **Field name, oneof, and enum-value rename.** Identifiers whose parent is `field`, `map_field`, `oneof_field`, `oneof`, or `enum_field` are now renameable. Each is single-site — these don't appear as type references in other `.proto` files — so the workspace pass is intentionally a no-op for them, avoiding accidental renames if a lowercase type happens to share a name. **Chained `rpc`/`Request`/`Response` rename.** When an `rpc` follows the `rpc <Name>(<Name>Request) returns (<Name>Response)` convention from the [Google API design guide](https://google.aip.dev/) (AIPs 131–136), renaming any one of the three triggers a chained rename of the other two. The chain only kicks in when all of these hold: - the matching message name follows the convention exactly (trailing segment of its qualified text), - the request/response is used by exactly one rpc in the workspace, and - the user's new name preserves the convention (e.g. renaming a request, the new name still ends with `Request`). If any check fails, only the symbol the user invoked rename on is renamed — the primary rename is never blocked by chain-detection failures. ## Implementation This implementation has been assisted by Claude Code using Opus 4.7. I have read and generally understood the code and sign off on it as a human being. I have previous experience with Rust, but would not consider myself an expert. I have no previous experience with writing LSP:s. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Ashar <coder3101@users.noreply.github.com> Co-authored-by: Ashar <ashar786khan@gmail.com>
1 parent 0e98894 commit c58d702

35 files changed

Lines changed: 1565 additions & 34 deletions

File tree

README.md

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
- [Configuration Sections](#configuration-sections)
3333
- [Basic Configuration](#basic-configuration)
3434
- [Path Configuration](#path-configuration)
35+
- [Rename Configuration](#rename-configuration)
3536
- [Usage](#-usage)
3637
- [Code Completion](#code-completion)
3738
- [Diagnostics](#diagnostics)
@@ -170,6 +171,9 @@ include_paths = ["foobar", "bazbaaz"] # Include paths to look for protofiles dur
170171
[config.path]
171172
clang_format = "clang-format"
172173
protoc = "protoc"
174+
175+
[config.rename]
176+
chain_rpc_request_response = false # Also rename <Rpc>Request/<Rpc>Response messages when renaming an rpc
173177
```
174178

175179
### Configuration Sections
@@ -197,6 +201,16 @@ The `[config.path]` section contains path for various tools used by LSP.
197201
- `clang_format`: Uses clang_format from this path for formatting
198202
- `protoc`: Uses protoc from this path for diagnostics
199203

204+
#### Rename Configuration
205+
206+
The `[config.rename]` section tunes rename behaviour.
207+
208+
- `chain_rpc_request_response` (default `false`): when enabled, renaming an `rpc`
209+
also renames its convention-named `<Rpc>Request` and `<Rpc>Response` messages —
210+
and renaming such a message renames the `rpc` and its sibling message. The chain
211+
only fires when the names follow the [Google API design guide](https://cloud.google.com/apis/design/naming_convention#request_and_response_messages)
212+
convention and the messages are used by exactly one `rpc`.
213+
200214
---
201215

202216
## 🛠 Usage
@@ -233,7 +247,9 @@ Hover over any symbol or imports to get detailed documentation and comments asso
233247

234248
### Rename Symbols
235249

236-
Rename symbols like messages or enums, and Propagate the changes throughout the codebase. Currently, field renaming within symbols is not supported.
250+
Rename symbols like messages, enums, services and RPC methods, and propagate the changes throughout the codebase. Rename also works when invoked on a type reference (e.g. the request or response type of an `rpc`) — the LSP pivots to the declaration and applies the rename from there. Field names, oneof names, and enum values can also be renamed at their declaration site (single-site rename, since they aren't referenced as types from other `.proto` files).
251+
252+
When an `rpc` follows the `rpc <Name>(<Name>Request) returns (<Name>Response)` convention from the [Google API design guide](https://google.aip.dev/) (AIPs 131–136), renaming any one of the three triggers a chained rename of the other two — but only when (a) the matching message name follows the convention exactly, (b) the request/response is used by exactly one rpc in the workspace, and (c) the user's new name preserves the convention. If any check fails, only the symbol the user invoked rename on is renamed.
237253

238254
### Find References
239255

protols.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
[config]
22
include_paths = ["src/workspace/input"]
3+
4+
[config.rename]
5+
chain_rpc_request_response = true

sample/simple.proto

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ service BookService {
5151
rpc GetBooksViaAuthor(GetBookViaAuthor) returns (stream Book) {}
5252
rpc GetGreatestBook(stream GetBookRequest) returns (Book) {}
5353
rpc GetBooks(stream GetBookRequest) returns (stream Book) {}
54+
rpc ReadBook(ReadBookRequest) returns (ReadBookResponse) {}
5455
}
5556

5657
message BookStore {
@@ -61,6 +62,9 @@ message BookStore {
6162
EnumSample sample = 4;
6263
}
6364

65+
message ReadBookRequest {}
66+
message ReadBookResponse {}
67+
6468
// These are enum options representing some operation in the proto
6569
// these are meant to be ony called from one place,
6670

src/config/input/protols-valid.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,6 @@ include_paths = ["foobar", "bazbaaz"]
44
[config.path]
55
protoc = "/usr/bin/protoc"
66
clang_format = "/usr/bin/clang-format"
7+
8+
[config.rename]
9+
chain_rpc_request_response = true

src/config/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub struct ProtolsConfig {
2121
pub struct Config {
2222
pub include_paths: Vec<String>,
2323
pub path: PathConfig,
24+
pub rename: RenameConfig,
2425
}
2526

2627
#[derive(Serialize, Deserialize, Debug, Clone)]
@@ -38,3 +39,14 @@ impl Default for PathConfig {
3839
}
3940
}
4041
}
42+
43+
#[derive(Serialize, Deserialize, Debug, Clone, Default)]
44+
#[serde(default)]
45+
pub struct RenameConfig {
46+
/// When renaming an `rpc`, also rename its convention-named
47+
/// `<Rpc>Request` / `<Rpc>Response` messages — and, when renaming such a
48+
/// message, the `rpc` and its sibling message. Opt-in: defaults to `false`
49+
/// since not every codebase follows the `<Rpc>Request`/`Response` naming
50+
/// convention.
51+
pub chain_rpc_request_response: bool,
52+
}

src/config/snapshots/protols__config__workspace__test__get_for_workspace-2.snap

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ config:
77
path:
88
clang_format: clang-format
99
protoc: protoc
10+
rename:
11+
chain_rpc_request_response: false

src/config/snapshots/protols__config__workspace__test__get_for_workspace.snap

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,5 @@ config:
99
path:
1010
clang_format: /usr/bin/clang-format
1111
protoc: /usr/bin/protoc
12+
rename:
13+
chain_rpc_request_response: true

src/lsp.rs

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use std::ops::ControlFlow;
2-
use std::{collections::HashMap, fs::read_to_string, path::PathBuf};
2+
use std::{fs::read_to_string, path::PathBuf};
33
use tracing::{error, info, warn};
44

55
use async_lsp::lsp_types::{
@@ -20,6 +20,7 @@ use async_lsp::{Error, LanguageClient, ResponseError};
2020
use futures::future::BoxFuture;
2121
use serde_json::Value;
2222

23+
use crate::context::jumpable::Jumpable;
2324
use crate::formatter::ProtoFormatter;
2425
use crate::server::ProtoLanguageServer;
2526
use crate::{docs, log};
@@ -244,7 +245,6 @@ impl ProtoLanguageServer {
244245
) -> BoxFuture<'static, Result<Option<WorkspaceEdit>, ResponseError>> {
245246
let uri = params.text_document_position.text_document.uri;
246247
let pos = params.text_document_position.position;
247-
248248
let new_name = params.new_name;
249249

250250
let Some(tree) = self.state.get_tree(&uri) else {
@@ -253,38 +253,72 @@ impl ProtoLanguageServer {
253253
};
254254

255255
let content = self.state.get_content(&uri);
256-
257256
let current_package = tree.get_package_name(content.as_bytes()).unwrap_or(".");
257+
let ipath = self.configs.get_include_paths(&uri).unwrap_or_default();
258258

259-
let Some((edit, otext, ntext)) = tree.rename_tree(&pos, &new_name, content.as_bytes())
260-
else {
261-
error!(uri=%uri, "failed to rename in a tree");
262-
return Box::pin(async move { Ok(None) });
259+
// If the cursor is on a type reference (inside a message_or_enum_type
260+
// node), pivot to the declaration and rename from there. The workspace
261+
// pass then handles all references — including the one the user is
262+
// standing on.
263+
let (decl_uri, decl_pos) = match tree.rename_pivot_identifier(&pos, content.as_bytes()) {
264+
Some(decl_path) => {
265+
let locations =
266+
self.state
267+
.definition(&ipath, current_package, Jumpable::Identifier(decl_path));
268+
let Some(decl) = locations.into_iter().next() else {
269+
error!(uri=%uri, "failed to resolve declaration for reference-site rename");
270+
return Box::pin(async move { Ok(None) });
271+
};
272+
(decl.uri, decl.range.start)
273+
}
274+
None => (uri.clone(), pos),
263275
};
264276

265-
let Some(workspace) = self.configs.get_workspace_for_uri(&uri) else {
266-
error!(uri=%uri, "failed to get workspace");
277+
let Some(workspace) = self.configs.get_workspace_for_uri(&decl_uri) else {
278+
error!(uri=%decl_uri, "failed to get workspace");
279+
return Box::pin(async move { Ok(None) });
280+
};
281+
let Ok(workspace_path) = workspace.to_file_path() else {
282+
error!(uri=%workspace, "workspace url is not a file path");
267283
return Box::pin(async move { Ok(None) });
268284
};
269285

270-
let work_done_token = params.work_done_progress_params.work_done_token;
271-
let progress_sender = work_done_token.map(|token| self.with_report_progress(token));
272-
273-
let mut h = HashMap::new();
274-
h.extend(self.state.rename_fields(
275-
current_package,
276-
&otext,
277-
&ntext,
278-
workspace.to_file_path().unwrap(),
279-
progress_sender,
280-
));
286+
let progress_sender = params
287+
.work_done_progress_params
288+
.work_done_token
289+
.map(|token| self.with_report_progress(token));
281290

282-
h.entry(tree.uri).or_insert(edit.clone()).extend(edit);
291+
// The rpc/request/response chain rename is opt-in via the workspace's
292+
// `[config.rename]` settings; without a config it stays off.
293+
let chain_rpc_request_response = self
294+
.configs
295+
.get_config_for_uri(&uri)
296+
.map(|c| c.config.rename.chain_rpc_request_response)
297+
.unwrap_or_default();
298+
299+
let ops = self.state.compute_rename_ops(
300+
&decl_uri,
301+
decl_pos,
302+
&new_name,
303+
&ipath,
304+
chain_rpc_request_response,
305+
);
306+
let Some(all_edits) = self
307+
.state
308+
.apply_rename_ops(&ops, workspace_path, progress_sender)
309+
else {
310+
error!(uri=%decl_uri, "failed to apply primary rename");
311+
return Box::pin(async move { Ok(None) });
312+
};
283313

284-
let response = Some(WorkspaceEdit {
285-
changes: Some(h),
286-
..Default::default()
287-
});
314+
let response = if all_edits.is_empty() {
315+
None
316+
} else {
317+
Some(WorkspaceEdit {
318+
changes: Some(all_edits),
319+
..Default::default()
320+
})
321+
};
288322

289323
Box::pin(async move { Ok(response) })
290324
}

src/nodekind.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,35 @@ impl NodeKind {
6363
n.kind() == Self::FieldName.as_str()
6464
}
6565

66+
pub fn is_rpc_name(n: &Node) -> bool {
67+
n.kind() == Self::RpcName.as_str()
68+
}
69+
6670
pub fn is_userdefined(n: &Node) -> bool {
6771
n.kind() == Self::EnumName.as_str() || n.kind() == Self::MessageName.as_str()
6872
}
6973

74+
pub fn is_renameable(n: &Node) -> bool {
75+
Self::is_userdefined(n)
76+
|| n.kind() == Self::ServiceName.as_str()
77+
|| n.kind() == Self::RpcName.as_str()
78+
|| n.kind() == Self::FieldName.as_str()
79+
|| Self::is_field_decl_parent(n)
80+
}
81+
82+
/// Kinds whose direct identifier child is the *name* of a field-like
83+
/// declaration: regular fields, map fields, oneof fields, the oneof itself,
84+
/// and enum values. For `string title = 1;`, the identifier `title` has
85+
/// parent `field` — that's what we match here. The type identifier (e.g.
86+
/// `Author` in `Author author = 2;`) is nested deeper under
87+
/// `message_or_enum_type`, so it isn't caught by this predicate.
88+
pub fn is_field_decl_parent(n: &Node) -> bool {
89+
matches!(
90+
n.kind(),
91+
"field" | "map_field" | "oneof_field" | "oneof" | "enum_field"
92+
)
93+
}
94+
7095
pub fn is_actionable(n: &Node) -> bool {
7196
n.kind() == Self::MessageName.as_str()
7297
|| n.kind() == Self::EnumName.as_str()
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
syntax = "proto3";
2+
3+
package com.parser;
4+
5+
enum Color {
6+
RED = 0;
7+
GREEN = 1;
8+
}
9+
10+
message Author {
11+
string name = 1;
12+
}
13+
14+
message Book {
15+
string title = 1;
16+
Author author = 2;
17+
map<string, int32> counts = 3;
18+
oneof body {
19+
string text = 4;
20+
bytes blob = 5;
21+
}
22+
}

0 commit comments

Comments
 (0)