Skip to content

Commit

Permalink
refactor: pkg import error message (kcl-lang#1631)
Browse files Browse the repository at this point in the history
Signed-off-by: peefy <[email protected]>
  • Loading branch information
Peefy authored Sep 9, 2024
1 parent c2ff2d3 commit d5f4a27
Show file tree
Hide file tree
Showing 15 changed files with 41 additions and 58 deletions.
4 changes: 2 additions & 2 deletions kclvm/api/src/testdata/parse-file.response.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions kclvm/parser/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)? {
Expand Down
27 changes: 11 additions & 16 deletions kclvm/parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
),
);
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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)
Expand All @@ -780,7 +775,7 @@ impl Loader {
Ok(Some(PkgInfo::new(
pkg_name.to_string(),
internal_pkg_root,
fullpath,
full_pkg_path,
k_files,
)))
}
Expand All @@ -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),
}
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions kclvm/parser/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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()) {
Expand Down
11 changes: 6 additions & 5 deletions kclvm/sema/src/resolver/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
),
);
Expand Down
10 changes: 5 additions & 5 deletions kclvm/sema/src/resolver/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions kclvm/sema/src/resolver/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
);
}

Expand Down
4 changes: 2 additions & 2 deletions kclvm/tools/src/lint/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
Empty file.
6 changes: 0 additions & 6 deletions test/grammar/import/empty_import_fail/main.k

This file was deleted.

1 change: 0 additions & 1 deletion test/grammar/import/empty_import_fail/pkg_empty/1.txt

This file was deleted.

6 changes: 0 additions & 6 deletions test/grammar/import/empty_import_fail/stderr.golden

This file was deleted.

4 changes: 2 additions & 2 deletions test/grammar/import/import_abs_fail_0/app-main/stderr.golden
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
attempted import folder with no kcl files
No KCL files found in the specified folder
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down

0 comments on commit d5f4a27

Please sign in to comment.