diff --git a/README.md b/README.md index c223273..a383592 100644 --- a/README.md +++ b/README.md @@ -233,7 +233,9 @@ Hover over any symbol or imports to get detailed documentation and comments asso ### Rename Symbols -Rename symbols like messages or enums, and Propagate the changes throughout the codebase. Currently, field renaming within symbols is not supported. +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). + +When an `rpc` follows the `rpc (Request) returns (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. ### Find References diff --git a/src/lsp.rs b/src/lsp.rs index 73450a3..05e3106 100644 --- a/src/lsp.rs +++ b/src/lsp.rs @@ -1,5 +1,5 @@ use std::ops::ControlFlow; -use std::{collections::HashMap, fs::read_to_string, path::PathBuf}; +use std::{fs::read_to_string, path::PathBuf}; use tracing::{error, info, warn}; use async_lsp::lsp_types::{ @@ -20,6 +20,7 @@ use async_lsp::{Error, LanguageClient, ResponseError}; use futures::future::BoxFuture; use serde_json::Value; +use crate::context::jumpable::Jumpable; use crate::formatter::ProtoFormatter; use crate::server::ProtoLanguageServer; use crate::{docs, log}; @@ -244,7 +245,6 @@ impl ProtoLanguageServer { ) -> BoxFuture<'static, Result, ResponseError>> { let uri = params.text_document_position.text_document.uri; let pos = params.text_document_position.position; - let new_name = params.new_name; let Some(tree) = self.state.get_tree(&uri) else { @@ -253,38 +253,60 @@ impl ProtoLanguageServer { }; let content = self.state.get_content(&uri); - let current_package = tree.get_package_name(content.as_bytes()).unwrap_or("."); + let ipath = self.configs.get_include_paths(&uri).unwrap_or_default(); - let Some((edit, otext, ntext)) = tree.rename_tree(&pos, &new_name, content.as_bytes()) - else { - error!(uri=%uri, "failed to rename in a tree"); - return Box::pin(async move { Ok(None) }); + // If the cursor is on a type reference (inside a message_or_enum_type + // node), pivot to the declaration and rename from there. The workspace + // pass then handles all references — including the one the user is + // standing on. + let (decl_uri, decl_pos) = match tree.rename_pivot_identifier(&pos, content.as_bytes()) { + Some(decl_path) => { + let locations = + self.state + .definition(&ipath, current_package, Jumpable::Identifier(decl_path)); + let Some(decl) = locations.into_iter().next() else { + error!(uri=%uri, "failed to resolve declaration for reference-site rename"); + return Box::pin(async move { Ok(None) }); + }; + (decl.uri, decl.range.start) + } + None => (uri.clone(), pos), }; - let Some(workspace) = self.configs.get_workspace_for_uri(&uri) else { - error!(uri=%uri, "failed to get workspace"); + let Some(workspace) = self.configs.get_workspace_for_uri(&decl_uri) else { + error!(uri=%decl_uri, "failed to get workspace"); + return Box::pin(async move { Ok(None) }); + }; + let Ok(workspace_path) = workspace.to_file_path() else { + error!(uri=%workspace, "workspace url is not a file path"); return Box::pin(async move { Ok(None) }); }; - let work_done_token = params.work_done_progress_params.work_done_token; - let progress_sender = work_done_token.map(|token| self.with_report_progress(token)); + let progress_sender = params + .work_done_progress_params + .work_done_token + .map(|token| self.with_report_progress(token)); - let mut h = HashMap::new(); - h.extend(self.state.rename_fields( - current_package, - &otext, - &ntext, - workspace.to_file_path().unwrap(), - progress_sender, - )); - - h.entry(tree.uri).or_insert(edit.clone()).extend(edit); + let ops = self + .state + .compute_rename_ops(&decl_uri, decl_pos, &new_name, &ipath); + let Some(all_edits) = self + .state + .apply_rename_ops(&ops, workspace_path, progress_sender) + else { + error!(uri=%decl_uri, "failed to apply primary rename"); + return Box::pin(async move { Ok(None) }); + }; - let response = Some(WorkspaceEdit { - changes: Some(h), - ..Default::default() - }); + let response = if all_edits.is_empty() { + None + } else { + Some(WorkspaceEdit { + changes: Some(all_edits), + ..Default::default() + }) + }; Box::pin(async move { Ok(response) }) } diff --git a/src/nodekind.rs b/src/nodekind.rs index fc042e4..4575e6a 100644 --- a/src/nodekind.rs +++ b/src/nodekind.rs @@ -63,10 +63,35 @@ impl NodeKind { n.kind() == Self::FieldName.as_str() } + pub fn is_rpc_name(n: &Node) -> bool { + n.kind() == Self::RpcName.as_str() + } + pub fn is_userdefined(n: &Node) -> bool { n.kind() == Self::EnumName.as_str() || n.kind() == Self::MessageName.as_str() } + pub fn is_renameable(n: &Node) -> bool { + Self::is_userdefined(n) + || n.kind() == Self::ServiceName.as_str() + || n.kind() == Self::RpcName.as_str() + || n.kind() == Self::FieldName.as_str() + || Self::is_field_decl_parent(n) + } + + /// Kinds whose direct identifier child is the *name* of a field-like + /// declaration: regular fields, map fields, oneof fields, the oneof itself, + /// and enum values. For `string title = 1;`, the identifier `title` has + /// parent `field` — that's what we match here. The type identifier (e.g. + /// `Author` in `Author author = 2;`) is nested deeper under + /// `message_or_enum_type`, so it isn't caught by this predicate. + pub fn is_field_decl_parent(n: &Node) -> bool { + matches!( + n.kind(), + "field" | "map_field" | "oneof_field" | "oneof" | "enum_field" + ) + } + pub fn is_actionable(n: &Node) -> bool { n.kind() == Self::MessageName.as_str() || n.kind() == Self::EnumName.as_str() diff --git a/src/parser/input/test_rename_field.proto b/src/parser/input/test_rename_field.proto new file mode 100644 index 0000000..8b124ec --- /dev/null +++ b/src/parser/input/test_rename_field.proto @@ -0,0 +1,22 @@ +syntax = "proto3"; + +package com.parser; + +enum Color { + RED = 0; + GREEN = 1; +} + +message Author { + string name = 1; +} + +message Book { + string title = 1; + Author author = 2; + map counts = 3; + oneof body { + string text = 4; + bytes blob = 5; + } +} diff --git a/src/parser/input/test_rename_service.proto b/src/parser/input/test_rename_service.proto new file mode 100644 index 0000000..c7a50c0 --- /dev/null +++ b/src/parser/input/test_rename_service.proto @@ -0,0 +1,14 @@ +syntax = "proto3"; + +package com.parser; + +message Empty {} + +message Book { + string title = 1; +} + +service Library { + rpc GetBook(Empty) returns (Book); + rpc ListBooks(Empty) returns (Book); +} diff --git a/src/parser/rename.rs b/src/parser/rename.rs index fee43bd..67454c2 100644 --- a/src/parser/rename.rs +++ b/src/parser/rename.rs @@ -11,7 +11,7 @@ impl ParsedTree { .filter(NodeKind::is_identifier) .and_then(|n| { if let Some(parent) = n.parent() - && NodeKind::is_userdefined(&parent) + && NodeKind::is_renameable(&parent) { Some(Range { start: ts_to_lsp_position(&n.start_position()), @@ -23,6 +23,119 @@ impl ParsedTree { }) } + /// When the cursor is on a type-reference identifier (inside a + /// `message_or_enum_type` node), return the partial qualified path up to + /// and including the segment under cursor. This is the identifier whose + /// declaration the rename should pivot to. + /// + /// For `Outer.Inner` with cursor on `Inner`, returns `Some("Outer.Inner")`. + /// For `Outer.Inner` with cursor on `Outer`, returns `Some("Outer")`. + /// For a non-reference position, returns `None`. + /// + /// The reconstruction walks every child of `message_or_enum_type` (named + /// *and* anonymous) and concatenates their `utf8_text`. This depends on + /// tree-sitter-proto emitting the `.` separators as anonymous children + /// whose `utf8_text` is literally `"."`, and on the grammar leaving no + /// whitespace between identifier tokens. Both invariants are exercised by + /// `test_rename_pivot_identifier_qualified`, which asserts the exact + /// `"Book.Author"` reconstruction — a grammar change that violated either + /// invariant would fail that test rather than silently produce e.g. + /// `"BookAuthor"`. + pub fn rename_pivot_identifier( + &self, + pos: &Position, + content: impl AsRef<[u8]>, + ) -> Option { + let n = self.get_node_at_position(pos)?; + if !NodeKind::is_identifier(&n) { + return None; + } + let parent = n.parent()?; + if !NodeKind::is_field_name(&parent) { + return None; + } + + let cursor_end = n.end_byte(); + let bytes = content.as_ref(); + let mut path = String::new(); + let mut cursor = parent.walk(); + for child in parent.children(&mut cursor) { + let text = child.utf8_text(bytes).ok()?; + path.push_str(text); + if child.end_byte() >= cursor_end { + break; + } + } + Some(path) + } + + /// If the given position is on the rpc name of an rpc declaration, returns + /// the rpc's name along with its declared request and response type texts. + /// Used to drive the rpc/request/response chained rename. + pub fn rpc_at_position( + &self, + pos: &Position, + content: impl AsRef<[u8]>, + ) -> Option<(String, String, String)> { + let n = self.get_node_at_position(pos)?; + if !NodeKind::is_identifier(&n) { + return None; + } + let parent = n.parent()?; + if parent.kind() != NodeKind::RpcName.as_str() { + return None; + } + let rpc = parent.parent()?; + let bytes = content.as_ref(); + let mut cursor = rpc.walk(); + let mut types = rpc + .children(&mut cursor) + .filter(|c| c.kind() == NodeKind::FieldName.as_str()); + let request = types.next()?.utf8_text(bytes).ok()?.to_owned(); + let response = types.next()?.utf8_text(bytes).ok()?.to_owned(); + let rpc_name = n.utf8_text(bytes).ok()?.to_owned(); + Some((rpc_name, request, response)) + } + + /// If the given position is on a message name, returns that name. + pub fn message_name_at_position( + &self, + pos: &Position, + content: impl AsRef<[u8]>, + ) -> Option { + let n = self.get_node_at_position(pos)?; + if !NodeKind::is_identifier(&n) { + return None; + } + let parent = n.parent()?; + if parent.kind() != NodeKind::MessageName.as_str() { + return None; + } + Some(n.utf8_text(content.as_ref()).ok()?.to_owned()) + } + + /// Returns the (request, response) type texts for every `rpc` node in this + /// tree. Used to verify that a request/response type is uniquely used by a + /// single rpc before chain-renaming it. + pub fn all_rpc_signatures(&self, content: impl AsRef<[u8]>) -> Vec<(String, String)> { + let bytes = content.as_ref(); + let mut out = vec![]; + for rpc in self.find_all_nodes(|n: &Node| n.kind() == "rpc") { + let mut cursor = rpc.walk(); + let mut types = rpc + .children(&mut cursor) + .filter(|c| c.kind() == NodeKind::FieldName.as_str()); + let Some(req) = types.next().and_then(|c| c.utf8_text(bytes).ok()) else { + continue; + }; + let Some(resp) = types.next().and_then(|c| c.utf8_text(bytes).ok()) else { + continue; + }; + out.push((req.to_owned(), resp.to_owned())); + } + out + } + fn nodes_within<'a>( &self, n: Node<'a>, @@ -84,6 +197,26 @@ impl ParsedTree { let nodes = self.get_ancestor_nodes_at_position(pos); + // Renameable symbols with no message ancestor: top-level enums, services, RPCs, + // and the various field-like declarations (regular fields, map fields, oneof, + // oneof fields, enum values). Only top-level enums are referenced as types + // from other files; the rest are single-site, so we hand the workspace pass + // a name it won't find — making it a harmless no-op without risking that a + // field named the same as some lowercase type triggers an unwanted rename. + if nodes.is_empty() { + let n = self.get_node_at_position(pos)?; + let identifier = n.utf8_text(content.as_ref()).ok()?.to_owned(); + let is_type_symbol = n + .parent() + .is_some_and(|p| p.kind() == NodeKind::EnumName.as_str()); + let (otext, ntext) = if is_type_symbol { + (identifier, new_name.to_owned()) + } else { + (new_name.to_owned(), new_name.to_owned()) + }; + return Some((v, otext, ntext)); + } + let mut i = 1; let mut otext = nodes.first()?.utf8_text(content.as_ref()).ok()?.to_owned(); let mut ntext = new_name.to_owned(); @@ -262,4 +395,264 @@ mod test { assert_yaml_snapshot!(tree.can_rename(&pos_inner_type)); assert_yaml_snapshot!(tree.can_rename(&pos_outer_type)); } + + #[test] + fn test_can_rename_service_and_rpc() { + let uri: Url = "file://foo/bar/test.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_service.proto"); + let parsed = ProtoParser::new().parse(uri, contents); + assert!(parsed.is_some()); + let tree = parsed.unwrap(); + + let pos_service = Position { + line: 10, + character: 10, + }; + let pos_rpc = Position { + line: 11, + character: 9, + }; + let pos_rpc_request_type = Position { + line: 11, + character: 17, + }; + + assert_yaml_snapshot!(tree.can_rename(&pos_service)); + assert_yaml_snapshot!(tree.can_rename(&pos_rpc)); + // Type references inside an RPC declaration are renameable from the + // reference site; the LSP layer pivots to the declaration. + assert_yaml_snapshot!(tree.can_rename(&pos_rpc_request_type)); + } + + #[test] + fn test_rename_service_and_rpc() { + let uri: Url = "file://foo/bar.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_service.proto"); + let parsed = ProtoParser::new().parse(uri, contents); + assert!(parsed.is_some()); + let tree = parsed.unwrap(); + + let pos_service = Position { + line: 10, + character: 10, + }; + let pos_rpc = Position { + line: 11, + character: 9, + }; + + let rename_fn = |nt: &str, pos: &Position| match tree.rename_tree(pos, nt, contents) { + Some(k) => { + let mut v = tree.rename_field(&k.1, &k.2, contents); + v.extend(k.0); + v + } + _ => vec![], + }; + + assert_yaml_snapshot!(rename_fn("Catalog", &pos_service)); + assert_yaml_snapshot!(rename_fn("FetchBook", &pos_rpc)); + } + + #[test] + fn test_rename_pivot_identifier() { + let uri: Url = "file://foo/bar/test.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_service.proto"); + let parsed = ProtoParser::new().parse(uri, contents).unwrap(); + + // `Empty` reference at line 11 (the rpc Get(Empty) ...): char 16 = 'E' + let pos_unqualified_ref = Position { + line: 11, + character: 17, + }; + // `Book` return type at line 11 char 32..36 + let pos_other_ref = Position { + line: 11, + character: 33, + }; + // Service declaration site — not a reference, so no pivot needed + let pos_decl = Position { + line: 10, + character: 10, + }; + + assert_eq!( + parsed.rename_pivot_identifier(&pos_unqualified_ref, contents), + Some("Empty".to_owned()) + ); + assert_eq!( + parsed.rename_pivot_identifier(&pos_other_ref, contents), + Some("Book".to_owned()) + ); + assert_eq!(parsed.rename_pivot_identifier(&pos_decl, contents), None); + } + + #[test] + fn test_rpc_at_position_and_signatures() { + let uri: Url = "file://foo/bar.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_service.proto"); + let parsed = ProtoParser::new().parse(uri, contents).unwrap(); + + // `GetBook` rpc at line 11 chars 8..15 + let pos_rpc = Position { + line: 11, + character: 10, + }; + assert_eq!( + parsed.rpc_at_position(&pos_rpc, contents), + Some(("GetBook".to_owned(), "Empty".to_owned(), "Book".to_owned(),)), + ); + + // Cursor on a non-rpc identifier should return None. + let pos_service = Position { + line: 10, + character: 10, + }; + assert_eq!(parsed.rpc_at_position(&pos_service, contents), None); + + // all_rpc_signatures should pick up both rpcs in the file. + let sigs = parsed.all_rpc_signatures(contents); + assert_eq!( + sigs, + vec![ + ("Empty".to_owned(), "Book".to_owned()), + ("Empty".to_owned(), "Book".to_owned()), + ] + ); + } + + #[test] + fn test_message_name_at_position() { + let uri: Url = "file://foo/bar.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_service.proto"); + let parsed = ProtoParser::new().parse(uri, contents).unwrap(); + + // `Book` declaration at line 6 chars 8..12 + let pos = Position { + line: 6, + character: 9, + }; + assert_eq!( + parsed.message_name_at_position(&pos, contents), + Some("Book".to_owned()) + ); + + // RPC name shouldn't match. + let pos_rpc = Position { + line: 11, + character: 10, + }; + assert_eq!(parsed.message_name_at_position(&pos_rpc, contents), None); + } + + #[test] + fn test_rename_field_and_enum_value() { + let uri: Url = "file://foo/bar.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_field.proto"); + let parsed = ProtoParser::new().parse(uri, contents).unwrap(); + let tree = parsed; + + let rename_fn = |nt: &str, pos: &Position| match tree.rename_tree(pos, nt, contents) { + Some(k) => { + let mut v = tree.rename_field(&k.1, &k.2, contents); + v.extend(k.0); + v + } + _ => vec![], + }; + + // Enum value: RED at line 5 chars 4..7 + let pos_enum_value = Position { + line: 5, + character: 5, + }; + // Plain field: title at line 14 chars 11..16 + let pos_plain_field = Position { + line: 14, + character: 12, + }; + // User-type field: author at line 15 chars 11..17 + let pos_user_type_field = Position { + line: 15, + character: 12, + }; + // Map field: counts at line 16 chars 23..29 + let pos_map_field = Position { + line: 16, + character: 24, + }; + // Oneof name: body at line 17 chars 10..14 + let pos_oneof_name = Position { + line: 17, + character: 11, + }; + // Oneof field: text at line 18 chars 15..19 + let pos_oneof_field = Position { + line: 18, + character: 16, + }; + + assert_yaml_snapshot!(rename_fn("CRIMSON", &pos_enum_value)); + assert_yaml_snapshot!(rename_fn("name", &pos_plain_field)); + assert_yaml_snapshot!(rename_fn("writer", &pos_user_type_field)); + assert_yaml_snapshot!(rename_fn("tallies", &pos_map_field)); + assert_yaml_snapshot!(rename_fn("content", &pos_oneof_name)); + assert_yaml_snapshot!(rename_fn("words", &pos_oneof_field)); + } + + #[test] + fn test_can_rename_field_and_enum_value() { + let uri: Url = "file://foo/bar.proto".parse().unwrap(); + let contents = include_str!("input/test_rename_field.proto"); + let parsed = ProtoParser::new().parse(uri, contents).unwrap(); + + // Cursor on a type identifier inside a field (`Author` at line 15 + // chars 4..10) is a reference site — already supported. + let pos_type_ref = Position { + line: 15, + character: 6, + }; + // Cursor on the field name should now be renameable. + let pos_plain_field = Position { + line: 14, + character: 12, + }; + // Cursor on the int_lit `1` is not a renameable identifier. + let pos_field_number = Position { + line: 14, + character: 19, + }; + + assert_yaml_snapshot!(parsed.can_rename(&pos_type_ref)); + assert_yaml_snapshot!(parsed.can_rename(&pos_plain_field)); + assert_yaml_snapshot!(parsed.can_rename(&pos_field_number)); + } + + #[test] + fn test_rename_pivot_identifier_qualified() { + // `Book.Author a = 1;` at line 19 of test_can_rename.proto + let uri: Url = "file://foo/bar/test.proto".parse().unwrap(); + let contents = include_str!("input/test_can_rename.proto"); + let parsed = ProtoParser::new().parse(uri, contents).unwrap(); + + // Cursor on `Book` (the outer segment): chars 4..8 + let pos_outer = Position { + line: 19, + character: 5, + }; + // Cursor on `Author` (the inner segment): chars 9..15 + let pos_inner = Position { + line: 19, + character: 11, + }; + + assert_eq!( + parsed.rename_pivot_identifier(&pos_outer, contents), + Some("Book".to_owned()) + ); + assert_eq!( + parsed.rename_pivot_identifier(&pos_inner, contents), + Some("Book.Author".to_owned()) + ); + } } diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename-3.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename-3.snap index 9292d37..25a8302 100644 --- a/src/parser/snapshots/protols__parser__rename__test__can_rename-3.snap +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename-3.snap @@ -2,4 +2,9 @@ source: src/parser/rename.rs expression: tree.can_rename(&pos_inner_type) --- -~ +start: + line: 19 + character: 9 +end: + line: 19 + character: 15 diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename-4.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename-4.snap index 9545453..491df1e 100644 --- a/src/parser/snapshots/protols__parser__rename__test__can_rename-4.snap +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename-4.snap @@ -2,4 +2,9 @@ source: src/parser/rename.rs expression: tree.can_rename(&pos_outer_type) --- -~ +start: + line: 19 + character: 4 +end: + line: 19 + character: 8 diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value-2.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value-2.snap new file mode 100644 index 0000000..d52c7fc --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value-2.snap @@ -0,0 +1,10 @@ +--- +source: src/parser/rename.rs +expression: parsed.can_rename(&pos_plain_field) +--- +start: + line: 14 + character: 11 +end: + line: 14 + character: 16 diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value-3.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value-3.snap new file mode 100644 index 0000000..4943cbd --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value-3.snap @@ -0,0 +1,5 @@ +--- +source: src/parser/rename.rs +expression: parsed.can_rename(&pos_field_number) +--- +~ diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value.snap new file mode 100644 index 0000000..f5227ad --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename_field_and_enum_value.snap @@ -0,0 +1,10 @@ +--- +source: src/parser/rename.rs +expression: parsed.can_rename(&pos_type_ref) +--- +start: + line: 15 + character: 4 +end: + line: 15 + character: 10 diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc-2.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc-2.snap new file mode 100644 index 0000000..e4e36a2 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc-2.snap @@ -0,0 +1,10 @@ +--- +source: src/parser/rename.rs +expression: tree.can_rename(&pos_rpc) +--- +start: + line: 11 + character: 8 +end: + line: 11 + character: 15 diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc-3.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc-3.snap new file mode 100644 index 0000000..27f212b --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc-3.snap @@ -0,0 +1,10 @@ +--- +source: src/parser/rename.rs +expression: tree.can_rename(&pos_rpc_request_type) +--- +start: + line: 11 + character: 16 +end: + line: 11 + character: 21 diff --git a/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc.snap b/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc.snap new file mode 100644 index 0000000..74b2212 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__can_rename_service_and_rpc.snap @@ -0,0 +1,10 @@ +--- +source: src/parser/rename.rs +expression: tree.can_rename(&pos_service) +--- +start: + line: 10 + character: 8 +end: + line: 10 + character: 15 diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-2.snap b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-2.snap new file mode 100644 index 0000000..19d2434 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-2.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"name\", &pos_plain_field)" +--- +- range: + start: + line: 14 + character: 11 + end: + line: 14 + character: 16 + newText: name diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-3.snap b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-3.snap new file mode 100644 index 0000000..0154bca --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-3.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"writer\", &pos_user_type_field)" +--- +- range: + start: + line: 15 + character: 11 + end: + line: 15 + character: 17 + newText: writer diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-4.snap b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-4.snap new file mode 100644 index 0000000..2b01e20 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-4.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"tallies\", &pos_map_field)" +--- +- range: + start: + line: 16 + character: 23 + end: + line: 16 + character: 29 + newText: tallies diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-5.snap b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-5.snap new file mode 100644 index 0000000..47afe03 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-5.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"content\", &pos_oneof_name)" +--- +- range: + start: + line: 17 + character: 10 + end: + line: 17 + character: 14 + newText: content diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-6.snap b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-6.snap new file mode 100644 index 0000000..e36760e --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value-6.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"words\", &pos_oneof_field)" +--- +- range: + start: + line: 18 + character: 15 + end: + line: 18 + character: 19 + newText: words diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value.snap b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value.snap new file mode 100644 index 0000000..faaf177 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_field_and_enum_value.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"CRIMSON\", &pos_enum_value)" +--- +- range: + start: + line: 5 + character: 4 + end: + line: 5 + character: 7 + newText: CRIMSON diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_service_and_rpc-2.snap b/src/parser/snapshots/protols__parser__rename__test__rename_service_and_rpc-2.snap new file mode 100644 index 0000000..6359cbc --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_service_and_rpc-2.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"FetchBook\", &pos_rpc)" +--- +- range: + start: + line: 11 + character: 8 + end: + line: 11 + character: 15 + newText: FetchBook diff --git a/src/parser/snapshots/protols__parser__rename__test__rename_service_and_rpc.snap b/src/parser/snapshots/protols__parser__rename__test__rename_service_and_rpc.snap new file mode 100644 index 0000000..b69ea55 --- /dev/null +++ b/src/parser/snapshots/protols__parser__rename__test__rename_service_and_rpc.snap @@ -0,0 +1,12 @@ +--- +source: src/parser/rename.rs +expression: "rename_fn(\"Catalog\", &pos_service)" +--- +- range: + start: + line: 10 + character: 8 + end: + line: 10 + character: 15 + newText: Catalog diff --git a/src/utils.rs b/src/utils.rs index d7bf3d8..2ab843a 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -36,6 +36,12 @@ pub fn is_inner_identifier(s: &str) -> bool { s.split('.').all(is_title_case) } +/// Returns the segment after the last `.` in a dotted identifier, or the whole +/// string if it contains no dot. +pub fn trailing_segment(qualified: &str) -> &str { + qualified.rsplit_once('.').map_or(qualified, |(_, t)| t) +} + pub fn split_identifier_package(s: &str) -> (&str, &str) { let s = s.trim_start_matches("."); if is_inner_identifier(s) || !s.contains('.') { @@ -59,7 +65,16 @@ pub fn split_identifier_package(s: &str) -> (&str, &str) { #[cfg(test)] mod test { - use crate::utils::{is_inner_identifier, split_identifier_package}; + use crate::utils::{is_inner_identifier, split_identifier_package, trailing_segment}; + + #[test] + fn test_trailing_segment() { + assert_eq!(trailing_segment("Foo"), "Foo"); + assert_eq!(trailing_segment("foo.Bar"), "Bar"); + assert_eq!(trailing_segment("foo.bar.Baz"), "Baz"); + assert_eq!(trailing_segment(".foo.Bar"), "Bar"); + assert_eq!(trailing_segment(""), ""); + } #[test] fn test_is_inner_identifier() { diff --git a/src/workspace/input/collision_bar.proto b/src/workspace/input/collision_bar.proto new file mode 100644 index 0000000..6904a34 --- /dev/null +++ b/src/workspace/input/collision_bar.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; + +package com.bar; + +message ListReq {} +message ListResp {} + +service Other { + rpc GetBook(ListReq) returns (ListResp); +} diff --git a/src/workspace/input/collision_foo.proto b/src/workspace/input/collision_foo.proto new file mode 100644 index 0000000..9b4bc27 --- /dev/null +++ b/src/workspace/input/collision_foo.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; + +package com.foo; + +message GetBookRequest {} +message GetBookResponse {} + +service Library { + rpc GetBook(GetBookRequest) returns (GetBookResponse); +} diff --git a/src/workspace/input/messages.proto b/src/workspace/input/messages.proto new file mode 100644 index 0000000..1950205 --- /dev/null +++ b/src/workspace/input/messages.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +package com.workspace; + +message GetBookRequest {} +message GetBookResponse { + string title = 1; +} +message FrobRequest {} +message Bar {} +message SharedReq {} +message SharedResp {} diff --git a/src/workspace/input/service.proto b/src/workspace/input/service.proto new file mode 100644 index 0000000..2a62b98 --- /dev/null +++ b/src/workspace/input/service.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +package com.workspace; + +import "messages.proto"; + +service Library { + rpc GetBook(GetBookRequest) returns (GetBookResponse); + rpc Frob(FrobRequest) returns (Bar); + rpc ListA(SharedReq) returns (SharedResp); + rpc ListB(SharedReq) returns (SharedResp); +} diff --git a/src/workspace/rename.rs b/src/workspace/rename.rs index 88a1d89..e68e76c 100644 --- a/src/workspace/rename.rs +++ b/src/workspace/rename.rs @@ -1,12 +1,24 @@ -use crate::utils::split_identifier_package; use std::collections::HashMap; use std::path::PathBuf; +use std::sync::mpsc::Sender; -use async_lsp::lsp_types::{Location, TextEdit, Url}; +use async_lsp::lsp_types::{Location, Position, ProgressParamsValue, Range, TextEdit, Url}; +use crate::context::jumpable::Jumpable; +use crate::nodekind::NodeKind; use crate::state::ProtoLanguageState; -use async_lsp::lsp_types::ProgressParamsValue; -use std::sync::mpsc::Sender; +use crate::utils::{split_identifier_package, trailing_segment, ts_to_lsp_position}; + +/// A single rename operation to apply against the workspace: rename whatever +/// symbol is declared at `(uri, pos)` to `new_name`. Multiple ops are merged +/// into one `WorkspaceEdit` when a single user invocation triggers chained +/// renames (e.g. rpc + request + response). +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct RenameOp { + pub uri: Url, + pub pos: Position, + pub new_name: String, +} impl ProtoLanguageState { pub fn rename_fields( @@ -128,16 +140,371 @@ impl ProtoLanguageState { }); if r.is_empty() { None } else { Some(r) } } + + /// Find every rpc declaration in the workspace whose simple name matches + /// `rpc_name`. Used by the rpc/request/response chain rename to enumerate + /// candidate rpcs when the user invokes rename on a convention-named + /// message; the caller then narrows the list by checking which candidate + /// actually references the user's primary message. + pub fn find_rpc_decls(&self, rpc_name: &str) -> Vec { + let mut out = vec![]; + for tree in self.get_trees() { + let content = self.get_content(&tree.uri); + for node in tree.find_all_nodes(NodeKind::is_rpc_name) { + let Ok(text) = node.utf8_text(content.as_bytes()) else { + continue; + }; + if text == rpc_name { + out.push(Location { + uri: tree.uri.clone(), + range: Range { + start: ts_to_lsp_position(&node.start_position()), + end: ts_to_lsp_position(&node.end_position()), + }, + }); + } + } + } + out + } + + /// Count the number of rpcs in the workspace whose request or response + /// type's trailing identifier segment matches `type_simple_name`. Used for + /// the uniqueness check before chain-renaming a request/response message. + pub fn count_rpc_uses_of_type(&self, type_simple_name: &str) -> usize { + let mut count = 0; + for tree in self.get_trees() { + let content = self.get_content(&tree.uri); + for (req, resp) in tree.all_rpc_signatures(content.as_bytes()) { + if trailing_segment(&req) == type_simple_name + || trailing_segment(&resp) == type_simple_name + { + count += 1; + } + } + } + count + } + + /// Compute every rename operation that should run for a user's rename + /// invocation. The first op is the user's *primary* rename (always + /// present); any additional ops are rpc/request/response chain siblings. + /// + /// `decl_uri`/`decl_pos` must already point at the *declaration* of the + /// symbol being renamed — the caller is responsible for pivoting from a + /// reference site to the declaration before calling this. + pub fn compute_rename_ops( + &self, + decl_uri: &Url, + decl_pos: Position, + new_name: &str, + ipath: &[PathBuf], + ) -> Vec { + let mut ops = vec![RenameOp { + uri: decl_uri.clone(), + pos: decl_pos, + new_name: new_name.to_owned(), + }]; + ops.extend(self.compute_chain_siblings(decl_uri, decl_pos, new_name, ipath)); + ops + } + + /// Apply a sequence of rename ops, merging their per-file edits into a + /// single map. Returns `None` if the *primary* (first) op fails — in that + /// case the user's invocation should produce no edit at all. Sibling + /// failures are silently skipped so the primary always lands. + pub fn apply_rename_ops( + &mut self, + ops: &[RenameOp], + workspace: PathBuf, + progress_sender: Option>, + ) -> Option>> { + let mut all: HashMap> = HashMap::new(); + let mut progress = progress_sender; + for (i, op) in ops.iter().enumerate() { + // Only the first op gets the progress sender; subsequent ops would + // double-report. + let sender = progress.take(); + match self.run_single_rename(op, workspace.clone(), sender) { + Some(edits) => { + for (u, e) in edits { + all.entry(u).or_default().extend(e); + } + } + None if i == 0 => return None, + None => continue, + } + } + Some(all) + } + + fn run_single_rename( + &mut self, + op: &RenameOp, + workspace: PathBuf, + progress_sender: Option>, + ) -> Option>> { + let tree = self.get_tree(&op.uri)?; + let content = self.get_content(&op.uri); + let package = tree.get_package_name(content.as_bytes()).unwrap_or("."); + + let (edit, otext, ntext) = tree.rename_tree(&op.pos, &op.new_name, content.as_bytes())?; + + let mut h: HashMap> = HashMap::new(); + h.extend(self.rename_fields(package, &otext, &ntext, workspace, progress_sender)); + h.entry(tree.uri.clone()).or_default().extend(edit); + Some(h) + } + + fn compute_chain_siblings( + &self, + decl_uri: &Url, + decl_pos: Position, + new_name: &str, + ipath: &[PathBuf], + ) -> Vec { + if new_name.is_empty() { + return vec![]; + } + let Some(tree) = self.get_tree(decl_uri) else { + return vec![]; + }; + let content = self.get_content(decl_uri); + let bytes = content.as_bytes(); + + if tree.rpc_at_position(&decl_pos, bytes).is_some() { + return self.chain_from_rpc_cursor(decl_uri, decl_pos, new_name, ipath); + } + if tree.message_name_at_position(&decl_pos, bytes).is_some() { + return self.chain_from_message_cursor(decl_uri, decl_pos, new_name, ipath); + } + vec![] + } + + /// Case A: user invoked rename on an `rpc_name`. The primary is the rpc + /// rename; siblings are the convention-matching request/response messages. + fn chain_from_rpc_cursor( + &self, + decl_uri: &Url, + decl_pos: Position, + new_name: &str, + ipath: &[PathBuf], + ) -> Vec { + let tree = self.get_tree(decl_uri).expect("checked by caller"); + let content = self.get_content(decl_uri); + let (old_rpc_name, request_text, response_text) = tree + .rpc_at_position(&decl_pos, content.as_bytes()) + .expect("checked by caller"); + self.sibling_message_ops( + decl_uri, + &old_rpc_name, + new_name, + ipath, + &[ + ("Request", request_text.as_str()), + ("Response", response_text.as_str()), + ], + ) + } + + /// Case B: user invoked rename on a `message_name` matching the + /// `Request` / `Response` convention. Primary is the message + /// rename; we additionally rename the rpc and the opposite sibling. + /// + /// To guard against unrelated rpcs in other packages that happen to share + /// the same simple name, we enumerate *all* candidate rpcs and pick the + /// unique one whose request/response slot resolves (via the workspace's + /// name-resolution rules) to the user's primary message. If zero or more + /// than one rpc matches, the chain is silently dropped. + fn chain_from_message_cursor( + &self, + decl_uri: &Url, + decl_pos: Position, + new_name: &str, + ipath: &[PathBuf], + ) -> Vec { + let tree = self.get_tree(decl_uri).expect("checked by caller"); + let content = self.get_content(decl_uri); + let msg_name = tree + .message_name_at_position(&decl_pos, content.as_bytes()) + .expect("checked by caller"); + + let Some((rpc_base, primary_suffix, new_rpc_base)) = + strip_convention_suffix(&msg_name, new_name) + else { + return vec![]; + }; + if rpc_base.is_empty() || new_rpc_base.is_empty() { + return vec![]; + } + + // Enumerate rpcs by simple name, then keep only those whose primary + // slot resolves to the user's actual declaration. + let mut matching: Vec<(Location, String, String)> = vec![]; + for rpc_loc in self.find_rpc_decls(&rpc_base) { + let Some(rpc_tree) = self.get_tree(&rpc_loc.uri) else { + continue; + }; + let rpc_content = self.get_content(&rpc_loc.uri); + let Some((_, rpc_req, rpc_resp)) = + rpc_tree.rpc_at_position(&rpc_loc.range.start, rpc_content.as_bytes()) + else { + continue; + }; + let slot_text = if primary_suffix == "Request" { + &rpc_req + } else { + &rpc_resp + }; + let rpc_pkg = rpc_tree + .get_package_name(rpc_content.as_bytes()) + .unwrap_or("."); + let resolves_to_primary = self + .definition(ipath, rpc_pkg, Jumpable::Identifier(slot_text.clone())) + .iter() + .any(|l| l.uri == *decl_uri && position_in_range(decl_pos, l.range)); + if resolves_to_primary { + matching.push((rpc_loc, rpc_req, rpc_resp)); + } + } + if matching.len() != 1 { + return vec![]; + } + let (rpc_loc, rpc_req, rpc_resp) = matching.into_iter().next().unwrap(); + + // Uniqueness: only chain if the user's primary message is referenced + // by exactly one rpc. Otherwise a chained rename would silently break + // another rpc that shares the type. + let expected_primary = format!("{rpc_base}{primary_suffix}"); + if self.count_rpc_uses_of_type(&expected_primary) != 1 { + return vec![]; + } + + let mut ops = vec![RenameOp { + uri: rpc_loc.uri.clone(), + pos: rpc_loc.range.start, + new_name: new_rpc_base.clone(), + }]; + let (opposite_suffix, opposite_text) = if primary_suffix == "Request" { + ("Response", rpc_resp.as_str()) + } else { + ("Request", rpc_req.as_str()) + }; + ops.extend(self.sibling_message_ops( + &rpc_loc.uri, + &rpc_base, + &new_rpc_base, + ipath, + &[(opposite_suffix, opposite_text)], + )); + ops + } + + /// Resolve each `(suffix, type_text)` slot to a message declaration and + /// build the corresponding rename op, gated on convention + uniqueness. + fn sibling_message_ops( + &self, + anchor_uri: &Url, + old_rpc_name: &str, + new_rpc_name: &str, + ipath: &[PathBuf], + slots: &[(&str, &str)], + ) -> Vec { + let Some(anchor_tree) = self.get_tree(anchor_uri) else { + return vec![]; + }; + let anchor_content = self.get_content(anchor_uri); + let anchor_package = anchor_tree + .get_package_name(anchor_content.as_bytes()) + .unwrap_or(".") + .to_owned(); + + let mut ops = vec![]; + for (suffix, type_text) in slots { + let expected_name = format!("{old_rpc_name}{suffix}"); + if trailing_segment(type_text) != expected_name { + continue; + } + if self.count_rpc_uses_of_type(&expected_name) != 1 { + continue; + } + let locations = self.definition( + ipath, + &anchor_package, + Jumpable::Identifier((*type_text).to_owned()), + ); + let Some(decl) = locations.into_iter().next() else { + continue; + }; + ops.push(RenameOp { + uri: decl.uri, + pos: decl.range.start, + new_name: format!("{new_rpc_name}{suffix}"), + }); + } + ops + } +} + +/// True iff `pos` falls within `range` (inclusive of both endpoints). Used to +/// match a possibly mid-identifier cursor against a definition's full-span +/// range when verifying that two locations refer to the same symbol. +fn position_in_range(pos: Position, range: Range) -> bool { + (pos.line, pos.character) >= (range.start.line, range.start.character) + && (pos.line, pos.character) <= (range.end.line, range.end.character) +} + +/// If `msg_name` ends with `Request` or `Response` and `new_name` ends with +/// the same suffix, return `(rpc_base, suffix, new_rpc_base)`. Otherwise +/// `None`. Used to detect when a message rename can plausibly drive a chain. +fn strip_convention_suffix( + msg_name: &str, + new_name: &str, +) -> Option<(String, &'static str, String)> { + for suffix in ["Request", "Response"] { + if let (Some(base), Some(new_base)) = + (msg_name.strip_suffix(suffix), new_name.strip_suffix(suffix)) + { + return Some((base.to_owned(), suffix, new_base.to_owned())); + } + } + None } #[cfg(test)] mod test { use std::path::PathBuf; + use async_lsp::lsp_types::Position; use insta::assert_yaml_snapshot; use crate::config::Config; use crate::state::ProtoLanguageState; + use crate::workspace::rename::RenameOp; + + fn make_state(files: &[(&str, &str)], ipath: &[PathBuf]) -> ProtoLanguageState { + let mut state = ProtoLanguageState::new(); + for (uri, content) in files { + let parsed_uri = uri.parse().unwrap(); + state.upsert_file( + &parsed_uri, + (*content).to_owned(), + ipath, + 2, + &Config::default(), + false, + ); + } + state + } + + fn op(uri: &str, line: u32, character: u32, new_name: &str) -> RenameOp { + RenameOp { + uri: uri.parse().unwrap(), + pos: Position { line, character }, + new_name: new_name.to_owned(), + } + } #[test] fn test_rename() { @@ -207,4 +574,329 @@ mod test { None )); } + + #[test] + fn test_find_rpc_decls_and_count_uses() { + let ipath = vec![PathBuf::from("src/workspace/input")]; + let svc_uri = "file://input/service.proto".parse().unwrap(); + let msg_uri = "file://input/messages.proto".parse().unwrap(); + let svc = include_str!("input/service.proto"); + let msg = include_str!("input/messages.proto"); + + let mut state: ProtoLanguageState = ProtoLanguageState::new(); + state.upsert_file( + &svc_uri, + svc.to_owned(), + &ipath, + 2, + &Config::default(), + false, + ); + state.upsert_file( + &msg_uri, + msg.to_owned(), + &ipath, + 2, + &Config::default(), + false, + ); + + // Lookup hits the rpc_name node in service.proto. + let mut locs = state.find_rpc_decls("GetBook"); + assert_eq!(locs.len(), 1, "expected exactly one GetBook rpc"); + let loc = locs.pop().unwrap(); + assert!(loc.uri.as_str().ends_with("service.proto")); + assert_eq!(loc.range.start.line, 7); + + assert!(state.find_rpc_decls("DoesNotExist").is_empty()); + + // Convention-following types are uniquely used. + assert_eq!(state.count_rpc_uses_of_type("GetBookRequest"), 1); + assert_eq!(state.count_rpc_uses_of_type("GetBookResponse"), 1); + // Shared types are seen twice. + assert_eq!(state.count_rpc_uses_of_type("SharedReq"), 2); + assert_eq!(state.count_rpc_uses_of_type("SharedResp"), 2); + // Non-convention type still uniquely used as a response. + assert_eq!(state.count_rpc_uses_of_type("Bar"), 1); + // Unrelated names see no uses. + assert_eq!(state.count_rpc_uses_of_type("Unrelated"), 0); + } + + #[test] + fn test_compute_rename_ops_cross_package_collision_blocks_chain() { + // Two services in different packages each declare `rpc GetBook`. Only + // pkg foo follows the convention; pkg bar uses unrelated message + // names. Without the per-candidate full-qualified resolution check, + // find_rpc_decls' iteration order could let bar's GetBook poison the + // chain. With the fix, foo's GetBook is uniquely identified by + // resolving its request slot back to the user's primary message. + let ipath = vec![PathBuf::from("src/workspace/input")]; + let foo_uri = "file://input/collision_foo.proto".parse().unwrap(); + let state = make_state( + &[ + ( + "file://input/collision_foo.proto", + include_str!("input/collision_foo.proto"), + ), + ( + "file://input/collision_bar.proto", + include_str!("input/collision_bar.proto"), + ), + ], + &ipath, + ); + + // Cursor on foo.GetBookRequest message_name (line 4, col 8..22). + let pos = Position { + line: 4, + character: 12, + }; + let ops = state.compute_rename_ops(&foo_uri, pos, "FetchBookRequest", &ipath); + + // Primary + foo's rpc + foo's response. bar's GetBook is NOT touched. + assert_eq!(ops.len(), 3, "expected exactly the foo trio, got {ops:?}"); + for op in &ops { + assert!( + op.uri.as_str().contains("collision_foo"), + "chain leaked into bar: {op:?}" + ); + } + } + + #[test] + fn test_compute_rename_ops_chain_from_rpc_cursor() { + let ipath = vec![PathBuf::from("src/workspace/input")]; + let svc_uri = "file://input/service.proto".parse().unwrap(); + let state = make_state( + &[ + ( + "file://input/service.proto", + include_str!("input/service.proto"), + ), + ( + "file://input/messages.proto", + include_str!("input/messages.proto"), + ), + ], + &ipath, + ); + + // Cursor on the `GetBook` rpc name (line 7, col 8..15). + let pos = Position { + line: 7, + character: 10, + }; + let ops = state.compute_rename_ops(&svc_uri, pos, "FetchBook", &ipath); + + // Primary + Request + Response. + assert_eq!( + ops.len(), + 3, + "expected primary rpc + two sibling ops, got {ops:?}" + ); + assert_eq!(ops[0], op("file://input/service.proto", 7, 10, "FetchBook")); + assert_eq!( + ops[1], + op("file://input/messages.proto", 4, 8, "FetchBookRequest") + ); + assert_eq!( + ops[2], + op("file://input/messages.proto", 5, 8, "FetchBookResponse") + ); + } + + #[test] + fn test_compute_rename_ops_chain_from_request_cursor() { + let ipath = vec![PathBuf::from("src/workspace/input")]; + let msg_uri = "file://input/messages.proto".parse().unwrap(); + let state = make_state( + &[ + ( + "file://input/service.proto", + include_str!("input/service.proto"), + ), + ( + "file://input/messages.proto", + include_str!("input/messages.proto"), + ), + ], + &ipath, + ); + + // Cursor on `GetBookRequest` message name (line 4, col 8..22). + let pos = Position { + line: 4, + character: 12, + }; + let ops = state.compute_rename_ops(&msg_uri, pos, "FetchBookRequest", &ipath); + + // Primary (request) + rpc + response. + assert_eq!( + ops.len(), + 3, + "expected primary + rpc + response sibling ops, got {ops:?}" + ); + assert_eq!( + ops[0], + op("file://input/messages.proto", 4, 12, "FetchBookRequest") + ); + assert_eq!(ops[1], op("file://input/service.proto", 7, 8, "FetchBook")); + assert_eq!( + ops[2], + op("file://input/messages.proto", 5, 8, "FetchBookResponse") + ); + } + + #[test] + fn test_compute_rename_ops_shared_request_blocks_chain() { + let ipath = vec![PathBuf::from("src/workspace/input")]; + let msg_uri = "file://input/messages.proto".parse().unwrap(); + let state = make_state( + &[ + ( + "file://input/service.proto", + include_str!("input/service.proto"), + ), + ( + "file://input/messages.proto", + include_str!("input/messages.proto"), + ), + ], + &ipath, + ); + + // Cursor on `SharedReq` message name (line 10). Both ListA and ListB + // reference SharedReq — chain must not fire. + let pos = Position { + line: 10, + character: 12, + }; + let ops = state.compute_rename_ops(&msg_uri, pos, "RenamedReq", &ipath); + assert_eq!( + ops.len(), + 1, + "expected primary-only (no chain), got {ops:?}" + ); + assert_eq!( + ops[0], + op("file://input/messages.proto", 10, 12, "RenamedReq") + ); + } + + #[test] + fn test_compute_rename_ops_new_name_breaks_convention() { + let ipath = vec![PathBuf::from("src/workspace/input")]; + let msg_uri = "file://input/messages.proto".parse().unwrap(); + let state = make_state( + &[ + ( + "file://input/service.proto", + include_str!("input/service.proto"), + ), + ( + "file://input/messages.proto", + include_str!("input/messages.proto"), + ), + ], + &ipath, + ); + + // Cursor on GetBookRequest, but new name doesn't preserve the + // `Request` convention — primary-only. + let pos = Position { + line: 4, + character: 12, + }; + let ops = state.compute_rename_ops(&msg_uri, pos, "Whatever", &ipath); + assert_eq!( + ops.len(), + 1, + "expected primary-only (no chain), got {ops:?}" + ); + assert_eq!(ops[0], op("file://input/messages.proto", 4, 12, "Whatever")); + } + + #[test] + fn test_compute_rename_ops_reference_site_pivot_unsupported() { + // compute_rename_ops requires the caller to have pivoted to the + // declaration. As a sanity check, calling it with a cursor on a + // reference site (not a declaration) yields a primary-only op that + // would no-op the workspace pass. The LSP layer is responsible for + // resolving the reference to its declaration *before* calling this. + let ipath = vec![PathBuf::from("src/workspace/input")]; + let svc_uri = "file://input/service.proto".parse().unwrap(); + let state = make_state( + &[ + ( + "file://input/service.proto", + include_str!("input/service.proto"), + ), + ( + "file://input/messages.proto", + include_str!("input/messages.proto"), + ), + ], + &ipath, + ); + + // Cursor on the `GetBookRequest` reference inside the rpc signature + // (line 7, col 16..30). + let pos = Position { + line: 7, + character: 20, + }; + let ops = state.compute_rename_ops(&svc_uri, pos, "RenamedRequest", &ipath); + // Single op (the primary at the reference site) — no chain. This + // documents the contract: the LSP layer must pivot first. + assert_eq!(ops.len(), 1, "{ops:?}"); + } + + #[test] + fn test_apply_rename_ops_chain_from_rpc_cursor() { + // End-to-end snapshot of the full `rpc (Request) returns + // (Response)` convention chain. Renaming `GetBook` → + // `FetchBook` should fan out into: + // - the rpc_name span in service.proto, + // - the request type-reference inside that rpc's signature, + // - the response type-reference inside that rpc's signature, + // - the `GetBookRequest` message decl in messages.proto, + // - the `GetBookResponse` message decl in messages.proto. + // The snapshot pins both the URIs and the exact edit ranges so any + // regression in chain detection, edit merging, or workspace-pass + // resolution will show up here. + let ipath = vec![PathBuf::from("src/workspace/input")]; + let svc_uri = "file://input/service.proto".parse().unwrap(); + let mut state = make_state( + &[ + ( + "file://input/service.proto", + include_str!("input/service.proto"), + ), + ( + "file://input/messages.proto", + include_str!("input/messages.proto"), + ), + ], + &ipath, + ); + + let pos = Position { + line: 7, + character: 10, + }; + let ops = state.compute_rename_ops(&svc_uri, pos, "FetchBook", &ipath); + let edits = state + .apply_rename_ops(&ops, PathBuf::from("src/workspace/input"), None) + .expect("primary rename should not fail"); + + // Sort within each file so the snapshot is order-independent across + // unrelated implementation changes (e.g. op evaluation order). + let mut normalized: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for (url, mut v) in edits { + v.sort_by_key(|e| (e.range.start.line, e.range.start.character)); + normalized.insert(url.to_string(), v); + } + assert_yaml_snapshot!(normalized); + } } diff --git a/src/workspace/snapshots/protols__workspace__rename__test__apply_rename_ops_chain_from_rpc_cursor.snap b/src/workspace/snapshots/protols__workspace__rename__test__apply_rename_ops_chain_from_rpc_cursor.snap new file mode 100644 index 0000000..9e38963 --- /dev/null +++ b/src/workspace/snapshots/protols__workspace__rename__test__apply_rename_ops_chain_from_rpc_cursor.snap @@ -0,0 +1,46 @@ +--- +source: src/workspace/rename.rs +expression: normalized +--- +"file://input/messages.proto": + - range: + start: + line: 4 + character: 8 + end: + line: 4 + character: 22 + newText: FetchBookRequest + - range: + start: + line: 5 + character: 8 + end: + line: 5 + character: 23 + newText: FetchBookResponse +"file://input/service.proto": + - range: + start: + line: 7 + character: 8 + end: + line: 7 + character: 15 + newText: FetchBook + - range: + start: + line: 7 + character: 16 + end: + line: 7 + character: 30 + newText: FetchBookRequest + - range: + start: + line: 7 + character: 41 + end: + line: 7 + character: 56 + newText: FetchBookResponse