feat: add parser error messages into the result of api ListVariables

kclvm/api/src/service/service_impl.rs

@@ -347,7 +347,7 @@ impl KclvmServiceImpl {
         let select_res = list_variables(k_file, specs)?;
 
         let variables: HashMap<String, Variable> = select_res
-            .select_result
+            .variables
             .iter()
             .map(|(key, var)| {
                 (
@@ -370,6 +370,11 @@ impl KclvmServiceImpl {
         return Ok(ListVariablesResult {
             variables,
             unsupported_codes,
+            parse_errs: select_res
+                .parse_errs
+                .into_iter()
+                .map(|e| e.into_error())
+                .collect(),
         });
     }
 

kclvm/error/src/diagnostic.rs

@@ -1,9 +1,12 @@
+use indexmap::IndexSet;
 use kclvm_span::Loc;
 use std::fmt;
 use std::hash::Hash;
 
 use crate::{ErrorKind, WarningKind};
 
+pub type Errors = IndexSet<Diagnostic>;
+
 /// Diagnostic structure.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct Diagnostic {

kclvm/parser/src/lib.rs

@@ -17,10 +17,10 @@ use compiler_base_macros::bug;
 use compiler_base_session::Session;
 use compiler_base_span::span::new_byte_pos;
 use file_graph::FileGraph;
-use indexmap::{IndexMap, IndexSet};
+use indexmap::IndexMap;
 use kclvm_ast::ast;
 use kclvm_config::modfile::{get_vendor_home, KCL_FILE_EXTENSION, KCL_FILE_SUFFIX, KCL_MOD_FILE};
-use kclvm_error::diagnostic::{Diagnostic, Range};
+use kclvm_error::diagnostic::{Errors, Range};
 use kclvm_error::{ErrorKind, Message, Position, Style};
 use kclvm_sema::plugin::PLUGIN_MODULE_PREFIX;
 use kclvm_utils::pkgpath::parse_external_pkg_name;
@@ -72,8 +72,6 @@ pub enum ParseMode {
     ParseComments,
 }
 
-type Errors = IndexSet<Diagnostic>;
-
 /// LoadProgramResult denotes the result of the whole program and a topological
 /// ordering of all known files,
 #[derive(Debug, Clone)]

kclvm/query/src/lib.rs

@@ -83,8 +83,7 @@ pub fn override_file(file: &str, specs: &[String], import_paths: &[String]) -> R
     let overrides = specs
         .iter()
         .map(|s| parse_override_spec(s))
-        .filter_map(Result::ok)
-        .collect::<Vec<ast::OverrideSpec>>();
+        .collect::<Result<Vec<ast::OverrideSpec>, _>>()?;
     // Parse file to AST module.
     let mut module = match parse_file(file, None) {
         Ok(module) => module.module,

kclvm/query/src/selector.rs

@@ -1,6 +1,7 @@
 use super::util::{invalid_symbol_selector_spec_error, split_field_path};
 use anyhow::Result;
 use kclvm_ast::ast;
+use kclvm_error::diagnostic::Errors;
 
 use std::collections::{HashMap, VecDeque};
 
@@ -383,8 +384,9 @@ impl<'ctx> MutSelfWalker for Selector {
 }
 
 pub struct ListVariablesResult {
-    pub select_result: HashMap<String, Variable>,
+    pub variables: HashMap<String, Variable>,
     pub unsupported: Vec<UnsupportedSelectee>,
+    pub parse_errs: Errors,
 }
 
 #[derive(Debug, PartialEq)]
@@ -408,23 +410,24 @@ impl Variable {
 /// calling information.
 pub fn list_variables(file: String, specs: Vec<String>) -> Result<ListVariablesResult> {
     let mut selector = Selector::new(specs)?;
-    let ast = parse_file(&file, None)?;
+    let parse_result = parse_file(&file, None)?;
 
     let mut opts = Options::default();
     opts.merge_program = true;
     pre_process_program(
         &mut ast::Program {
             root: file,
-            pkgs: hashmap! { MAIN_PKG.to_string() => vec![ast.module.clone()] },
+            pkgs: hashmap! { MAIN_PKG.to_string() => vec![parse_result.module.clone()] },
         },
         &opts,
     );
 
-    selector.walk_module(&ast.module);
+    selector.walk_module(&parse_result.module);
 
     Ok(ListVariablesResult {
-        select_result: selector.select_result,
+        variables: selector.select_result,
         unsupported: selector.unsupported,
+        parse_errs: parse_result.errors,
     })
 }
 

kclvm/query/src/test_data/test_list_variables/invalid.k

@@ -0,0 +1 @@
+"a": "b"

kclvm/query/src/tests.rs

@@ -3,6 +3,7 @@ use std::{fs, path::PathBuf};
 use super::{r#override::apply_override_on_module, *};
 use crate::{path::parse_attribute_path, selector::list_variables};
 use kclvm_ast::ast;
+use kclvm_error::{DiagnosticId, ErrorKind, Level};
 use kclvm_parser::parse_file_force_errors;
 use pretty_assertions::assert_eq;
 
@@ -362,12 +363,9 @@ fn test_list_variables() {
     for (spec, expected, expected_name, op_sym) in test_cases {
         let specs = vec![spec.to_string()];
         let result = list_variables(file.clone(), specs).unwrap();
-        assert_eq!(result.select_result.get(spec).unwrap().value, expected);
-        assert_eq!(
-            result.select_result.get(spec).unwrap().type_name,
-            expected_name
-        );
-        assert_eq!(result.select_result.get(spec).unwrap().op_sym, op_sym);
+        assert_eq!(result.variables.get(spec).unwrap().value, expected);
+        assert_eq!(result.variables.get(spec).unwrap().type_name, expected_name);
+        assert_eq!(result.variables.get(spec).unwrap().op_sym, op_sym);
     }
 }
 
@@ -440,12 +438,10 @@ fn test_list_all_variables() {
 
     for (spec, expected, expected_name, op_sym) in test_cases {
         let result = list_variables(file.clone(), vec![]).unwrap();
-        assert_eq!(result.select_result.get(spec).unwrap().value, expected);
-        assert_eq!(
-            result.select_result.get(spec).unwrap().type_name,
-            expected_name
-        );
-        assert_eq!(result.select_result.get(spec).unwrap().op_sym, op_sym);
+        assert_eq!(result.variables.get(spec).unwrap().value, expected);
+        assert_eq!(result.variables.get(spec).unwrap().type_name, expected_name);
+        assert_eq!(result.variables.get(spec).unwrap().op_sym, op_sym);
+        assert_eq!(result.parse_errs.len(), 0);
     }
 }
 
@@ -481,8 +477,9 @@ fn test_list_unsupported_variables() {
     for (spec, expected_code) in test_cases {
         let specs = vec![spec.to_string()];
         let result = list_variables(file.clone(), specs).unwrap();
-        assert_eq!(result.select_result.get(spec), None);
+        assert_eq!(result.variables.get(spec), None);
         assert_eq!(result.unsupported[0].code, expected_code);
+        assert_eq!(result.parse_errs.len(), 0);
     }
 
     // test list variables from unsupported code
@@ -504,7 +501,7 @@ fn test_list_unsupported_variables() {
     for (spec, expected_code) in test_cases {
         let specs = vec![spec.to_string()];
         let result = list_variables(file.clone(), specs).unwrap();
-        assert_eq!(result.select_result.get(spec).unwrap().value, expected_code);
+        assert_eq!(result.variables.get(spec).unwrap().value, expected_code);
     }
 }
 
@@ -573,3 +570,55 @@ fn test_overridefile_insert() {
 
     fs::copy(simple_bk_path.clone(), simple_path.clone()).unwrap();
 }
+
+#[test]
+fn test_list_variable_with_invalid_kcl() {
+    let file = PathBuf::from("./src/test_data/test_list_variables/invalid.k")
+        .canonicalize()
+        .unwrap()
+        .display()
+        .to_string();
+    let specs = vec!["a".to_string()];
+    let result = list_variables(file.clone(), specs).unwrap();
+    assert_eq!(result.variables.get("a"), None);
+    assert_eq!(result.parse_errs.len(), 1);
+    assert_eq!(result.parse_errs[0].level, Level::Error);
+    assert_eq!(
+        result.parse_errs[0].code,
+        Some(DiagnosticId::Error(ErrorKind::InvalidSyntax))
+    );
+    assert_eq!(
+        result.parse_errs[0].messages[0].message,
+        "unexpected token ':'"
+    );
+    assert_eq!(result.parse_errs[0].messages[0].range.0.filename, file);
+    assert_eq!(result.parse_errs[0].messages[0].range.0.line, 1);
+    assert_eq!(result.parse_errs[0].messages[0].range.0.column, Some(3));
+}
+
+#[test]
+fn test_list_variables_with_file_noexist() {
+    let file = PathBuf::from("./src/test_data/test_list_variables/noexist.k")
+        .display()
+        .to_string();
+    let specs = vec!["a".to_string()];
+    let result = list_variables(file.clone(), specs);
+    assert!(result.is_err());
+    let err = result.err().unwrap();
+    assert_eq!(err.to_string(), "Cannot find the kcl file, please check the file path ./src/test_data/test_list_variables/noexist.k");
+}
+
+#[test]
+fn test_override_file_with_invalid_spec() {
+    let specs = vec!["....".to_string()];
+    let import_paths = vec![];
+    let file = PathBuf::from("./src/test_data/test_override_file/main.k")
+        .canonicalize()
+        .unwrap()
+        .display()
+        .to_string();
+    let result = override_file(&file, &specs, &import_paths);
+    assert!(result.is_err());
+    let err = result.err().unwrap();
+    assert_eq!(err.to_string(), "Invalid spec format '....', expected <pkgpath>:<field_path>=<filed_value> or <pkgpath>:<field_path>-");
+}

kclvm/spec/gpyrpc/gpyrpc.proto

@@ -299,6 +299,7 @@ message ListVariables_Args {
 message ListVariables_Result {
 	map<string, Variable> variables = 1;
 	repeated string unsupported_codes = 2; 
+	repeated Error parse_errs = 3;
 }
 
 message Variable {