Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: pkg import error message #1631

Merged
merged 1 commit into from
Sep 9, 2024
Merged

refactor: pkg import error message #1631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions kclvm/api/src/testdata/parse-file.response.json
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 removed test/grammar/import/empty_import_fail/kcl.mod
View file
Open in desktop
Empty file.
6 changes: 0 additions & 6 deletions test/grammar/import/empty_import_fail/main.k
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion test/grammar/import/empty_import_fail/pkg_empty/1.txt
View file
Open in desktop

This file was deleted.

6 changes: 0 additions & 6 deletions test/grammar/import/empty_import_fail/stderr.golden
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions test/grammar/import/import_abs_fail_0/app-main/stderr.golden
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion test/grammar/import/import_abs_fail_1/app-main/stderr.golden
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions test/grammar/import/import_syntax_error_0/app-main/stderr.golden
View file
Open in desktop
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
Loading