fix: package variable resolve

Signed-off-by: peefy <[email protected]>
kcl-lang · Oct 10, 2024 · 038e8da · 038e8da
1 parent 3e84ea4
commit 038e8da
Show file tree

Hide file tree

Showing 12 changed files with 81 additions and 68 deletions.
diff --git a/kclvm/api/src/capi_test.rs b/kclvm/api/src/capi_test.rs
@@ -137,7 +137,7 @@ fn test_c_api_lint_path() {
 
 #[test]
 fn test_c_api_call_exec_program_with_compile_only() {
-    test_c_api_paniced::<ExecProgramArgs>(
+    test_c_api_panic::<ExecProgramArgs>(
         "KclvmService.ExecProgram",
         "exec-program-with-compile-only.json",
         "exec-program-with-compile-only.response.panic",
@@ -311,7 +311,7 @@ where
     }
 }
 
-fn test_c_api_paniced<A>(svc_name: &str, input: &str, output: &str)
+fn test_c_api_panic<A>(svc_name: &str, input: &str, output: &str)
 where
     A: Message + DeserializeOwned,
 {
@@ -341,7 +341,11 @@ where
                         except_result_path.display()
                     )
                 });
-            assert!(result.to_string_lossy().contains(&except_result_panic_msg));
+            assert!(
+                result.to_string_lossy().contains(&except_result_panic_msg),
+                "{}",
+                result.to_string_lossy()
+            );
             unsafe {
                 kclvm_service_delete(serv);
                 kclvm_service_free_string(result_ptr as *mut c_char);

diff --git a/kclvm/api/src/testdata/exec-program-with-compile-only.response.panic b/kclvm/api/src/testdata/exec-program-with-compile-only.response.panic
@@ -1 +1 @@
-Module 'external' imported but unused
+Module 'ext' imported but unused
diff --git a/kclvm/cmd/src/test_data/instances/test_inst_11/sub/main.k b/kclvm/cmd/src/test_data/instances/test_inst_11/sub/main.k
@@ -1,5 +1,5 @@
-import model as m
+import model as subm
 
-k11_inst: m.K11 {
+k11_inst: subm.K11 {
     msg: "k11_in_sub"
 }
diff --git a/kclvm/sema/src/lint/lints_def.rs b/kclvm/sema/src/lint/lints_def.rs
@@ -57,7 +57,7 @@ impl LintPass for ImportPosition {
                             range: stmt.get_span_pos(),
                             style: Style::Line,
                             message: format!(
-                                "Importstmt should be placed at the top of the module"
+                                "The import stmt should be placed at the top of the module"
                             ),
                             note: Some(
                                 "Consider moving tihs statement to the top of the file".to_string(),

diff --git a/kclvm/sema/src/pre_process/identifier.rs b/kclvm/sema/src/pre_process/identifier.rs
@@ -233,12 +233,12 @@ pub fn fix_qualified_identifier<'ctx>(
             import_names.insert(import_stmt.name.clone(), import_stmt.path.node.clone());
         }
     }
-    // 1. fix_global_ident
-    let mut global_names_walker = QualifiedIdentifierTransformer {
+    // 1. fix qualified identifier
+    let mut walker = QualifiedIdentifierTransformer {
         import_names: import_names.clone(),
         ..Default::default()
     };
-    global_names_walker.walk_module(module);
+    walker.walk_module(module);
 }
 
 /// Fix AST raw identifier prefix `$`, e.g., $filter -> filter

diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs
@@ -119,57 +119,63 @@ impl<'ctx> Resolver<'ctx> {
         let modules = self.program.pkgs.get(&self.ctx.pkgpath);
         match modules {
             Some(modules) => {
+                let mut import_table: IndexMap<String, String> = IndexMap::default();
                 for module in modules {
                     self.ctx.filename = module.filename.clone();
                     self.ctx.pkgpath = module.pkg.clone();
                     for stmt in &module.body {
                         if let ast::Stmt::Import(import_stmt) = &stmt.node {
-                            {
-                                match self.ctx.import_names.get_mut(&self.ctx.filename) {
-                                    Some(mapping) => {
-                                        // 'import sub as s' and 'import sub.sub as s' will raise this error.
-                                        // 'import sub' and 'import sub' will not raise this error.
-                                        // 'import sub as s' and 'import sub as s' will not raise this error.
-                                        if let Some(path) = mapping.get(&import_stmt.name) {
-                                            if path != &import_stmt.path.node {
-                                                self.handler.add_compile_error(
-                                                    &format!(
-                                                        "the name '{}' is defined multiple times, '{}' must be defined only once",
-                                                        import_stmt.name, import_stmt.name
-                                                    ),
-                                                    stmt.get_span_pos(),
-                                                );
-                                            }
-                                        }
-                                        mapping.insert(
-                                            import_stmt.name.to_string(),
-                                            import_stmt.path.node.to_string(),
-                                        );
-                                    }
-                                    None => {
-                                        let mut mapping = IndexMap::default();
-                                        mapping.insert(
-                                            import_stmt.name.to_string(),
-                                            import_stmt.path.node.to_string(),
-                                        );
-                                        self.ctx
-                                            .import_names
-                                            .insert(self.ctx.filename.clone(), mapping);
-                                    }
+                            // 'import sub as s' and 'import sub.sub as s' will raise this error.
+                            // 'import sub' and 'import sub' will not raise this error.
+                            // 'import sub as s' and 'import sub as s' will not raise this error.
+                            if let Some(path) = import_table.get(&import_stmt.name) {
+                                if path != &import_stmt.path.node {
+                                    self.handler.add_compile_error(
+                                        &format!(
+                                            "the name '{}' is defined multiple times, '{}' must be defined only once",
+                                            import_stmt.name, import_stmt.name
+                                        ),
+                                        stmt.get_span_pos(),
+                                    );
                                 }
+                            } else {
+                                import_table.insert(
+                                    import_stmt.name.clone(),
+                                    import_stmt.path.node.clone(),
+                                );
+                            }
+                            match self.ctx.import_names.get_mut(&self.ctx.filename) {
+                                Some(mapping) => {
+                                    mapping.insert(
+                                        import_stmt.name.to_string(),
+                                        import_stmt.path.node.to_string(),
+                                    );
+                                }
+                                None => {
+                                    let mut mapping = IndexMap::default();
+                                    mapping.insert(
+                                        import_stmt.name.to_string(),
+                                        import_stmt.path.node.to_string(),
+                                    );
+                                    self.ctx
+                                        .import_names
+                                        .insert(self.ctx.filename.clone(), mapping);
+                                }
+                            }
+                            {
                                 let mut scope = self.scope.borrow_mut();
-                                let is_user_module = match scope.elems.get(&import_stmt.path.node) {
+                                let is_user_module = match scope.elems.get(&import_stmt.name) {
                                     Some(scope_obj) => {
                                         let mut obj = scope_obj.borrow_mut();
                                         match &mut obj.kind {
-                                                    ScopeObjectKind::Module(m) => {
-                                                        m.import_stmts.push((stmt.clone(), false))
-                                                    },
-                                                    _ => bug!(
-                                                        "invalid module type in the import check function {}",
-                                                        scope_obj.borrow().ty.ty_str()
-                                                    )
-                                                }
+                                                ScopeObjectKind::Module(m) => {
+                                                    m.import_stmts.push((stmt.clone(), false))
+                                                },
+                                                _ => bug!(
+                                                    "invalid module type in the import check function {}",
+                                                    scope_obj.borrow().ty.ty_str()
+                                                )
+                                            }
                                         match &obj.ty.kind {
                                             TypeKind::Module(module_ty) => {
                                                 let mut module_ty = module_ty.clone();
@@ -211,9 +217,9 @@ impl<'ctx> Resolver<'ctx> {
                                         let (start, end) = stmt.get_span_pos();
 
                                         scope.elems.insert(
-                                            import_stmt.path.node.to_string(),
+                                            import_stmt.name.clone(),
                                             Rc::new(RefCell::new(ScopeObject {
-                                                name: import_stmt.path.node.to_string(),
+                                                name: import_stmt.name.clone(),
                                                 start,
                                                 end,
                                                 ty: Arc::new(ty),

diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs
@@ -414,7 +414,7 @@ fn test_lint() {
                 },
             ),
             style: Style::Line,
-            message: format!("Importstmt should be placed at the top of the module"),
+            message: format!("The import stmt should be placed at the top of the module"),
             note: Some("Consider moving tihs statement to the top of the file".to_string()),
             suggested_replacement: None,
         }],
@@ -456,7 +456,7 @@ fn test_lint() {
                 },
             ),
             style: Style::Line,
-            message: format!("Module 'import_test.a' imported but unused"),
+            message: format!("Module 'a' imported but unused"),
             note: Some("Consider removing this statement".to_string()),
             suggested_replacement: None,
         }],

diff --git a/kclvm/sema/src/resolver/var.rs b/kclvm/sema/src/resolver/var.rs
@@ -113,7 +113,7 @@ impl<'ctx> Resolver<'ctx> {
             // Lookup pkgpath scope object and record it as "used". When enter child scope, e.g., in a schema scope, cant find module object.
             // It should be recursively search whole scope to lookup scope object, not the current scope.element.
             if !pkgpath.is_empty() {
-                if let Some(obj) = self.scope.borrow().lookup(pkgpath) {
+                if let Some(obj) = self.scope.borrow().lookup(&names[0]) {
                     if let ScopeObjectKind::Module(m) = &mut obj.borrow_mut().kind {
                         for (stmt, used) in m.import_stmts.iter_mut() {
                             if stmt.get_pos().filename == range.0.filename {
@@ -124,15 +124,7 @@ impl<'ctx> Resolver<'ctx> {
                 }
             }
             // Load type
-            let mut tys = self.resolve_var(
-                &[if !pkgpath.is_empty() {
-                    pkgpath.to_string()
-                } else {
-                    names[0].clone()
-                }],
-                pkgpath,
-                range.clone(),
-            );
+            let mut tys = self.resolve_var(&[names[0].clone()], pkgpath, range.clone());
             let mut ty = tys[0].clone();
 
             for name in &names[1..] {

diff --git a/kclvm/tools/src/lint/tests.rs b/kclvm/tools/src/lint/tests.rs
@@ -5,7 +5,7 @@ use std::path::PathBuf;
 fn test_lint() {
     let (errors, warnings) = lint_files(&["./src/lint/test_data/lint.k"], None);
     let msgs = [
-        "Importstmt should be placed at the top of the module",
+        "The import stmt should be placed at the top of the module",
         "Module 'a' is reimported multiple times",
         "Module 'import_test.a' imported but unused",
         "Module 'import_test.a' imported but unused",

diff --git a/test/grammar/import/import_name_same_with_schema_attr/main.k b/test/grammar/import/import_name_same_with_schema_attr/main.k
@@ -0,0 +1,9 @@
+import file as libfile
+
+schema FilesSchema:
+    file: str
+
+    check:
+        libfile.exists(file) if file, "file not found ${file}"
+
+v = FilesSchema {file = ""}
diff --git a/test/grammar/import/import_name_same_with_schema_attr/stdout.golden b/test/grammar/import/import_name_same_with_schema_attr/stdout.golden
@@ -0,0 +1,2 @@
+v:
+  file: ''
diff --git a/test/grammar/import/import_same_as_name_0/pkg/temp.k b/test/grammar/import/import_same_as_name_0/pkg/temp.k
@@ -1,6 +1,6 @@
-import pkg.core.v1 as v1
+import pkg.core.v1 as corev1
 
 schema CafeDeploy:
-    data: v1.Deploy = v1.Deploy {
+    data: corev1.Deploy = corev1.Deploy {
         name: "deploy"
     }