Skip to content

Commit

Permalink
fix: fix lsp goto def of system pkg. (kcl-lang#1376)
Browse files Browse the repository at this point in the history
fix: fix lsp goto def of system pkg. refactor some unit test

Signed-off-by: he1pa <[email protected]>
  • Loading branch information
He1pa authored May 28, 2024
1 parent 847a96a commit 67e8c28
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 4 deletions.
8 changes: 7 additions & 1 deletion kclvm/sema/src/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,16 @@ pub struct PackageInfo {
pub(crate) fully_qualified_name: String,
pub(crate) pkg_filepath: String,
pub(crate) kfile_paths: IndexSet<String>,
pub(crate) is_system: bool,
}

impl PackageInfo {
pub fn new(fully_qualified_name: String, pkg_filepath: String) -> Self {
pub fn new(fully_qualified_name: String, pkg_filepath: String, is_system: bool) -> Self {
Self {
fully_qualified_name,
pkg_filepath,
kfile_paths: IndexSet::default(),
is_system,
}
}

Expand All @@ -63,6 +65,10 @@ impl PackageInfo {
pub fn get_pkg_filepath(&self) -> &String {
&self.pkg_filepath
}

pub fn is_system(&self) -> bool {
self.is_system
}
}
#[allow(unused)]
#[derive(Debug, Clone)]
Expand Down
7 changes: 6 additions & 1 deletion kclvm/sema/src/namer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,13 @@ impl<'ctx> Namer<'ctx> {
namer.ctx.current_package_info = Some(PackageInfo::new(
BUILTIN_SYMBOL_PKG_PATH.to_string(),
"".to_string(),
true,
));
namer.init_builtin_symbols();
namer
.gs
.get_packages_mut()
.add_package(namer.ctx.current_package_info.take().unwrap());

for (name, modules) in namer.ctx.program.pkgs.iter() {
{
Expand All @@ -130,7 +135,7 @@ impl<'ctx> Namer<'ctx> {
namer.ctx.owner_symbols.push(symbol_ref);

namer.ctx.current_package_info =
Some(PackageInfo::new(name.to_string(), real_path));
Some(PackageInfo::new(name.to_string(), real_path, false));
}

for module in modules.iter() {
Expand Down
34 changes: 33 additions & 1 deletion kclvm/tools/src/LSP/src/goto_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ pub(crate) fn goto_def(
Some(def_ref) => match gs.get_symbols().get_symbol(def_ref) {
Some(def) => match def_ref.get_kind() {
kclvm_sema::core::symbol::SymbolKind::Package => {
let pkg_info = gs.get_packages().get_package_info(&def.get_name()).unwrap();
let pkg_info = match gs.get_packages().get_package_info(&def.get_name()) {
Some(pkg_info) => pkg_info,
None => return None,
};
if pkg_info.is_system() {
return None;
}
for file in pkg_info.get_kfile_paths() {
let dummy_pos = KCLPos {
filename: file.clone(),
Expand Down Expand Up @@ -688,4 +694,30 @@ mod tests {
let res = goto_def(&pos, &gs);
compare_goto_res(res, (&file, 94, 11, 94, 12));
}

#[macro_export]
macro_rules! goto_def_test_snapshot {
($name:ident, $file:expr, $line:expr, $column: expr) => {
#[test]
fn $name() {
insta::assert_snapshot!(format!("{:?}", {
let (file, _program, _, gs) = compile_test_file($file);

let pos = KCLPos {
filename: file.clone(),
line: $line,
column: Some($column),
};
goto_def(&pos, &gs)
}));
}
};
}

goto_def_test_snapshot!(
goto_system_pkg_test,
"src/test_data/goto_def_test/goto_def.k",
3,
1
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: tools/src/LSP/src/goto_def.rs
expression: "format!(\"{:?}\", res)"
---
None
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import yaml

yaml.decode("")
2 changes: 1 addition & 1 deletion kclvm/tools/src/LSP/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub(crate) fn compile_with_params(
let gs = Namer::find_symbols(&program, gs);
match AdvancedResolver::resolve_program(&program, gs, prog_scope.node_ty_map.clone()) {
Ok(gs) => (diags, Ok((program, gs))),
Err(e) => (diags, Err(anyhow::anyhow!("Parse failed: {:?}", e))),
Err(e) => (diags, Err(anyhow::anyhow!("Resolve failed: {:?}", e))),
}
}

Expand Down

0 comments on commit 67e8c28

Please sign in to comment.