Skip to content

Commit

Permalink
fix select symbol bug, return range instead of symbol ref
Browse files Browse the repository at this point in the history 
Signed-off-by: xiarui.xr <xiarui1994@gmail.com>
  • Loading branch information
@amyXia1994
amyXia1994 committed Nov 17, 2023
1 parent 638cedf commit e47f586
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 119 deletions.
254 changes: 138 additions & 116 deletions kclvm/tools/src/LSP/src/rename.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ use crate::{
goto_def::find_def_with_gs,
util::{build_word_index_for_file_paths, parse_param_and_compile, Param},
};
use chumsky::chain::Chain;
use kclvm_ast::{ast, token::LitKind::Err};
use kclvm_ast::ast;
use kclvm_error::diagnostic;
use kclvm_query::selector::parse_symbol_selector_spec;
use kclvm_sema::core::symbol::{Symbol, SymbolKind, SymbolRef};
use kclvm_sema::resolver::doc::Attribute;
use lsp_types::{Location, TextEdit, Url};
use std::collections::HashMap;
use std::path::PathBuf;
use std::{collections::HashMap, ops::Deref};

/// the rename_symbol API
/// find all the occurrences of the target symbol and return the text edit actions to rename them
Expand All @@ -27,83 +25,66 @@ pub fn rename_symbol(
// 1. from symbol path to the symbol
match parse_symbol_selector_spec(pkg_root, symbol_path) {
Ok(symbol_spec) => {
if let Some((name, def)) = select_symbol(&symbol_spec) {
if def.is_none() {
return Result::Err(format!(
"can not find definition for symbol {}",
symbol_path
));
}
match def.unwrap().get_kind() {
SymbolKind::Unresolved => {
return Result::Err(format!(
"can not resolve target symbol {}",
symbol_path
));
}
_ => {
// 3. build word index on file_paths, find refs within file_paths scope
if let Ok(word_index) = build_word_index_for_file_paths(file_paths) {
if let Some(locations) = word_index.get(&name) {
// 4. filter out the matched refs
// 4.1 collect matched words(names) and remove Duplicates of the file paths
let file_map = locations.iter().fold(
HashMap::<Url, Vec<&Location>>::new(),
|mut acc, loc| {
acc.entry(loc.uri.clone()).or_insert(Vec::new()).push(loc);
acc
// 2. get the symbol name and definition range from symbol path
if let Some((name, range)) = select_symbol(&symbol_spec) {
// 3. build word index on file_paths, find refs within file_paths scope
if let Ok(word_index) = build_word_index_for_file_paths(file_paths) {
if let Some(locations) = word_index.get(&name) {
// 4. filter out the matched refs
// 4.1 collect matched words(names) and remove Duplicates of the file paths
let file_map = locations.iter().fold(
HashMap::<Url, Vec<&Location>>::new(),
|mut acc, loc| {
acc.entry(loc.uri.clone()).or_insert(Vec::new()).push(loc);
acc
},
);
let refs = file_map
.iter()
.flat_map(|(_, locs)| locs.iter())
.filter(|&&loc| {
// 4.2 filter out the words and remain those whose definition is the target def
let p = loc.uri.path();
if let Ok((_, _, _, gs)) = parse_param_and_compile(
Param {
file: p.to_string(),
},
);
let refs = file_map
.iter()
.flat_map(|(_, locs)| locs.iter())
.filter(|&&loc| {
// 4.2 filter out the words and remain those whose definition is the target def
let p = loc.uri.path();
if let Ok((_, _, _, gs)) = parse_param_and_compile(
Param {
file: p.to_string(),
},
None,
) {
let kcl_pos = kcl_pos(p, loc.range.start);
if let Some(symbol_ref) =
find_def_with_gs(&kcl_pos, &gs, true)
{
if let Some(real_def) =
gs.get_symbols().get_symbol(symbol_ref)
{
return real_def.get_definition() == def;
}
}
None,
) {
let kcl_pos = kcl_pos(p, loc.range.start);
if let Some(symbol_ref) = find_def_with_gs(&kcl_pos, &gs, true)
{
if let Some(real_def) =
gs.get_symbols().get_symbol(symbol_ref)
{
return real_def.get_range() == range;
}
false
})
.cloned()
.collect::<Vec<&Location>>();
// 5. refs to rename actions
let changes =
refs.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
});
return Ok(changes);
}
}
}
}
false
})
.cloned()
.collect::<Vec<&Location>>();
// 5. refs to rename actions
let changes = refs.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
});
return Ok(changes);
}
}
}
Result::Err("rename failed".to_string())
}
Result::Err(err) => {
return Result::Err(format!(
"can not parse symbol path {}, {}",
"get symbol from symbol path failed, {}, {}",
symbol_path, err
));
}
Expand All @@ -112,7 +93,8 @@ pub fn rename_symbol(

/// Select a symbol by the symbol path
/// The symbol path should be in the format of: `pkg.sub_pkg:name.sub_name`
pub fn select_symbol(selector: &ast::SymbolSelectorSpec) -> Option<(String, Option<SymbolRef>)> {
/// returns the symbol name and definition range
pub fn select_symbol(selector: &ast::SymbolSelectorSpec) -> Option<(String, diagnostic::Range)> {
let mut pkg = PathBuf::from(&selector.pkg_root);
let pkg_names = selector.pkgpath.split(".");
for n in pkg_names {
Expand All @@ -129,46 +111,34 @@ pub fn select_symbol(selector: &ast::SymbolSelectorSpec) -> Option<(String, Opti
},
None,
) {
if let Some(symbol_ref) = gs.get_symbols().get_symbol_by_fully_qualified_name(
&format!("{}.{}", prog.main, fields[0]),
) {
let outer_symbol = gs.get_symbols().get_symbol(symbol_ref).unwrap();
if fields.len() == 1 {
return Some((outer_symbol.get_name(), outer_symbol.get_definition()));
}
match symbol_ref.get_kind() {
SymbolKind::Schema => {
let schema = gs.get_symbols().get_schema_symbol(symbol_ref).unwrap();
if let Some(attr) =
schema.get_attribute(fields[1], gs.get_symbols(), None)
{
let sym = gs.get_symbols().get_attribue_symbol(attr).unwrap();
if fields.len() == 2 {
return Some((sym.get_name(), sym.get_definition()));
}
}
return None;
}
_ => {
// not supported by global state
return None;
if let Some(symbol_ref) = gs
.get_symbols()
.get_symbol_by_fully_qualified_name(&prog.main)
{
let mut owner_ref = symbol_ref;
let mut target = None;
for field in fields {
let owner = gs.get_symbols().get_symbol(owner_ref).unwrap();
target = owner.get_attribute(field, gs.get_symbols(), None);
if let Some(target) = target {
owner_ref = target;
}
}

let target_symbol = gs.get_symbols().get_symbol(target?)?;
return Some((target_symbol.get_name(), target_symbol.get_range()));
}
}
None
}
None => None,
}
}

#[cfg(test)]
mod tests {
use lsp_types::{Location, Position, Range, Url};

use kclvm_ast::ast::{self, Pos};
use std::collections::HashMap;
use std::fs::rename;
use kclvm_ast::ast;
use kclvm_error::diagnostic;
use lsp_types::{Position, Url};
use std::path::PathBuf;

use super::{rename_symbol, select_symbol};
Expand All @@ -179,52 +149,105 @@ mod tests {
root.push("src/test_data/rename_test/");
let pkg_root = root.to_str().unwrap().to_string();

if let Some((name, Some(def))) = select_symbol(&ast::SymbolSelectorSpec {
let mut main_path = root.clone();
main_path = main_path.join("base").join("person.k");

if let Some((name, range)) = select_symbol(&ast::SymbolSelectorSpec {
pkg_root: pkg_root.clone(),
pkgpath: "base".to_string(),
field_path: "Person.name".to_string(),
}) {
assert_eq!(name, "name");
assert_eq!(
def.get_kind(),
kclvm_sema::core::symbol::SymbolKind::Attribute
range,
(
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 2,
column: Some(4),
},
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 2,
column: Some(8),
},
)
);
} else {
assert!(false, "select symbol failed")
}

if let Some((name, Some(def))) = select_symbol(&ast::SymbolSelectorSpec {
if let Some((name, range)) = select_symbol(&ast::SymbolSelectorSpec {
pkg_root: pkg_root.clone(),
pkgpath: "base".to_string(),
field_path: "Name.first".to_string(),
}) {
assert_eq!(name, "first");
assert_eq!(
def.get_kind(),
kclvm_sema::core::symbol::SymbolKind::Attribute
range,
(
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 6,
column: Some(4),
},
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 6,
column: Some(9),
},
)
);
} else {
assert!(false, "select symbol failed")
}

if let Some((name, Some(def))) = select_symbol(&ast::SymbolSelectorSpec {
if let Some((name, range)) = select_symbol(&ast::SymbolSelectorSpec {
pkg_root: pkg_root.clone(),
pkgpath: "base".to_string(),
field_path: "Person".to_string(),
}) {
assert_eq!(name, "Person");
assert_eq!(def.get_kind(), kclvm_sema::core::symbol::SymbolKind::Schema);
assert_eq!(
range,
(
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 1,
column: Some(7),
},
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 1,
column: Some(13),
},
)
);
} else {
assert!(false, "select symbol failed")
}

if let Some((name, Some(def))) = select_symbol(&ast::SymbolSelectorSpec {
if let Some((name, range)) = select_symbol(&ast::SymbolSelectorSpec {
pkg_root: pkg_root.clone(),
pkgpath: "base".to_string(),
field_path: "a".to_string(),
}) {
assert_eq!(name, "a");
assert_eq!(def.get_kind(), kclvm_sema::core::symbol::SymbolKind::Value);
assert_eq!(
range,
(
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 8,
column: Some(0),
},
diagnostic::Position {
filename: main_path.as_path().to_str().unwrap().to_string(),
line: 8,
column: Some(1),
},
)
);
} else {
assert!(false, "select symbol failed")
}
Expand Down Expand Up @@ -258,7 +281,7 @@ mod tests {

if let Ok(changes) = rename_symbol(
root.to_str().unwrap(),
vec![
&vec![
base_path.to_str().unwrap().to_string(),
main_path.to_str().unwrap().to_string(),
],
Expand All @@ -279,14 +302,13 @@ mod tests {

if let Ok(changes) = rename_symbol(
root.to_str().unwrap(),
vec![
&vec![
base_path.to_str().unwrap().to_string(),
main_path.to_str().unwrap().to_string(),
],
"base:Person.name",
"new_name".to_string(),
) {
println!("{:?}", changes);
assert_eq!(changes.len(), 2);
assert!(changes.contains_key(&base_url));
assert!(changes.contains_key(&main_url));
Expand Down
6 changes: 3 additions & 3 deletions kclvm/tools/src/LSP/src/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -803,10 +803,10 @@ pub(crate) fn get_pkg_scope(
}

pub(crate) fn build_word_index_for_file_paths(
paths: Vec<String>,
paths: &[String],
) -> anyhow::Result<HashMap<String, Vec<Location>>> {
let mut index: HashMap<String, Vec<Location>> = HashMap::new();
for p in &paths {
for p in paths {
// str path to url
if let Ok(url) = Url::from_file_path(p) {
// read file content and save the word to word index
Expand All @@ -822,7 +822,7 @@ pub(crate) fn build_word_index_for_file_paths(
/// scan and build a word -> Locations index map
pub(crate) fn build_word_index(path: String) -> anyhow::Result<HashMap<String, Vec<Location>>> {
if let Ok(files) = get_kcl_files(path.clone(), true) {
return build_word_index_for_file_paths(files);
return build_word_index_for_file_paths(&files);
}
Ok(HashMap::new())
}
Expand Down

0 comments on commit e47f586

Please sign in to comment.