refactor: pkg import error message

Signed-off-by: peefy <[email protected]>
kcl-lang · Sep 9, 2024 · 1f8b5f5 · 1f8b5f5
1 parent def5a56
commit 1f8b5f5
Show file tree

Hide file tree

Showing 15 changed files with 41 additions and 58 deletions.
diff --git a/kclvm/api/src/testdata/parse-file.response.json b/kclvm/api/src/testdata/parse-file.response.json
@@ -21,7 +21,7 @@
             "code": "Suggestions",
             "messages": [
                 {
-                    "msg": "try 'kcl mod add data' to download the package not found",
+                    "msg": "try 'kcl mod add data' to download the missing package",
                     "pos": {
                         "line": 0,
                         "column": 0,
@@ -35,7 +35,7 @@
             "code": "Suggestions",
             "messages": [
                 {
-                    "msg": "find more package on 'https://artifacthub.io'",
+                    "msg": "browse more packages at 'https://artifacthub.io'",
                     "pos": {
                         "line": 0,
                         "column": 0,

diff --git a/kclvm/parser/src/entry.rs b/kclvm/parser/src/entry.rs
@@ -428,9 +428,9 @@ fn get_main_files_from_pkg_path(
 
     for (i, path) in path_list.iter().enumerate() {
         // read dir/*.k
-        if is_dir(path) {
+        if !path.is_empty() && is_dir(path) {
             if opts.k_code_list.len() > i {
-                return Err(anyhow::anyhow!("Invalid code list"));
+                return Err(anyhow::anyhow!("Invalid code list for the path {}", path));
             }
             // k_code_list
             for s in get_dir_files(path, false)? {

diff --git a/kclvm/parser/src/lib.rs b/kclvm/parser/src/lib.rs
@@ -505,13 +505,13 @@ impl Loader {
                     }],
                 );
                 let mut suggestions =
-                    vec![format!("find more package on 'https://artifacthub.io'")];
+                    vec![format!("browse more packages at 'https://artifacthub.io'")];
 
                 if let Ok(pkg_name) = parse_external_pkg_name(pkg_path) {
                     suggestions.insert(
                         0,
                         format!(
-                            "try 'kcl mod add {}' to download the package not found",
+                            "try 'kcl mod add {}' to download the missing package",
                             pkg_name
                         ),
                     );
@@ -626,11 +626,6 @@ impl Loader {
             return Ok(Some(pkg_info));
         }
 
-        if pkg_info.k_files.is_empty() {
-            self.missing_pkgs.push(pkgpath);
-            return Ok(Some(pkg_info));
-        }
-
         if !self.opts.load_packages {
             return Ok(Some(pkg_info));
         }
@@ -769,9 +764,9 @@ impl Loader {
         pkg_root: &str,
         pkg_path: &str,
     ) -> Result<Option<PkgInfo>> {
-        match self.pkg_exists(vec![pkg_root.to_string()], pkg_path) {
+        match self.pkg_exists(&[pkg_root.to_string()], pkg_path) {
             Some(internal_pkg_root) => {
-                let fullpath = if pkg_name == kclvm_ast::MAIN_PKG {
+                let full_pkg_path = if pkg_name == kclvm_ast::MAIN_PKG {
                     pkg_path.to_string()
                 } else {
                     format!("{}.{}", pkg_name, pkg_path)
@@ -780,7 +775,7 @@ impl Loader {
                 Ok(Some(PkgInfo::new(
                     pkg_name.to_string(),
                     internal_pkg_root,
-                    fullpath,
+                    full_pkg_path,
                     k_files,
                 )))
             }
@@ -800,7 +795,7 @@ impl Loader {
         let external_pkg_root = if let Some(root) = self.opts.package_maps.get(&pkg_name) {
             PathBuf::from(root).join(KCL_MOD_FILE)
         } else {
-            match self.pkg_exists(self.opts.vendor_dirs.clone(), pkg_path) {
+            match self.pkg_exists(&self.opts.vendor_dirs, pkg_path) {
                 Some(path) => PathBuf::from(path).join(&pkg_name).join(KCL_MOD_FILE),
                 None => return Ok(None),
             }
@@ -831,17 +826,17 @@ impl Loader {
     ///
     /// # Notes
     ///
-    /// All paths in [`pkgpath`] must contain the kcl.mod file.
     /// It returns the parent directory of kcl.mod if present, or none if not.
-    fn pkg_exists(&self, pkgroots: Vec<String>, pkgpath: &str) -> Option<String> {
+    fn pkg_exists(&self, pkgroots: &[String], pkgpath: &str) -> Option<String> {
         pkgroots
             .into_iter()
-            .find(|root| self.pkg_exists_in_path(root.to_string(), pkgpath))
+            .find(|root| self.pkg_exists_in_path(root, pkgpath))
+            .cloned()
     }
 
     /// Search for [`pkgpath`] under [`path`].
-    /// It only returns [`true`] if [`path`]/[`pkgpath`] or [`path`]/[`kcl.mod`] exists.
-    fn pkg_exists_in_path(&self, path: String, pkgpath: &str) -> bool {
+    /// It only returns [`true`] if [`path`]/[`pkgpath`] or [`path`]/[`pkgpath.k`] exists.
+    fn pkg_exists_in_path(&self, path: &str, pkgpath: &str) -> bool {
         let mut pathbuf = PathBuf::from(path);
         pkgpath.split('.').for_each(|s| pathbuf.push(s));
         pathbuf.exists() || pathbuf.with_extension(KCL_FILE_EXTENSION).exists()

diff --git a/kclvm/parser/src/tests.rs b/kclvm/parser/src/tests.rs
@@ -376,8 +376,8 @@ pub fn test_import_vendor_without_vendor_home() {
             let errors = sess.classification().0;
             let msgs = [
                 "pkgpath assign not found in the program",
-                "try 'kcl mod add assign' to download the package not found",
-                "find more package on 'https://artifacthub.io'",
+                "try 'kcl mod add assign' to download the missing package",
+                "browse more packages at 'https://artifacthub.io'",
                 "pkgpath assign.assign not found in the program",
             ];
             assert_eq!(errors.len(), msgs.len());
@@ -400,8 +400,8 @@ pub fn test_import_vendor_without_vendor_home() {
             let errors = sess.classification().0;
             let msgs = [
                 "pkgpath assign not found in the program",
-                "try 'kcl mod add assign' to download the package not found",
-                "find more package on 'https://artifacthub.io'",
+                "try 'kcl mod add assign' to download the missing package",
+                "browse more packages at 'https://artifacthub.io'",
                 "pkgpath assign.assign not found in the program",
             ];
             assert_eq!(errors.len(), msgs.len());
@@ -724,8 +724,8 @@ pub fn test_pkg_not_found_suggestion() {
             let errors = sess.classification().0;
             let msgs = [
                 "pkgpath k9s not found in the program",
-                "try 'kcl mod add k9s' to download the package not found",
-                "find more package on 'https://artifacthub.io'",
+                "try 'kcl mod add k9s' to download the missing package",
+                "browse more packages at 'https://artifacthub.io'",
             ];
             assert_eq!(errors.len(), msgs.len());
             for (diag, m) in errors.iter().zip(msgs.iter()) {

diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs
@@ -44,9 +44,9 @@ impl<'ctx> Resolver<'ctx> {
                                         range: stmt.get_span_pos(),
                                         style: Style::Line,
                                         message: format!(
-                                            "Cannot import the module {} from {}, attempted import folder with no kcl files",
+                                            "Failed to import the module {} from {}. No KCL files found in the specified folder",
                                             import_stmt.rawpath,
-                                            real_path.to_str().unwrap()
+                                            real_path.to_str().unwrap(),
                                         ),
                                         note: None,
                                         suggested_replacement: None,
@@ -67,14 +67,15 @@ impl<'ctx> Resolver<'ctx> {
                                         suggested_replacement: None,
                                     }],
                                 );
-                                let mut suggestions =
-                                    vec![format!("find more package on 'https://artifacthub.io'")];
+                                let mut suggestions = vec![format!(
+                                    "browse more packages at 'https://artifacthub.io'"
+                                )];
 
                                 if let Ok(pkg_name) = parse_external_pkg_name(pkgpath) {
                                     suggestions.insert(
                                         0,
                                         format!(
-                                            "try 'kcl mod add {}' to download the package not found",
+                                            "try 'kcl mod add {}' to download the missing package",
                                             pkg_name
                                         ),
                                     );

diff --git a/kclvm/sema/src/resolver/node.rs b/kclvm/sema/src/resolver/node.rs
@@ -29,8 +29,8 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
 
     fn walk_expr_stmt(&mut self, expr_stmt: &'ctx ast::ExprStmt) -> Self::Result {
         let expr_types = self.exprs(&expr_stmt.exprs);
-        if !expr_types.is_empty() {
-            let ty = expr_types.last().unwrap().clone();
+        if let Some(last) = expr_types.last() {
+            let ty = last.clone();
             if expr_types.len() > 1 {
                 self.handler.add_compile_error(
                     "expression statement can only have one expression",
@@ -296,9 +296,9 @@ impl<'ctx> MutSelfTypedResultWalker<'ctx> for Resolver<'ctx> {
 
     fn walk_if_stmt(&mut self, if_stmt: &'ctx ast::IfStmt) -> Self::Result {
         self.expr(&if_stmt.cond);
-        self.stmts(&if_stmt.body);
-        self.stmts(&if_stmt.orelse);
-        self.any_ty()
+        let if_ty = self.stmts(&if_stmt.body);
+        let orelse_ty = self.stmts(&if_stmt.orelse);
+        sup(&[if_ty, orelse_ty])
     }
 
     fn walk_import_stmt(&mut self, _import_stmt: &'ctx ast::ImportStmt) -> Self::Result {

diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs
@@ -749,14 +749,14 @@ fn test_pkg_not_found_suggestion() {
     assert_eq!(diag.messages.len(), 1);
     assert_eq!(
         diag.messages[0].message,
-        "try 'kcl mod add k9s' to download the package not found"
+        "try 'kcl mod add k9s' to download the missing package"
     );
     let diag = &scope.handler.diagnostics[2];
     assert_eq!(diag.code, Some(DiagnosticId::Suggestions));
     assert_eq!(diag.messages.len(), 1);
     assert_eq!(
         diag.messages[0].message,
-        "find more package on 'https://artifacthub.io'"
+        "browse more packages at 'https://artifacthub.io'"
     );
 }
 

diff --git a/kclvm/tools/src/lint/tests.rs b/kclvm/tools/src/lint/tests.rs
@@ -24,8 +24,8 @@ fn test_lint() {
 
     let msgs = [
         "pkgpath abc not found in the program",
-        "try 'kcl mod add abc' to download the package not found",
-        "find more package on 'https://artifacthub.io'",
+        "try 'kcl mod add abc' to download the missing package",
+        "browse more packages at 'https://artifacthub.io'",
         &format!("Cannot find the module abc from {}", path.to_str().unwrap()),
     ];
     assert_eq!(

diff --git a/test/grammar/import/empty_import_fail/kcl.mod b/test/grammar/import/empty_import_fail/kcl.mod
diff --git a/test/grammar/import/empty_import_fail/main.k b/test/grammar/import/empty_import_fail/main.k
diff --git a/test/grammar/import/empty_import_fail/pkg_empty/1.txt b/test/grammar/import/empty_import_fail/pkg_empty/1.txt
diff --git a/test/grammar/import/empty_import_fail/stderr.golden b/test/grammar/import/empty_import_fail/stderr.golden
diff --git a/test/grammar/import/import_abs_fail_0/app-main/stderr.golden b/test/grammar/import/import_abs_fail_0/app-main/stderr.golden
@@ -4,8 +4,8 @@ error[E2F04]: CannotFindModule
 1 | import .some0.pkg1 as some00  # some0 not found in app-main package
   | ^ Cannot find the module .some0.pkg1 from ${CWD}/some0/pkg1
   |
-suggestion: try 'kcl mod add app-main' to download the package not found
-suggestion: find more package on 'https://artifacthub.io'
+suggestion: try 'kcl mod add app-main' to download the missing package
+suggestion: browse more packages at 'https://artifacthub.io'
 error[E2G22]: TypeError
  --> ${CWD}/main.k:3:9
   |

diff --git a/test/grammar/import/import_abs_fail_1/app-main/stderr.golden b/test/grammar/import/import_abs_fail_1/app-main/stderr.golden
@@ -1 +1 @@
-attempted import folder with no kcl files
+No KCL files found in the specified folder
diff --git a/test/grammar/import/import_syntax_error_0/app-main/stderr.golden b/test/grammar/import/import_syntax_error_0/app-main/stderr.golden
@@ -10,8 +10,8 @@ error[E2F04]: CannotFindModule
 1 | import .some0..pkg1 as some00  # some0 not found in app-main package
   | ^ Cannot find the module .some0..pkg1 from ${CWD}/some0//pkg1
   |
-suggestion: try 'kcl mod add app-main' to download the package not found
-suggestion: find more package on 'https://artifacthub.io'
+suggestion: try 'kcl mod add app-main' to download the missing package
+suggestion: browse more packages at 'https://artifacthub.io'
 error[E2G22]: TypeError
  --> ${CWD}/main.k:3:9
   |