From b8cdc93dc6ce73de566dc9524e6dea758df7d968 Mon Sep 17 00:00:00 2001 From: "xiarui.xr" Date: Tue, 19 Sep 2023 09:09:05 +0800 Subject: [PATCH] find refs: add unit test; fix kcl_pos bug --- kclvm/Cargo.lock | 4 +- kclvm/parser/src/lib.rs | 2 +- kclvm/tools/src/LSP/src/capabilities.rs | 1 + kclvm/tools/src/LSP/src/find_refs.rs | 326 ++++++++++++------ kclvm/tools/src/LSP/src/from_lsp.rs | 6 +- kclvm/tools/src/LSP/src/goto_def.rs | 22 +- kclvm/tools/src/LSP/src/main.rs | 2 +- kclvm/tools/src/LSP/src/notification.rs | 5 +- kclvm/tools/src/LSP/src/request.rs | 55 ++- kclvm/tools/src/LSP/src/state.rs | 5 +- .../LSP/src/test_data/find_refs_test/main.k | 16 +- kclvm/tools/src/LSP/src/tests.rs | 83 +++++ kclvm/tools/src/LSP/src/util.rs | 29 +- 13 files changed, 407 insertions(+), 149 deletions(-) diff --git a/kclvm/Cargo.lock b/kclvm/Cargo.lock index 035c684be..396095451 100644 --- a/kclvm/Cargo.lock +++ b/kclvm/Cargo.lock @@ -1961,9 +1961,9 @@ checksum = "490cc448043f947bae3cbee9c203358d62dbee0db12107a74be5c30ccfd09771" [[package]] name = "memchr" -version = "2.6.3" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f232d6ef707e1956a43342693d2a31e72989554d58299d7a88738cc95b0d35c" +checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" [[package]] name = "memmap2" diff --git a/kclvm/parser/src/lib.rs b/kclvm/parser/src/lib.rs index 2c5fa8bce..0cd42ecb8 100644 --- a/kclvm/parser/src/lib.rs +++ b/kclvm/parser/src/lib.rs @@ -313,7 +313,7 @@ impl Loader { fn _load_main(&mut self) -> Result { let compile_entries = get_compile_entries_from_paths(&self.paths, &self.opts)?; let mut pkgs = HashMap::new(); - let workdir = compile_entries.get_root_path().to_string();// get package root + let workdir = compile_entries.get_root_path().to_string(); debug_assert_eq!(compile_entries.len(), self.paths.len()); diff --git a/kclvm/tools/src/LSP/src/capabilities.rs b/kclvm/tools/src/LSP/src/capabilities.rs index 801bcd91a..ed5c707b9 100644 --- a/kclvm/tools/src/LSP/src/capabilities.rs +++ b/kclvm/tools/src/LSP/src/capabilities.rs @@ -38,6 +38,7 @@ pub fn server_capabilities(client_caps: &ClientCapabilities) -> ServerCapabiliti ), document_formatting_provider: Some(OneOf::Left(true)), document_range_formatting_provider: Some(OneOf::Left(true)), + references_provider: Some(OneOf::Left(true)), ..Default::default() } } diff --git a/kclvm/tools/src/LSP/src/find_refs.rs b/kclvm/tools/src/LSP/src/find_refs.rs index e8c8ee300..a5828f47a 100644 --- a/kclvm/tools/src/LSP/src/find_refs.rs +++ b/kclvm/tools/src/LSP/src/find_refs.rs @@ -1,116 +1,244 @@ +use crate::from_lsp::kcl_pos; +use crate::goto_def::goto_definition; +use crate::util::{build_word_index, parse_param_and_compile, Param}; use anyhow; -use std::collections::HashMap; -use crate::{ - util::{build_word_index, parse_param_and_compile, Param}, - state::{LanguageServerSnapshot, Task, log_message}, - from_lsp::{self, file_path_from_url, kcl_pos}, - goto_def::{goto_definition, find_def,}, -}; -use lsp_types; -use crossbeam_channel::Sender; use kclvm_config::modfile::get_pkg_root; -use kclvm_ast::ast::Stmt; +use lsp_types::Location; - -pub(crate) fn find_references ( - snapshot: LanguageServerSnapshot, - params: lsp_types::ReferenceParams, - sender: Sender, -) -> anyhow::Result>> { - // 1. find definition of current token - let file = file_path_from_url(¶ms.text_document_position.text_document.uri)?; - let path = from_lsp::abs_path(¶ms.text_document_position.text_document.uri)?; - let db = snapshot.get_db(&path.clone().into())?; - let pos = kcl_pos(&file, params.text_document_position.position); - - if let Some(def_resp) = goto_definition(&db.prog, &pos, &db.scope) { - match def_resp { - lsp_types::GotoDefinitionResponse::Scalar(def_loc) => { - // get the def location - if let Some(def_name) = match db.prog.pos_to_stmt(&pos) { - Some(node) => match node.node { - Stmt::Import(_) => None, - _ => match find_def(node.clone(), &pos, &db.scope) { - Some(def) => Some(def.get_name()), - None => None, - }, - }, - None => None, - } { - // 2. find all occurrence of current token - // todo: decide the scope by the workspace root and the kcl.mod both, use the narrower scope - if let Some(root) = get_pkg_root(path.display().to_string().as_str()) { - match build_word_index(root) { - Ok(word_index) => { - return find_refs(def_loc, def_name, word_index); - }, - Err(_) => { - let _ = log_message("build word index failed".to_string(), &sender); - return anyhow::Ok(None); - } - } - } else { - return Ok(None) - } +pub(crate) fn find_refs Result<(), anyhow::Error>>( + def_loc: Location, + name: String, + cursor_path: String, + logger: F, +) -> anyhow::Result>> { + // todo: decide the scope by the workspace root and the kcl.mod both, use the narrower scope + // todo: should use the current file path + if let Some(root) = get_pkg_root(def_loc.uri.path()) { + match build_word_index(root) { + std::result::Result::Ok(word_index) => { + if let Some(locs) = word_index.get(name.as_str()).cloned() { + return anyhow::Ok(Some( + locs.into_iter() + .filter(|ref_loc| { + // from location to real def + // return if the real def location matches the def_loc + let file_path = ref_loc.uri.path().to_string(); + match parse_param_and_compile( + Param { + file: file_path.clone(), + }, + None, + ) { + Ok((prog, scope, _)) => { + let ref_pos = kcl_pos(&file_path, ref_loc.range.start); + // find def from the ref_pos + if let Some(real_def) = + goto_definition(&prog, &ref_pos, &scope) + { + match real_def { + lsp_types::GotoDefinitionResponse::Scalar( + real_def_loc, + ) => real_def_loc == def_loc, + _ => false, + } + } else { + false + } + } + Err(_) => { + let _ = logger(format!("{cursor_path} compilation failed")); + return false; + } + } + }) + .collect(), + )); + } else { + return Ok(None); } - }, - _=> return Ok(None), + } + Err(_) => { + logger("build word index failed".to_string())?; + return Ok(None); + } } } else { - log_message("Definition item not found, result in no reference".to_string(), &sender)?; + return Ok(None); } - - return Ok(None) } -pub(crate) fn find_refs(def_loc:lsp_types::Location, name: String, word_index: HashMap>) --> anyhow::Result>>{ - if let Some(locs) = word_index.get(name.as_str()).cloned() { - return anyhow::Ok(Some(locs.into_iter().filter(|ref_loc|{ - // from location to real def - // return if the real def location matches the def_loc - let file_path = ref_loc.uri.path().to_string(); - match parse_param_and_compile( - Param { - file: file_path.clone(), - }, - None, - ) { - Ok((prog, scope, _)) => { - let ref_pos = kcl_pos(&file_path, ref_loc.range.start); - // find def from the ref_pos - if let Some(real_def) = goto_definition(&prog, &ref_pos, &scope) { - match real_def { - lsp_types::GotoDefinitionResponse::Scalar(real_def_loc) => { - real_def_loc == def_loc - }, - _ => false - } - } else { - false - } +#[cfg(test)] +mod tests { + use super::find_refs; + use lsp_types::{Location, Position, Range}; + use proc_macro_crate::bench_test; + use std::{path::PathBuf, vec}; + + fn logger(msg: String) -> Result<(), anyhow::Error> { + println!("{}", msg); + anyhow::Ok(()) + } + fn check_locations_match(expect: Vec, actual: anyhow::Result>>) { + match actual { + Ok(act) => { + if let Some(locations) = act { + assert_eq!(expect, locations) + } else { + assert!(false, "got empty result. expect: {:?}", expect) } - Err(_) => { - // todo log compilation error - return false; - }, } - }).collect())); - } else { - return Ok(None) + Err(_) => assert!(false), + } } - -} -#[cfg(test)] -mod tests { - //todo - // todo assert #[test] - fn test_find_refs() { - + #[bench_test] + fn find_refs_from_variable_test() { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let mut path = root.clone(); + path.push("src/test_data/find_refs_test/main.k"); + let path = path.to_str().unwrap(); + match lsp_types::Url::from_file_path(path) { + Ok(url) => { + let def_loc = Location { + uri: url.clone(), + range: Range { + start: Position::new(0, 0), + end: Position::new(0, 1), + }, + }; + let expect = vec![ + Location { + uri: url.clone(), + range: Range { + start: Position::new(0, 0), + end: Position::new(0, 1), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(1, 4), + end: Position::new(1, 5), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(2, 4), + end: Position::new(2, 5), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(12, 14), + end: Position::new(12, 15), + }, + }, + ]; + check_locations_match( + expect, + find_refs(def_loc, "a".to_string(), path.to_string(), logger), + ); + } + Err(_) => assert!(false, "file not found"), + } } - -} \ No newline at end of file + #[test] + #[bench_test] + fn find_refs_from_schema_name_test() { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let mut path = root.clone(); + path.push("src/test_data/find_refs_test/main.k"); + let path = path.to_str().unwrap(); + match lsp_types::Url::from_file_path(path) { + Ok(url) => { + let def_loc = Location { + uri: url.clone(), + range: Range { + start: Position::new(4, 0), + end: Position::new(7, 0), + }, + }; + let expect = vec![ + Location { + uri: url.clone(), + range: Range { + start: Position::new(4, 7), + end: Position::new(4, 11), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(8, 4), + end: Position::new(8, 8), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(11, 7), + end: Position::new(11, 11), + }, + }, + ]; + check_locations_match( + expect, + find_refs(def_loc, "Name".to_string(), path.to_string(), logger), + ); + } + Err(_) => assert!(false, "file not found"), + } + } + #[test] + #[bench_test] + fn find_refs_from_schema_name_test() { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let mut path = root.clone(); + path.push("src/test_data/find_refs_test/main.k"); + let path = path.to_str().unwrap(); + match lsp_types::Url::from_file_path(path) { + Ok(url) => { + let def_loc = Location { + uri: url.clone(), + range: Range { + start: Position::new(4, 0), + end: Position::new(7, 0), + }, + }; + let expect = vec![ + Location { + uri: url.clone(), + range: Range { + start: Position::new(4, 7), + end: Position::new(4, 11), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(8, 4), + end: Position::new(8, 8), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(11, 7), + end: Position::new(11, 11), + }, + }, + ]; + check_locations_match( + expect, + find_refs(def_loc, "Name".to_string(), path.to_string(), logger), + ); + } + Err(_) => assert!(false, "file not found"), + } + } +} diff --git a/kclvm/tools/src/LSP/src/from_lsp.rs b/kclvm/tools/src/LSP/src/from_lsp.rs index 09f103810..eab37b973 100644 --- a/kclvm/tools/src/LSP/src/from_lsp.rs +++ b/kclvm/tools/src/LSP/src/from_lsp.rs @@ -19,11 +19,7 @@ pub(crate) fn kcl_pos(file: &str, pos: Position) -> KCLPos { KCLPos { filename: file.to_string(), line: (pos.line + 1) as u64, - column: if pos.character == 0 { - None - } else { - Some(pos.character as u64) - }, + column: Some(pos.character as u64), } } diff --git a/kclvm/tools/src/LSP/src/goto_def.rs b/kclvm/tools/src/LSP/src/goto_def.rs index 21dcb03ab..d8c49f006 100644 --- a/kclvm/tools/src/LSP/src/goto_def.rs +++ b/kclvm/tools/src/LSP/src/goto_def.rs @@ -105,7 +105,6 @@ impl Definition { } } - pub(crate) fn find_def( node: Node, kcl_pos: &KCLPos, @@ -241,15 +240,18 @@ pub(crate) fn resolve_var( let ty = get_system_member_function_ty(&name, &func_name); match &ty.kind { kclvm_sema::ty::TypeKind::Function(func_ty) => { - return Some(Definition::Object(ScopeObject { - name: func_name.clone(), - start: func_name_node.get_pos(), - end: func_name_node.get_end_pos(), - ty: ty.clone(), - kind: ScopeObjectKind::FunctionCall, - used: false, - doc: Some(func_ty.doc.clone()), - }, func_name)) + return Some(Definition::Object( + ScopeObject { + name: func_name.clone(), + start: func_name_node.get_pos(), + end: func_name_node.get_end_pos(), + ty: ty.clone(), + kind: ScopeObjectKind::FunctionCall, + used: false, + doc: Some(func_ty.doc.clone()), + }, + func_name, + )) } _ => return None, } diff --git a/kclvm/tools/src/LSP/src/main.rs b/kclvm/tools/src/LSP/src/main.rs index c605e967b..ac76e7019 100644 --- a/kclvm/tools/src/LSP/src/main.rs +++ b/kclvm/tools/src/LSP/src/main.rs @@ -7,9 +7,9 @@ mod config; mod db; mod dispatcher; mod document_symbol; +mod find_refs; mod from_lsp; mod goto_def; -mod find_refs; mod hover; mod main_loop; mod notification; diff --git a/kclvm/tools/src/LSP/src/notification.rs b/kclvm/tools/src/LSP/src/notification.rs index b86c46e55..8b4d2761b 100644 --- a/kclvm/tools/src/LSP/src/notification.rs +++ b/kclvm/tools/src/LSP/src/notification.rs @@ -1,5 +1,8 @@ use lsp_types::notification::{ - DidChangeTextDocument, DidChangeWatchedFiles, DidCloseTextDocument, DidOpenTextDocument, + DidChangeTextDocument, + DidChangeWatchedFiles, + DidCloseTextDocument, + DidOpenTextDocument, DidSaveTextDocument, //Initialized, }; diff --git a/kclvm/tools/src/LSP/src/request.rs b/kclvm/tools/src/LSP/src/request.rs index 4e41b9964..e694c74a3 100644 --- a/kclvm/tools/src/LSP/src/request.rs +++ b/kclvm/tools/src/LSP/src/request.rs @@ -1,6 +1,7 @@ use anyhow::Ok; use crossbeam_channel::Sender; -use lsp_types::{TextEdit, Location}; +use kclvm_ast::ast::Stmt; +use lsp_types::{Location, TextEdit}; use ra_ap_vfs::VfsPath; use std::time::Instant; @@ -9,10 +10,10 @@ use crate::{ db::AnalysisDatabase, dispatcher::RequestDispatcher, document_symbol::document_symbol, + find_refs::find_refs, formatting::format, from_lsp::{self, file_path_from_url, kcl_pos}, - goto_def::goto_definition, - find_refs::find_references, + goto_def::{find_def, goto_definition}, hover, quick_fix, state::{log_message, LanguageServerSnapshot, LanguageServerState, Task}, }; @@ -72,16 +73,6 @@ impl LanguageServerSnapshot { } } -// pub(crate) fn handle_initialize( -// _snapshot: LanguageServerSnapshot, -// params: lsp_types::InitializeParams, -// _sender: Sender -// ) -> anyhow::Result{ -// if let Some(uri) = params.root_uri { -// self.word_index = build_word_index(uri.path().to_string()) -// } -// } - pub(crate) fn handle_formatting( _snapshot: LanguageServerSnapshot, params: lsp_types::DocumentFormattingParams, @@ -146,12 +137,46 @@ pub(crate) fn handle_goto_definition( } /// Called when a `FindReferences` request was received -pub(crate) fn handle_reference ( +pub(crate) fn handle_reference( snapshot: LanguageServerSnapshot, params: lsp_types::ReferenceParams, sender: Sender, ) -> anyhow::Result>> { - find_references(snapshot, params, sender) + // 1. find definition of current token + let file = file_path_from_url(¶ms.text_document_position.text_document.uri)?; + let path = from_lsp::abs_path(¶ms.text_document_position.text_document.uri)?; + let db = snapshot.get_db(&path.clone().into())?; + let pos = kcl_pos(&file, params.text_document_position.position); + + let log = |msg: String| log_message(msg, &sender); + + if let Some(def_resp) = goto_definition(&db.prog, &pos, &db.scope) { + match def_resp { + lsp_types::GotoDefinitionResponse::Scalar(def_loc) => { + // get the def location + if let Some(def_name) = match db.prog.pos_to_stmt(&pos) { + Some(node) => match node.node { + Stmt::Import(_) => None, + _ => match find_def(node.clone(), &pos, &db.scope) { + Some(def) => Some(def.get_name()), + None => None, + }, + }, + None => None, + } { + return find_refs(def_loc, def_name, file, log); + } + } + _ => return Ok(None), + } + } else { + log_message( + "Definition item not found, result in no reference".to_string(), + &sender, + )?; + } + + return Ok(None); } /// Called when a `Completion` request was received. diff --git a/kclvm/tools/src/LSP/src/state.rs b/kclvm/tools/src/LSP/src/state.rs index 20ec3dec0..e64a485dd 100644 --- a/kclvm/tools/src/LSP/src/state.rs +++ b/kclvm/tools/src/LSP/src/state.rs @@ -2,14 +2,13 @@ use crate::analysis::Analysis; use crate::config::Config; use crate::db::AnalysisDatabase; use crate::to_lsp::{kcl_diag_to_lsp_diags, url}; -use crate::util::{get_file_name, parse_param_and_compile, to_json, Param, self}; +use crate::util::{self, get_file_name, parse_param_and_compile, to_json, Param}; use crossbeam_channel::{select, unbounded, Receiver, Sender}; use indexmap::IndexSet; use lsp_server::{ReqQueue, Response}; use lsp_types::{ notification::{Notification, PublishDiagnostics}, - Diagnostic, PublishDiagnosticsParams, - Location, + Diagnostic, Location, PublishDiagnosticsParams, }; use parking_lot::RwLock; use ra_ap_vfs::{FileId, Vfs}; diff --git a/kclvm/tools/src/LSP/src/test_data/find_refs_test/main.k b/kclvm/tools/src/LSP/src/test_data/find_refs_test/main.k index bf5812c28..ad2c3f8d9 100644 --- a/kclvm/tools/src/LSP/src/test_data/find_refs_test/main.k +++ b/kclvm/tools/src/LSP/src/test_data/find_refs_test/main.k @@ -1,3 +1,15 @@ -a = 1 +a = "demo" b = a -c = a \ No newline at end of file +c = a + +schema Name: + name: str + +schema Person: + n: Name + +p2 = Person { + n: Name{ + name: a + } +} \ No newline at end of file diff --git a/kclvm/tools/src/LSP/src/tests.rs b/kclvm/tools/src/LSP/src/tests.rs index d21168b74..3633870c1 100644 --- a/kclvm/tools/src/LSP/src/tests.rs +++ b/kclvm/tools/src/LSP/src/tests.rs @@ -16,6 +16,8 @@ use lsp_types::HoverContents; use lsp_types::HoverParams; use lsp_types::MarkedString; use lsp_types::PublishDiagnosticsParams; +use lsp_types::ReferenceContext; +use lsp_types::ReferenceParams; use lsp_types::TextDocumentIdentifier; use lsp_types::TextDocumentItem; use lsp_types::TextDocumentPositionParams; @@ -772,3 +774,84 @@ fn formatting_test() { .unwrap() ) } + +#[test] +fn test_find_refs() { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let mut path = root.clone(); + path.push("src/test_data/find_refs_test/main.k"); + + let path = path.to_str().unwrap(); + let src = std::fs::read_to_string(path.clone()).unwrap(); + let server = Project {}.server(); + let url = Url::from_file_path(path).unwrap(); + + // Mock open file + server.notification::( + lsp_types::DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: url.clone(), + language_id: "KCL".to_string(), + version: 0, + text: src, + }, + }, + ); + + let id = server.next_request_id.get(); + server.next_request_id.set(id.wrapping_add(1)); + + let r: Request = Request::new( + id.into(), + "textDocument/references".to_string(), + ReferenceParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: url.clone() }, + position: Position::new(0, 1), + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + context: ReferenceContext { + include_declaration: true, + }, + }, + ); + + // Send request and wait for it's response + let res = server.send_and_receive(r); + + assert_eq!( + res.result.unwrap(), + to_json(vec![ + Location { + uri: url.clone(), + range: Range { + start: Position::new(0, 0), + end: Position::new(0, 1), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(1, 4), + end: Position::new(1, 5), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(2, 4), + end: Position::new(2, 5), + }, + }, + Location { + uri: url.clone(), + range: Range { + start: Position::new(12, 14), + end: Position::new(12, 15), + }, + }, + ]) + .unwrap() + ); +} diff --git a/kclvm/tools/src/LSP/src/util.rs b/kclvm/tools/src/LSP/src/util.rs index fdbd8a0a6..45ebe9ad5 100644 --- a/kclvm/tools/src/LSP/src/util.rs +++ b/kclvm/tools/src/LSP/src/util.rs @@ -15,15 +15,15 @@ use kclvm_parser::{load_program, ParseSession}; use kclvm_sema::resolver::scope::Scope; use kclvm_sema::resolver::{resolve_program, scope::ProgramScope}; use kclvm_utils::pkgpath::rm_external_pkg_name; -use lsp_types::{Location, Range, Position, Url}; +use lsp_types::{Location, Position, Range, Url}; use parking_lot::{RwLock, RwLockReadGuard}; use ra_ap_vfs::{FileId, Vfs}; use serde::{de::DeserializeOwned, Serialize}; use std::cell::RefCell; +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::{fs, sync::Arc}; -use std::collections::HashMap; use crate::from_lsp; @@ -740,7 +740,7 @@ pub(crate) fn get_pkg_scope( } /// scan and build a word -> Locations index map -pub fn build_word_index(path: String)-> anyhow::Result>> { +pub fn build_word_index(path: String) -> anyhow::Result>> { let mut index: HashMap> = HashMap::new(); if let Ok(files) = get_kcl_files(path.clone(), true) { for file_path in &files { @@ -752,17 +752,22 @@ pub fn build_word_index(path: String)-> anyhow::Result HashMap> { false => { // Find out the end position. if continue_pos + 1 == i { - words.push(Word::new(start_pos as u32,i as u32,chars[start_pos..i].iter().collect::().clone())); + words.push(Word::new( + start_pos as u32, + i as u32, + chars[start_pos..i].iter().collect::().clone(), + )); } // Reset the start position. start_pos = usize::MAX; @@ -832,7 +841,7 @@ fn line_to_words(text: String) -> HashMap> { #[cfg(test)] mod tests { - use super::{line_to_words,build_word_index, Word}; + use super::{build_word_index, line_to_words, Word}; // todo assert #[test] fn test_build_word_index() {