Skip to content

Commit a5f0b70

Browse files
feat: Add support for relative package resolution (#85)
fixes #75 Prior to this `protols` could not resolve relative package names, this fixes it and now LSP should be able to properly jump/hover and find references/rename even in packages with field name relative to current package.
1 parent dd71d54 commit a5f0b70

13 files changed

+143
-87
lines changed

src/workspace/definition.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,22 @@ impl ProtoLanguageState {
3434
package = curr_package;
3535
}
3636

37-
self.get_trees_for_package(package)
38-
.into_iter()
39-
.fold(vec![], |mut v, tree| {
40-
v.extend(tree.definition(identifier, self.get_content(&tree.uri)));
41-
v
42-
})
37+
let mut trees = vec![];
38+
39+
// If package != curr_package, either identifier is from a completely new package
40+
// or relative package from within. As per name resolution first resolve relative
41+
// packages, add all relative trees in search list
42+
if curr_package != package {
43+
let fullpackage = format!("{curr_package}.{package}");
44+
trees.append(&mut self.get_trees_for_package(&fullpackage));
45+
}
46+
47+
// Add all direct package trees
48+
trees.append(&mut self.get_trees_for_package(package));
49+
trees.into_iter().fold(vec![], |mut v, tree| {
50+
v.extend(tree.definition(identifier, self.get_content(&tree.uri)));
51+
v
52+
})
4353
}
4454
}
4555
}

src/workspace/hover.rs

Lines changed: 74 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::path::PathBuf;
1+
use std::path::{Path, PathBuf};
22

33
use async_lsp::lsp_types::{MarkupContent, MarkupKind};
44

@@ -7,6 +7,23 @@ use crate::{
77
utils::split_identifier_package,
88
};
99

