From 4576f6b9fd56c87234e0128340743545e0e2f11f Mon Sep 17 00:00:00 2001 From: mohammadkhan Date: Fri, 6 Jun 2025 17:54:27 +0530 Subject: [PATCH] feat: Add support for relative package resolution fixes #75 --- src/workspace/definition.rs | 22 +++- src/workspace/hover.rs | 116 +++++++++++------- src/workspace/input/a.proto | 1 + src/workspace/input/inner/secret/y.proto | 2 +- src/workspace/input/inner/x.proto | 3 +- src/workspace/rename.rs | 37 ++++-- ...__hover__test__workspace_test_hover-8.snap | 5 +- ...__hover__test__workspace_test_hover-9.snap | 5 +- ..._workspace__rename__test__reference-2.snap | 7 +- ...s__workspace__rename__test__reference.snap | 7 +- ...ls__workspace__rename__test__rename-2.snap | 7 +- ...ls__workspace__rename__test__rename-3.snap | 7 +- ...tols__workspace__rename__test__rename.snap | 11 +- 13 files changed, 143 insertions(+), 87 deletions(-) diff --git a/src/workspace/definition.rs b/src/workspace/definition.rs index cce9987..f23131e 100644 --- a/src/workspace/definition.rs +++ b/src/workspace/definition.rs @@ -34,12 +34,22 @@ impl ProtoLanguageState { package = curr_package; } - self.get_trees_for_package(package) - .into_iter() - .fold(vec![], |mut v, tree| { - v.extend(tree.definition(identifier, self.get_content(&tree.uri))); - v - }) + let mut trees = vec![]; + + // If package != curr_package, either identifier is from a completely new package + // or relative package from within. As per name resolution first resolve relative + // packages, add all relative trees in search list + if curr_package != package { + let fullpackage = format!("{curr_package}.{package}"); + trees.append(&mut self.get_trees_for_package(&fullpackage)); + } + + // Add all direct package trees + trees.append(&mut self.get_trees_for_package(package)); + trees.into_iter().fold(vec![], |mut v, tree| { + v.extend(tree.definition(identifier, self.get_content(&tree.uri))); + v + }) } } } diff --git a/src/workspace/hover.rs b/src/workspace/hover.rs index b83b8e0..e047369 100644 --- a/src/workspace/hover.rs +++ b/src/workspace/hover.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use async_lsp::lsp_types::{MarkupContent, MarkupKind}; @@ -7,6 +7,23 @@ use crate::{ utils::split_identifier_package, }; +fn format_import_path_hover_text(path: &str, p: &Path) -> String { + format!( + r#"Import: `{path}` protobuf file, +--- +Included from {}"#, + p.to_string_lossy() + ) +} + +fn format_identifier_hover_text(identifier: &str, package: &str, result: &str) -> String { + format!( + r#"`{identifier}` message or enum type, package: `{package}` +--- +{result}"# + ) +} + impl ProtoLanguageState { pub fn hover( &self, @@ -15,58 +32,67 @@ impl ProtoLanguageState { hv: Hoverables, ) -> Option { let v = match hv { - Hoverables::FieldType(field) => { - // Type is a builtin - match docs::BUITIN.get(field.as_str()) { - Some(docs) => docs.to_string(), - _ => String::new(), - } - } - Hoverables::ImportPath(path) => { - if let Some(p) = ipath.iter().map(|p| p.join(&path)).find(|p| p.exists()) { - format!( - r#"Import: `{path}` protobuf file, ---- -Included from {}"#, - p.to_string_lossy(), - ) - } else { - String::new() - } - } + Hoverables::FieldType(field) => docs::BUITIN + .get(field.as_str()) + .map(ToString::to_string) + .unwrap_or_default(), + + Hoverables::ImportPath(path) => ipath + .iter() + .map(|p| p.join(&path)) + .find(|p| p.exists()) + .map(|p| format_import_path_hover_text(&path, &p)) + .unwrap_or_default(), + Hoverables::Identifier(identifier) => { let (mut package, identifier) = split_identifier_package(identifier.as_str()); if package.is_empty() { package = curr_package; } - // Node is user defined type or well known type - // If user defined, - let mut result = docs::WELLKNOWN + // Identifier is user defined type or well known type + + // If well known types, check in wellknown docs, + // otherwise check in trees + match docs::WELLKNOWN .get(format!("{package}.{identifier}").as_str()) .map(|&s| s.to_string()) - .unwrap_or_default(); - - // If no well known was found; try parsing from trees. - if result.is_empty() { - for tree in self.get_trees_for_package(package) { - let res = tree.hover(identifier, self.get_content(&tree.uri)); + { + Some(res) => res, + None => { + let mut trees = vec![]; - if res.is_empty() { - continue; + // If package != curr_package, either identifier is from a completely new package + // or relative package from within. As per name resolution first resolve relative + // packages, add all relative trees in search list + if curr_package != package { + let fullpackage = format!("{curr_package}.{package}"); + trees.append(&mut self.get_trees_for_package(&fullpackage)); } - result = format!( - r#"`{identifier}` message or enum type, package: `{package}` ---- -{}"#, - res[0].clone() - ); - break; + // Add all direct package trees + trees.append(&mut self.get_trees_for_package(package)); + + // Find the first field hovered in the trees + let res = trees.iter().find_map(|tree| { + let content = self.get_content(&tree.uri); + let res = tree.hover(identifier, content); + if res.is_empty() { + None + } else { + Some(res[0].clone()) + } + }); + + // Format the hover text and return + // TODO: package here is literally what was hovered, incase of + // relative it is only the relative part, should be full path, should + // probably figure out the package from the tree which provides hover and + // pass here. + res.map(|r| format_identifier_hover_text(identifier, package, &r)) + .unwrap_or_default() } } - - result } }; @@ -140,8 +166,14 @@ mod test { )); assert_yaml_snapshot!(state.hover( &ipath, - "com.workspace", - Hoverables::Identifier("com.super.secret.SomeSecret".to_string()) + "com.inner", + Hoverables::Identifier(".com.inner.secret.SomeSecret".to_string()) )); + // relative path hover + assert_yaml_snapshot!(state.hover( + &ipath, + "com.inner", + Hoverables::Identifier("secret.SomeSecret".to_string()) + )) } } diff --git a/src/workspace/input/a.proto b/src/workspace/input/a.proto index c70cb95..c28d867 100644 --- a/src/workspace/input/a.proto +++ b/src/workspace/input/a.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package com.workspace; import "c.proto"; +import "b.proto"; import "inner/x.proto"; // A Book is a book diff --git a/src/workspace/input/inner/secret/y.proto b/src/workspace/input/inner/secret/y.proto index bee46a3..21b039a 100644 --- a/src/workspace/input/inner/secret/y.proto +++ b/src/workspace/input/inner/secret/y.proto @@ -1,6 +1,6 @@ syntax = "proto3"; -package com.super.secret; +package com.inner.secret; // SomeSecret is a real secret with hidden string message SomeSecret { diff --git a/src/workspace/input/inner/x.proto b/src/workspace/input/inner/x.proto index 1dde688..5b7a3c0 100644 --- a/src/workspace/input/inner/x.proto +++ b/src/workspace/input/inner/x.proto @@ -7,6 +7,7 @@ import "inner/secret/y.proto"; // Why is a reason with secret message Why { string reason = 1; - com.super.secret.SomeSecret secret = 2; + .com.inner.secret.SomeSecret secret = 2; + secret.SomeSecret secret2 = 3; } diff --git a/src/workspace/rename.rs b/src/workspace/rename.rs index 4cc220c..88a1d89 100644 --- a/src/workspace/rename.rs +++ b/src/workspace/rename.rs @@ -53,6 +53,16 @@ impl ProtoLanguageState { // Current package: Reference by full or relative name or directly if current_package == package { v.extend(tree.rename_field(&old, &new, content.as_str())); + } else if current_package.starts_with(package) { + // Safety: prefix check already done + // get the relative part of the package + let packagepart = current_package + .strip_prefix(package) + .unwrap() + .trim_start_matches('.'); + let relative_old = format!("{packagepart}.{old}"); + let relative_new = format!("{packagepart}.{new}"); + v.extend(tree.rename_field(&relative_old, &relative_new, content.as_str())); } // Otherwise, full reference @@ -81,30 +91,39 @@ impl ProtoLanguageState { .fold(Vec::::new(), |mut v, tree| { let content = self.get_content(&tree.uri); let package = tree.get_package_name(content.as_ref()).unwrap_or("."); - let mut old = identifier.to_owned(); + let mut ident = identifier.to_owned(); // Global scope: Reference by only . or within global directly if current_package == "." { if package == "." { - v.extend(tree.reference_field(&old, content.as_str())); + v.extend(tree.reference_field(&ident, content.as_str())); } - old = format!(".{old}"); - v.extend(tree.reference_field(&old, content.as_str())); + ident = format!(".{ident}"); + v.extend(tree.reference_field(&ident, content.as_str())); return v; } - let full_old = format!("{current_package}.{old}"); - let global_full_old = format!(".{current_package}.{old}"); + let full_ident = format!("{current_package}.{ident}"); + let global_full_ident = format!(".{current_package}.{ident}"); // Current package: Reference by full or relative name or directly if current_package == package { - v.extend(tree.reference_field(&old, content.as_str())); + v.extend(tree.reference_field(&ident, content.as_str())); + } else if current_package.starts_with(package) { + // Safety: prefix check already done + // get the relative part of the package + let packagepart = current_package + .strip_prefix(package) + .unwrap() + .trim_start_matches('.'); + let relative = format!("{packagepart}.{ident}"); + v.extend(tree.reference_field(&relative, content.as_str())); } // Otherwise, full reference - v.extend(tree.reference_field(&full_old, content.as_str())); - v.extend(tree.reference_field(&global_full_old, content.as_str())); + v.extend(tree.reference_field(&full_ident, content.as_str())); + v.extend(tree.reference_field(&global_full_ident, content.as_str())); v }); if r.is_empty() { None } else { Some(r) } diff --git a/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-8.snap b/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-8.snap index 669973d..0cda512 100644 --- a/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-8.snap +++ b/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-8.snap @@ -1,7 +1,6 @@ --- source: src/workspace/hover.rs -expression: "state.hover(&ipath, \"com.workspace\",\nHoverables::Identifier(\"com.super.secret.SomeSecret\".to_string()))" -snapshot_kind: text +expression: "state.hover(&ipath, \"com.inner\",\nHoverables::Identifier(\".com.inner.secret.SomeSecret\".to_string()))" --- kind: markdown -value: "`SomeSecret` message or enum type, package: `com.super.secret`\n---\nSomeSecret is a real secret with hidden string" +value: "`SomeSecret` message or enum type, package: `com.inner.secret`\n---\nSomeSecret is a real secret with hidden string" diff --git a/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-9.snap b/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-9.snap index 669973d..6053c11 100644 --- a/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-9.snap +++ b/src/workspace/snapshots/protols__workspace__hover__test__workspace_test_hover-9.snap @@ -1,7 +1,6 @@ --- source: src/workspace/hover.rs -expression: "state.hover(&ipath, \"com.workspace\",\nHoverables::Identifier(\"com.super.secret.SomeSecret\".to_string()))" -snapshot_kind: text +expression: "state.hover(&ipath, \"com.inner\",\nHoverables::Identifier(\"secret.SomeSecret\".to_string()))" --- kind: markdown -value: "`SomeSecret` message or enum type, package: `com.super.secret`\n---\nSomeSecret is a real secret with hidden string" +value: "`SomeSecret` message or enum type, package: `secret`\n---\nSomeSecret is a real secret with hidden string" diff --git a/src/workspace/snapshots/protols__workspace__rename__test__reference-2.snap b/src/workspace/snapshots/protols__workspace__rename__test__reference-2.snap index 904df03..8e5eec9 100644 --- a/src/workspace/snapshots/protols__workspace__rename__test__reference-2.snap +++ b/src/workspace/snapshots/protols__workspace__rename__test__reference-2.snap @@ -1,13 +1,12 @@ --- source: src/workspace/rename.rs -expression: "state.reference_fields(\"com.workspace\", \"Author.Address\")" -snapshot_kind: text +expression: "state.reference_fields(\"com.workspace\", \"Author.Address\",\nPathBuf::from(\"src/workspace/input\"), None)" --- - uri: "file://input/a.proto" range: start: - line: 10 + line: 11 character: 3 end: - line: 10 + line: 11 character: 17 diff --git a/src/workspace/snapshots/protols__workspace__rename__test__reference.snap b/src/workspace/snapshots/protols__workspace__rename__test__reference.snap index fa1bd58..0d8ac4a 100644 --- a/src/workspace/snapshots/protols__workspace__rename__test__reference.snap +++ b/src/workspace/snapshots/protols__workspace__rename__test__reference.snap @@ -1,13 +1,12 @@ --- source: src/workspace/rename.rs -expression: "state.reference_fields(\"com.workspace\", \"Author\")" -snapshot_kind: text +expression: "state.reference_fields(\"com.workspace\", \"Author\",\nPathBuf::from(\"src/workspace/input\"), None)" --- - uri: "file://input/a.proto" range: start: - line: 9 + line: 10 character: 3 end: - line: 9 + line: 10 character: 9 diff --git a/src/workspace/snapshots/protols__workspace__rename__test__rename-2.snap b/src/workspace/snapshots/protols__workspace__rename__test__rename-2.snap index bbe2aa5..44b22da 100644 --- a/src/workspace/snapshots/protols__workspace__rename__test__rename-2.snap +++ b/src/workspace/snapshots/protols__workspace__rename__test__rename-2.snap @@ -1,14 +1,13 @@ --- source: src/workspace/rename.rs -expression: "state.rename_fields(\"com.workspace\", \"Author.Address\", \"Author.Location\")" -snapshot_kind: text +expression: "state.rename_fields(\"com.workspace\", \"Author.Address\", \"Author.Location\",\nPathBuf::from(\"src/workspace/input\"), None)" --- "file://input/a.proto": - range: start: - line: 10 + line: 11 character: 3 end: - line: 10 + line: 11 character: 17 newText: Author.Location diff --git a/src/workspace/snapshots/protols__workspace__rename__test__rename-3.snap b/src/workspace/snapshots/protols__workspace__rename__test__rename-3.snap index 24afb2d..976f242 100644 --- a/src/workspace/snapshots/protols__workspace__rename__test__rename-3.snap +++ b/src/workspace/snapshots/protols__workspace__rename__test__rename-3.snap @@ -1,14 +1,13 @@ --- source: src/workspace/rename.rs -expression: "state.rename_fields(\"com.utility\", \"Foobar.Baz\", \"Foobar.Baaz\")" -snapshot_kind: text +expression: "state.rename_fields(\"com.utility\", \"Foobar.Baz\", \"Foobar.Baaz\",\nPathBuf::from(\"src/workspace/input\"), None)" --- "file://input/a.proto": - range: start: - line: 11 + line: 12 character: 3 end: - line: 11 + line: 12 character: 25 newText: com.utility.Foobar.Baaz diff --git a/src/workspace/snapshots/protols__workspace__rename__test__rename.snap b/src/workspace/snapshots/protols__workspace__rename__test__rename.snap index c86a70a..8cfe396 100644 --- a/src/workspace/snapshots/protols__workspace__rename__test__rename.snap +++ b/src/workspace/snapshots/protols__workspace__rename__test__rename.snap @@ -1,22 +1,21 @@ --- source: src/workspace/rename.rs -expression: "state.rename_fields(\"com.workspace\", \"Author\", \"Writer\")" -snapshot_kind: text +expression: "state.rename_fields(\"com.workspace\", \"Author\", \"Writer\",\nPathBuf::from(\"src/workspace/input\"), None)" --- "file://input/a.proto": - range: start: - line: 9 + line: 10 character: 3 end: - line: 9 + line: 10 character: 9 newText: Writer - range: start: - line: 10 + line: 11 character: 3 end: - line: 10 + line: 11 character: 17 newText: Writer.Address