Fix nest schema expr type check (#1475)

* fix: fix nest schema expr type check Signed-off-by: he1pa <[email protected]> * fix: fix schema index signature check and schema symbol init in advanced_resolver Signed-off-by: he1pa <[email protected]> --------- Signed-off-by: he1pa <[email protected]>
kcl-lang · Jul 4, 2024 · 4a5951e · 4a5951e
1 parent 34d0e46
commit 4a5951e
Show file tree

Hide file tree

Showing 12 changed files with 165 additions and 12 deletions.
diff --git a/kclvm/sema/src/advanced_resolver/mod.rs b/kclvm/sema/src/advanced_resolver/mod.rs
@@ -49,8 +49,8 @@ use crate::{
     resolver::scope::{NodeKey, NodeTyMap},
 };
 
-use kclvm_ast::ast::AstIndex;
 use kclvm_ast::ast::Program;
+use kclvm_ast::ast::{AstIndex, Stmt};
 use kclvm_ast::walker::MutSelfTypedResultWalker;
 mod node;
 
@@ -113,6 +113,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
                 maybe_def: false,
             },
         };
+        // Scan all scehma symbol
         for (name, modules) in advanced_resolver.ctx.program.pkgs.iter() {
             if !advanced_resolver.gs.new_or_invalidate_pkgs.contains(name) {
                 continue;
@@ -130,6 +131,38 @@ impl<'ctx> AdvancedResolver<'ctx> {
                     pkg_info.pkg_filepath.clone(),
                     pkg_info.kfile_paths.clone(),
                 );
+                for module in modules.iter() {
+                    advanced_resolver.ctx.current_filename = Some(module.filename.clone());
+                    for stmt in &module.body {
+                        if matches!(stmt.node, Stmt::Schema(_)) {
+                            advanced_resolver.stmt(stmt)?;
+                        }
+                    }
+                }
+                advanced_resolver.leave_scope()
+            }
+        }
+
+        for (name, modules) in advanced_resolver.ctx.program.pkgs.iter() {
+            if !advanced_resolver.gs.new_or_invalidate_pkgs.contains(name) {
+                continue;
+            }
+            advanced_resolver.ctx.current_pkgpath = Some(name.clone());
+            if let Some(_) = advanced_resolver.gs.get_packages().get_package_info(name) {
+                if modules.is_empty() {
+                    continue;
+                }
+                if !advanced_resolver.ctx.scopes.is_empty() {
+                    advanced_resolver.ctx.scopes.clear();
+                }
+
+                let scope_ref = advanced_resolver
+                    .gs
+                    .get_scopes_mut()
+                    .get_root_scope(name.to_string())
+                    .unwrap();
+
+                advanced_resolver.ctx.scopes.push(scope_ref);
                 for module in modules.iter() {
                     advanced_resolver.ctx.current_filename = Some(module.filename.clone());
                     advanced_resolver.walk_module(module)?;

diff --git a/kclvm/sema/src/resolver/config.rs b/kclvm/sema/src/resolver/config.rs
@@ -88,12 +88,33 @@ impl<'ctx> Resolver<'ctx> {
                             )),
                             TypeKind::Schema(schema_ty) => {
                                 match schema_ty.get_obj_of_attr(key_name) {
-                                    Some(attr_ty_obj) => Some(self.new_config_expr_context_item(
-                                        key_name,
-                                        attr_ty_obj.ty.clone(),
-                                        attr_ty_obj.range.0.clone(),
-                                        attr_ty_obj.range.1.clone(),
-                                    )),
+                                    Some(attr_ty_obj) => {
+                                        let ty = match &attr_ty_obj.ty.kind {
+                                            TypeKind::Schema(schema_ty) => {
+                                                let runtime_type =
+                                                    kclvm_runtime::schema_runtime_type(
+                                                        &schema_ty.name,
+                                                        &schema_ty.pkgpath,
+                                                    );
+                                                if let Some(runtime_scehma_ty) =
+                                                    self.ctx.schema_mapping.get(&runtime_type)
+                                                {
+                                                    Arc::new(Type::schema(
+                                                        runtime_scehma_ty.borrow().clone(),
+                                                    ))
+                                                } else {
+                                                    attr_ty_obj.ty.clone()
+                                                }
+                                            }
+                                            _ => attr_ty_obj.ty.clone(),
+                                        };
+                                        Some(self.new_config_expr_context_item(
+                                            key_name,
+                                            ty,
+                                            attr_ty_obj.range.0.clone(),
+                                            attr_ty_obj.range.1.clone(),
+                                        ))
+                                    }
                                     None => match &schema_ty.index_signature {
                                         Some(index_signature) => {
                                             Some(self.new_config_expr_context_item(

diff --git a/kclvm/sema/src/resolver/ty.rs b/kclvm/sema/src/resolver/ty.rs
@@ -352,23 +352,39 @@ impl<'ctx> Resolver<'ctx> {
             // empty dict {}
             TypeKind::Any => true,
             // single key: {key1: value1}
-            TypeKind::StrLit(s) => schema_ty.attrs.len() == 1 && schema_ty.attrs.contains_key(s),
+            TypeKind::StrLit(s) => schema_ty.attrs.len() >= 1 && schema_ty.attrs.contains_key(s),
             // multi key: {
             // key1: value1
             // key2: value2
             // ...
             // }
             TypeKind::Union(types) => {
-                schema_ty.attrs.len() == types.len()
-                    && types.iter().all(|ty| match &ty.kind {
-                        TypeKind::StrLit(s) => schema_ty.attrs.contains_key(s),
+                let (attrs, has_index_signature) = Self::get_schema_attrs(schema_ty);
+                match (attrs.len() >= types.len(), has_index_signature) {
+                    (true, _) => types.iter().all(|ty| match &ty.kind {
+                        TypeKind::StrLit(s) => attrs.contains(s),
                         _ => false,
-                    })
+                    }),
+                    // Todo: do more index_signature check
+                    (false, true) => true,
+                    (false, false) => false,
+                }
             }
             _ => false,
         }
     }
 
+    fn get_schema_attrs(schema_ty: &SchemaType) -> (Vec<String>, bool) {
+        let mut attrs: Vec<String> = schema_ty.attrs.keys().map(|attr| attr.clone()).collect();
+        let mut has_index_signature = schema_ty.index_signature.is_some();
+        if let Some(base) = &schema_ty.base {
+            let (base_attrs, index_signature) = Self::get_schema_attrs(base);
+            attrs.extend(base_attrs);
+            has_index_signature &= index_signature;
+        }
+        (attrs, has_index_signature)
+    }
+
     fn upgrade_named_ty_with_scope(
         &mut self,
         ty: TypeRef,

diff --git a/kclvm/tools/src/LSP/src/goto_def.rs b/kclvm/tools/src/LSP/src/goto_def.rs
@@ -434,4 +434,25 @@ mod tests {
         6,
         10
     );
+
+    goto_def_test_snapshot!(
+        goto_nested_schema_attr_test,
+        "src/test_data/goto_def_test/goto_nested_schema_attr_test/goto_nested_schema_attr_test.k",
+        22,
+        22
+    );
+
+    goto_def_test_snapshot!(
+        goto_base_schema_attr_test,
+        "src/test_data/goto_def_test/goto_base_schema_attr_test/goto_base_schema_attr_test.k",
+        8,
+        12
+    );
+
+    goto_def_test_snapshot!(
+        goto_base_schema_attr_1_test,
+        "src/test_data/goto_def_test/goto_base_schema_attr_1_test/goto_base_schema_attr_1_test.k",
+        4,
+        12
+    );
 }
diff --git a/...LSP/src/snapshots/kcl_language_server__goto_def__tests__goto_base_schema_attr_1_test.snap b/...LSP/src/snapshots/kcl_language_server__goto_def__tests__goto_base_schema_attr_1_test.snap
@@ -0,0 +1,5 @@
+---
+source: tools/src/LSP/src/goto_def.rs
+expression: "format!(\"{:?}\", { fmt_resp(& res) })"
+---
+"path: \"src/test_data/goto_def_test/goto_base_schema_attr_1_test/types/host.k\", range: Range { start: Position { line: 1, character: 4 }, end: Position { line: 1, character: 8 } }"
diff --git a/...c/LSP/src/snapshots/kcl_language_server__goto_def__tests__goto_base_schema_attr_test.snap b/...c/LSP/src/snapshots/kcl_language_server__goto_def__tests__goto_base_schema_attr_test.snap
@@ -0,0 +1,5 @@
+---
+source: tools/src/LSP/src/goto_def.rs
+expression: "format!(\"{:?}\", { fmt_resp(& res) })"
+---
+"path: \"src/test_data/goto_def_test/goto_base_schema_attr_test/goto_base_schema_attr_test.k\", range: Range { start: Position { line: 1, character: 4 }, end: Position { line: 1, character: 8 } }"
diff --git a/...LSP/src/snapshots/kcl_language_server__goto_def__tests__goto_nested_schema_attr_test.snap b/...LSP/src/snapshots/kcl_language_server__goto_def__tests__goto_nested_schema_attr_test.snap
@@ -0,0 +1,5 @@
+---
+source: tools/src/LSP/src/goto_def.rs
+expression: "format!(\"{:?}\", { fmt_resp(& res) })"
+---
+"path: \"src/test_data/goto_def_test/goto_nested_schema_attr_test/goto_nested_schema_attr_test.k\", range: Range { start: Position { line: 14, character: 4 }, end: Position { line: 14, character: 8 } }"
diff --git a/...P/src/test_data/goto_def_test/goto_base_schema_attr_1_test/goto_base_schema_attr_1_test.k b/...P/src/test_data/goto_def_test/goto_base_schema_attr_1_test/goto_base_schema_attr_1_test.k
@@ -0,0 +1,6 @@
+import types
+
+hosts: {str: types.HostPort} = {
+    foo: { host: "foo.example.net", port: 80}
+}
+
diff --git a/kclvm/tools/src/LSP/src/test_data/goto_def_test/goto_base_schema_attr_1_test/kcl.mod b/kclvm/tools/src/LSP/src/test_data/goto_def_test/goto_base_schema_attr_1_test/kcl.mod
diff --git a/kclvm/tools/src/LSP/src/test_data/goto_def_test/goto_base_schema_attr_1_test/types/host.k b/kclvm/tools/src/LSP/src/test_data/goto_def_test/goto_base_schema_attr_1_test/types/host.k
@@ -0,0 +1,5 @@
+schema Host:
+    host: str
+
+schema HostPort(Host):
+    port: int
diff --git a/...c/LSP/src/test_data/goto_def_test/goto_base_schema_attr_test/goto_base_schema_attr_test.k b/...c/LSP/src/test_data/goto_def_test/goto_base_schema_attr_test/goto_base_schema_attr_test.k
@@ -0,0 +1,9 @@
+schema Host:
+    host: str
+
+schema HostPort(Host):
+    port: int
+
+hosts: {str:HostPort} = {
+    foo: {host: "foo.example.net", port: 80}
+}
diff --git a/...P/src/test_data/goto_def_test/goto_nested_schema_attr_test/goto_nested_schema_attr_test.k b/...P/src/test_data/goto_def_test/goto_nested_schema_attr_test/goto_nested_schema_attr_test.k
@@ -0,0 +1,27 @@
+schema Foo:
+    data: Data
+
+
+schema Data:
+    spec: Spec
+
+schema Spec:
+    config: Config
+
+schema Config:
+    template: Template
+
+schema Template:
+    name: str
+
+foo = Foo {
+    data: {
+        spec: {
+            config: {
+                template: {
+                    name: "template"
+                }
+            }
+        }
+    }
+}