fix: schema index signature type unsoundness (#1007)

* fix: schema index signature type unsoundess Signed-off-by: peefy <[email protected]> * fix: test cases Signed-off-by: peefy <[email protected]> --------- Signed-off-by: peefy <[email protected]>
kcl-lang · Jan 26, 2024 · 6cf7389 · 6cf7389
1 parent 5b40623
commit 6cf7389
Show file tree

Hide file tree

Showing 33 changed files with 286 additions and 29 deletions.
diff --git a/kclvm/runtime/src/_kclvm.h b/kclvm/runtime/src/_kclvm.h
@@ -120,7 +120,7 @@ kclvm_value_ref_t* kclvm_builtin_oct(kclvm_context_t* ctx, kclvm_value_ref_t* ar
 
 kclvm_value_ref_t* kclvm_builtin_option(kclvm_context_t* ctx, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs);
 
-void kclvm_builtin_option_init(kclvm_context_t* ctx, int8_t* key, int8_t* value);
+void kclvm_builtin_option_init(kclvm_context_t* ctx, char* key, char* value);
 
 kclvm_value_ref_t* kclvm_builtin_option_reset(kclvm_context_t* ctx, kclvm_value_ref_t* _args, kclvm_value_ref_t* _kwargs);
 
@@ -224,13 +224,13 @@ void kclvm_context_set_disable_schema_check(kclvm_context_t* p, kclvm_bool_t v);
 
 void kclvm_context_set_import_names(kclvm_context_t* p, kclvm_value_ref_t* import_names);
 
-void kclvm_context_set_kcl_filename(kclvm_context_t* ctx, int8_t* filename);
+void kclvm_context_set_kcl_filename(kclvm_context_t* ctx, char* filename);
 
 void kclvm_context_set_kcl_line_col(kclvm_context_t* ctx, int32_t line, int32_t col);
 
-void kclvm_context_set_kcl_location(kclvm_context_t* p, int8_t* filename, int32_t line, int32_t col);
+void kclvm_context_set_kcl_location(kclvm_context_t* p, char* filename, int32_t line, int32_t col);
 
-void kclvm_context_set_kcl_pkgpath(kclvm_context_t* p, int8_t* pkgpath);
+void kclvm_context_set_kcl_pkgpath(kclvm_context_t* p, char* pkgpath);
 
 void kclvm_context_set_list_option_mode(kclvm_context_t* p, kclvm_bool_t v);
 
@@ -422,9 +422,9 @@ kclvm_value_ref_t* kclvm_net_to_IP4(kclvm_context_t* ctx, kclvm_value_ref_t* arg
 
 void kclvm_plugin_init(void* fn_ptr);
 
-kclvm_value_ref_t* kclvm_plugin_invoke(kclvm_context_t* ctx, int8_t* method, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs);
+kclvm_value_ref_t* kclvm_plugin_invoke(kclvm_context_t* ctx, char* method, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs);
 
-char* kclvm_plugin_invoke_json(int8_t* method, char* args, char* kwargs);
+char* kclvm_plugin_invoke_json(char* method, char* args, char* kwargs);
 
 kclvm_value_ref_t* kclvm_regex_compile(kclvm_context_t* ctx, kclvm_value_ref_t* args, kclvm_value_ref_t* _kwargs);
 

diff --git a/kclvm/runtime/src/_kclvm_api_spec.rs b/kclvm/runtime/src/_kclvm_api_spec.rs
@@ -19,15 +19,15 @@
 // api-spec(llvm): declare %kclvm_value_ref_t* @kclvm_context_main_end_hook(%kclvm_context_t* %p, %kclvm_value_ref_t* %return_value);
 
 // api-spec:       kclvm_context_set_kcl_location
-// api-spec(c):    void kclvm_context_set_kcl_location(kclvm_context_t* p, int8_t* filename, int32_t line, int32_t col);
+// api-spec(c):    void kclvm_context_set_kcl_location(kclvm_context_t* p, char* filename, int32_t line, int32_t col);
 // api-spec(llvm): declare void @kclvm_context_set_kcl_location(%kclvm_context_t* %p, i8* %filename, i32 %line, i32 %col);
 
 // api-spec:       kclvm_context_set_kcl_pkgpath
-// api-spec(c):    void kclvm_context_set_kcl_pkgpath(kclvm_context_t* p, int8_t* pkgpath);
+// api-spec(c):    void kclvm_context_set_kcl_pkgpath(kclvm_context_t* p, char* pkgpath);
 // api-spec(llvm): declare void @kclvm_context_set_kcl_pkgpath(%kclvm_context_t* %p, i8* %pkgpath);
 
 // api-spec:       kclvm_context_set_kcl_filename
-// api-spec(c):    void kclvm_context_set_kcl_filename(kclvm_context_t* ctx, int8_t* filename);
+// api-spec(c):    void kclvm_context_set_kcl_filename(kclvm_context_t* ctx, char* filename);
 // api-spec(llvm): declare void @kclvm_context_set_kcl_filename(%kclvm_context_t* %ctx, i8* %filename);
 
 // api-spec:       kclvm_context_set_kcl_line_col
@@ -967,7 +967,7 @@
 // api-spec(llvm): declare void @kclvm_assert(%kclvm_context_t* %ctx, %kclvm_value_ref_t* %value, %kclvm_value_ref_t* %msg);
 
 // api-spec:       kclvm_builtin_option_init
-// api-spec(c):    void kclvm_builtin_option_init(kclvm_context_t* ctx, int8_t* key, int8_t* value);
+// api-spec(c):    void kclvm_builtin_option_init(kclvm_context_t* ctx, char* key, char* value);
 // api-spec(llvm): declare void @kclvm_builtin_option_init(%kclvm_context_t* %ctx, i8* %key, i8* %value);
 
 // api-spec:       kclvm_builtin_option_reset
@@ -1087,11 +1087,11 @@
 // api-spec(llvm): declare void @kclvm_plugin_init(i8* %fn_ptr);
 
 // api-spec:       kclvm_plugin_invoke
-// api-spec(c):    kclvm_value_ref_t* kclvm_plugin_invoke(kclvm_context_t* ctx, int8_t* method, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs);
+// api-spec(c):    kclvm_value_ref_t* kclvm_plugin_invoke(kclvm_context_t* ctx, char* method, kclvm_value_ref_t* args, kclvm_value_ref_t* kwargs);
 // api-spec(llvm): declare %kclvm_value_ref_t* @kclvm_plugin_invoke(%kclvm_context_t* %ctx, i8* %method, %kclvm_value_ref_t* %args, %kclvm_value_ref_t* %kwargs);
 
 // api-spec:       kclvm_plugin_invoke_json
-// api-spec(c):    char* kclvm_plugin_invoke_json(int8_t* method, char* args, char* kwargs);
+// api-spec(c):    char* kclvm_plugin_invoke_json(char* method, char* args, char* kwargs);
 // api-spec(llvm): declare i8* @kclvm_plugin_invoke_json(i8* %method, i8* %args, i8* %kwargs);
 
 // api-spec:       kclvm_units_to_n

diff --git a/kclvm/runtime/src/value/api.rs b/kclvm/runtime/src/value/api.rs
@@ -1086,18 +1086,23 @@ pub unsafe extern "C" fn kclvm_dict_set_value(
     let val = ptr_as_ref(val);
     if p.is_config() {
         p.dict_update_key_value(key, val.clone());
-    }
-    if p.is_schema() {
-        let schema: ValueRef;
-        {
-            let schema_value = p.as_schema();
-            let mut config_keys = schema_value.config_keys.clone();
-            config_keys.push(key.to_string());
-            schema = resolve_schema(mut_ptr_as_ref(ctx), p, &config_keys);
+        if p.is_schema() {
+            let schema: ValueRef;
+            {
+                let schema_value = p.as_schema();
+                let mut config_keys = schema_value.config_keys.clone();
+                config_keys.push(key.to_string());
+                schema = resolve_schema(mut_ptr_as_ref(ctx), p, &config_keys);
+            }
+            p.schema_update_with_schema(&schema);
         }
-        p.schema_update_with_schema(&schema);
+    } else {
+        panic!(
+            "failed to update the dict. An iterable of key-value pairs was expected, but got {}. Check if the syntax for updating the dictionary with the attribute '{}' is correct",
+            p.type_str(),
+            key
+        );
     }
-    /*panic*/
 }
 
 #[no_mangle]

diff --git a/kclvm/runtime/src/value/val_dict.rs b/kclvm/runtime/src/value/val_dict.rs
@@ -248,7 +248,11 @@ impl ValueRef {
             Value::schema_value(schema) => {
                 schema.config.values.insert(key.to_string(), val);
             }
-            _ => panic!("invalid dict update value: {}", self.type_str()),
+            _ => panic!(
+                "failed to update the dict. An iterable of key-value pairs was expected, but got {}. Check if the syntax for updating the dictionary with the attribute '{}' is correct",
+                self.type_str(),
+                key
+            ),
         }
     }
 

diff --git a/kclvm/runtime/src/value/val_schema.rs b/kclvm/runtime/src/value/val_schema.rs
@@ -134,7 +134,7 @@ impl ValueRef {
         let attr_map = match &*binding {
             Value::schema_value(schema) => &schema.config.values,
             Value::dict_value(schema) => &schema.values,
-            _ => panic!("Invalid schema/dict value, got {}", self.type_str()),
+            _ => panic!("invalid schema or dict value, got {}", self.type_str()),
         };
         let optional_mapping = self.schema_optional_mapping();
         let optional_mapping_ref = optional_mapping.rc.borrow();

diff --git a/kclvm/runtime/src/value/val_union.rs b/kclvm/runtime/src/value/val_union.rs
@@ -227,7 +227,11 @@ impl ValueRef {
         }
         if union_schema {
             let result = self.clone();
-            let optional_mapping = self.schema_optional_mapping();
+            let optional_mapping = if self.is_schema() {
+                self.schema_optional_mapping()
+            } else {
+                x.schema_optional_mapping()
+            };
             let schema = result.dict_to_schema(
                 name.as_str(),
                 pkgpath.as_str(),

diff --git a/kclvm/sema/src/resolver/schema.rs b/kclvm/sema/src/resolver/schema.rs
@@ -74,6 +74,7 @@ impl<'ctx> Resolver<'ctx> {
         if let (Some(index_signature), Some(index_signature_node)) =
             (scope_ty.index_signature, &schema_stmt.index_signature)
         {
+            // Insert the schema index signature key name into the scope.
             if let Some(key_name) = index_signature.key_name {
                 let (start, end) = index_signature_node.get_span_pos();
                 self.insert_object(
@@ -88,6 +89,17 @@ impl<'ctx> Resolver<'ctx> {
                     },
                 )
             }
+            // Check index signature default value type.
+            if let Some(value) = &index_signature_node.node.value {
+                let expected_ty = index_signature.val_ty;
+                let value_ty = self.expr(value);
+                self.must_assignable_to(
+                    value_ty,
+                    expected_ty,
+                    index_signature_node.get_span_pos(),
+                    None,
+                );
+            }
         }
         let schema_attr_names = schema_stmt.get_left_identifier_list();
         for (line, column, name) in schema_attr_names {

diff --git a/kclvm/sema/src/resolver/test_fail_data/unmatched_index_sign_default_value.k b/kclvm/sema/src/resolver/test_fail_data/unmatched_index_sign_default_value.k
@@ -0,0 +1,5 @@
+schema WithComponent:
+    name: str
+
+schema WithComponents:
+    [name: str]: WithComponent = 1
diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs
@@ -155,6 +155,7 @@ fn test_resolve_program_fail() {
         "mutable_error_1.k",
         "unique_key_error_0.k",
         "unique_key_error_1.k",
+        "unmatched_index_sign_default_value.k",
         "unmatched_args.k",
     ];
     for case in cases {

diff --git a/kclvm/sema/src/resolver/ty.rs b/kclvm/sema/src/resolver/ty.rs
@@ -237,7 +237,7 @@ impl<'ctx> Resolver<'ctx> {
         range: &Range,
     ) -> bool {
         if let Some(index_signature) = &schema_ty.index_signature {
-            if !assignable_to(val_ty.clone(), index_signature.val_ty.clone()) {
+            if !self.check_type(val_ty.clone(), index_signature.val_ty.clone(), range) {
                 self.handler.add_type_error(
                     &format!(
                         "expected schema index signature value type {}, got {}",
@@ -248,8 +248,8 @@ impl<'ctx> Resolver<'ctx> {
                 );
             }
             if index_signature.any_other {
-                return assignable_to(key_ty, index_signature.key_ty.clone())
-                    && assignable_to(val_ty, index_signature.val_ty.clone());
+                return self.check_type(key_ty, index_signature.key_ty.clone(), range)
+                    && self.check_type(val_ty, index_signature.val_ty.clone(), range);
             }
             true
         } else {

diff --git a/test/grammar/schema/assign_stmt/fail_0/main.k b/test/grammar/schema/assign_stmt/fail_0/main.k
@@ -0,0 +1,4 @@
+schema Schema:
+    a.b = 1
+
+s = Schema {}
diff --git a/test/grammar/schema/assign_stmt/fail_0/stderr.golden.py b/test/grammar/schema/assign_stmt/fail_0/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=cwd + "/main.k",
+                                    line_no=2,
+                                ),
+                            ],
+                            arg_msg="failed to update the dict. An iterable of key-value pairs was expected, but got UndefinedType. Check if the syntax for updating the dictionary with the attribute 'b' is correct")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/index_signature/fail_10/main.k b/test/grammar/schema/index_signature/fail_10/main.k
@@ -0,0 +1,10 @@
+schema WithComponent:
+    name: str
+    $type: str
+
+schema WithComponents:
+    [name: str]: WithComponent = WithComponent {name: name}
+
+components: WithComponents {
+    ws: {}
+}
diff --git a/test/grammar/schema/index_signature/fail_10/stderr.golden.py b/test/grammar/schema/index_signature/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=cwd + "/main.k",
+                                    line_no=9,
+                                ),
+                            ],
+                            arg_msg="attribute 'type' of WithComponent is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/index_signature/fail_11/main.k b/test/grammar/schema/index_signature/fail_11/main.k
@@ -0,0 +1,10 @@
+schema WithComponent:
+    name: str
+    $type: str
+
+schema WithComponents:
+    [name: str]: WithComponent = WithComponent {name: name}
+
+components: WithComponents {
+    ws = {}
+}
diff --git a/test/grammar/schema/index_signature/fail_11/stderr.golden.py b/test/grammar/schema/index_signature/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=cwd + "/main.k",
+                                    line_no=9,
+                                ),
+                            ],
+                            arg_msg="attribute 'name' of WithComponent is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/index_signature/fail_7/main.k b/test/grammar/schema/index_signature/fail_7/main.k
@@ -0,0 +1,10 @@
+schema WithComponent:
+    name: str
+    $type: str
+
+schema WithComponents:
+    [name: str]: WithComponent = {name: name}
+
+components: WithComponents {
+    ws: {}
+}
diff --git a/test/grammar/schema/index_signature/fail_7/stderr.golden.py b/test/grammar/schema/index_signature/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=cwd + "/main.k",
+                                    line_no=9,
+                                ),
+                            ],
+                            arg_msg="attribute 'type' of WithComponent is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/index_signature/fail_8/main.k b/test/grammar/schema/index_signature/fail_8/main.k
@@ -0,0 +1,10 @@
+schema WithComponent:
+    name: str
+    $type: str
+
+schema WithComponents:
+    [name: str]: WithComponent = {name: name}
+
+components: WithComponents {
+    ws: WithComponent {}
+}
diff --git a/test/grammar/schema/index_signature/fail_8/stderr.golden.py b/test/grammar/schema/index_signature/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=cwd + "/main.k",
+                                    line_no=9,
+                                ),
+                            ],
+                            arg_msg="attribute 'type' of WithComponent is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/index_signature/fail_9/main.k b/test/grammar/schema/index_signature/fail_9/main.k
@@ -0,0 +1,10 @@
+schema WithComponent:
+    name: str
+    $type: str
+
+schema WithComponents:
+    [name: str]: WithComponent = WithComponent {name: name}
+
+components: WithComponents {
+    ws: WithComponent {}
+}
diff --git a/test/grammar/schema/index_signature/fail_9/stderr.golden.py b/test/grammar/schema/index_signature/fail_9/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=cwd + "/main.k",
+                                    line_no=9,
+                                ),
+                            ],
+                            arg_msg="attribute 'type' of WithComponent is required and can't be None or Undefined")
+    , file=sys.stdout
+)
diff --git a/test/grammar/schema/index_signature/key_alias_1/main.k b/test/grammar/schema/index_signature/key_alias_1/main.k
@@ -1,6 +1,6 @@
 schema TeamSpec:
     fullName: str
-    name = id
+    name: str = id
     shortName: str = name
 
 schema TeamMap: