From 7cc84a31f08d6bd48cdbb180cd9535b0d55e043e Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 3 Jun 2024 14:11:05 +0800 Subject: [PATCH 1/3] feat: api list_variables supports get variables from multi-files Signed-off-by: zongz --- kclvm/api/src/service/service_impl.rs | 6 +- kclvm/api/src/testdata/list-variables.json | 2 +- kclvm/api/src/testdata/rename/main.k | 0 kclvm/query/src/selector.rs | 34 +++++----- .../test_list_merged_variables/kcl.mod | 5 ++ .../override/base.k | 11 +++ .../override/main.k | 6 ++ .../test_list_merged_variables/test.k | 7 ++ .../test_list_merged_variables/union/base.k | 11 +++ .../test_list_merged_variables/union/main.k | 6 ++ kclvm/query/src/tests.rs | 67 +++++++++++++++++-- kclvm/spec/gpyrpc/gpyrpc.proto | 2 +- 12 files changed, 129 insertions(+), 28 deletions(-) create mode 100644 kclvm/api/src/testdata/rename/main.k create mode 100644 kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/kcl.mod create mode 100644 kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/base.k create mode 100644 kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/main.k create mode 100644 kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/test.k create mode 100644 kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/base.k create mode 100644 kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/main.k diff --git a/kclvm/api/src/service/service_impl.rs b/kclvm/api/src/service/service_impl.rs index cce3184e5..18a01a42d 100644 --- a/kclvm/api/src/service/service_impl.rs +++ b/kclvm/api/src/service/service_impl.rs @@ -351,7 +351,7 @@ impl KclvmServiceImpl { /// /// let serv = KclvmServiceImpl::default(); /// let args = &ListVariablesArgs { - /// file: Path::new(".").join("src").join("testdata").join("variables").join("main.k").canonicalize().unwrap().display().to_string(), + /// files: vec![Path::new(".").join("src").join("testdata").join("variables").join("main.k").canonicalize().unwrap().display().to_string()], /// specs: vec!["a".to_string()] /// }; /// let result = serv.list_variables(args).unwrap(); @@ -359,10 +359,10 @@ impl KclvmServiceImpl { /// assert_eq!(result.variables.get("a").unwrap().value, "1"); /// ``` pub fn list_variables(&self, args: &ListVariablesArgs) -> anyhow::Result { - let k_file = args.file.to_string(); + let k_files = args.files.iter().map(|s| s.as_str()).collect(); let specs = args.specs.clone(); - let select_res = list_variables(k_file, specs)?; + let select_res = list_variables(k_files, specs)?; let variables: HashMap = select_res .variables diff --git a/kclvm/api/src/testdata/list-variables.json b/kclvm/api/src/testdata/list-variables.json index b03e74c3e..db27f88b9 100644 --- a/kclvm/api/src/testdata/list-variables.json +++ b/kclvm/api/src/testdata/list-variables.json @@ -1,4 +1,4 @@ { - "file": "./src/testdata/variables/main.k", + "files": ["./src/testdata/variables/main.k"], "specs": ["a", "b", "c"] } diff --git a/kclvm/api/src/testdata/rename/main.k b/kclvm/api/src/testdata/rename/main.k new file mode 100644 index 000000000..e69de29bb diff --git a/kclvm/query/src/selector.rs b/kclvm/query/src/selector.rs index 01627bcd4..f6998471a 100644 --- a/kclvm/query/src/selector.rs +++ b/kclvm/query/src/selector.rs @@ -2,21 +2,22 @@ use super::util::{invalid_symbol_selector_spec_error, split_field_path}; use anyhow::Result; use kclvm_ast::ast; use kclvm_error::diagnostic::Errors; +use kclvm_parser::ParseSession; use serde::{Deserialize, Serialize}; -use std::collections::{HashMap, VecDeque}; +use std::{ + collections::{HashMap, VecDeque}, + sync::Arc, +}; use kclvm_ast::path::get_key_path; use kclvm_ast::walker::MutSelfWalker; use kclvm_ast_pretty::{print_ast_node, ASTNode}; -use kclvm_parser::parse_file; +use kclvm_parser::load_program; -use kclvm_sema::resolver::Options; - -use kclvm_ast::MAIN_PKG; use kclvm_sema::pre_process::pre_process_program; -use maplit::hashmap; +use kclvm_sema::resolver::Options; #[derive(Debug, Default)] /// UnsupportedSelectee is used to store the unsupported selectee, such as if, for, etc. pub struct UnsupportedSelectee { @@ -461,26 +462,25 @@ impl Variable { /// list_options provides users with the ability to parse kcl program and get all option /// calling information. -pub fn list_variables(file: String, specs: Vec) -> Result { +pub fn list_variables(files: Vec<&str>, specs: Vec) -> Result { let mut selector = Selector::new(specs)?; - let parse_result = parse_file(&file, None)?; + // let parse_result = parse_file(&file, None)?; + let mut load_result = load_program(Arc::new(ParseSession::default()), &files, None, None)?; let mut opts = Options::default(); opts.merge_program = true; - pre_process_program( - &mut ast::Program { - root: file, - pkgs: hashmap! { MAIN_PKG.to_string() => vec![parse_result.module.clone()] }, - }, - &opts, - ); + pre_process_program(&mut load_result.program, &opts); - selector.walk_module(&parse_result.module); + for (_, modules) in load_result.program.pkgs.iter() { + for module in modules.iter() { + selector.walk_module(&module); + } + } Ok(ListVariablesResult { variables: selector.select_result, unsupported: selector.unsupported, - parse_errors: parse_result.errors, + parse_errors: load_result.errors, }) } diff --git a/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/kcl.mod b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/kcl.mod new file mode 100644 index 000000000..f893ac172 --- /dev/null +++ b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/kcl.mod @@ -0,0 +1,5 @@ +[package] +name = "test_list_merged_variables" +edition = "v0.9.0" +version = "0.0.1" + diff --git a/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/base.k b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/base.k new file mode 100644 index 000000000..cc4c5914b --- /dev/null +++ b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/base.k @@ -0,0 +1,11 @@ + +import test + +_tests = test.Test { + pots: [ + test.Pot { + name: "http" + number: 90 + } + ] +} \ No newline at end of file diff --git a/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/main.k b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/main.k new file mode 100644 index 000000000..074e18a18 --- /dev/null +++ b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/override/main.k @@ -0,0 +1,6 @@ + +import test + +_tests = test.Test { + aType: "Internet" +} diff --git a/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/test.k b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/test.k new file mode 100644 index 000000000..a484f7f47 --- /dev/null +++ b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/test.k @@ -0,0 +1,7 @@ +schema Pot: + name: str + number: int + +schema Test: + aType?: str + pots?: [Pot] diff --git a/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/base.k b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/base.k new file mode 100644 index 000000000..6f908c51e --- /dev/null +++ b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/base.k @@ -0,0 +1,11 @@ + +import test + +tests = test.Test { + pots: [ + test.Pot { + name: "http" + number: 90 + } + ] +} \ No newline at end of file diff --git a/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/main.k b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/main.k new file mode 100644 index 000000000..2c7f7aa3b --- /dev/null +++ b/kclvm/query/src/test_data/test_list_variables/test_list_merged_variables/union/main.k @@ -0,0 +1,6 @@ + +import test + +tests : test.Test { + aType: "Internet" +} diff --git a/kclvm/query/src/tests.rs b/kclvm/query/src/tests.rs index 0aaf84c29..3002ad3ba 100644 --- a/kclvm/query/src/tests.rs +++ b/kclvm/query/src/tests.rs @@ -381,7 +381,7 @@ fn test_list_variables() { for (spec, expected, expected_name, op_sym) in test_cases { let specs = vec![spec.to_string()]; - let result = list_variables(file.clone(), specs).unwrap(); + let result = list_variables(vec![&file.clone()], specs).unwrap(); assert_eq!(result.variables.get(spec).unwrap().value, expected); assert_eq!(result.variables.get(spec).unwrap().type_name, expected_name); assert_eq!(result.variables.get(spec).unwrap().op_sym, op_sym); @@ -470,7 +470,7 @@ fn test_list_all_variables() { ]; for (spec, expected, expected_name, op_sym) in test_cases { - let result = list_variables(file.clone(), vec![]).unwrap(); + let result = list_variables(vec![&file.clone()], vec![]).unwrap(); assert_eq!(result.variables.get(spec).unwrap().value, expected); assert_eq!(result.variables.get(spec).unwrap().type_name, expected_name); assert_eq!(result.variables.get(spec).unwrap().op_sym, op_sym); @@ -521,7 +521,7 @@ fn test_list_unsupported_variables() { for (spec, expected_code) in test_cases { let specs = vec![spec.to_string()]; - let result = list_variables(file.clone(), specs).unwrap(); + let result = list_variables(vec![&file.clone()], specs).unwrap(); assert_eq!(result.variables.get(spec), None); assert_eq!(result.unsupported[0].code, expected_code); assert_eq!(result.parse_errors.len(), 0); @@ -545,7 +545,7 @@ fn test_list_unsupported_variables() { for (spec, expected_code) in test_cases { let specs = vec![spec.to_string()]; - let result = list_variables(file.clone(), specs).unwrap(); + let result = list_variables(vec![&file.clone()], specs).unwrap(); assert_eq!(result.variables.get(spec).unwrap().value, expected_code); } } @@ -626,7 +626,7 @@ fn test_list_variable_with_invalid_kcl() { .display() .to_string(); let specs = vec!["a".to_string()]; - let result = list_variables(file.clone(), specs).unwrap(); + let result = list_variables(vec![&file.clone()], specs).unwrap(); assert_eq!(result.variables.get("a"), None); assert_eq!(result.parse_errors.len(), 2); assert_eq!(result.parse_errors[0].level, Level::Error); @@ -682,7 +682,7 @@ fn test_list_variables_with_file_noexist() { .display() .to_string(); let specs = vec!["a".to_string()]; - let result = list_variables(file.clone(), specs); + let result = list_variables(vec![&file.clone()], specs); assert!(result.is_err()); let err = result.err().unwrap(); assert_eq!(err.to_string(), "Cannot find the kcl file, please check the file path ./src/test_data/test_list_variables/noexist.k"); @@ -702,3 +702,58 @@ fn test_override_file_with_invalid_spec() { let err = result.err().unwrap(); assert_eq!(err.to_string(), "Invalid spec format '....', expected =filed_value>, :filed_value>, +=filed_value> or -"); } + +#[test] +fn test_list_merged_variables() { + let file = PathBuf::from("./src/test_data/test_list_variables/test_list_merged_variables") + .canonicalize() + .unwrap(); + + let test_cases = vec![ + ( + "override/base.k", + "override/main.k", + vec!["_tests.aType".to_string(), "_tests.pots".to_string()], + r#""Internet""#.to_string(), + r#"[test.Pot { + name: "http" + number: 90 +}]"# + .to_string(), + ), + ( + "union/base.k", + "union/main.k", + vec!["tests.aType".to_string(), "tests.pots".to_string()], + r#""Internet""#.to_string(), + r#"[test.Pot { + name: "http" + number: 90 +}]"# + .to_string(), + ), + ]; + + for (base_file_name, main_file_name, specs, expected_value1, expected_value2) in test_cases { + let base_file = file.join(base_file_name).display().to_string(); + let main_file = file.join(main_file_name).display().to_string(); + + let result = list_variables(vec![&main_file, &base_file], specs.clone()).unwrap(); + assert_eq!( + result + .variables + .get(&specs.get(0).unwrap().to_string()) + .unwrap() + .value, + expected_value1 + ); + assert_eq!( + result + .variables + .get(&specs.get(1).unwrap().to_string()) + .unwrap() + .value, + expected_value2 + ); + } +} diff --git a/kclvm/spec/gpyrpc/gpyrpc.proto b/kclvm/spec/gpyrpc/gpyrpc.proto index 4225fa926..d33e944fc 100644 --- a/kclvm/spec/gpyrpc/gpyrpc.proto +++ b/kclvm/spec/gpyrpc/gpyrpc.proto @@ -285,7 +285,7 @@ message OverrideFile_Result { } message ListVariables_Args { - string file = 1; + repeated string files = 1; repeated string specs = 2; } From 1047480950669aec60a4683c3eec50eb2318eb36 Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 3 Jun 2024 17:18:46 +0800 Subject: [PATCH 2/3] fix: fix CR comments Signed-off-by: zongz --- kclvm/api/src/service/service_impl.rs | 2 +- kclvm/query/src/selector.rs | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/kclvm/api/src/service/service_impl.rs b/kclvm/api/src/service/service_impl.rs index 18a01a42d..6be18b2e6 100644 --- a/kclvm/api/src/service/service_impl.rs +++ b/kclvm/api/src/service/service_impl.rs @@ -359,7 +359,7 @@ impl KclvmServiceImpl { /// assert_eq!(result.variables.get("a").unwrap().value, "1"); /// ``` pub fn list_variables(&self, args: &ListVariablesArgs) -> anyhow::Result { - let k_files = args.files.iter().map(|s| s.as_str()).collect(); + let k_files = args.files.clone(); let specs = args.specs.clone(); let select_res = list_variables(k_files, specs)?; diff --git a/kclvm/query/src/selector.rs b/kclvm/query/src/selector.rs index f6998471a..0096d61cb 100644 --- a/kclvm/query/src/selector.rs +++ b/kclvm/query/src/selector.rs @@ -462,10 +462,14 @@ impl Variable { /// list_options provides users with the ability to parse kcl program and get all option /// calling information. -pub fn list_variables(files: Vec<&str>, specs: Vec) -> Result { +pub fn list_variables(files: Vec, specs: Vec) -> Result { let mut selector = Selector::new(specs)?; - // let parse_result = parse_file(&file, None)?; - let mut load_result = load_program(Arc::new(ParseSession::default()), &files, None, None)?; + let mut load_result = load_program( + Arc::new(ParseSession::default()), + &files.iter().map(AsRef::as_ref).collect::>(), + None, + None, + )?; let mut opts = Options::default(); opts.merge_program = true; From 0796f29bc330e30f3b6ed943ce6512b1ab03ffbd Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 3 Jun 2024 17:33:45 +0800 Subject: [PATCH 3/3] fix: fix test cases Signed-off-by: zongz --- kclvm/query/src/tests.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/kclvm/query/src/tests.rs b/kclvm/query/src/tests.rs index 3002ad3ba..28b905136 100644 --- a/kclvm/query/src/tests.rs +++ b/kclvm/query/src/tests.rs @@ -381,7 +381,7 @@ fn test_list_variables() { for (spec, expected, expected_name, op_sym) in test_cases { let specs = vec![spec.to_string()]; - let result = list_variables(vec![&file.clone()], specs).unwrap(); + let result = list_variables(vec![file.clone()], specs).unwrap(); assert_eq!(result.variables.get(spec).unwrap().value, expected); assert_eq!(result.variables.get(spec).unwrap().type_name, expected_name); assert_eq!(result.variables.get(spec).unwrap().op_sym, op_sym); @@ -470,7 +470,7 @@ fn test_list_all_variables() { ]; for (spec, expected, expected_name, op_sym) in test_cases { - let result = list_variables(vec![&file.clone()], vec![]).unwrap(); + let result = list_variables(vec![file.clone()], vec![]).unwrap(); assert_eq!(result.variables.get(spec).unwrap().value, expected); assert_eq!(result.variables.get(spec).unwrap().type_name, expected_name); assert_eq!(result.variables.get(spec).unwrap().op_sym, op_sym); @@ -521,7 +521,7 @@ fn test_list_unsupported_variables() { for (spec, expected_code) in test_cases { let specs = vec![spec.to_string()]; - let result = list_variables(vec![&file.clone()], specs).unwrap(); + let result = list_variables(vec![file.clone()], specs).unwrap(); assert_eq!(result.variables.get(spec), None); assert_eq!(result.unsupported[0].code, expected_code); assert_eq!(result.parse_errors.len(), 0); @@ -545,7 +545,7 @@ fn test_list_unsupported_variables() { for (spec, expected_code) in test_cases { let specs = vec![spec.to_string()]; - let result = list_variables(vec![&file.clone()], specs).unwrap(); + let result = list_variables(vec![file.clone()], specs).unwrap(); assert_eq!(result.variables.get(spec).unwrap().value, expected_code); } } @@ -626,7 +626,7 @@ fn test_list_variable_with_invalid_kcl() { .display() .to_string(); let specs = vec!["a".to_string()]; - let result = list_variables(vec![&file.clone()], specs).unwrap(); + let result = list_variables(vec![file.clone()], specs).unwrap(); assert_eq!(result.variables.get("a"), None); assert_eq!(result.parse_errors.len(), 2); assert_eq!(result.parse_errors[0].level, Level::Error); @@ -682,7 +682,7 @@ fn test_list_variables_with_file_noexist() { .display() .to_string(); let specs = vec!["a".to_string()]; - let result = list_variables(vec![&file.clone()], specs); + let result = list_variables(vec![file.clone()], specs); assert!(result.is_err()); let err = result.err().unwrap(); assert_eq!(err.to_string(), "Cannot find the kcl file, please check the file path ./src/test_data/test_list_variables/noexist.k"); @@ -738,7 +738,7 @@ fn test_list_merged_variables() { let base_file = file.join(base_file_name).display().to_string(); let main_file = file.join(main_file_name).display().to_string(); - let result = list_variables(vec![&main_file, &base_file], specs.clone()).unwrap(); + let result = list_variables(vec![main_file, base_file], specs.clone()).unwrap(); assert_eq!( result .variables