refactor: refine parse and resolve function error returned and add te…

…st cases (#508)

refactor: refine parse and resolve function error returned and add test cases.

kclvm/api/src/service/service_impl.rs

@@ -60,15 +60,39 @@ impl KclvmServiceImpl {
     /// use kclvm_api::service::service_impl::KclvmServiceImpl;
     /// use kclvm_api::gpyrpc::*;
     /// use std::path::Path;
-    ///
+    /// // File case
     /// let serv = KclvmServiceImpl::default();
     /// let args = &ExecProgramArgs {
     ///     work_dir: Path::new(".").join("src").join("testdata").canonicalize().unwrap().display().to_string(),
     ///     k_filename_list: vec!["test.k".to_string()],
     ///     ..Default::default()
     /// };
     /// let exec_result = serv.exec_program(args).unwrap();
-    /// println!("{}",exec_result.json_result);
+    /// assert_eq!(exec_result.yaml_result, "alice:\n  age: 18");
+    ///
+    /// // Code case
+    /// let args = &ExecProgramArgs {
+    ///     k_filename_list: vec!["file.k".to_string()],
+    ///     k_code_list: vec!["alice = {age = 18}".to_string()],
+    ///     ..Default::default()
+    /// };
+    /// let exec_result = serv.exec_program(args).unwrap();
+    /// assert_eq!(exec_result.yaml_result, "alice:\n  age: 18");
+    ///
+    /// // Error case
+    /// let args = &ExecProgramArgs {
+    ///     k_filename_list: vec!["invalid_file.k".to_string()],
+    ///     ..Default::default()
+    /// };
+    /// let error = serv.exec_program(args).unwrap_err();
+    /// assert!(error.contains("Cannot find the kcl file"), "{error}");
+    ///
+    /// let args = &ExecProgramArgs {
+    ///     k_filename_list: vec![],
+    ///     ..Default::default()
+    /// };
+    /// let error = serv.exec_program(args).unwrap_err();
+    /// assert!(error.contains("No input KCL files or paths"), "{error}");
     /// ```
     pub fn exec_program(&self, args: &ExecProgramArgs) -> Result<ExecProgramResult, String> {
         // transform args to json
@@ -414,8 +438,12 @@ impl KclvmServiceImpl {
         let settings_files = args.files.iter().map(|f| f.as_str()).collect::<Vec<&str>>();
         let settings_pathbuf = build_settings_pathbuf(&[], Some(settings_files), None)?;
         let files = if !settings_pathbuf.settings().input().is_empty() {
-            canonicalize_input_files(&settings_pathbuf.settings().input(), args.work_dir.clone())
-                .map_err(|e| anyhow!(e))?
+            canonicalize_input_files(
+                &settings_pathbuf.settings().input(),
+                args.work_dir.clone(),
+                false,
+            )
+            .map_err(|e| anyhow!(e))?
         } else {
             vec![]
         };

kclvm/cmd/src/run.rs

@@ -1,9 +1,8 @@
 use anyhow::Result;
 use clap::ArgMatches;
-use kclvm_error::Diagnostic;
+use kclvm_error::StringError;
 use kclvm_parser::ParseSession;
 use kclvm_runner::exec_program;
-use kclvm_runtime::PanicInfo;
 use std::sync::Arc;
 
 use crate::settings::must_build_settings;
@@ -23,8 +22,7 @@ pub fn run_command(matches: &ArgMatches) -> Result<()> {
         },
         Err(msg) => {
             if !sess.0.diag_handler.has_errors()? {
-                sess.0
-                    .add_err(<PanicInfo as Into<Diagnostic>>::into(PanicInfo::from(msg)))?;
+                sess.0.add_err(StringError(msg))?;
             }
             sess.0.emit_stashed_diagnostics_and_abort()?;
         }

kclvm/driver/src/lib.rs

@@ -11,7 +11,6 @@ use kclvm_config::{
     settings::{build_settings_pathbuf, DEFAULT_SETTING_FILE},
 };
 use kclvm_parser::LoadProgramOptions;
-use kclvm_runtime::PanicInfo;
 use kclvm_utils::path::PathPrefix;
 use std::{
     fs::read_dir,
@@ -22,8 +21,9 @@ use walkdir::WalkDir;
 
 /// Normalize input files with the working directory and replace ${KCL_MOD} with the module root path.
 pub fn canonicalize_input_files(
-    k_files: &Vec<String>,
+    k_files: &[String],
     work_dir: String,
+    check_exist: bool,
 ) -> Result<Vec<String>, String> {
     let mut kcl_paths = Vec::<String>::new();
 
@@ -32,14 +32,17 @@ pub fn canonicalize_input_files(
         // If the input file or path is a relative path and it is not a absolute path in the KCL module VFS,
         // join with the work directory path and convert it to a absolute path.
         if !file.starts_with(KCL_MOD_PATH_ENV) && !Path::new(file).is_absolute() {
-            match Path::new(&work_dir).join(file).canonicalize() {
+            let filepath = Path::new(&work_dir).join(file);
+            match filepath.canonicalize() {
                 Ok(path) => kcl_paths.push(path.adjust_canonicalization()),
                 Err(_) => {
-                    return Err(PanicInfo::from_string(&format!(
-                        "Cannot find the kcl file, please check whether the file path {}",
-                        file
-                    ))
-                    .to_json_string())
+                    kcl_paths.push(filepath.to_string_lossy().to_string());
+                    if check_exist {
+                        return Err(format!(
+                            "Cannot find the kcl file, please check whether the file path {}",
+                            file
+                        ));
+                    }
                 }
             }
         } else {
@@ -113,7 +116,7 @@ pub fn lookup_compile_unit(
                         },
                         ..Default::default()
                     };
-                    match canonicalize_input_files(&files, work_dir) {
+                    match canonicalize_input_files(&files, work_dir, true) {
                         Ok(kcl_paths) => (kcl_paths, Some(load_opt)),
                         Err(_) => (vec![file.to_string()], None),
                     }

kclvm/driver/src/tests.rs

@@ -1,6 +1,24 @@
+use std::path::Path;
+
 use kclvm_config::settings::KeyValuePair;
 
 use crate::arguments::parse_key_value_pair;
+use crate::canonicalize_input_files;
+
+#[test]
+fn test_canonicalize_input_files() {
+    let input_files = vec!["file1.k".to_string(), "file2.k".to_string()];
+    let work_dir = ".".to_string();
+    let expected_files = vec![
+        Path::new(".").join("file1.k").to_string_lossy().to_string(),
+        Path::new(".").join("file2.k").to_string_lossy().to_string(),
+    ];
+    assert_eq!(
+        canonicalize_input_files(&input_files, work_dir.clone(), false).unwrap(),
+        expected_files
+    );
+    assert!(canonicalize_input_files(&input_files, work_dir, true).is_err());
+}
 
 #[test]
 fn test_parse_key_value_pair() {

kclvm/error/src/lib.rs

@@ -64,31 +64,18 @@ impl Handler {
         Ok(self.has_errors())
     }
 
-    /// Emit all diagnostics but do not abort and return the error json string format.
-    #[inline]
-    pub fn alert_if_any_errors(&self) -> Result<(), String> {
-        if self.has_errors() {
-            for diag in &self.diagnostics {
-                if !diag.messages.is_empty() {
-                    let pos = diag.messages[0].pos.clone();
-
-                    let mut panic_info = PanicInfo::from(diag.messages[0].message.clone());
-                    panic_info.kcl_file = pos.filename.clone();
-                    panic_info.kcl_line = pos.line as i32;
-                    panic_info.kcl_col = pos.column.unwrap_or(0) as i32;
-
-                    if diag.messages.len() >= 2 {
-                        let pos = diag.messages[1].pos.clone();
-                        panic_info.kcl_config_meta_file = pos.filename.clone();
-                        panic_info.kcl_config_meta_line = pos.line as i32;
-                        panic_info.kcl_config_meta_col = pos.column.unwrap_or(0) as i32;
-                    }
-
-                    return Err(panic_info.to_json_string());
-                }
-            }
+    /// Emit diagnostic to string.
+    pub fn emit_to_string(&mut self) -> Result<String> {
+        let sess = Session::default();
+        for diag in &self.diagnostics {
+            sess.add_err(diag.clone())?;
+        }
+        let errors = sess.emit_all_diags_into_string()?;
+        let mut error_strings = vec![];
+        for error in errors {
+            error_strings.push(error?);
         }
-        Ok(())
+        Ok(error_strings.join("\n"))
     }
 
     /// Emit all diagnostics and abort if has any errors.
@@ -277,6 +264,9 @@ pub enum ParseError {
     },
 }
 
+/// A single string error.
+pub struct StringError(pub String);
+
 impl ParseError {
     /// New a unexpected token parse error with span and token information.
     pub fn unexpected_token(expected: &[&str], got: &str, span: Span) -> Self {
@@ -434,6 +424,15 @@ impl SessionDiagnostic for Diagnostic {
     }
 }
 
+impl SessionDiagnostic for StringError {
+    fn into_diagnostic(self, _: &Session) -> Result<DiagnosticTrait<DiagnosticStyle>> {
+        let mut diag = DiagnosticTrait::<DiagnosticStyle>::new();
+        diag.append_component(Box::new(Label::Error(E3M38.code.to_string())));
+        diag.append_component(Box::new(format!(": {}\n", self.0)));
+        Ok(diag)
+    }
+}
+
 /// Convert an error to string.
 ///
 /// ```

kclvm/parser/src/lib.rs

@@ -19,7 +19,6 @@ use kclvm_config::modfile::{
     KCL_MOD_PATH_ENV,
 };
 use kclvm_error::{ErrorKind, Message, Position, Style};
-use kclvm_runtime::PanicInfo;
 use kclvm_sema::plugin::PLUGIN_MODULE_PREFIX;
 use kclvm_utils::path::PathPrefix;
 
@@ -309,26 +308,23 @@ impl Loader {
         // get k files
         let mut k_files: Vec<String> = Vec::new();
         for (i, path) in path_list.iter().enumerate() {
-            if path.ends_with(KCL_FILE_SUFFIX) {
-                k_files.push(path.to_string());
-                continue;
-            }
-
             // read dir/*.k
             if self.is_dir(path) {
                 if self.opts.k_code_list.len() > i {
-                    return Err(PanicInfo::from("Invalid code list").to_json_string());
+                    return Err("Invalid code list".to_string());
                 }
                 //k_code_list
                 for s in self.get_dir_files(path)? {
                     k_files.push(s);
                 }
                 continue;
+            } else {
+                k_files.push(path.to_string());
             }
         }
 
         if k_files.is_empty() {
-            return Err(PanicInfo::from("No input KCL files").to_json_string());
+            return Err("No input KCL files".to_string());
         }
 
         // check all file exists
@@ -338,11 +334,10 @@ impl Loader {
             }
 
             if !self.path_exist(filename.as_str()) {
-                return Err(PanicInfo::from(format!(
+                return Err(format!(
                     "Cannot find the kcl file, please check whether the file path {}",
                     filename.as_str(),
-                ))
-                .to_json_string());
+                ));
             }
         }
         Ok(k_files)
@@ -384,15 +379,18 @@ impl Loader {
 
         // plugin pkgs
         if self.is_plugin_pkg(pkgpath.as_str()) {
-            if self.opts.load_plugins {
-                return Ok(());
-            } else {
-                return Err(PanicInfo::from_ast_pos(
-                    format!("the plugin package `{}` is not found, please confirm if plugin mode is enabled", pkgpath),
-                    pos.into(),
-                )
-                .to_json_string());
+            if !self.opts.load_plugins {
+                self.sess.1.borrow_mut().add_error(
+                    ErrorKind::CannotFindModule,
+                    &[Message {
+                        pos: Into::<(Position, Position)>::into(pos).0,
+                        style: Style::Line,
+                        message: format!("the plugin package `{}` is not found, please confirm if plugin mode is enabled", pkgpath),
+                        note: None,
+                    }],
+                );
             }
+            return Ok(());
         }
 
         // builtin pkgs
@@ -407,14 +405,19 @@ impl Loader {
         let is_external = self.is_external_pkg(&pkgpath)?;
 
         if is_external.is_some() && is_internal.is_some() {
-            return Err(PanicInfo::from_ast_pos(
-                format!(
-                    "the `{}` is found multiple times in the current package and vendor package",
-                    pkgpath
-                ),
-                pos.into(),
-            )
-            .to_json_string());
+            self.sess.1.borrow_mut().add_error(
+                ErrorKind::CannotFindModule,
+                &[Message {
+                    pos: Into::<(Position, Position)>::into(pos).0,
+                    style: Style::Line,
+                    message: format!(
+                        "the `{}` is found multiple times in the current package and vendor package",
+                        pkgpath
+                    ),
+                    note: None,
+                }],
+            );
+            return Ok(());
         }
 
         let origin_pkg_path = pkgpath.to_string();
@@ -635,10 +638,7 @@ impl Loader {
         let mut names = pkgpath.splitn(2, '.');
         match names.next() {
             Some(it) => Ok(it.to_string()),
-            None => Err(
-                PanicInfo::from(format!("Invalid external package name `{}`", pkgpath))
-                    .to_json_string(),
-            ),
+            None => Err(format!("Invalid external package name `{}`", pkgpath)),
         }
     }