From 73187576fd161de904c6f9ecbfe299e82cb4073e Mon Sep 17 00:00:00 2001 From: "xiarui.xr" Date: Mon, 16 Oct 2023 22:25:15 +0800 Subject: [PATCH 1/4] feat: rename symbol Signed-off-by: xiarui.xr --- kclvm/tools/src/LSP/src/capabilities.rs | 1 + kclvm/tools/src/LSP/src/find_refs.rs | 78 ++++++++---- kclvm/tools/src/LSP/src/goto_def.rs | 25 ++-- kclvm/tools/src/LSP/src/request.rs | 112 ++++++++++++------ .../src/LSP/src/test_data/rename_test/main.k | 6 + .../LSP/src/test_data/rename_test/pkg/vars.k | 13 ++ kclvm/tools/src/LSP/src/tests.rs | 95 +++++++++++++++ kclvm/tools/src/LSP/src/to_lsp.rs | 10 ++ 8 files changed, 261 insertions(+), 79 deletions(-) create mode 100644 kclvm/tools/src/LSP/src/test_data/rename_test/main.k create mode 100644 kclvm/tools/src/LSP/src/test_data/rename_test/pkg/vars.k diff --git a/kclvm/tools/src/LSP/src/capabilities.rs b/kclvm/tools/src/LSP/src/capabilities.rs index ed5c707b9..fc3ce5f7a 100644 --- a/kclvm/tools/src/LSP/src/capabilities.rs +++ b/kclvm/tools/src/LSP/src/capabilities.rs @@ -39,6 +39,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)), + rename_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 1ddf1db0f..2a5d4b539 100644 --- a/kclvm/tools/src/LSP/src/find_refs.rs +++ b/kclvm/tools/src/LSP/src/find_refs.rs @@ -1,7 +1,11 @@ use crate::from_lsp::kcl_pos; -use crate::goto_def::goto_definition; +use crate::goto_def::{find_def, goto_definition}; +use crate::to_lsp::lsp_location; use crate::util::{parse_param_and_compile, Param}; use anyhow; +use kclvm_ast::ast::{Program, Stmt}; +use kclvm_error::Position as KCLPos; +use kclvm_sema::resolver::scope::ProgramScope; use lsp_types::{Location, Url}; use parking_lot::RwLock; use ra_ap_vfs::Vfs; @@ -9,15 +13,52 @@ use std::collections::HashMap; use std::sync::Arc; pub(crate) fn find_refs Result<(), anyhow::Error>>( + program: &Program, + kcl_pos: &KCLPos, + prog_scope: &ProgramScope, + word_index_map: HashMap>>, + vfs: Option>>, + logger: F, +) -> Result, String> { + // find the definition at the position + let def = match program.pos_to_stmt(kcl_pos) { + Some(node) => match node.node { + Stmt::Import(_) => None, + _ => find_def(node.clone(), kcl_pos, prog_scope), + }, + None => None, + }; + if def.is_none() { + return Err(String::from( + "Definition item not found, result in no reference", + )); + } + let def = def.unwrap(); + if def.get_positions().len() != 1 { + return Err(String::from( + "Found more than one definitions, reference not supported", + )); + } + let (start, end) = def.get_positions().iter().next().unwrap().clone(); + let def_loc = lsp_location(start.filename.clone(), &start, &end); + // find all the refs of the def + Ok(find_refs_from_def( + vfs, + word_index_map, + def_loc, + def.get_name(), + logger, + )) +} + +pub(crate) fn find_refs_from_def Result<(), anyhow::Error>>( vfs: Option>>, word_index_map: HashMap>>, def_loc: Location, name: String, - cursor_path: String, logger: F, -) -> anyhow::Result>> { +) -> Vec { let mut ref_locations = vec![]; - for (_, word_index) in word_index_map { if let Some(locs) = word_index.get(name.as_str()).cloned() { let matched_locs: Vec = locs @@ -47,7 +88,8 @@ pub(crate) fn find_refs Result<(), anyhow::Error>>( } } Err(_) => { - let _ = logger(format!("{cursor_path} compilation failed")); + let file_path = def_loc.uri.path(); + let _ = logger(format!("{file_path} compilation failed")); return false; } } @@ -56,12 +98,12 @@ pub(crate) fn find_refs Result<(), anyhow::Error>>( ref_locations.extend(matched_locs); } } - anyhow::Ok(Some(ref_locations)) + ref_locations } #[cfg(test)] mod tests { - use super::find_refs; + use super::find_refs_from_def; use crate::util::build_word_index; use lsp_types::{Location, Position, Range, Url}; use std::collections::HashMap; @@ -72,17 +114,8 @@ mod tests { 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(_) => assert!(false), - } + fn check_locations_match(expect: Vec, actual: Vec) { + assert_eq!(expect, actual) } fn setup_word_index_map(root: &str) -> HashMap>> { @@ -139,12 +172,11 @@ mod tests { ]; check_locations_match( expect, - find_refs( + find_refs_from_def( None, setup_word_index_map(path), def_loc, "a".to_string(), - path.to_string(), logger, ), ); @@ -193,12 +225,11 @@ mod tests { ]; check_locations_match( expect, - find_refs( + find_refs_from_def( None, setup_word_index_map(path), def_loc, "Name".to_string(), - path.to_string(), logger, ), ); @@ -240,12 +271,11 @@ mod tests { ]; check_locations_match( expect, - find_refs( + find_refs_from_def( None, setup_word_index_map(path), def_loc, "name".to_string(), - path.to_string(), logger, ), ); diff --git a/kclvm/tools/src/LSP/src/goto_def.rs b/kclvm/tools/src/LSP/src/goto_def.rs index c5a243db4..2eb1c99ed 100644 --- a/kclvm/tools/src/LSP/src/goto_def.rs +++ b/kclvm/tools/src/LSP/src/goto_def.rs @@ -19,13 +19,12 @@ use kclvm_sema::resolver::scope::{ builtin_scope, ProgramScope, Scope, ScopeObject, ScopeObjectKind, }; use kclvm_sema::ty::{DictType, SchemaType}; -use lsp_types::{GotoDefinitionResponse, Url}; -use lsp_types::{Location, Range}; +use lsp_types::GotoDefinitionResponse; use std::cell::RefCell; use std::path::Path; use std::rc::Rc; -use crate::to_lsp::lsp_pos; +use crate::to_lsp::lsp_location; use crate::util::{ fix_missing_identifier, get_pkg_scope, get_pos_from_real_path, get_real_path_from_external, inner_most_expr_in_stmt, @@ -408,24 +407,16 @@ fn positions_to_goto_def_resp( 0 => None, 1 => { let (start, end) = positions.iter().next().unwrap().clone(); - Some(lsp_types::GotoDefinitionResponse::Scalar(Location { - uri: Url::from_file_path(start.filename.clone()).unwrap(), - range: Range { - start: lsp_pos(&start), - end: lsp_pos(&end), - }, - })) + Some(lsp_types::GotoDefinitionResponse::Scalar(lsp_location( + start.filename.clone(), + &start, + &end, + ))) } _ => { let mut res = vec![]; for (start, end) in positions { - res.push(Location { - uri: Url::from_file_path(start.filename.clone()).unwrap(), - range: Range { - start: lsp_pos(start), - end: lsp_pos(end), - }, - }) + res.push(lsp_location(start.filename.clone(), &start, &end)) } Some(lsp_types::GotoDefinitionResponse::Array(res)) } diff --git a/kclvm/tools/src/LSP/src/request.rs b/kclvm/tools/src/LSP/src/request.rs index 4c24e3dfa..5c3436c28 100644 --- a/kclvm/tools/src/LSP/src/request.rs +++ b/kclvm/tools/src/LSP/src/request.rs @@ -1,8 +1,8 @@ use anyhow::Ok; use crossbeam_channel::Sender; -use kclvm_ast::ast::Stmt; use lsp_types::{Location, TextEdit}; use ra_ap_vfs::VfsPath; +use std::collections::HashMap; use std::time::Instant; use crate::{ @@ -13,7 +13,7 @@ use crate::{ find_refs::find_refs, formatting::format, from_lsp::{self, file_path_from_url, kcl_pos}, - goto_def::{find_def, goto_definition}, + goto_def::goto_definition, hover, quick_fix, state::{log_message, LanguageServerSnapshot, LanguageServerState, Task}, }; @@ -52,6 +52,7 @@ impl LanguageServerState { .on::(handle_code_action)? .on::(handle_formatting)? .on::(handle_range_formatting)? + .on::(handle_rename)? .finish(); Ok(()) @@ -141,49 +142,25 @@ pub(crate) fn handle_reference( 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); - let word_index_map = snapshot.word_index_map.clone(); - 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( - Some(snapshot.vfs), - word_index_map, - def_loc, - def_name, - file, - log, - ); - } - } - _ => return Ok(None), + match find_refs( + &db.prog, + &pos, + &db.scope, + snapshot.word_index_map.clone(), + Some(snapshot.vfs.clone()), + log, + ) { + core::result::Result::Ok(locations) => Ok(Some(locations)), + Err(msg) => { + log(format!("Find references failed: {msg}"))?; + 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. @@ -239,3 +216,62 @@ pub(crate) fn handle_document_symbol( } Ok(res) } + +/// Called when a `Rename` request was received. +pub(crate) fn handle_rename( + snapshot: LanguageServerSnapshot, + params: lsp_types::RenameParams, + sender: Sender, +) -> anyhow::Result> { + // 1. check the new name validity, todo + // let new_name = params.new_name; + + // 2. find all the references of the symbol + 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.into())?; + let kcl_pos = kcl_pos(&file, params.text_document_position.position); + let log = |msg: String| log_message(msg, &sender); + let references = find_refs( + &db.prog, + &kcl_pos, + &db.scope, + snapshot.word_index_map.clone(), + Some(snapshot.vfs.clone()), + log, + ); + match references { + core::result::Result::Ok(locations) => { + match locations.len() { + 0 => { + let _ = log("Symbol not found".to_string()); + Ok(None) + } + _ => { + // 3. return the workspaceEdit to rename all the references with the new name + let mut workspace_edit = lsp_types::WorkspaceEdit::default(); + + let changes = + locations + .into_iter() + .fold(HashMap::new(), |mut map, location| { + let uri = location.uri; + map.entry(uri.clone()) + .or_insert_with(Vec::new) + .push(TextEdit { + range: location.range, + new_text: params.new_name.clone(), + }); + map + }); + workspace_edit.changes = Some(changes); + Ok(Some(workspace_edit)) + } + } + } + Err(msg) => { + log(format!("Can not rename symbol: {msg}"))?; + Ok(None) + } + } +} diff --git a/kclvm/tools/src/LSP/src/test_data/rename_test/main.k b/kclvm/tools/src/LSP/src/test_data/rename_test/main.k new file mode 100644 index 000000000..efd22bf88 --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/rename_test/main.k @@ -0,0 +1,6 @@ +import .pkg.vars + +Bob = vars.Person { + name: "Bob" + age: 30 +} \ No newline at end of file diff --git a/kclvm/tools/src/LSP/src/test_data/rename_test/pkg/vars.k b/kclvm/tools/src/LSP/src/test_data/rename_test/pkg/vars.k new file mode 100644 index 000000000..6f32c0650 --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/rename_test/pkg/vars.k @@ -0,0 +1,13 @@ +schema Person: + name: str + age: int + +John = Person { + name: "John" + age: 20 +} + +Alice = Person { + name: "Alice" + age: 30 +} \ 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 c2979ac11..bb8663ba5 100644 --- a/kclvm/tools/src/LSP/src/tests.rs +++ b/kclvm/tools/src/LSP/src/tests.rs @@ -20,16 +20,19 @@ use lsp_types::MarkedString; use lsp_types::PublishDiagnosticsParams; use lsp_types::ReferenceContext; use lsp_types::ReferenceParams; +use lsp_types::RenameParams; use lsp_types::TextDocumentIdentifier; use lsp_types::TextDocumentItem; use lsp_types::TextDocumentPositionParams; use lsp_types::TextEdit; use lsp_types::Url; +use lsp_types::WorkspaceEdit; use lsp_types::WorkspaceFolder; use serde::Serialize; use std::cell::Cell; use std::cell::RefCell; +use std::collections::HashMap; use std::env; use std::path::Path; use std::path::PathBuf; @@ -1491,3 +1494,95 @@ p2 = Person { .unwrap() ); } + +#[test] +fn rename_test() { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let mut path = root.clone(); + let mut main_path = root.clone(); + path.push("src/test_data/rename_test/pkg/vars.k"); + main_path.push("src/test_data/rename_test/main.k"); + + let path = path.to_str().unwrap(); + let src = std::fs::read_to_string(path.clone()).unwrap(); + let mut initialize_params = InitializeParams::default(); + initialize_params.workspace_folders = Some(vec![WorkspaceFolder { + uri: Url::from_file_path(root.clone()).unwrap(), + name: "test".to_string(), + }]); + let server = Project {}.server(initialize_params); + let url = Url::from_file_path(path).unwrap(); + let main_url = Url::from_file_path(main_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 new_name = String::from("Person2"); + let r: Request = Request::new( + id.into(), + "textDocument/rename".to_string(), + RenameParams { + text_document_position: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri: url.clone() }, + position: Position::new(0, 7), + }, + new_name: new_name.clone(), + work_done_progress_params: Default::default(), + }, + ); + + // Send request and wait for it's response + let res = server.send_and_receive(r); + let mut expect = WorkspaceEdit::default(); + expect.changes = Some(HashMap::from_iter(vec![ + ( + url.clone(), + vec![ + TextEdit { + range: Range { + start: Position::new(0, 7), + end: Position::new(0, 13), + }, + new_text: new_name.clone(), + }, + TextEdit { + range: Range { + start: Position::new(4, 7), + end: Position::new(4, 13), + }, + new_text: new_name.clone(), + }, + TextEdit { + range: Range { + start: Position::new(9, 8), + end: Position::new(9, 14), + }, + new_text: new_name.clone(), + }, + ], + ), + ( + main_url.clone(), + vec![TextEdit { + range: Range { + start: Position::new(2, 11), + end: Position::new(2, 17), + }, + new_text: new_name.clone(), + }], + ), + ])); + assert_eq!(res.result.unwrap(), to_json(expect).unwrap()); +} diff --git a/kclvm/tools/src/LSP/src/to_lsp.rs b/kclvm/tools/src/LSP/src/to_lsp.rs index 36940ff11..2a7423799 100644 --- a/kclvm/tools/src/LSP/src/to_lsp.rs +++ b/kclvm/tools/src/LSP/src/to_lsp.rs @@ -21,6 +21,16 @@ pub fn lsp_pos(pos: &KCLPos) -> Position { } } +pub fn lsp_location(file_path: String, start: &KCLPos, end: &KCLPos) -> Location { + Location { + uri: Url::from_file_path(file_path).unwrap(), + range: Range { + start: lsp_pos(start), + end: lsp_pos(end), + }, + } +} + /// Convert KCL Message to LSP Diagnostic fn kcl_msg_to_lsp_diags( msg: &Message, From ed39272e5c8f9f214137114069c63f34f83925ad Mon Sep 17 00:00:00 2001 From: "xiarui.xr" Date: Tue, 17 Oct 2023 13:37:06 +0800 Subject: [PATCH 2/4] rename: support new name validity check Signed-off-by: xiarui.xr --- kclvm/Cargo.lock | 1 + kclvm/tools/src/LSP/Cargo.toml | 1 + kclvm/tools/src/LSP/src/request.rs | 23 ++++++++++++++--------- kclvm/tools/src/LSP/src/util.rs | 6 ++++++ 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/kclvm/Cargo.lock b/kclvm/Cargo.lock index f83b0d867..bc96c9f70 100644 --- a/kclvm/Cargo.lock +++ b/kclvm/Cargo.lock @@ -1416,6 +1416,7 @@ dependencies = [ "proc_macro_crate", "ra_ap_vfs", "ra_ap_vfs-notify", + "regex", "ropey", "rustc-hash", "rustc_lexer", diff --git a/kclvm/tools/src/LSP/Cargo.toml b/kclvm/tools/src/LSP/Cargo.toml index f78291226..b4f4cde0a 100644 --- a/kclvm/tools/src/LSP/Cargo.toml +++ b/kclvm/tools/src/LSP/Cargo.toml @@ -19,6 +19,7 @@ log = "0.4.14" im-rc = "15.0.0" rustc_lexer = "0.1.0" clap = "4.3.0" +regex = "1.4" kclvm-tools = { path = "../../../tools" } kclvm-error = { path = "../../../error" } diff --git a/kclvm/tools/src/LSP/src/request.rs b/kclvm/tools/src/LSP/src/request.rs index 5c3436c28..10b63f820 100644 --- a/kclvm/tools/src/LSP/src/request.rs +++ b/kclvm/tools/src/LSP/src/request.rs @@ -1,4 +1,4 @@ -use anyhow::Ok; +use anyhow::{anyhow, Ok}; use crossbeam_channel::Sender; use lsp_types::{Location, TextEdit}; use ra_ap_vfs::VfsPath; @@ -16,6 +16,7 @@ use crate::{ goto_def::goto_definition, hover, quick_fix, state::{log_message, LanguageServerSnapshot, LanguageServerState, Task}, + util, }; impl LanguageServerState { @@ -223,8 +224,11 @@ pub(crate) fn handle_rename( params: lsp_types::RenameParams, sender: Sender, ) -> anyhow::Result> { - // 1. check the new name validity, todo - // let new_name = params.new_name; + // 1. check the new name validity + let new_name = params.new_name; + if !util::is_valid_kcl_name(new_name.clone()) { + return Err(anyhow!("Can not rename to: {new_name}, invalid name")); + } // 2. find all the references of the symbol let file = file_path_from_url(¶ms.text_document_position.text_document.uri)?; @@ -241,11 +245,11 @@ pub(crate) fn handle_rename( log, ); match references { - core::result::Result::Ok(locations) => { + Result::Ok(locations) => { match locations.len() { 0 => { let _ = log("Symbol not found".to_string()); - Ok(None) + anyhow::Ok(None) } _ => { // 3. return the workspaceEdit to rename all the references with the new name @@ -260,18 +264,19 @@ pub(crate) fn handle_rename( .or_insert_with(Vec::new) .push(TextEdit { range: location.range, - new_text: params.new_name.clone(), + new_text: new_name.clone(), }); map }); workspace_edit.changes = Some(changes); - Ok(Some(workspace_edit)) + anyhow::Ok(Some(workspace_edit)) } } } Err(msg) => { - log(format!("Can not rename symbol: {msg}"))?; - Ok(None) + let err_msg = format!("Can not rename symbol: {msg}"); + log(err_msg.clone())?; + return Err(anyhow!(err_msg)); } } } diff --git a/kclvm/tools/src/LSP/src/util.rs b/kclvm/tools/src/LSP/src/util.rs index e93f184e5..cc2b1c0ef 100644 --- a/kclvm/tools/src/LSP/src/util.rs +++ b/kclvm/tools/src/LSP/src/util.rs @@ -19,6 +19,7 @@ use kclvm_utils::pkgpath::rm_external_pkg_name; use lsp_types::{Location, Position, Range, Url}; use parking_lot::{RwLock, RwLockReadGuard}; use ra_ap_vfs::{FileId, Vfs}; +use regex::Regex; use serde::{de::DeserializeOwned, Serialize}; use std::cell::RefCell; use std::collections::HashMap; @@ -894,6 +895,11 @@ fn line_to_words(text: String) -> HashMap> { result } +pub fn is_valid_kcl_name(name: String) -> bool { + let re = Regex::new(r#"^[a-zA-Z_][a-zA-Z0-9_]*$"#).unwrap(); + re.is_match(&name) +} + #[cfg(test)] mod tests { use super::{build_word_index, line_to_words, word_index_add, word_index_subtract, Word}; From ea4c1694f1ac37d14569a4b822a9d7503d12d987 Mon Sep 17 00:00:00 2001 From: "xiarui.xr" Date: Tue, 17 Oct 2023 14:06:56 +0800 Subject: [PATCH 3/4] minor fixes Signed-off-by: xiarui.xr --- kclvm/tools/src/LSP/src/find_refs.rs | 21 ++++++++------ kclvm/tools/src/LSP/src/goto_def.rs | 10 +++---- kclvm/tools/src/LSP/src/request.rs | 42 +++++++++++++--------------- kclvm/tools/src/LSP/src/to_lsp.rs | 9 +++--- 4 files changed, 40 insertions(+), 42 deletions(-) diff --git a/kclvm/tools/src/LSP/src/find_refs.rs b/kclvm/tools/src/LSP/src/find_refs.rs index 2a5d4b539..0b7d454ae 100644 --- a/kclvm/tools/src/LSP/src/find_refs.rs +++ b/kclvm/tools/src/LSP/src/find_refs.rs @@ -34,21 +34,24 @@ pub(crate) fn find_refs Result<(), anyhow::Error>>( )); } let def = def.unwrap(); - if def.get_positions().len() != 1 { + if def.get_positions().len() > 1 { return Err(String::from( "Found more than one definitions, reference not supported", )); } let (start, end) = def.get_positions().iter().next().unwrap().clone(); - let def_loc = lsp_location(start.filename.clone(), &start, &end); // find all the refs of the def - Ok(find_refs_from_def( - vfs, - word_index_map, - def_loc, - def.get_name(), - logger, - )) + if let Some(def_loc) = lsp_location(start.filename.clone(), &start, &end) { + Ok(find_refs_from_def( + vfs, + word_index_map, + def_loc, + def.get_name(), + logger, + )) + } else { + Err(format!("Invalid file path: {0}", start.filename)) + } } pub(crate) fn find_refs_from_def Result<(), anyhow::Error>>( diff --git a/kclvm/tools/src/LSP/src/goto_def.rs b/kclvm/tools/src/LSP/src/goto_def.rs index 2eb1c99ed..800dbf089 100644 --- a/kclvm/tools/src/LSP/src/goto_def.rs +++ b/kclvm/tools/src/LSP/src/goto_def.rs @@ -407,16 +407,14 @@ fn positions_to_goto_def_resp( 0 => None, 1 => { let (start, end) = positions.iter().next().unwrap().clone(); - Some(lsp_types::GotoDefinitionResponse::Scalar(lsp_location( - start.filename.clone(), - &start, - &end, - ))) + let loc = lsp_location(start.filename.clone(), &start, &end)?; + Some(lsp_types::GotoDefinitionResponse::Scalar(loc)) } _ => { let mut res = vec![]; for (start, end) in positions { - res.push(lsp_location(start.filename.clone(), &start, &end)) + let loc = lsp_location(start.filename.clone(), &start, &end)?; + res.push(loc) } Some(lsp_types::GotoDefinitionResponse::Array(res)) } diff --git a/kclvm/tools/src/LSP/src/request.rs b/kclvm/tools/src/LSP/src/request.rs index 10b63f820..d0dcc1826 100644 --- a/kclvm/tools/src/LSP/src/request.rs +++ b/kclvm/tools/src/LSP/src/request.rs @@ -246,31 +246,27 @@ pub(crate) fn handle_rename( ); match references { Result::Ok(locations) => { - match locations.len() { - 0 => { - let _ = log("Symbol not found".to_string()); - anyhow::Ok(None) - } - _ => { - // 3. return the workspaceEdit to rename all the references with the new name - let mut workspace_edit = lsp_types::WorkspaceEdit::default(); + if locations.is_empty() { + let _ = log("Symbol not found".to_string()); + anyhow::Ok(None) + } else { + // 3. return the workspaceEdit to rename all the references with the new name + let mut workspace_edit = lsp_types::WorkspaceEdit::default(); - let changes = - locations - .into_iter() - .fold(HashMap::new(), |mut map, location| { - let uri = location.uri; - map.entry(uri.clone()) - .or_insert_with(Vec::new) - .push(TextEdit { - range: location.range, - new_text: new_name.clone(), - }); - map + let changes = locations + .into_iter() + .fold(HashMap::new(), |mut map, location| { + let uri = location.uri; + map.entry(uri.clone()) + .or_insert_with(Vec::new) + .push(TextEdit { + range: location.range, + new_text: new_name.clone(), }); - workspace_edit.changes = Some(changes); - anyhow::Ok(Some(workspace_edit)) - } + map + }); + workspace_edit.changes = Some(changes); + anyhow::Ok(Some(workspace_edit)) } } Err(msg) => { diff --git a/kclvm/tools/src/LSP/src/to_lsp.rs b/kclvm/tools/src/LSP/src/to_lsp.rs index 2a7423799..0c54ea8c3 100644 --- a/kclvm/tools/src/LSP/src/to_lsp.rs +++ b/kclvm/tools/src/LSP/src/to_lsp.rs @@ -21,14 +21,15 @@ pub fn lsp_pos(pos: &KCLPos) -> Position { } } -pub fn lsp_location(file_path: String, start: &KCLPos, end: &KCLPos) -> Location { - Location { - uri: Url::from_file_path(file_path).unwrap(), +pub fn lsp_location(file_path: String, start: &KCLPos, end: &KCLPos) -> Option { + let uri = Url::from_file_path(file_path).ok()?; + Some(Location { + uri, range: Range { start: lsp_pos(start), end: lsp_pos(end), }, - } + }) } /// Convert KCL Message to LSP Diagnostic From 74f1efab13819604b8c889340f6a8472ffb9956b Mon Sep 17 00:00:00 2001 From: "xiarui.xr" Date: Tue, 17 Oct 2023 14:12:01 +0800 Subject: [PATCH 4/4] minor updates Signed-off-by: xiarui.xr --- kclvm/Cargo.lock | 1 - kclvm/sema/src/info/mod.rs | 8 ++++++++ kclvm/tools/src/LSP/Cargo.toml | 1 - kclvm/tools/src/LSP/src/request.rs | 4 ++-- kclvm/tools/src/LSP/src/util.rs | 6 ------ 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/kclvm/Cargo.lock b/kclvm/Cargo.lock index bc96c9f70..f83b0d867 100644 --- a/kclvm/Cargo.lock +++ b/kclvm/Cargo.lock @@ -1416,7 +1416,6 @@ dependencies = [ "proc_macro_crate", "ra_ap_vfs", "ra_ap_vfs-notify", - "regex", "ropey", "rustc-hash", "rustc_lexer", diff --git a/kclvm/sema/src/info/mod.rs b/kclvm/sema/src/info/mod.rs index f025de680..c57844717 100644 --- a/kclvm/sema/src/info/mod.rs +++ b/kclvm/sema/src/info/mod.rs @@ -1,4 +1,12 @@ +use regex::Regex; + #[inline] pub fn is_private_field(name: &str) -> bool { name.starts_with('_') } + +#[inline] +pub fn is_valid_kcl_name(name: &str) -> bool { + let re = Regex::new(r#"^[a-zA-Z_][a-zA-Z0-9_]*$"#).unwrap(); + re.is_match(name) +} diff --git a/kclvm/tools/src/LSP/Cargo.toml b/kclvm/tools/src/LSP/Cargo.toml index b4f4cde0a..f78291226 100644 --- a/kclvm/tools/src/LSP/Cargo.toml +++ b/kclvm/tools/src/LSP/Cargo.toml @@ -19,7 +19,6 @@ log = "0.4.14" im-rc = "15.0.0" rustc_lexer = "0.1.0" clap = "4.3.0" -regex = "1.4" kclvm-tools = { path = "../../../tools" } kclvm-error = { path = "../../../error" } diff --git a/kclvm/tools/src/LSP/src/request.rs b/kclvm/tools/src/LSP/src/request.rs index d0dcc1826..bcdce48e2 100644 --- a/kclvm/tools/src/LSP/src/request.rs +++ b/kclvm/tools/src/LSP/src/request.rs @@ -1,5 +1,6 @@ use anyhow::{anyhow, Ok}; use crossbeam_channel::Sender; +use kclvm_sema::info::is_valid_kcl_name; use lsp_types::{Location, TextEdit}; use ra_ap_vfs::VfsPath; use std::collections::HashMap; @@ -16,7 +17,6 @@ use crate::{ goto_def::goto_definition, hover, quick_fix, state::{log_message, LanguageServerSnapshot, LanguageServerState, Task}, - util, }; impl LanguageServerState { @@ -226,7 +226,7 @@ pub(crate) fn handle_rename( ) -> anyhow::Result> { // 1. check the new name validity let new_name = params.new_name; - if !util::is_valid_kcl_name(new_name.clone()) { + if !is_valid_kcl_name(new_name.as_str()) { return Err(anyhow!("Can not rename to: {new_name}, invalid name")); } diff --git a/kclvm/tools/src/LSP/src/util.rs b/kclvm/tools/src/LSP/src/util.rs index cc2b1c0ef..e93f184e5 100644 --- a/kclvm/tools/src/LSP/src/util.rs +++ b/kclvm/tools/src/LSP/src/util.rs @@ -19,7 +19,6 @@ use kclvm_utils::pkgpath::rm_external_pkg_name; use lsp_types::{Location, Position, Range, Url}; use parking_lot::{RwLock, RwLockReadGuard}; use ra_ap_vfs::{FileId, Vfs}; -use regex::Regex; use serde::{de::DeserializeOwned, Serialize}; use std::cell::RefCell; use std::collections::HashMap; @@ -895,11 +894,6 @@ fn line_to_words(text: String) -> HashMap> { result } -pub fn is_valid_kcl_name(name: String) -> bool { - let re = Regex::new(r#"^[a-zA-Z_][a-zA-Z0-9_]*$"#).unwrap(); - re.is_match(&name) -} - #[cfg(test)] mod tests { use super::{build_word_index, line_to_words, word_index_add, word_index_subtract, Word};