Skip to content

Commit

Permalink
fix: add parse errors in reture value of OverrideFile api
Browse files Browse the repository at this point in the history
Signed-off-by: zongz <[email protected]>
  • Loading branch information
zong-zhe committed May 21, 2024
1 parent 69095ae commit c686b2e
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 11 deletions.
9 changes: 8 additions & 1 deletion kclvm/api/src/service/service_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,14 @@ impl KclvmServiceImpl {
pub fn override_file(&self, args: &OverrideFileArgs) -> Result<OverrideFileResult, String> {
override_file(&args.file, &args.specs, &args.import_paths)
.map_err(|err| err.to_string())
.map(|result| OverrideFileResult { result })
.map(|result| OverrideFileResult {
result: result.result,
parse_errors: result
.parse_errors
.into_iter()
.map(|e| e.into_error())
.collect(),
})
}

/// Service for getting the schema type list.
Expand Down
27 changes: 20 additions & 7 deletions kclvm/query/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod util;
use anyhow::{anyhow, Result};
use kclvm_ast::ast;
use kclvm_ast_pretty::print_ast_module;
use kclvm_error::diagnostic::Errors;
use kclvm_parser::parse_file;

use kclvm_sema::pre_process::fix_config_expr_nest_attr;
Expand Down Expand Up @@ -78,31 +79,43 @@ use self::r#override::parse_override_spec;
/// age = 18
/// }
/// ```
pub fn override_file(file: &str, specs: &[String], import_paths: &[String]) -> Result<bool> {
pub fn override_file(
file: &str,
specs: &[String],
import_paths: &[String],
) -> Result<OverrideFileResult> {
// Parse override spec strings.
let overrides = specs
.iter()
.map(|s| parse_override_spec(s))
.collect::<Result<Vec<ast::OverrideSpec>, _>>()?;
// Parse file to AST module.
let mut module = match parse_file(file, None) {
Ok(module) => module.module,
let mut parse_result = match parse_file(file, None) {
Ok(module) => module,
Err(msg) => return Err(anyhow!("{}", msg)),
};
let mut result = false;
// Override AST module.
for o in &overrides {
if apply_override_on_module(&mut module, o, import_paths)? {
if apply_override_on_module(&mut parse_result.module, o, import_paths)? {
result = true;
}
}

// Transform config expr to simplify the config path query and override.
fix_config_expr_nest_attr(&mut module);
fix_config_expr_nest_attr(&mut parse_result.module);
// Print AST module.
if result {
let code_str = print_ast_module(&module);
let code_str = print_ast_module(&parse_result.module);
std::fs::write(file, code_str)?
}
Ok(result)
Ok(OverrideFileResult {
result,
parse_errors: parse_result.errors,
})
}

pub struct OverrideFileResult {
pub result: bool,
pub parse_errors: Errors,
}
1 change: 1 addition & 0 deletions kclvm/query/src/test_data/invalid.bk.k
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"a": "b"
1 change: 1 addition & 0 deletions kclvm/query/src/test_data/test_override_file/invalid.k
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"a": "b"
45 changes: 42 additions & 3 deletions kclvm/query/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ fn test_override_file_simple() {

let import_paths = vec![];
assert_eq!(
override_file(simple_path.clone().to_str().unwrap(), &specs, &import_paths).unwrap(),
override_file(simple_path.clone().to_str().unwrap(), &specs, &import_paths)
.unwrap()
.result,
true
);

Expand Down Expand Up @@ -79,7 +81,9 @@ fn test_override_file_import_paths() {
let abs_path = cargo_file_path.to_str().unwrap();

assert_eq!(
override_file(abs_path, &specs, &import_paths).unwrap(),
override_file(abs_path, &specs, &import_paths)
.unwrap()
.result,
true
)
}
Expand Down Expand Up @@ -555,7 +559,9 @@ fn test_overridefile_insert() {
// test insert multiple times
for _ in 1..=5 {
assert_eq!(
override_file(&simple_path.display().to_string(), &specs, &import_paths).unwrap(),
override_file(&simple_path.display().to_string(), &specs, &import_paths)
.unwrap()
.result,
true
);

Expand Down Expand Up @@ -596,6 +602,39 @@ fn test_list_variable_with_invalid_kcl() {
assert_eq!(result.parse_errors[0].messages[0].range.0.column, Some(3));
}

#[test]
fn test_overridefile_with_invalid_kcl() {
let simple_path = get_test_dir("test_override_file/invalid.k".to_string());
let simple_bk_path = get_test_dir("invalid.bk.k".to_string());
fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();

let result = override_file(
&simple_path.display().to_string(),
&vec!["a=b".to_string()],
&vec![],
)
.unwrap();

fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
assert_eq!(result.result, true);
assert_eq!(result.parse_errors.len(), 1);
assert_eq!(result.parse_errors[0].level, Level::Error);
assert_eq!(
result.parse_errors[0].code,
Some(DiagnosticId::Error(ErrorKind::InvalidSyntax))
);
assert_eq!(
result.parse_errors[0].messages[0].message,
"unexpected token ':'"
);
assert_eq!(
result.parse_errors[0].messages[0].range.0.filename,
simple_path.display().to_string()
);
assert_eq!(result.parse_errors[0].messages[0].range.0.line, 1);
assert_eq!(result.parse_errors[0].messages[0].range.0.column, Some(3));
}

#[test]
fn test_list_variables_with_file_noexist() {
let file = PathBuf::from("./src/test_data/test_list_variables/noexist.k")
Expand Down
1 change: 1 addition & 0 deletions kclvm/spec/gpyrpc/gpyrpc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ message OverrideFile_Args {

message OverrideFile_Result {
bool result = 1;
repeated Error parse_errors = 2;
}

message ListVariables_Args {
Expand Down

0 comments on commit c686b2e

Please sign in to comment.