Skip to content

Commit

Permalink
feat: rename symbol (#773)
Browse files Browse the repository at this point in the history 
* feat: rename symbol

Signed-off-by: xiarui.xr <[email protected]>

* rename: support new name validity check

Signed-off-by: xiarui.xr <[email protected]>

* minor fixes

Signed-off-by: xiarui.xr <[email protected]>

* minor updates

Signed-off-by: xiarui.xr <[email protected]>

---------

Signed-off-by: xiarui.xr <[email protected]>
  • Loading branch information
@amyXia1994
amyXia1994 authored Oct 17, 2023
1 parent db59124 commit 4486c08
Show file tree
Hide file tree
Showing 9 changed files with 273 additions and 80 deletions.
8 changes: 8 additions & 0 deletions kclvm/sema/src/info/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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)
}
1 change: 1 addition & 0 deletions kclvm/tools/src/LSP/src/capabilities.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
81 changes: 57 additions & 24 deletions kclvm/tools/src/LSP/src/find_refs.rs
View file
Original file line number Diff line number Diff line change
@@ -1,23 +1,67 @@
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;
use std::collections::HashMap;
use std::sync::Arc;

pub(crate) fn find_refs<F: Fn(String) -> Result<(), anyhow::Error>>(
program: &Program,
kcl_pos: &KCLPos,
prog_scope: &ProgramScope,
word_index_map: HashMap<Url, HashMap<String, Vec<Location>>>,
vfs: Option<Arc<RwLock<Vfs>>>,
logger: F,
) -> Result<Vec<Location>, 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();
// find all the refs of the def
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<F: Fn(String) -> Result<(), anyhow::Error>>(
vfs: Option<Arc<RwLock<Vfs>>>,
word_index_map: HashMap<Url, HashMap<String, Vec<Location>>>,
def_loc: Location,
name: String,
cursor_path: String,
logger: F,
) -> anyhow::Result<Option<Vec<Location>>> {
) -> Vec<Location> {
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<Location> = locs
Expand Down Expand Up @@ -47,7 +91,8 @@ pub(crate) fn find_refs<F: Fn(String) -> 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;
}
}
Expand All @@ -56,12 +101,12 @@ pub(crate) fn find_refs<F: Fn(String) -> 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;
Expand All @@ -72,17 +117,8 @@ mod tests {
anyhow::Ok(())
}

fn check_locations_match(expect: Vec<Location>, actual: anyhow::Result<Option<Vec<Location>>>) {
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<Location>, actual: Vec<Location>) {
assert_eq!(expect, actual)
}

fn setup_word_index_map(root: &str) -> HashMap<Url, HashMap<String, Vec<Location>>> {
Expand Down Expand Up @@ -139,12 +175,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,
),
);
Expand Down Expand Up @@ -193,12 +228,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,
),
);
Expand Down Expand Up @@ -240,12 +274,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,
),
);
Expand Down
23 changes: 6 additions & 17 deletions kclvm/tools/src/LSP/src/goto_def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -408,24 +407,14 @@ 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),
},
}))
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(Location {
uri: Url::from_file_path(start.filename.clone()).unwrap(),
range: Range {
start: lsp_pos(start),
end: lsp_pos(end),
},
})
let loc = lsp_location(start.filename.clone(), &start, &end)?;
res.push(loc)
}
Some(lsp_types::GotoDefinitionResponse::Array(res))
}
Expand Down
115 changes: 76 additions & 39 deletions kclvm/tools/src/LSP/src/request.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use anyhow::Ok;
use anyhow::{anyhow, Ok};
use crossbeam_channel::Sender;
use kclvm_ast::ast::Stmt;
use kclvm_sema::info::is_valid_kcl_name;
use lsp_types::{Location, TextEdit};
use ra_ap_vfs::VfsPath;
use std::collections::HashMap;
use std::time::Instant;

use crate::{
Expand All @@ -13,7 +14,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},
};
Expand Down Expand Up @@ -52,6 +53,7 @@ impl LanguageServerState {
.on::<lsp_types::request::CodeActionRequest>(handle_code_action)?
.on::<lsp_types::request::Formatting>(handle_formatting)?
.on::<lsp_types::request::RangeFormatting>(handle_range_formatting)?
.on::<lsp_types::request::Rename>(handle_rename)?
.finish();

Ok(())
Expand Down Expand Up @@ -141,49 +143,25 @@ pub(crate) fn handle_reference(
params: lsp_types::ReferenceParams,
sender: Sender<Task>,
) -> anyhow::Result<Option<Vec<Location>>> {
// 1. find definition of current token
let file = file_path_from_url(&params.text_document_position.text_document.uri)?;
let path = from_lsp::abs_path(&params.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.
Expand Down Expand Up @@ -239,3 +217,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<Task>,
) -> anyhow::Result<Option<lsp_types::WorkspaceEdit>> {
// 1. check the new name validity
let new_name = params.new_name;
if !is_valid_kcl_name(new_name.as_str()) {
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(&params.text_document_position.text_document.uri)?;
let path = from_lsp::abs_path(&params.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 {
Result::Ok(locations) => {
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
});
workspace_edit.changes = Some(changes);
anyhow::Ok(Some(workspace_edit))
}
}
Err(msg) => {
let err_msg = format!("Can not rename symbol: {msg}");
log(err_msg.clone())?;
return Err(anyhow!(err_msg));
}
}
}
6 changes: 6 additions & 0 deletions kclvm/tools/src/LSP/src/test_data/rename_test/main.k
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import .pkg.vars

Bob = vars.Person {
name: "Bob"
age: 30
}
Loading

0 comments on commit 4486c08

Please sign in to comment.