feat: enhance required attr check (#587)

* chore: bump version to 0.5.0-beta.1 * feat: enhance schema optional attribute check for index signature and add more test cases.
kcl-lang · Jun 27, 2023 · a5ba758 · a5ba758
1 parent 40de454
commit a5ba758
Show file tree

Hide file tree

Showing 29 changed files with 280 additions and 36 deletions.
diff --git a/kclvm/compiler/src/codegen/llvm/node.rs b/kclvm/compiler/src/codegen/llvm/node.rs
@@ -733,7 +733,7 @@ impl<'ctx> TypedResultWalker<'ctx> for LLVMCodeGenContext<'ctx> {
                 let index_sign_value = if let Some(value) = &index_signature.node.value {
                     self.walk_expr(value).expect(kcl_error::COMPILE_ERROR_MSG)
                 } else {
-                    self.none_value()
+                    self.undefined_value()
                 };
                 let key_name_str_ptr = if let Some(key_name) = &index_signature.node.key_name {
                     self.native_global_string(key_name.as_str(), "")
@@ -757,7 +757,6 @@ impl<'ctx> TypedResultWalker<'ctx> for LLVMCodeGenContext<'ctx> {
                         )
                         .into(),
                         self.native_i8(index_signature.node.any_other as i8).into(),
-                        self.native_i8(false as i8).into(),
                     ],
                 );
             } else {
@@ -773,7 +772,6 @@ impl<'ctx> TypedResultWalker<'ctx> for LLVMCodeGenContext<'ctx> {
                         self.native_global_string("", "").into(),
                         self.native_global_string("", "").into(),
                         self.native_i8(0).into(),
-                        self.native_i8(false as i8).into(),
                     ],
                 );
             }

