Skip to content

Commit 9ac22e5

Browse files
committed
Improve goto definition across packages.
1 parent 2e76069 commit 9ac22e5

File tree

3 files changed

+95
-12
lines changed

3 files changed

+95
-12
lines changed

src/workspace.rs

+63-11
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,54 @@ pub struct Workspace {
3737
files: std::collections::HashMap<Url, file::File>,
3838
}
3939

40+
// Return the possible package qualifiers to_pkg could use for a type imported from from_pkg
41+
fn possible_qualifiers<'a>(from_pkg: &'a str, to_pkg: &'a str) -> Vec<&'a str> {
42+
log::trace!("possible_qualifiers({from_pkg}, {to_pkg})");
43+
if to_pkg == "" {
44+
return vec![from_pkg];
45+
}
46+
47+
let mut res = vec![];
48+
if let Some(pkg) = from_pkg.strip_prefix(to_pkg) {
49+
let pkg = pkg.strip_prefix(".").unwrap_or(pkg);
50+
res.push(pkg);
51+
}
52+
53+
if let Some((to_pkg, _)) = to_pkg.rsplit_once(".") {
54+
res.append(&mut possible_qualifiers(from_pkg, to_pkg));
55+
} else {
56+
res.push(from_pkg);
57+
}
58+
return res;
59+
}
60+
61+
#[test]
62+
fn test_possible_qualifiers() {
63+
let _ = env_logger::builder().is_test(true).try_init();
64+
assert_eq!(possible_qualifiers("", ""), vec![""]);
65+
assert_eq!(possible_qualifiers("foo", ""), vec!["foo"]);
66+
assert_eq!(possible_qualifiers("foo.bar", ""), vec!["foo.bar"]);
67+
assert_eq!(possible_qualifiers("foo", "bar"), vec!["foo"]);
68+
assert_eq!(possible_qualifiers("foo.bar", "bar"), vec!["foo.bar"]);
69+
assert_eq!(possible_qualifiers("foo", "foo"), vec!["", "foo"]);
70+
assert_eq!(
71+
possible_qualifiers("foo.bar", "foo"),
72+
vec!["bar", "foo.bar"]
73+
);
74+
assert_eq!(
75+
possible_qualifiers("foo.bar.baz", "foo.bar"),
76+
vec!["baz", "bar.baz", "foo.bar.baz",]
77+
);
78+
assert_eq!(
79+
possible_qualifiers("foo.bar.baz", "foo.bar.baz"),
80+
vec!["", "baz", "bar.baz", "foo.bar.baz",]
81+
);
82+
assert_eq!(
83+
possible_qualifiers("folder.stuff", "folder.what"),
84+
vec!["stuff", "folder.stuff"]
85+
);
86+
}
87+
4088
impl Workspace {
4189
pub fn new(proto_paths: Vec<std::path::PathBuf>) -> Workspace {
4290
Workspace {
@@ -362,21 +410,25 @@ impl Workspace {
362410
for (uri, file) in imports {
363411
let package = file.package();
364412
if let Some(sym) = if package == local_package {
365-
log::trace!("Searching for {} in {uri}", typ.name);
413+
log::trace!("Searching for {} in {uri} (same package)", typ.name);
366414
// same package, match the name without the package prefix
367415
file.symbols(&mut qc).find(|sym| sym.name == typ.name)
368416
} else if let Some(package) = package {
417+
log::trace!("Searching for {} in {uri} (different package)", typ.name);
369418
// different package, fully qualify the name
370-
log::trace!("Stripping {package} from {}", typ.name);
371-
let qualified = typ
372-
.name
373-
.strip_prefix(package)
374-
.unwrap_or(typ.name)
375-
.strip_prefix(".")
376-
.unwrap_or(typ.name)
377-
.to_string();
378-
log::trace!("Searching for {} in {uri} (different package)", qualified);
379-
file.symbols(&mut qc).find(|sym| sym.name == qualified)
419+
let local_package = local_package.unwrap_or("");
420+
file.symbols(&mut qc).find(|sym| {
421+
let quals = possible_qualifiers(package, local_package);
422+
log::trace!("Qualifiers: {quals:?}");
423+
quals
424+
.iter()
425+
.inspect(|q| log::trace!("Qual == {q}"))
426+
.filter_map(|qual| typ.name.strip_prefix(qual))
427+
.inspect(|q| log::trace!("stripped == {q}"))
428+
.filter_map(|name| name.strip_prefix("."))
429+
.inspect(|q| log::trace!("name == {q} == {}", sym.name))
430+
.any(|name| name == sym.name)
431+
})
380432
} else {
381433
// target file has no package
382434
log::trace!("Searching for {} in {uri}", typ.name);

testdata/folder/what.proto

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package folder.what;
55
import "folder/stuff.proto";
66

77
message What{
8-
stuff.Stuff stuff = 1;
8+
stuff.Stuff a = 1;
9+
folder.stuff.Stuff b = 2;
910
}
1011

tests/integration_test.rs

+30
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ fn stuff_uri() -> Url {
4040
Url::from_file_path(std::fs::canonicalize("./testdata/folder/stuff.proto").unwrap()).unwrap()
4141
}
4242

43+
fn what_uri() -> Url {
44+
Url::from_file_path(std::fs::canonicalize("./testdata/folder/what.proto").unwrap()).unwrap()
45+
}
46+
4347
fn diag(uri: Url, target: &str, message: &str) -> Diagnostic {
4448
Diagnostic {
4549
range: locate_sym(uri, target).range,
@@ -653,6 +657,32 @@ fn test_goto_definition_different_file_nested() -> pbls::Result<()> {
653657
Ok(())
654658
}
655659

660+
#[test]
661+
fn test_goto_definition_partially_qualified_package() -> pbls::Result<()> {
662+
let mut client = TestClient::new()?;
663+
client.open(what_uri())?;
664+
665+
let resp = client.request::<GotoDefinition>(goto(what_uri(), "stuff.Stuff a =", 0))?;
666+
assert_eq!(
667+
resp,
668+
Some(GotoDefinitionResponse::Scalar(locate_sym(
669+
stuff_uri(),
670+
"message Stuff",
671+
)))
672+
);
673+
674+
let resp = client.request::<GotoDefinition>(goto(what_uri(), "folder.stuff.Stuff b =", 0))?;
675+
assert_eq!(
676+
resp,
677+
Some(GotoDefinitionResponse::Scalar(locate_sym(
678+
stuff_uri(),
679+
"message Stuff",
680+
)))
681+
);
682+
683+
Ok(())
684+
}
685+
656686
#[test]
657687
fn test_message_references() -> pbls::Result<()> {
658688
let mut client = TestClient::new()?;

0 commit comments

Comments
 (0)