From 22e472f66657575a647026973738184a4a8b79bc Mon Sep 17 00:00:00 2001 From: He1pa <56333845+He1pa@users.noreply.github.com> Date: Tue, 28 May 2024 14:25:58 +0800 Subject: [PATCH] enhance lsp diags. (#1371) * enhance lsp diags. Change the return type of function `compile_with_params` from Result<(Program, IndexSet, GlobalState)> to (IndexSet, Result<(Program, GlobalState)>)). Returns the discovered diags when compilation fails. Signed-off-by: he1pa <18012015693@163.com> * fmt code Signed-off-by: he1pa <18012015693@163.com> --------- Signed-off-by: he1pa <18012015693@163.com> --- kclvm/tools/src/LSP/src/find_refs.rs | 6 +- kclvm/tools/src/LSP/src/quick_fix.rs | 4 +- kclvm/tools/src/LSP/src/state.rs | 89 ++++++++++++++-------------- kclvm/tools/src/LSP/src/tests.rs | 37 +++++++----- kclvm/tools/src/LSP/src/util.rs | 69 +++++++++++---------- 5 files changed, 112 insertions(+), 93 deletions(-) diff --git a/kclvm/tools/src/LSP/src/find_refs.rs b/kclvm/tools/src/LSP/src/find_refs.rs index 54279ed36..f62a29344 100644 --- a/kclvm/tools/src/LSP/src/find_refs.rs +++ b/kclvm/tools/src/LSP/src/find_refs.rs @@ -99,8 +99,10 @@ pub(crate) fn find_refs_from_def Result<(), anyhow::Error>>( vfs: vfs.clone(), entry_cache: entry_cache.clone(), tool: Arc::new(RwLock::new(toolchain::default())), - }) { - Ok((_, _, gs)) => { + }) + .1 + { + Ok((_, gs)) => { let ref_pos = kcl_pos(&file_path, ref_loc.range.start); if *ref_loc == def_loc && !include_declaration { return false; diff --git a/kclvm/tools/src/LSP/src/quick_fix.rs b/kclvm/tools/src/LSP/src/quick_fix.rs index a978073c1..00c8f050c 100644 --- a/kclvm/tools/src/LSP/src/quick_fix.rs +++ b/kclvm/tools/src/LSP/src/quick_fix.rs @@ -183,7 +183,7 @@ mod tests { test_file.push("src/test_data/quick_fix.k"); let file = test_file.to_str().unwrap(); - let (_, diags, _) = compile_with_params(Params { + let diags = compile_with_params(Params { file: file.to_string(), module_cache: None, scope_cache: None, @@ -191,7 +191,7 @@ mod tests { entry_cache: None, tool: Arc::new(RwLock::new(toolchain::default())), }) - .unwrap(); + .0; let diagnostics = diags .iter() diff --git a/kclvm/tools/src/LSP/src/state.rs b/kclvm/tools/src/LSP/src/state.rs index 379e15d46..b7e257eef 100644 --- a/kclvm/tools/src/LSP/src/state.rs +++ b/kclvm/tools/src/LSP/src/state.rs @@ -228,24 +228,46 @@ impl LanguageServerState { let version = snapshot.opened_files.read().get(&file.file_id).cloned(); let mut db = snapshot.db.write(); - match compile_with_params(Params { + let (diags, compile_res) = compile_with_params(Params { file: filename.clone(), module_cache: Some(module_cache), scope_cache: Some(scope_cache), vfs: Some(snapshot.vfs), entry_cache: Some(entry), tool, - }) { - Ok((prog, diags, gs)) => { - let current_version = snapshot - .opened_files - .read() - .get(&file.file_id) - .cloned(); - match (version, current_version) { - (Some(version), Some(current_version)) => { - // If the text is updated during compilation(current_version > version), the current compilation result will not be output. - if current_version == version { + }); + + let current_version = + snapshot.opened_files.read().get(&file.file_id).cloned(); + + match (version, current_version) { + (Some(version), Some(current_version)) => { + // If the text is updated during compilation(current_version > version), the current compilation result will not be output. + if current_version == version { + let diagnostics = diags + .iter() + .flat_map(|diag| { + kcl_diag_to_lsp_diags( + diag, + filename.as_str(), + ) + }) + .collect::>(); + sender.send(Task::Notify( + lsp_server::Notification { + method: PublishDiagnostics::METHOD + .to_owned(), + params: to_json(PublishDiagnosticsParams { + uri, + diagnostics, + version: None, + }) + .unwrap(), + }, + )); + + match compile_res { + Ok((prog, gs)) => { db.insert( file.file_id, Arc::new(AnalysisDatabase { @@ -254,42 +276,21 @@ impl LanguageServerState { version, }), ); - - let diagnostics = diags - .iter() - .flat_map(|diag| { - kcl_diag_to_lsp_diags( - diag, - filename.as_str(), - ) - }) - .collect::>(); - sender.send(Task::Notify( - lsp_server::Notification { - method: PublishDiagnostics::METHOD - .to_owned(), - params: to_json( - PublishDiagnosticsParams { - uri, - diagnostics, - version: None, - }, - ) - .unwrap(), - }, - )); + } + Err(err) => { + db.remove(&file.file_id); + log_message( + format!( + "Compile failed: {:?}", + err.to_string() + ), + &sender, + ); } } - _ => {} } } - Err(err) => { - db.remove(&file.file_id); - log_message( - format!("Compile failed: {:?}", err.to_string()), - &sender, - ); - } + _ => {} } } Err(_) => { diff --git a/kclvm/tools/src/LSP/src/tests.rs b/kclvm/tools/src/LSP/src/tests.rs index c6fce288d..7f4ba0cd8 100644 --- a/kclvm/tools/src/LSP/src/tests.rs +++ b/kclvm/tools/src/LSP/src/tests.rs @@ -127,15 +127,15 @@ pub(crate) fn compile_test_file( let file = test_file.to_str().unwrap().to_string(); - let (program, diags, gs) = compile_with_params(Params { + let (diags, compile_res) = compile_with_params(Params { file: file.clone(), module_cache: Some(KCLModuleCache::default()), scope_cache: Some(KCLScopeCache::default()), vfs: Some(KCLVfs::default()), entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), - }) - .unwrap(); + }); + let (program, gs) = compile_res.unwrap(); (file, program, diags, gs) } @@ -290,7 +290,7 @@ fn diagnostics_test() { test_file.push("src/test_data/diagnostics.k"); let file = test_file.to_str().unwrap(); - let (_, diags, _) = compile_with_params(Params { + let diags = compile_with_params(Params { file: file.to_string(), module_cache: None, scope_cache: None, @@ -298,7 +298,7 @@ fn diagnostics_test() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) - .unwrap(); + .0; let diagnostics = diags .iter() @@ -478,7 +478,7 @@ fn complete_import_external_file_test() { .output() .unwrap(); - let (program, _, gs) = compile_with_params(Params { + let (program, gs) = compile_with_params(Params { file: path.to_string(), module_cache: None, scope_cache: None, @@ -486,6 +486,7 @@ fn complete_import_external_file_test() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) + .1 .unwrap(); let pos = KCLPos { @@ -538,15 +539,15 @@ fn goto_import_external_file_test() { .output() .unwrap(); - let (_program, diags, gs) = compile_with_params(Params { + let (diags, compile_res) = compile_with_params(Params { file: path.to_string(), module_cache: None, scope_cache: None, vfs: Some(KCLVfs::default()), entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), - }) - .unwrap(); + }); + let gs = compile_res.unwrap().1; assert_eq!(diags.len(), 0); @@ -1393,7 +1394,7 @@ fn konfig_goto_def_test_base() { let mut base_path = konfig_path.clone(); base_path.push("appops/nginx-example/base/base.k"); let base_path_str = base_path.to_str().unwrap().to_string(); - let (_program, _, gs) = compile_with_params(Params { + let (_program, gs) = compile_with_params(Params { file: base_path_str.clone(), module_cache: None, scope_cache: None, @@ -1401,6 +1402,7 @@ fn konfig_goto_def_test_base() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) + .1 .unwrap(); // schema def @@ -1486,7 +1488,7 @@ fn konfig_goto_def_test_main() { let mut main_path = konfig_path.clone(); main_path.push("appops/nginx-example/dev/main.k"); let main_path_str = main_path.to_str().unwrap().to_string(); - let (_program, _, gs) = compile_with_params(Params { + let (_program, gs) = compile_with_params(Params { file: main_path_str.clone(), module_cache: None, scope_cache: None, @@ -1494,6 +1496,7 @@ fn konfig_goto_def_test_main() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) + .1 .unwrap(); // schema def @@ -1551,7 +1554,7 @@ fn konfig_completion_test_main() { let mut main_path = konfig_path.clone(); main_path.push("appops/nginx-example/dev/main.k"); let main_path_str = main_path.to_str().unwrap().to_string(); - let (program, _, gs) = compile_with_params(Params { + let (program, gs) = compile_with_params(Params { file: main_path_str.clone(), module_cache: None, scope_cache: None, @@ -1559,6 +1562,7 @@ fn konfig_completion_test_main() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) + .1 .unwrap(); // pkg's definition(schema) completion @@ -1665,7 +1669,7 @@ fn konfig_hover_test_main() { let mut main_path = konfig_path.clone(); main_path.push("appops/nginx-example/dev/main.k"); let main_path_str = main_path.to_str().unwrap().to_string(); - let (_program, _, gs) = compile_with_params(Params { + let (_program, gs) = compile_with_params(Params { file: main_path_str.clone(), module_cache: None, scope_cache: None, @@ -1673,6 +1677,7 @@ fn konfig_hover_test_main() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) + .1 .unwrap(); // schema def hover @@ -2099,7 +2104,7 @@ fn compile_unit_test() { test_file.push("src/test_data/compile_unit/b.k"); let file = test_file.to_str().unwrap(); - let (prog, ..) = compile_with_params(Params { + let prog = compile_with_params(Params { file: file.to_string(), module_cache: None, scope_cache: None, @@ -2107,7 +2112,9 @@ fn compile_unit_test() { entry_cache: Some(KCLEntryCache::default()), tool: Arc::new(RwLock::new(toolchain::default())), }) - .unwrap(); + .1 + .unwrap() + .0; // b.k is not contained in kcl.yaml but need to be contained in main pkg assert!(prog .pkgs diff --git a/kclvm/tools/src/LSP/src/util.rs b/kclvm/tools/src/LSP/src/util.rs index fc73bd476..d23feb627 100644 --- a/kclvm/tools/src/LSP/src/util.rs +++ b/kclvm/tools/src/LSP/src/util.rs @@ -26,8 +26,8 @@ use parking_lot::RwLockReadGuard; use ra_ap_vfs::{FileId, Vfs}; use serde::{de::DeserializeOwned, Serialize}; +use std::fs; use std::path::Path; -use std::{fs, panic}; #[allow(unused)] /// Deserializes a `T` from a json value. @@ -92,7 +92,7 @@ pub(crate) fn lookup_compile_unit_with_cache( pub(crate) fn compile_with_params( params: Params, -) -> anyhow::Result<(Program, IndexSet, GlobalState)> { +) -> (IndexSet, anyhow::Result<(Program, GlobalState)>) { // Lookup compile unit (kcl.mod or kcl.yaml) from the entry file. let (mut files, opt) = lookup_compile_unit_with_cache(&*params.tool.read(), ¶ms.entry_cache, ¶ms.file); @@ -105,36 +105,45 @@ pub(crate) fn compile_with_params( opt.load_plugins = true; // Update opt.k_code_list if let Some(vfs) = params.vfs { - let mut k_code_list = load_files_code_from_vfs(&files, vfs)?; + let mut k_code_list = match load_files_code_from_vfs(&files, vfs) { + Ok(code_list) => code_list, + Err(e) => { + return ( + IndexSet::new(), + Err(anyhow::anyhow!("Compile failed: {:?}", e)), + ) + } + }; opt.k_code_list.append(&mut k_code_list); } - match panic::catch_unwind(move || { - // Parser - let sess = ParseSessionRef::default(); - let mut program = load_program(sess.clone(), &files, Some(opt), params.module_cache) - .unwrap() - .program; - // Resolver - let prog_scope = resolve_program_with_opts( - &mut program, - kclvm_sema::resolver::Options { - merge_program: false, - type_erasure: false, - ..Default::default() - }, - params.scope_cache, - ); - // Please note that there is no global state cache at this stage. - let gs = GlobalState::default(); - let gs = Namer::find_symbols(&program, gs); - let gs = AdvancedResolver::resolve_program(&program, gs, prog_scope.node_ty_map.clone())?; - // Merge parse diagnostic and resolve diagnostic - sess.append_diagnostic(prog_scope.handler.diagnostics.clone()); - let diags = sess.1.borrow().diagnostics.clone(); - Ok((program, diags, gs)) - }) { - Ok(res) => res, - Err(e) => Err(anyhow::anyhow!("Compile failed: {:?}", e)), + + let mut diags = IndexSet::new(); + + // Parser + let sess = ParseSessionRef::default(); + let mut program = match load_program(sess.clone(), &files, Some(opt), params.module_cache) { + Ok(r) => r.program, + Err(e) => return (diags, Err(anyhow::anyhow!("Parse failed: {:?}", e))), + }; + diags.extend(sess.1.borrow().diagnostics.clone()); + // Resolver + let prog_scope = resolve_program_with_opts( + &mut program, + kclvm_sema::resolver::Options { + merge_program: false, + type_erasure: false, + ..Default::default() + }, + params.scope_cache, + ); + diags.extend(prog_scope.handler.diagnostics); + + // Please note that there is no global state cache at this stage. + let gs = GlobalState::default(); + 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))), } }