diff --git a/kclvm/runtime/src/_kclvm.bc b/kclvm/runtime/src/_kclvm.bc
diff --git a/kclvm/runtime/src/_kclvm.h b/kclvm/runtime/src/_kclvm.h
@@ -480,7 +480,7 @@ kclvm_value_ref_t* kclvm_schema_instances(kclvm_context_t* ctx, kclvm_value_ref_
 
 void kclvm_schema_optional_check(kclvm_value_ref_t* p);
 
-void kclvm_schema_value_check(kclvm_value_ref_t* schema_value, kclvm_value_ref_t* schema_config, kclvm_value_ref_t* _config_meta, kclvm_char_t* schema_name, kclvm_value_ref_t* index_sign_value, kclvm_char_t* _key_name, kclvm_char_t* key_type, kclvm_char_t* _value_type, kclvm_bool_t _any_other, kclvm_bool_t is_relaxed);
+void kclvm_schema_value_check(kclvm_value_ref_t* schema_value, kclvm_value_ref_t* schema_config, kclvm_value_ref_t* _config_meta, kclvm_char_t* schema_name, kclvm_value_ref_t* index_sign_value, kclvm_char_t* key_name, kclvm_char_t* key_type, kclvm_char_t* value_type, kclvm_bool_t _any_other);
 
 kclvm_value_ref_t* kclvm_schema_value_new(kclvm_context_t* ctx, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs, kclvm_value_ref_t* schema_value_or_func, kclvm_value_ref_t* config, kclvm_value_ref_t* config_meta, kclvm_char_t* pkgpath);
 

diff --git a/kclvm/runtime/src/_kclvm.ll b/kclvm/runtime/src/_kclvm.ll
@@ -428,7 +428,7 @@ declare %kclvm_value_ref_t* @kclvm_schema_instances(%kclvm_context_t* %ctx, %kcl
 
 declare void @kclvm_schema_optional_check(%kclvm_value_ref_t* %p);
 
-declare void @kclvm_schema_value_check(%kclvm_value_ref_t* %schema_value, %kclvm_value_ref_t* %schema_config, %kclvm_value_ref_t* %_config_meta, %kclvm_char_t* %schema_name, %kclvm_value_ref_t* %index_sign_value, %kclvm_char_t* %_key_name, %kclvm_char_t* %key_type, %kclvm_char_t* %_value_type, %kclvm_bool_t %_any_other, %kclvm_bool_t %is_relaxed);
+declare void @kclvm_schema_value_check(%kclvm_value_ref_t* %schema_value, %kclvm_value_ref_t* %schema_config, %kclvm_value_ref_t* %_config_meta, %kclvm_char_t* %schema_name, %kclvm_value_ref_t* %index_sign_value, %kclvm_char_t* %key_name, %kclvm_char_t* %key_type, %kclvm_char_t* %value_type, %kclvm_bool_t %_any_other);
 
 declare %kclvm_value_ref_t* @kclvm_schema_value_new(%kclvm_context_t* %ctx, %kclvm_value_ref_t* %args, %kclvm_value_ref_t* %kwargs, %kclvm_value_ref_t* %schema_value_or_func, %kclvm_value_ref_t* %config, %kclvm_value_ref_t* %config_meta, %kclvm_char_t* %pkgpath);
 

diff --git a/kclvm/runtime/src/_kclvm_api_spec.rs b/kclvm/runtime/src/_kclvm_api_spec.rs
@@ -763,8 +763,8 @@
 // api-spec(llvm): declare %kclvm_value_ref_t* @kclvm_schema_instances(%kclvm_context_t* %ctx, %kclvm_value_ref_t* %args, %kclvm_value_ref_t* %kwargs);
 
 // api-spec:       kclvm_schema_value_check
-// api-spec(c):    void kclvm_schema_value_check(kclvm_value_ref_t* schema_value, kclvm_value_ref_t* schema_config, kclvm_value_ref_t* _config_meta, kclvm_char_t* schema_name, kclvm_value_ref_t* index_sign_value, kclvm_char_t* _key_name, kclvm_char_t* key_type, kclvm_char_t* _value_type, kclvm_bool_t _any_other, kclvm_bool_t is_relaxed);
-// api-spec(llvm): declare void @kclvm_schema_value_check(%kclvm_value_ref_t* %schema_value, %kclvm_value_ref_t* %schema_config, %kclvm_value_ref_t* %_config_meta, %kclvm_char_t* %schema_name, %kclvm_value_ref_t* %index_sign_value, %kclvm_char_t* %_key_name, %kclvm_char_t* %key_type, %kclvm_char_t* %_value_type, %kclvm_bool_t %_any_other, %kclvm_bool_t %is_relaxed);
+// api-spec(c):    void kclvm_schema_value_check(kclvm_value_ref_t* schema_value, kclvm_value_ref_t* schema_config, kclvm_value_ref_t* _config_meta, kclvm_char_t* schema_name, kclvm_value_ref_t* index_sign_value, kclvm_char_t* key_name, kclvm_char_t* key_type, kclvm_char_t* value_type, kclvm_bool_t _any_other);
+// api-spec(llvm): declare void @kclvm_schema_value_check(%kclvm_value_ref_t* %schema_value, %kclvm_value_ref_t* %schema_config, %kclvm_value_ref_t* %_config_meta, %kclvm_char_t* %schema_name, %kclvm_value_ref_t* %index_sign_value, %kclvm_char_t* %key_name, %kclvm_char_t* %key_type, %kclvm_char_t* %value_type, %kclvm_bool_t %_any_other);
 
 // api-spec:       kclvm_schema_do_check_with_index_sign_attr
 // api-spec(c):    void kclvm_schema_do_check_with_index_sign_attr(kclvm_context_t* ctx, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs, uint64_t* check_fn_ptr, kclvm_char_t* attr_name);

diff --git a/kclvm/runtime/src/value/api.rs b/kclvm/runtime/src/value/api.rs
@@ -1821,19 +1821,16 @@ pub unsafe extern "C" fn kclvm_value_union(
     };
     if b.is_config() {
         let dict = b.as_dict_ref();
-        let mut result = schema;
-        for (k, v) in &dict.values {
-            if attr_map.contains_key(k) {
-                let v = type_pack_and_check(v, vec![attr_map.get(k).unwrap()]);
-                let mut entry = b.dict_get_entry(k).unwrap().deep_copy();
-                entry.dict_update_key_value(k, v);
-                result = a.union_entry(&entry, true, &opts).clone().into_raw();
-            } else {
-                let entry = b.dict_get_entry(k).unwrap();
-                result = a.union_entry(&entry, true, &opts).clone().into_raw();
+        for k in dict.values.keys() {
+            let entry = b.dict_get_entry(k).unwrap();
+            a.union_entry(&entry, true, &opts);
+            // Has type annotation
+            if let Some(ty) = attr_map.get(k) {
+                let value = a.dict_get_value(k).unwrap();
+                a.dict_update_key_value(k, type_pack_and_check(&value, vec![ty]));
             }
         }
-        result
+        a.clone().into_raw()
     } else {
         a.union_entry(b, true, &opts).into_raw()
     }
@@ -2155,26 +2152,25 @@ pub unsafe extern "C" fn kclvm_schema_value_check(
     index_sign_value: *const kclvm_value_ref_t,
     key_name: *const kclvm_char_t,
     key_type: *const kclvm_char_t,
-    _value_type: *const kclvm_char_t,
+    value_type: *const kclvm_char_t,
     _any_other: kclvm_bool_t,
-    is_relaxed: kclvm_bool_t,
 ) {
     let schema_value = mut_ptr_as_ref(schema_value);
     let schema_config = ptr_as_ref(schema_config);
     let index_sign_value = ptr_as_ref(index_sign_value);
     let key_type = c2str(key_type);
+    let value_type = c2str(value_type);
     let index_key_name = c2str(key_name);
     let has_index_signature = !key_type.is_empty();
-    let should_add_attr = is_relaxed != 0 || has_index_signature;
 
     let ctx = Context::current_context_mut();
     if ctx.cfg.disable_schema_check {
         return;
     }
     let config = schema_config.as_dict_ref();
     for (key, value) in &config.values {
-        let is_not_in_schema = schema_value.dict_get_value(key).is_none();
-        if should_add_attr && is_not_in_schema {
+        let no_such_attr = schema_value.dict_get_value(key).is_none();
+        if has_index_signature && no_such_attr {
             // Allow index signature value has different values
             // related to the index signature key name.
             let should_update =
@@ -2194,13 +2190,18 @@ pub unsafe extern "C" fn kclvm_schema_value_check(
                     .unwrap_or(&ConfigEntryOperationKind::Union);
                 schema_value.dict_update_entry(
                     key.as_str(),
-                    &index_sign_value,
+                    &index_sign_value.deep_copy(),
                     &ConfigEntryOperationKind::Override,
                     &-1,
                 );
                 schema_value.dict_insert(key.as_str(), &value, op.clone(), -1);
+                let value = schema_value.dict_get_value(key).unwrap();
+                schema_value.dict_update_key_value(
+                    key.as_str(),
+                    type_pack_and_check(&value, vec![value_type]),
+                );
             }
-        } else if !should_add_attr && is_not_in_schema {
+        } else if !has_index_signature && no_such_attr {
             let schema_name = c2str(schema_name);
             panic!("No attribute named '{key}' in the schema '{schema_name}'");
         }

diff --git a/kclvm/runtime/src/value/val_schema.rs b/kclvm/runtime/src/value/val_schema.rs
@@ -133,8 +133,13 @@ impl ValueRef {
                             self.schema_name()
                         );
                     }
-                    if recursive && value.is_schema() {
-                        value.schema_check_attr_optional(recursive);
+                }
+                // Recursive check schema values for every attributes.
+                if recursive {
+                    for value in attr_map.values() {
+                        if value.is_schema() {
+                            value.schema_check_attr_optional(recursive);
+                        }
                     }
                 }
             }

diff --git a/kclvm/runtime/src/value/val_type.rs b/kclvm/runtime/src/value/val_type.rs
@@ -160,7 +160,7 @@ pub fn type_pack_and_check(value: &ValueRef, expected_types: Vec<&str>) -> Value
     let is_schema = value.is_schema();
     let value_tpe = value.type_str();
     let mut checked = false;
-    let mut convertted_value = value.clone();
+    let mut converted_value = value.clone();
     let expected_type = &expected_types.join(" | ").replace('@', "");
     for tpe in expected_types {
         let tpe = if !tpe.contains('.') {
@@ -173,18 +173,18 @@ pub fn type_pack_and_check(value: &ValueRef, expected_types: Vec<&str>) -> Value
             tpe
         };
         if !is_schema {
-            convertted_value = convert_collection_value(value, tpe);
+            converted_value = convert_collection_value(value, tpe);
         }
         // Runtime type check
-        checked = check_type(&convertted_value, tpe);
+        checked = check_type(&converted_value, tpe);
         if checked {
             break;
         }
     }
     if !checked {
         panic!("expect {expected_type}, got {value_tpe}");
     }
-    convertted_value
+    converted_value
 }
 
 /// Convert collection value including dict/list to the potential schema

diff --git a/kclvm/version/src/lib.rs b/kclvm/version/src/lib.rs
@@ -1,7 +1,7 @@
 // Copyright 2021 The KCL Authors. All rights reserved.
 
-pub const VERSION: &str = "0.5.0-alpha.4";
-pub const CHECK_SUM: &str = "eb13b547c76bd1eaf9b1ea0caa917fb1";
+pub const VERSION: &str = "0.5.0-beta.1";
+pub const CHECK_SUM: &str = "20ab3eb4b9179219d6837a57f5d35286";
 
 /// Get kCL full version string with the format `{version}-{check_sum}`.
 #[inline]

diff --git a/test/grammar/schema/index_signature/key_alias_0/main.k b/test/grammar/schema/index_signature/key_alias_0/main.k
@@ -4,7 +4,7 @@ schema TeamSpec[id: str]:
     shortName: str = name
 
 schema TeamMap:
-    [n: str]: TeamSpec = TeamSpec(n)  # `n` does not work currently
+    [n: str]: TeamSpec = TeamSpec(n)
 
 teamMap = TeamMap {
     a.fullName = "alpha"

diff --git a/test/grammar/schema/index_signature/normal_1/stdout.golden b/test/grammar/schema/index_signature/normal_1/stdout.golden
@@ -5,8 +5,6 @@ namedMap:
   key1:
     default_key: default_value
     key1: value1
-    key2: value2
   key2:
     default_key: default_value
-    key1: value1
     key2: value2
diff --git a/test/grammar/schema/optional_attr/fail_10/main.k b/test/grammar/schema/optional_attr/fail_10/main.k
@@ -0,0 +1,16 @@
+schema TeamSpec[id: str]:
+    fullName: str
+    name: str = id
+    shortName: str = name
+
+schema Team:
+    [id: str]: TeamSpec = TeamSpec(id)
+
+Teams = Team {
+    a.fullName = "alpha"
+    b.fullName = "bravo"
+    c = {
+        fullName = "charlie"
+        shortName = "cc"
+    }
+}
diff --git a/test/grammar/schema/optional_attr/fail_10/stderr.golden.py b/test/grammar/schema/optional_attr/fail_10/stderr.golden.py
@@ -0,0 +1,18 @@
+import sys
+import os
+
+import kclvm.kcl.error as kcl_error
+
+cwd = os.path.dirname(os.path.realpath(__file__))
+
+kcl_error.print_kcl_error_message(
+    kcl_error.get_exception(err_type=kcl_error.ErrType.EvaluationError_TYPE,
+                            file_msgs=[
+                                kcl_error.ErrFileMsg(
+                                    filename=os.path.join(cwd, "main.k"),
+                                    line_no=14,
+                                ),
+                            ],
+                            arg_msg="attribute 'name' of TeamSpec is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/optional_attr/fail_11/main.k b/test/grammar/schema/optional_attr/fail_11/main.k
@@ -0,0 +1,14 @@
+schema TeamSpec[id: str]:
+    fullName: str
+    name: str = id
+    shortName: str = name
+
+schema Team:
+    d: TeamSpec = TeamSpec("d")
+
+Teams = Team {
+    d = {
+        fullName = "charlie"
+        shortName = "dd"
+    }
+}
diff --git a/test/grammar/schema/optional_attr/fail_11/stderr.golden.py b/test/grammar/schema/optional_attr/fail_11/stderr.golden.py
@@ -0,0 +1,18 @@
+import sys
+import os
+
+import kclvm.kcl.error as kcl_error
+
+cwd = os.path.dirname(os.path.realpath(__file__))
+
+kcl_error.print_kcl_error_message(
+    kcl_error.get_exception(err_type=kcl_error.ErrType.EvaluationError_TYPE,
+                            file_msgs=[
+                                kcl_error.ErrFileMsg(
+                                    filename=os.path.join(cwd, "main.k"),
+                                    line_no=12,
+                                ),
+                            ],
+                            arg_msg="attribute 'name' of TeamSpec is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/optional_attr/fail_12/main.k b/test/grammar/schema/optional_attr/fail_12/main.k
@@ -0,0 +1,14 @@
+schema TeamSpec[id: str]:
+    fullName: str
+    name: str = id
+    shortName: str = name
+
+schema Team:
+    d: TeamSpec
+
+Teams = Team {
+    d = {
+        fullName = "charlie"
+        shortName = "dd"
+    }
+}
diff --git a/test/grammar/schema/optional_attr/fail_12/stderr.golden.py b/test/grammar/schema/optional_attr/fail_12/stderr.golden.py
@@ -0,0 +1,18 @@
+import sys
+import os
+
+import kclvm.kcl.error as kcl_error
+
+cwd = os.path.dirname(os.path.realpath(__file__))
+
+kcl_error.print_kcl_error_message(
+    kcl_error.get_exception(err_type=kcl_error.ErrType.EvaluationError_TYPE,
+                            file_msgs=[
+                                kcl_error.ErrFileMsg(
+                                    filename=os.path.join(cwd, "main.k"),
+                                    line_no=12,
+                                ),
+                            ],
+                            arg_msg="attribute 'name' of TeamSpec is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/optional_attr/fail_7/main.k b/test/grammar/schema/optional_attr/fail_7/main.k
@@ -0,0 +1,7 @@
+schema Values:
+    a: int
+
+schema Data:
+    values = Values {}
+
+d = Data {}
diff --git a/test/grammar/schema/optional_attr/fail_7/stderr.golden.py b/test/grammar/schema/optional_attr/fail_7/stderr.golden.py
@@ -0,0 +1,18 @@
+import sys
+import os
+
+import kclvm.kcl.error as kcl_error
+
+cwd = os.path.dirname(os.path.realpath(__file__))
+
+kcl_error.print_kcl_error_message(
+    kcl_error.get_exception(err_type=kcl_error.ErrType.EvaluationError_TYPE,
+                            file_msgs=[
+                                kcl_error.ErrFileMsg(
+                                    filename=os.path.join(cwd, "main.k"),
+                                    line_no=5,
+                                ),
+                            ],
+                            arg_msg=" attribute 'a' of Values is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/optional_attr/fail_8/main.k b/test/grammar/schema/optional_attr/fail_8/main.k
@@ -0,0 +1,16 @@
+schema TeamSpec[id: str]:
+    fullName: str
+    name: str = id
+    shortName: str = name
+
+schema Team:
+    [id: str]: TeamSpec = TeamSpec(id)
+
+Teams = Team {
+    a.fullName = "alpha"
+    b.fullName = "bravo"
+    c = TeamSpec {
+        fullName = "charlie"
+        shortName = "cc"
+    }
+}
diff --git a/test/grammar/schema/optional_attr/fail_8/stderr.golden.py b/test/grammar/schema/optional_attr/fail_8/stderr.golden.py
@@ -0,0 +1,18 @@
+import sys
+import os
+
+import kclvm.kcl.error as kcl_error
+
+cwd = os.path.dirname(os.path.realpath(__file__))
+
+kcl_error.print_kcl_error_message(
+    kcl_error.get_exception(err_type=kcl_error.ErrType.EvaluationError_TYPE,
+                            file_msgs=[
+                                kcl_error.ErrFileMsg(
+                                    filename=os.path.join(cwd, "main.k"),
+                                    line_no=12,
+                                ),
+                            ],
+                            arg_msg="attribute 'name' of TeamSpec is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/optional_attr/fail_9/main.k b/test/grammar/schema/optional_attr/fail_9/main.k
@@ -0,0 +1,13 @@
+schema TeamSpec[id: str]:
+    fullName: str
+    name: str = id
+    shortName: str = name
+
+schema Team:
+    [id: str]: TeamSpec = TeamSpec(id)
+
+Teams = Team {
+    a.fullName = "alpha"
+    b.fullName = "bravo"
+    c.shortName = "cc"
+}