Skip to content

Commit 76f84b3

Browse files
authored
fix: nothing works in a file without a package (#77)
Prior to this, a proto file without a package declaration didn't supported any operation as parser required that a package be present. For missing package we now consider . as package
1 parent 89a878f commit 76f84b3

File tree

8 files changed

+98
-49
lines changed

8 files changed

+98
-49
lines changed

sample/test.proto

+14
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,17 @@ message SomeMessage {
99

1010
CustomType another = 2;
1111
}
12+
13+
message CapitalA {
14+
// B is a b
15+
message CapitalB {
16+
17+
}
18+
19+
a.b.c.CapitalA.CapitalB b = 1;
20+
}
21+
22+
message C {
23+
CapitalA.CapitalB ab = 1;
24+
.a.b.c.CapitalA a = 2;
25+
}

src/config/mod.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ pub struct ProtolsConfig {
1717
}
1818

1919
#[derive(Serialize, Deserialize, Debug, Clone)]
20-
pub struct FormatterConfig {
21-
}
20+
pub struct FormatterConfig {}
2221

2322
#[derive(Serialize, Deserialize, Debug, Clone, Default)]
2423
#[serde(default)]
@@ -38,7 +37,7 @@ impl Default for PathConfig {
3837
fn default() -> Self {
3938
Self {
4039
clang_format: default_clang_format_path(),
41-
protoc: default_protoc_path()
40+
protoc: default_protoc_path(),
4241
}
4342
}
4443
}

src/config/workspace.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ impl WorkspaceProtoConfigs {
149149
mod test {
150150
use async_lsp::lsp_types::{Url, WorkspaceFolder};
151151
use insta::assert_yaml_snapshot;
152-
use tempfile::tempdir;
153152
use std::path::PathBuf;
153+
use tempfile::tempdir;
154154

155155
use super::{CONFIG_FILE_NAMES, WorkspaceProtoConfigs};
156156

@@ -263,13 +263,16 @@ mod test {
263263
let include_paths = ws.get_include_paths(&inworkspace).unwrap();
264264

265265
// Check that CLI paths are included in the result
266-
assert!(include_paths.iter().any(|p| p.ends_with("relative/path") ||
267-
p == &PathBuf::from("/path/to/protos")));
268-
266+
assert!(
267+
include_paths
268+
.iter()
269+
.any(|p| p.ends_with("relative/path") || p == &PathBuf::from("/path/to/protos"))
270+
);
271+
269272
// The relative path should be resolved relative to the workspace
270273
let resolved_relative_path = tmpdir.path().join("relative/path");
271274
assert!(include_paths.contains(&resolved_relative_path));
272-
275+
273276
// The absolute path should be included as is
274277
assert!(include_paths.contains(&PathBuf::from("/path/to/protos")));
275278
}

src/lsp.rs

+16-28
Original file line numberDiff line numberDiff line change
@@ -135,18 +135,13 @@ impl LanguageServer for ProtoLanguageServer {
135135

136136
let content = self.state.get_content(&uri);
137137
let hv = tree.get_hoverable_at_position(&pos, content.as_bytes());
138-
let current_package_name = tree.get_package_name(content.as_bytes());
138+
let current_package_name = tree.get_package_name(content.as_bytes()).unwrap_or(".");
139139

140140
let Some(hv) = hv else {
141141
error!(uri=%uri, "failed to get hoverable identifier");
142142
return Box::pin(async move { Ok(None) });
143143
};
144144

145-
let Some(current_package_name) = current_package_name else {
146-
error!(uri=%uri, "failed to get package name");
147-
return Box::pin(async move { Ok(None) });
148-
};
149-
150145
let ipath = self.configs.get_include_paths(&uri).unwrap_or_default();
151146
let result = self.state.hover(&ipath, current_package_name.as_ref(), hv);
152147

@@ -220,10 +215,7 @@ impl LanguageServer for ProtoLanguageServer {
220215

221216
let content = self.state.get_content(&uri);
222217

223-
let Some(current_package) = tree.get_package_name(content.as_bytes()) else {
224-
error!(uri=%uri, "failed to get package name");
225-
return Box::pin(async move { Ok(None) });
226-
};
218+
let current_package = tree.get_package_name(content.as_bytes()).unwrap_or(".");
227219

228220
let Some((edit, otext, ntext)) = tree.rename_tree(&pos, &new_name, content.as_bytes())
229221
else {
@@ -240,7 +232,6 @@ impl LanguageServer for ProtoLanguageServer {
240232
let progress_sender = work_done_token.map(|token| self.with_report_progress(token));
241233

242234
let mut h = HashMap::new();
243-
h.insert(tree.uri.clone(), edit);
244235
h.extend(self.state.rename_fields(
245236
current_package,
246237
&otext,
@@ -249,6 +240,8 @@ impl LanguageServer for ProtoLanguageServer {
249240
progress_sender,
250241
));
251242

243+
h.entry(tree.uri).or_insert(edit.clone()).extend(edit);
244+
252245
let response = Some(WorkspaceEdit {
253246
changes: Some(h),
254247
..Default::default()
@@ -272,10 +265,7 @@ impl LanguageServer for ProtoLanguageServer {
272265

273266
let content = self.state.get_content(&uri);
274267

275-
let Some(current_package) = tree.get_package_name(content.as_bytes()) else {
276-
error!(uri=%uri, "failed to get package name");
277-
return Box::pin(async move { Ok(None) });
278-
};
268+
let current_package = tree.get_package_name(content.as_bytes()).unwrap_or(".");
279269

280270
let Some((mut refs, otext)) = tree.reference_tree(&pos, content.as_bytes()) else {
281271
error!(uri=%uri, "failed to find references in a tree");
@@ -321,18 +311,13 @@ impl LanguageServer for ProtoLanguageServer {
321311

322312
let content = self.state.get_content(&uri);
323313
let jump = tree.get_jumpable_at_position(&pos, content.as_bytes());
324-
let current_package_name = tree.get_package_name(content.as_bytes());
314+
let current_package_name = tree.get_package_name(content.as_bytes()).unwrap_or(".");
325315

326316
let Some(jump) = jump else {
327317
error!(uri=%uri, "failed to get jump identifier");
328318
return Box::pin(async move { Ok(None) });
329319
};
330320

331-
let Some(current_package_name) = current_package_name else {
332-
error!(uri=%uri, "failed to get package name");
333-
return Box::pin(async move { Ok(None) });
334-
};
335-
336321
let ipath = self.configs.get_include_paths(&uri).unwrap_or_default();
337322
let locations = self
338323
.state
@@ -407,9 +392,9 @@ impl LanguageServer for ProtoLanguageServer {
407392
return ControlFlow::Continue(());
408393
};
409394

410-
if let Some(diagnostics) = self
411-
.state
412-
.upsert_file(&uri, content, &ipath, 8, &pconf.config, true)
395+
if let Some(diagnostics) =
396+
self.state
397+
.upsert_file(&uri, content, &ipath, 8, &pconf.config, true)
413398
{
414399
if let Err(e) = self.client.publish_diagnostics(diagnostics) {
415400
error!(error=%e, "failed to publish diagnostics")
@@ -434,9 +419,9 @@ impl LanguageServer for ProtoLanguageServer {
434419
return ControlFlow::Continue(());
435420
};
436421

437-
if let Some(diagnostics) = self
438-
.state
439-
.upsert_file(&uri, content, &ipath, 8, &pconf.config, true)
422+
if let Some(diagnostics) =
423+
self.state
424+
.upsert_file(&uri, content, &ipath, 8, &pconf.config, true)
440425
{
441426
if let Err(e) = self.client.publish_diagnostics(diagnostics) {
442427
error!(error=%e, "failed to publish diagnostics")
@@ -457,7 +442,10 @@ impl LanguageServer for ProtoLanguageServer {
457442
return ControlFlow::Continue(());
458443
};
459444

460-
if let Some(diagnostics) = self.state.upsert_file(&uri, content, &ipath, 8, &pconf.config, false) {
445+
if let Some(diagnostics) =
446+
self.state
447+
.upsert_file(&uri, content, &ipath, 8, &pconf.config, false)
448+
{
461449
if let Err(e) = self.client.publish_diagnostics(diagnostics) {
462450
error!(error=%e, "failed to publish diagnostics")
463451
}

src/parser/definition.rs

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ impl ParsedTree {
1111
self.definition_impl(identifier, self.tree.root_node(), &mut results, content);
1212
results
1313
}
14+
1415
fn definition_impl(
1516
&self,
1617
identifier: &str,

src/state.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ impl ProtoLanguageState {
6767
.values()
6868
.filter(|tree| {
6969
let content = self.get_content(&tree.uri);
70-
tree.get_package_name(content.as_bytes())
71-
.unwrap_or_default()
72-
== package
70+
tree.get_package_name(content.as_bytes()).unwrap_or(".") == package
7371
})
7472
.map(ToOwned::to_owned)
7573
.collect()

src/utils.rs

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ pub fn is_inner_identifier(s: &str) -> bool {
3737
}
3838

3939
pub fn split_identifier_package(s: &str) -> (&str, &str) {
40-
// trim beginning dots, some use `.` prefix for fully qualified field names
4140
let s = s.trim_start_matches(".");
4241
if is_inner_identifier(s) || !s.contains('.') {
4342
return ("", s);

src/workspace/rename.rs

+56-9
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,42 @@ impl ProtoLanguageState {
2323
.into_iter()
2424
.fold(HashMap::new(), |mut h, tree| {
2525
let content = self.get_content(&tree.uri);
26-
let package = tree.get_package_name(content.as_ref()).unwrap_or_default();
26+
let package = tree.get_package_name(content.as_ref()).unwrap_or(".");
2727
let mut old = identifier.to_string();
2828
let mut new = new_text.to_string();
29-
if current_package != package {
30-
old = format!("{current_package}.{old}");
31-
new = format!("{current_package}.{new}");
29+
let mut v = vec![];
30+
31+
// Global scope: Reference by only . or within global directly
32+
if current_package == "." {
33+
if package == "." {
34+
v.extend(tree.rename_field(&old, &new, content.as_str()));
35+
}
36+
37+
old = format!(".{old}");
38+
new = format!(".{new}");
39+
40+
v.extend(tree.rename_field(&old, &new, content.as_str()));
41+
42+
if !v.is_empty() {
43+
h.insert(tree.uri.clone(), v);
44+
}
45+
return h;
3246
}
33-
let v = tree.rename_field(&old, &new, content.as_str());
47+
48+
let full_old = format!("{current_package}.{old}");
49+
let full_new = format!("{current_package}.{new}");
50+
let global_full_old = format!(".{current_package}.{old}");
51+
let global_full_new = format!(".{current_package}.{new}");
52+
53+
// Current package: Reference by full or relative name or directly
54+
if current_package == package {
55+
v.extend(tree.rename_field(&old, &new, content.as_str()));
56+
}
57+
58+
// Otherwise, full reference
59+
v.extend(tree.rename_field(&full_old, &full_new, content.as_str()));
60+
v.extend(tree.rename_field(&global_full_old, &global_full_new, content.as_str()));
61+
3462
if !v.is_empty() {
3563
h.insert(tree.uri.clone(), v);
3664
}
@@ -52,12 +80,31 @@ impl ProtoLanguageState {
5280
.into_iter()
5381
.fold(Vec::<Location>::new(), |mut v, tree| {
5482
let content = self.get_content(&tree.uri);
55-
let package = tree.get_package_name(content.as_ref()).unwrap_or_default();
83+
let package = tree.get_package_name(content.as_ref()).unwrap_or(".");
5684
let mut old = identifier.to_owned();
57-
if current_package != package {
58-
old = format!("{current_package}.{old}");
85+
// Global scope: Reference by only . or within global directly
86+
if current_package == "." {
87+
if package == "." {
88+
v.extend(tree.reference_field(&old, content.as_str()));
89+
}
90+
91+
old = format!(".{old}");
92+
v.extend(tree.reference_field(&old, content.as_str()));
93+
94+
return v;
5995
}
60-
v.extend(tree.reference_field(&old, content.as_str()));
96+
97+
let full_old = format!("{current_package}.{old}");
98+
let global_full_old = format!(".{current_package}.{old}");
99+
100+
// Current package: Reference by full or relative name or directly
101+
if current_package == package {
102+
v.extend(tree.reference_field(&old, content.as_str()));
103+
}
104+
105+
// 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()));
61108
v
62109
});
63110
if r.is_empty() { None } else { Some(r) }

0 commit comments

Comments
 (0)