diff --git a/kclvm/config/src/settings.rs b/kclvm/config/src/settings.rs index 2daae6237..04f29a6ce 100644 --- a/kclvm/config/src/settings.rs +++ b/kclvm/config/src/settings.rs @@ -1,5 +1,5 @@ // Copyright 2021 The KCL Authors. All rights reserved. -use anyhow::Result; +use anyhow::{Context, Result}; use serde::{ de::{DeserializeSeed, Error, MapAccess, SeqAccess, Unexpected, Visitor}, Deserialize, Serialize, @@ -344,8 +344,10 @@ pub struct TestSettingsFile { /// Load kcl settings file. pub fn load_file(filename: &str) -> Result { - let f = std::fs::File::open(filename)?; - let data: SettingsFile = serde_yaml::from_reader(f)?; + let f = std::fs::File::open(filename) + .with_context(|| format!("Failed to load '{}', no such file or directory", filename))?; + let data: SettingsFile = serde_yaml::from_reader(f) + .with_context(|| format!("Failed to load '{}', invalid setting file format", filename))?; Ok(data) } diff --git a/kclvm/runtime/src/value/val_plan.rs b/kclvm/runtime/src/value/val_plan.rs index 7ddfe2121..6aafbcae0 100644 --- a/kclvm/runtime/src/value/val_plan.rs +++ b/kclvm/runtime/src/value/val_plan.rs @@ -172,7 +172,7 @@ fn handle_schema(value: &ValueRef) -> (Vec, bool) { impl ValueRef { fn is_planned_empty(&self) -> bool { - self.is_dict() && !self.is_truthy() + (self.is_dict() && !self.is_truthy()) || self.is_undefined() } pub fn plan_to_json_string(&self) -> String { diff --git a/kclvm/tools/src/vet/expr_builder.rs b/kclvm/tools/src/vet/expr_builder.rs index 7fdfa1540..36b5bb654 100644 --- a/kclvm/tools/src/vet/expr_builder.rs +++ b/kclvm/tools/src/vet/expr_builder.rs @@ -10,6 +10,8 @@ use kclvm_ast::{ use crate::util::loader::{DataLoader, Loader, LoaderKind}; use anyhow::{bail, Context, Result}; +const FAIL_LOAD_VALIDATED_ERR_MSG: &str = "Failed to load the validated file"; + trait ExprGenerator { fn generate(&self, value: &T, schema_name: &Option) -> Result>; } @@ -71,8 +73,8 @@ impl ExprGenerator for ExprBuilder { serde_yaml::Value::Bool(j_bool) => { let name_const = match NameConstant::try_from(*j_bool) { Ok(nc) => nc, - Err(_) => { - bail!("Failed to Load Validated File") + Err(err) => { + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}, {err}") } }; @@ -85,7 +87,7 @@ impl ExprGenerator for ExprBuilder { let number_lit = match j_num.as_f64() { Some(num_f64) => num_f64, None => { - bail!("Failed to Load Validated File") + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}") } }; @@ -97,7 +99,7 @@ impl ExprGenerator for ExprBuilder { let number_lit = match j_num.as_i64() { Some(j_num) => j_num, None => { - bail!("Failed to Load Validated File") + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}") } }; @@ -106,14 +108,14 @@ impl ExprGenerator for ExprBuilder { value: NumberLitValue::Int(number_lit) }))) } else { - bail!("Failed to Load Validated File, Unsupported Unsigned 64"); + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}, Unsupported Unsigned 64"); } } serde_yaml::Value::String(j_string) => { let str_lit = match StringLit::try_from(j_string.to_string()) { Ok(s) => s, Err(_) => { - bail!("Failed to Load Validated File") + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}") } }; Ok(node_ref!(Expr::StringLit(str_lit))) @@ -123,7 +125,7 @@ impl ExprGenerator for ExprBuilder { for j_arr_item in j_arr { j_arr_ast_nodes.push( self.generate(j_arr_item, schema_name) - .with_context(|| "Failed to Load Validated File".to_string())?, + .with_context(|| FAIL_LOAD_VALIDATED_ERR_MSG)?, ); } Ok(node_ref!(Expr::List(ListExpr { @@ -138,10 +140,10 @@ impl ExprGenerator for ExprBuilder { // The configuration builder already in the schema no longer needs a schema name let k = self .generate(k, &None) - .with_context(|| "Failed to Load Validated File".to_string())?; + .with_context(|| FAIL_LOAD_VALIDATED_ERR_MSG)?; let v = self .generate(v, &None) - .with_context(|| "Failed to Load Validated File".to_string())?; + .with_context(|| FAIL_LOAD_VALIDATED_ERR_MSG)?; let config_entry = node_ref!(ConfigEntry { key: Some(k), @@ -173,8 +175,11 @@ impl ExprGenerator for ExprBuilder { None => Ok(config_expr), } } - serde_yaml::Value::Tagged(_) => { - bail!("Failed to Load Validated File, Unsupported Yaml Tagged.") + serde_yaml::Value::Tagged(v) => { + bail!( + "{FAIL_LOAD_VALIDATED_ERR_MSG}, Unsupported Yaml tag {}", + v.tag + ) } } } @@ -193,8 +198,8 @@ impl ExprGenerator for ExprBuilder { serde_json::Value::Bool(j_bool) => { let name_const = match NameConstant::try_from(*j_bool) { Ok(nc) => nc, - Err(_) => { - bail!("Failed to Load Validated File") + Err(err) => { + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}, {err}") } }; @@ -207,7 +212,7 @@ impl ExprGenerator for ExprBuilder { let number_lit = match j_num.as_f64() { Some(num_f64) => num_f64, None => { - bail!("Failed to Load Validated File") + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}") } }; @@ -219,7 +224,7 @@ impl ExprGenerator for ExprBuilder { let number_lit = match j_num.as_i64() { Some(j_num) => j_num, None => { - bail!("Failed to Load Validated File") + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}") } }; @@ -228,14 +233,14 @@ impl ExprGenerator for ExprBuilder { value: NumberLitValue::Int(number_lit) }))) } else { - bail!("Failed to Load Validated File, Unsupported Unsigned 64"); + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}, Unsupported Unsigned 64"); } } serde_json::Value::String(j_string) => { let str_lit = match StringLit::try_from(j_string.to_string()) { Ok(s) => s, Err(_) => { - bail!("Failed to Load Validated File") + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}") } }; @@ -246,7 +251,7 @@ impl ExprGenerator for ExprBuilder { for j_arr_item in j_arr { j_arr_ast_nodes.push( self.generate(j_arr_item, schema_name) - .with_context(|| "Failed to Load Validated File".to_string())?, + .with_context(|| FAIL_LOAD_VALIDATED_ERR_MSG)?, ); } Ok(node_ref!(Expr::List(ListExpr { @@ -260,13 +265,13 @@ impl ExprGenerator for ExprBuilder { for (k, v) in j_map.iter() { let k = match StringLit::try_from(k.to_string()) { Ok(s) => s, - Err(_) => { - bail!("Failed to Load Validated File") + Err(err) => { + bail!("{FAIL_LOAD_VALIDATED_ERR_MSG}, {err}") } }; let v = self .generate(v, &None) - .with_context(|| "Failed to Load Validated File".to_string())?; + .with_context(|| FAIL_LOAD_VALIDATED_ERR_MSG)?; let config_entry = node_ref!(ConfigEntry { key: Some(node_ref!(Expr::StringLit(k))), diff --git a/kclvm/tools/src/vet/tests.rs b/kclvm/tools/src/vet/tests.rs index 741d62c2b..3d89865b1 100644 --- a/kclvm/tools/src/vet/tests.rs +++ b/kclvm/tools/src/vet/tests.rs @@ -303,7 +303,7 @@ mod test_expr_builder { panic!("unreachable") } Err(err) => { - assert_eq!(format!("{:?}", err), "Failed to Load JSON\n\nCaused by:\n 0: Failed to Load Validated File\n 1: Failed to Load Validated File, Unsupported Unsigned 64"); + assert_eq!(format!("{:?}", err), "Failed to Load JSON\n\nCaused by:\n 0: Failed to load the validated file\n 1: Failed to load the validated file, Unsupported Unsigned 64"); } }; } @@ -318,7 +318,7 @@ mod test_expr_builder { panic!("unreachable") } Err(err) => { - assert_eq!(format!("{:?}", err), "Failed to Load YAML\n\nCaused by:\n 0: Failed to Load Validated File\n 1: Failed to Load Validated File, Unsupported Unsigned 64"); + assert_eq!(format!("{:?}", err), "Failed to Load YAML\n\nCaused by:\n 0: Failed to load the validated file\n 1: Failed to load the validated file, Unsupported Unsigned 64"); } }; } @@ -333,7 +333,7 @@ mod test_expr_builder { panic!("unreachable") } Err(err) => { - assert_eq!(format!("{:?}", err), "Failed to Load YAML\n\nCaused by:\n Failed to Load Validated File, Unsupported Yaml Tagged."); + assert_eq!(format!("{:?}", err), "Failed to Load YAML\n\nCaused by:\n Failed to load the validated file, Unsupported Yaml tag !mytag"); } }; } diff --git a/kclvm/tools/src/vet/validator.rs b/kclvm/tools/src/vet/validator.rs index 8626a9817..568b09a64 100644 --- a/kclvm/tools/src/vet/validator.rs +++ b/kclvm/tools/src/vet/validator.rs @@ -176,10 +176,7 @@ pub fn validate(val_opt: ValidateOption) -> Result { None => TMP_FILE.to_string(), }; - let mut module: Module = match kclvm_parser::parse_file(&k_path, val_opt.kcl_code) { - Ok(ast_m) => ast_m, - Err(err_msg) => return Err(err_msg), - }; + let mut module = kclvm_parser::parse_file(&k_path, val_opt.kcl_code)?; let schemas = filter_schema_stmt(&module); let schema_name = match val_opt.schema_name { @@ -187,27 +184,19 @@ pub fn validate(val_opt: ValidateOption) -> Result { None => schemas.get(0).map(|schema| schema.name.node.clone()), }; - let expr_builder = match ExprBuilder::new_with_file_path( - val_opt.validated_file_kind, - val_opt.validated_file_path, - ) { - Ok(builder) => builder, - Err(_) => return Err("Failed to load validated file.".to_string()), - }; + let expr_builder = + ExprBuilder::new_with_file_path(val_opt.validated_file_kind, val_opt.validated_file_path) + .map_err(|_| "Failed to load validated file.".to_string())?; - let validated_expr = match expr_builder.build(schema_name) { - Ok(expr) => expr, - Err(_) => return Err("Failed to load validated file.".to_string()), - }; + let validated_expr = expr_builder + .build(schema_name) + .map_err(|_| "Failed to load validated file.".to_string())?; let assign_stmt = build_assign(&val_opt.attribute_name, validated_expr); module.body.insert(0, assign_stmt); - match execute_module(module) { - Ok(_) => Ok(true), - Err(err) => Err(err), - } + execute_module(module).map(|_| true) } fn build_assign(attr_name: &str, node: NodeRef) -> NodeRef {