diff --git a/kclvm/evaluator/src/node.rs b/kclvm/evaluator/src/node.rs index 5ff871a6f..797d2f90c 100644 --- a/kclvm/evaluator/src/node.rs +++ b/kclvm/evaluator/src/node.rs @@ -9,7 +9,6 @@ use generational_arena::Index; use kclvm_ast::ast::{self, CallExpr, ConfigEntry, NodeRef}; use kclvm_ast::walker::TypedResultWalker; use kclvm_runtime::val_func::invoke_function; -use kclvm_runtime::walker::walk_value_mut; use kclvm_runtime::{ schema_assert, schema_runtime_type, ConfigEntryOperationKind, DecoratorValue, RuntimeErrorType, UnionOptions, ValueRef, PKG_PATH_PREFIX, @@ -138,16 +137,7 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> { let mut value = self.walk_expr(&assign_stmt.value)?; // Runtime type cast if exists the type annotation. if let Some(ty) = &assign_stmt.ty { - let is_in_schema = self.is_in_schema() || self.is_in_schema_expr(); value = type_pack_and_check(self, &value, vec![&ty.node.to_string()], false); - // Schema required attribute validating. - if !is_in_schema { - walk_value_mut(&value, &mut |value: &ValueRef| { - if value.is_schema() { - value.schema_check_attr_optional(&mut self.runtime_ctx.borrow_mut(), true); - } - }) - } } if assign_stmt.targets.len() == 1 { // Store the single target @@ -843,7 +833,6 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> { fn walk_schema_expr(&self, schema_expr: &'ctx ast::SchemaExpr) -> Self::Result { // Check the required attributes only when the values of all attributes // in the final schema are solved. - let is_in_schema = self.is_in_schema() || self.is_in_schema_expr(); self.push_schema_expr(); let config_value = self.walk_expr(&schema_expr.config)?; let schema_type = self.walk_identifier_with_ctx( @@ -888,7 +877,6 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> { &list_value, &dict_value, ); - {} self.pop_backtrace(); self.pop_pkgpath(); value @@ -916,9 +904,6 @@ impl<'ctx> TypedResultWalker<'ctx> for Evaluator<'ctx> { &UnionOptions::default(), ) }; - if !is_in_schema { - schema.schema_check_attr_optional(&mut self.runtime_ctx.borrow_mut(), true) - } self.pop_schema_expr(); Ok(schema) } diff --git a/kclvm/evaluator/src/proxy.rs b/kclvm/evaluator/src/proxy.rs index 1a48c115b..fb29c1217 100644 --- a/kclvm/evaluator/src/proxy.rs +++ b/kclvm/evaluator/src/proxy.rs @@ -107,6 +107,7 @@ pub(crate) fn call_schema_body_from_rule( pub(crate) fn call_schema_check( s: &Evaluator, func: &ValueRef, + schema_value: &ValueRef, args: &ValueRef, kwargs: &ValueRef, ctx: Option<&SchemaEvalContextRef>, @@ -125,7 +126,7 @@ pub(crate) fn call_schema_check( if let Some(ctx) = ctx { schema.ctx.borrow_mut().set_info_with_schema(&ctx.borrow()) } - (schema.check)(s, &schema.ctx, args, kwargs); + (schema.check)(s, &schema.ctx, schema_value, args, kwargs); s.pop_backtrace(); s.pop_pkgpath(); } diff --git a/kclvm/evaluator/src/schema.rs b/kclvm/evaluator/src/schema.rs index ac37ee5e8..ede41b97c 100644 --- a/kclvm/evaluator/src/schema.rs +++ b/kclvm/evaluator/src/schema.rs @@ -18,6 +18,9 @@ use crate::{Evaluator, INNER_LEVEL}; pub type SchemaBodyHandler = Arc ValueRef>; +pub type SchemaCheckHandler = + Arc ()>; + pub type SchemaEvalContextRef = Rc>; /// Proxy functions represent the saved functions of the runtime its, @@ -332,7 +335,7 @@ pub struct ConfigMeta { pub struct SchemaCaller { pub ctx: SchemaEvalContextRef, pub body: SchemaBodyHandler, - pub check: SchemaBodyHandler, + pub check: SchemaCheckHandler, } /// Init or reset the schema lazy eval scope. @@ -353,7 +356,7 @@ pub(crate) fn schema_body( ) -> ValueRef { init_lazy_scope(s, &mut ctx.borrow_mut()); // Schema self value or parent schema value; - let mut schema_value = if let Some(parent_name) = &ctx.borrow().node.parent_name { + let mut schema_ctx_value = if let Some(parent_name) = &ctx.borrow().node.parent_name { let base_constructor_func = s .walk_identifier_with_ctx(&parent_name.node, &ast::ExprContext::Load, None) .expect(kcl_error::RUNTIME_ERROR_MSG); @@ -385,9 +388,9 @@ pub(crate) fn schema_body( .expect(kcl_error::RUNTIME_ERROR_MSG); } let runtime_type = kclvm_runtime::schema_runtime_type(&schema_name, schema_pkgpath); - schema_value.set_potential_schema_type(&runtime_type); + schema_ctx_value.set_potential_schema_type(&runtime_type); // Set schema arguments and keyword arguments - schema_value.set_schema_args(args, kwargs); + schema_ctx_value.set_schema_args(args, kwargs); } // Schema Mixins { @@ -396,7 +399,7 @@ pub(crate) fn schema_body( let mixin_func = s .walk_identifier_with_ctx(&mixin.node, &ast::ExprContext::Load, None) .expect(kcl_error::RUNTIME_ERROR_MSG); - schema_value = call_schema_body(s, &mixin_func, args, kwargs, ctx); + schema_ctx_value = call_schema_body(s, &mixin_func, args, kwargs, ctx); } } // Schema Attribute optional check @@ -415,7 +418,7 @@ pub(crate) fn schema_body( } // Do schema check for the sub schema. let is_sub_schema = { ctx.borrow().is_sub_schema }; - if is_sub_schema { + let schema = if is_sub_schema { let index_sign_key_name = if let Some(index_signature) = &ctx.borrow().node.index_signature { if let Some(key_name) = &index_signature.node.key_name { @@ -427,8 +430,15 @@ pub(crate) fn schema_body( "".to_string() }; if index_sign_key_name.is_empty() { + // Update schema relaxed attribute + update_schema_relaxed_attr(s, ctx, &mut schema_ctx_value); + // Construct schema instance + let schema = schema_with_config(s, ctx, &schema_ctx_value, args, kwargs); + // Do schema optional attribute check recursively before evaluate check expressions. + check_schema_optional_attr(s, &schema); // Call schema check block function - schema_check(s, ctx, args, kwargs); + schema_check(s, ctx, &schema, args, kwargs); + schema } else { // Do check function for every index signature key let config = { @@ -437,26 +447,97 @@ pub(crate) fn schema_body( }; for (k, _) in &config.as_dict_ref().values { // relaxed keys - if schema_value.attr_map_get(k).is_none() { + if schema_ctx_value.attr_map_get(k).is_none() { // Update index signature key value let value = ValueRef::str(k); - schema_value.dict_update_key_value(&index_sign_key_name, value); + schema_ctx_value.dict_update_key_value(&index_sign_key_name, value.clone()); + // Update schema relaxed attribute + update_schema_relaxed_attr(s, ctx, &mut schema_ctx_value); // Call schema check block function - schema_check(s, ctx, args, kwargs); + schema_check(s, ctx, &schema_ctx_value, args, kwargs); } } - schema_value.dict_remove(&index_sign_key_name); + schema_ctx_value.dict_remove(&index_sign_key_name); + // Construct schema instance + let schema = schema_with_config(s, ctx, &schema_ctx_value, args, kwargs); + // Do schema optional attribute check recursively before evaluate check expressions. + check_schema_optional_attr(s, &schema); + schema } - } + } else { + // Record base schema instances. + schema_with_config(s, ctx, &schema_ctx_value, args, kwargs) + }; s.leave_scope(); s.pop_schema(); - schema_with_config(s, ctx, &schema_value, args, kwargs) + schema +} + +// Schema check and index sign value update function +pub(crate) fn schema_check( + s: &Evaluator, + ctx: &SchemaEvalContextRef, + schema_value: &ValueRef, + args: &ValueRef, + kwargs: &ValueRef, +) { + // Call base check function + { + let ctx_ref = ctx.borrow(); + if let Some(parent_name) = &ctx_ref.node.parent_name { + let base_constructor_func = s + .walk_identifier_with_ctx(&parent_name.node, &ast::ExprContext::Load, None) + .expect(kcl_error::RUNTIME_ERROR_MSG); + call_schema_check( + s, + &base_constructor_func, + schema_value, + args, + kwargs, + Some(ctx), + ) + } + } + // Call self check function + { + let ctx = ctx.borrow(); + for check_expr in &ctx.node.checks { + s.walk_check_expr(&check_expr.node) + .expect(kcl_error::RUNTIME_ERROR_MSG); + } + } + + // Call mixin check functions + { + let ctx = ctx.borrow(); + for mixin in &ctx.node.mixins { + let mixin_func = s + .walk_identifier_with_ctx(&mixin.node, &ast::ExprContext::Load, None) + .expect(kcl_error::RUNTIME_ERROR_MSG); + if let Some(index) = mixin_func.try_get_proxy() { + let frame = { + let frames = s.frames.borrow(); + frames + .get(index) + .expect(kcl_error::INTERNAL_ERROR_MSG) + .clone() + }; + if let Proxy::Schema(schema) = &frame.proxy { + s.push_pkgpath(&frame.pkgpath); + s.push_backtrace(&frame); + (schema.check)(s, &schema.ctx, schema_value, args, kwargs); + s.pop_backtrace(); + s.pop_pkgpath(); + } + } + } + } } pub(crate) fn schema_with_config( s: &Evaluator, ctx: &SchemaEvalContextRef, - schema_dict: &ValueRef, + schema_ctx_value: &ValueRef, args: &ValueRef, kwargs: &ValueRef, ) -> ValueRef { @@ -471,18 +552,6 @@ pub(crate) fn schema_with_config( .cloned() .collect() }; - let schema = { - let ctx = ctx.borrow(); - schema_dict.dict_to_schema( - &name, - &pkgpath, - &config_keys, - &ctx.config_meta, - &ctx.optional_mapping, - Some(args.clone()), - Some(kwargs.clone()), - ) - }; let runtime_type = schema_runtime_type(&name, &pkgpath); // Instance package path is the last frame calling package path. let instance_pkgpath = s.last_pkgpath(); @@ -504,26 +573,33 @@ pub(crate) fn schema_with_config( pkg_instance_map .get_mut(&instance_pkgpath) .unwrap() - .push(schema_dict.clone()); + .push(schema_ctx_value.clone()); } // Dict to schema let is_sub_schema = { ctx.borrow().is_sub_schema }; if is_sub_schema { - schema + let ctx = ctx.borrow(); + // Record instance copy and convert it to schema value. + schema_ctx_value.dict_to_schema( + &name, + &pkgpath, + &config_keys, + &ctx.config_meta, + &ctx.optional_mapping, + Some(args.clone()), + Some(kwargs.clone()), + ) } else { - schema_dict.clone() + schema_ctx_value.clone() } } -// Schema check function -pub(crate) fn schema_check( +fn update_schema_relaxed_attr( s: &Evaluator, ctx: &SchemaEvalContextRef, - args: &ValueRef, - kwargs: &ValueRef, -) -> ValueRef { + schema_value: &mut ValueRef, +) { let schema_name = { ctx.borrow().node.name.node.to_string() }; - let mut schema_value = { ctx.borrow().value.clone() }; // Do check function // Schema runtime index signature and relaxed check { @@ -539,9 +615,9 @@ pub(crate) fn schema_check( } else { "" }; - schema_value_check( + schema_relaxed_attr_update_and_check( s, - &mut schema_value, + schema_value, &ctx.config, &schema_name, &index_sign_value, @@ -550,9 +626,9 @@ pub(crate) fn schema_check( index_signature.node.value_ty.node.to_string().as_str(), ); } else { - schema_value_check( + schema_relaxed_attr_update_and_check( s, - &mut schema_value, + schema_value, &ctx.config, &schema_name, &s.undefined_value(), @@ -562,55 +638,16 @@ pub(crate) fn schema_check( ); } } - // Call base check function - { - let ctx_ref = ctx.borrow(); - if let Some(parent_name) = &ctx_ref.node.parent_name { - let base_constructor_func = s - .walk_identifier_with_ctx(&parent_name.node, &ast::ExprContext::Load, None) - .expect(kcl_error::RUNTIME_ERROR_MSG); - call_schema_check(s, &base_constructor_func, args, kwargs, Some(ctx)) - } - } - // Call self check function - { - let ctx = ctx.borrow(); - for check_expr in &ctx.node.checks { - s.walk_check_expr(&check_expr.node) - .expect(kcl_error::RUNTIME_ERROR_MSG); - } - } +} - // Call mixin check functions - { - let ctx = ctx.borrow(); - for mixin in &ctx.node.mixins { - let mixin_func = s - .walk_identifier_with_ctx(&mixin.node, &ast::ExprContext::Load, None) - .expect(kcl_error::RUNTIME_ERROR_MSG); - if let Some(index) = mixin_func.try_get_proxy() { - let frame = { - let frames = s.frames.borrow(); - frames - .get(index) - .expect(kcl_error::INTERNAL_ERROR_MSG) - .clone() - }; - if let Proxy::Schema(schema) = &frame.proxy { - s.push_pkgpath(&frame.pkgpath); - s.push_backtrace(&frame); - (schema.check)(s, &schema.ctx, args, kwargs); - s.pop_backtrace(); - s.pop_pkgpath(); - } - } - } +fn check_schema_optional_attr(s: &Evaluator, schema_value: &ValueRef) { + if is_top_level_schema_instance(s) { + schema_value.schema_check_attr_optional(&mut s.runtime_ctx.borrow_mut(), true); } - schema_value } /// Schema additional value check -fn schema_value_check( +fn schema_relaxed_attr_update_and_check( s: &Evaluator, schema_value: &mut ValueRef, schema_config: &ValueRef, @@ -664,6 +701,13 @@ fn schema_value_check( } } +/// For a schema instance returned by the schema body. Its schema and schema expr stack +/// length are both 1, if > 1, it's not a top level schema instance. +#[inline] +fn is_top_level_schema_instance(s: &Evaluator) -> bool { + !(s.schema_stack.borrow().len() > 1 || s.schema_expr_stack.borrow().len() > 1) +} + impl<'ctx> Evaluator<'ctx> { pub(crate) fn construct_schema_config_meta( &self, diff --git a/kclvm/runtime/src/api/kclvm.rs b/kclvm/runtime/src/api/kclvm.rs index 976ad1476..fdba18c6a 100644 --- a/kclvm/runtime/src/api/kclvm.rs +++ b/kclvm/runtime/src/api/kclvm.rs @@ -229,8 +229,9 @@ pub struct DictValue { pub values: IndexMap, pub ops: IndexMap, pub insert_indexs: IndexMap, + /// Attribute type annotation string mapping. pub attr_map: IndexMap, - // The runtime dict to schema reflect type string. + /// The runtime dict to schema reflect type string. pub potential_schema: Option, } diff --git a/kclvm/runtime/src/value/api.rs b/kclvm/runtime/src/value/api.rs index 160a5787f..109f50954 100644 --- a/kclvm/runtime/src/value/api.rs +++ b/kclvm/runtime/src/value/api.rs @@ -281,15 +281,6 @@ pub unsafe extern "C" fn kclvm_value_schema_with_config( let optional_mapping = ptr_as_ref(optional_mapping); let args = ptr_as_ref(args); let kwargs = ptr_as_ref(kwargs); - let schema = schema_dict.dict_to_schema( - name, - pkgpath, - &config_keys, - config_meta, - optional_mapping, - Some(args.clone()), - Some(kwargs.clone()), - ); if record_instance.is_truthy() { // Record schema instance in the context if !ctx.instances.contains_key(&runtime_type) { @@ -307,6 +298,15 @@ pub unsafe extern "C" fn kclvm_value_schema_with_config( } // Dict to schema if is_sub_schema.is_truthy() { + let schema = schema_dict.dict_to_schema( + name, + pkgpath, + &config_keys, + config_meta, + optional_mapping, + Some(args.clone()), + Some(kwargs.clone()), + ); schema.into_raw(ctx) } else { schema_dict.clone().into_raw(ctx) diff --git a/kclvm/runtime/src/value/val_args.rs b/kclvm/runtime/src/value/val_args.rs index eef166c8f..211dbaf8c 100644 --- a/kclvm/runtime/src/value/val_args.rs +++ b/kclvm/runtime/src/value/val_args.rs @@ -71,7 +71,7 @@ impl ValueRef { if let Some(x) = self.arg_i(i) { match *x.rc.borrow() { Value::bool_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -82,7 +82,7 @@ impl ValueRef { if let Some(x) = self.arg_i(i) { match *x.rc.borrow() { Value::int_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -94,7 +94,7 @@ impl ValueRef { match *x.rc.borrow() { Value::bool_value(v) => return Some(v as i64), Value::int_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -105,7 +105,7 @@ impl ValueRef { if let Some(x) = self.arg_i(i) { match *x.rc.borrow() { Value::float_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -117,7 +117,7 @@ impl ValueRef { match *x.rc.borrow() { Value::float_value(v) => return Some(v), Value::int_value(v) => return Some(v as f64), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -128,7 +128,7 @@ impl ValueRef { if let Some(x) = self.arg_i(i) { match &*x.rc.borrow() { Value::str_value(s) => return Some(s.to_string()), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -160,7 +160,7 @@ impl ValueRef { if let Some(x) = self.kwarg(name) { match *x.rc.borrow() { Value::bool_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -171,7 +171,7 @@ impl ValueRef { if let Some(x) = self.kwarg(name) { match *x.rc.borrow() { Value::int_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -182,7 +182,7 @@ impl ValueRef { if let Some(x) = self.kwarg(name) { match *x.rc.borrow() { Value::float_value(v) => return Some(v), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } @@ -193,7 +193,7 @@ impl ValueRef { if let Some(x) = self.kwarg(name) { match &*x.rc.borrow() { Value::str_value(s) => return Some(s.to_string()), - Value::none => return default, + Value::none | Value::undefined => return default, _ => return None, } } diff --git a/kclvm/runtime/src/value/val_schema.rs b/kclvm/runtime/src/value/val_schema.rs index 01bdd0f2e..207cfc8a5 100644 --- a/kclvm/runtime/src/value/val_schema.rs +++ b/kclvm/runtime/src/value/val_schema.rs @@ -271,7 +271,7 @@ impl ValueRef { } } - pub fn attr_map_get(&mut self, name: &str) -> Option { + pub fn attr_map_get(&self, name: &str) -> Option { match &*self.rc.borrow() { Value::dict_value(dict) => dict.attr_map.get(name).cloned(), Value::schema_value(schema) => schema.config.attr_map.get(name).cloned(), diff --git a/test/grammar/schema/optional_attr/fail_21/main.k b/test/grammar/schema/optional_attr/fail_21/main.k new file mode 100644 index 000000000..45787d999 --- /dev/null +++ b/test/grammar/schema/optional_attr/fail_21/main.k @@ -0,0 +1,10 @@ +import regex +pattern = 'xxxx' + +schema Name: + name: str + + check: + regex.match(name, pattern) + +n = Name {} diff --git a/test/grammar/schema/optional_attr/fail_21/stderr.golden b/test/grammar/schema/optional_attr/fail_21/stderr.golden new file mode 100644 index 000000000..0094f4cb2 --- /dev/null +++ b/test/grammar/schema/optional_attr/fail_21/stderr.golden @@ -0,0 +1 @@ +attribute 'name' of Name is required and can't be None or Undefined \ No newline at end of file