10+
fn format_import_path_hover_text(path: &str, p: &Path) -> String {
11+
format!(
12+
r#"Import: `{path}` protobuf file,
13+
---
14+
Included from {}"#,
15+
p.to_string_lossy()
16+
)
17+
}
18+
19+
fn format_identifier_hover_text(identifier: &str, package: &str, result: &str) -> String {
20+
format!(
21+
r#"`{identifier}` message or enum type, package: `{package}`
22+
---
23+
{result}"#
24+
)
25+
}
26+
1027
impl ProtoLanguageState {
1128
pub fn hover(
1229
&self,
@@ -15,58 +32,67 @@ impl ProtoLanguageState {
1532
hv: Hoverables,
1633
) -> Option<MarkupContent> {
1734
let v = match hv {
18-
Hoverables::FieldType(field) => {
19-
// Type is a builtin
20-
match docs::BUITIN.get(field.as_str()) {
21-
Some(docs) => docs.to_string(),
22-
_ => String::new(),
23-
}
24-
}
25-
Hoverables::ImportPath(path) => {
26-
if let Some(p) = ipath.iter().map(|p| p.join(&path)).find(|p| p.exists()) {
27-
format!(
28-
r#"Import: `{path}` protobuf file,
29-
---
30-
Included from {}"#,
31-
p.to_string_lossy(),
32-
)
33-
} else {
34-
String::new()
35-
}
36-
}
35+
Hoverables::FieldType(field) => docs::BUITIN
36+
.get(field.as_str())
37+
.map(ToString::to_string)
38+
.unwrap_or_default(),
39+
40+
Hoverables::ImportPath(path) => ipath
41+
.iter()
42+
.map(|p| p.join(&path))
43+
.find(|p| p.exists())
44+
.map(|p| format_import_path_hover_text(&path, &p))
45+
.unwrap_or_default(),
46+
3747
Hoverables::Identifier(identifier) => {
3848
let (mut package, identifier) = split_identifier_package(identifier.as_str());
3949
if package.is_empty() {
4050
package = curr_package;
4151
}
4252

43-
// Node is user defined type or well known type
44-
// If user defined,
45-
let mut result = docs::WELLKNOWN
53+
// Identifier is user defined type or well known type
54+
55+
// If well known types, check in wellknown docs,
56+
// otherwise check in trees
57+
match docs::WELLKNOWN
4658
.get(format!("{package}.{identifier}").as_str())
4759
.map(|&s| s.to_string())
48-
.unwrap_or_default();
49-
50-
// If no well known was found; try parsing from trees.
51-
if result.is_empty() {
52-
for tree in self.get_trees_for_package(package) {
53-
let res = tree.hover(identifier, self.get_content(&tree.uri));
60+
{
61+
Some(res) => res,
62+
None => {
63+
let mut trees = vec![];
5464

55-
if res.is_empty() {
56-
continue;
65+
// If package != curr_package, either identifier is from a completely new package
66+
// or relative package from within. As per name resolution first resolve relative
67+
// packages, add all relative trees in search list
68+
if curr_package != package {
69+
let fullpackage = format!("{curr_package}.{package}");
70+
trees.append(&mut self.get_trees_for_package(&fullpackage));
5771
}
5872

59-
result = format!(
60-
r#"`{identifier}` message or enum type, package: `{package}`
61-
---
62-
{}"#,
63-
res[0].clone()
64-
);
65-
break;
73+
// Add all direct package trees
74+
trees.append(&mut self.get_trees_for_package(package));
75+
76+
// Find the first field hovered in the trees
77+
let res = trees.iter().find_map(|tree| {
78+
let content = self.get_content(&tree.uri);
79+
let res = tree.hover(identifier, content);
80+
if res.is_empty() {
81+
None
82+
} else {
83+
Some(res[0].clone())
84+
}
85+
});
86+
87+
// Format the hover text and return
88+
// TODO: package here is literally what was hovered, incase of
89+
// relative it is only the relative part, should be full path, should
90+
// probably figure out the package from the tree which provides hover and
91+
// pass here.
92+
res.map(|r| format_identifier_hover_text(identifier, package, &r))
93+
.unwrap_or_default()
6694
}
6795
}
68-
69-
result
7096
}
7197
};
7298

@@ -140,8 +166,14 @@ mod test {
140166
));
141167
assert_yaml_snapshot!(state.hover(
142168
&ipath,
143-
"com.workspace",
144-
Hoverables::Identifier("com.super.secret.SomeSecret".to_string())
169+
"com.inner",
170+
Hoverables::Identifier(".com.inner.secret.SomeSecret".to_string())
145171
));
172+
// relative path hover
173+
assert_yaml_snapshot!(state.hover(
174+
&ipath,
175+
"com.inner",
176+
Hoverables::Identifier("secret.SomeSecret".to_string())
177+
))
146178
}
147179
}

src/workspace/input/a.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ syntax = "proto3";
33
package com.workspace;
44

55
import "c.proto";
6+
import "b.proto";
67
import "inner/x.proto";
78

89
// A Book is a book

src/workspace/input/inner/secret/y.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
syntax = "proto3";
22

3-
package com.super.secret;
3+
package com.inner.secret;
44

55
// SomeSecret is a real secret with hidden string
66
message SomeSecret {

src/workspace/input/inner/x.proto

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import "inner/secret/y.proto";
77
// Why is a reason with secret
88
message Why {
99
string reason = 1;
10-
com.super.secret.SomeSecret secret = 2;
10+
.com.inner.secret.SomeSecret secret = 2;
11+
secret.SomeSecret secret2 = 3;
1112
}
1213

src/workspace/rename.rs

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ impl ProtoLanguageState {
5353
// Current package: Reference by full or relative name or directly
5454
if current_package == package {
5555
v.extend(tree.rename_field(&old, &new, content.as_str()));
56+
} else if current_package.starts_with(package) {
57+
// Safety: prefix check already done
58+
// get the relative part of the package
59+
let packagepart = current_package
60+
.strip_prefix(package)
61+
.unwrap()
62+
.trim_start_matches('.');
63+
let relative_old = format!("{packagepart}.{old}");
64+
let relative_new = format!("{packagepart}.{new}");
65+
v.extend(tree.rename_field(&relative_old, &relative_new, content.as_str()));
5666
}
5767

5868
// Otherwise, full reference
@@ -81,30 +91,39 @@ impl ProtoLanguageState {
8191
.fold(Vec::<Location>::new(), |mut v, tree| {
8292
let content = self.get_content(&tree.uri);
8393
let package = tree.get_package_name(content.as_ref()).unwrap_or(".");
84-
let mut old = identifier.to_owned();
94+
let mut ident = identifier.to_owned();
8595
// Global scope: Reference by only . or within global directly
8696
if current_package == "." {
8797
if package == "." {
88-
v.extend(tree.reference_field(&old, content.as_str()));
98+
v.extend(tree.reference_field(&ident, content.as_str()));
8999
}
90100

91-
old = format!(".{old}");
92-
v.extend(tree.reference_field(&old, content.as_str()));
101+
ident = format!(".{ident}");
102+
v.extend(tree.reference_field(&ident, content.as_str()));
93103

94104
return v;
95105
}
96106

97-
let full_old = format!("{current_package}.{old}");
98-
let global_full_old = format!(".{current_package}.{old}");
107+
let full_ident = format!("{current_package}.{ident}");
108+
let global_full_ident = format!(".{current_package}.{ident}");
99109

100110
// Current package: Reference by full or relative name or directly
101111
if current_package == package {
102-
v.extend(tree.reference_field(&old, content.as_str()));
112+
v.extend(tree.reference_field(&ident, content.as_str()));
113+
} else if current_package.starts_with(package) {
114+
// Safety: prefix check already done
115+
// get the relative part of the package
116+
let packagepart = current_package
117+
.strip_prefix(package)
118+
.unwrap()
119+
.trim_start_matches('.');
120+
let relative = format!("{packagepart}.{ident}");
121+
v.extend(tree.reference_field(&relative, content.as_str()));
103122
}
104123

105124
// Otherwise, full reference
106-
v.extend(tree.reference_field(&full_old, content.as_str()));
107-
v.extend(tree.reference_field(&global_full_old, content.as_str()));
125+
v.extend(tree.reference_field(&full_ident, content.as_str()));
126+
v.extend(tree.reference_field(&global_full_ident, content.as_str()));
108127
v
109128
});
110129
if r.is_empty() { None } else { Some(r) }
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
source: src/workspace/hover.rs
3-
expression: "state.hover(&ipath, \"com.workspace\",\nHoverables::Identifier(\"com.super.secret.SomeSecret\".to_string()))"
4-
snapshot_kind: text
3+
expression: "state.hover(&ipath, \"com.inner\",\nHoverables::Identifier(\".com.inner.secret.SomeSecret\".to_string()))"
54
---
65
kind: markdown
7-
value: "`SomeSecret` message or enum type, package: `com.super.secret`\n---\nSomeSecret is a real secret with hidden string"
6+
value: "`SomeSecret` message or enum type, package: `com.inner.secret`\n---\nSomeSecret is a real secret with hidden string"
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
source: src/workspace/hover.rs
3-
expression: "state.hover(&ipath, \"com.workspace\",\nHoverables::Identifier(\"com.super.secret.SomeSecret\".to_string()))"
4-
snapshot_kind: text
3+
expression: "state.hover(&ipath, \"com.inner\",\nHoverables::Identifier(\"secret.SomeSecret\".to_string()))"
54
---
65
kind: markdown
7-
value: "`SomeSecret` message or enum type, package: `com.super.secret`\n---\nSomeSecret is a real secret with hidden string"
6+
value: "`SomeSecret` message or enum type, package: `secret`\n---\nSomeSecret is a real secret with hidden string"
Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
---
22
source: src/workspace/rename.rs
3-
expression: "state.reference_fields(\"com.workspace\", \"Author.Address\")"
4-
snapshot_kind: text
3+
expression: "state.reference_fields(\"com.workspace\", \"Author.Address\",\nPathBuf::from(\"src/workspace/input\"), None)"
54
---
65
- uri: "file://input/a.proto"
76
range:
87
start:
9-
line: 10
8+
line: 11
109
character: 3
1110
end:
12-
line: 10
11+
line: 11
1312
character: 17
Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
---
22
source: src/workspace/rename.rs
3-
expression: "state.reference_fields(\"com.workspace\", \"Author\")"
4-
snapshot_kind: text
3+
expression: "state.reference_fields(\"com.workspace\", \"Author\",\nPathBuf::from(\"src/workspace/input\"), None)"
54
---
65
- uri: "file://input/a.proto"
76
range:
87
start:
9-
line: 9
8+
line: 10
109
character: 3
1110
end:
12-
line: 9
11+
line: 10
1312
character: 9

0 commit comments

Comments
 (0)