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.

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

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

Merged
merged 3 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 5 additions & 0 deletions kclvm/api/src/service/service_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,11 @@ impl KclvmServiceImpl {
return Ok(ListVariablesResult {
variables,
unsupported_codes,
errs: select_res
.errs
.into_iter()
.map(|e| e.into_error())
.collect(),
});
}

Expand Down
3 changes: 3 additions & 0 deletions kclvm/error/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions kclvm/parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
9 changes: 6 additions & 3 deletions kclvm/query/src/selector.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -385,6 +386,7 @@ impl<'ctx> MutSelfWalker for Selector {
pub struct ListVariablesResult {
pub select_result: HashMap<String, Variable>,
Peefy marked this conversation as resolved.
Show resolved Hide resolved
pub unsupported: Vec<UnsupportedSelectee>,
pub errs: Errors,
}

#[derive(Debug, PartialEq)]
Expand All @@ -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,
unsupported: selector.unsupported,
errs: parse_result.errors,
})
}

Expand Down
1 change: 1 addition & 0 deletions kclvm/query/src/test_data/test_list_variables/invalid.k
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"a": "b"
52 changes: 52 additions & 0 deletions kclvm/query/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -446,6 +447,7 @@ fn test_list_all_variables() {
expected_name
);
assert_eq!(result.select_result.get(spec).unwrap().op_sym, op_sym);
assert_eq!(result.errs.len(), 0);
}
}

Expand Down Expand Up @@ -483,6 +485,7 @@ fn test_list_unsupported_variables() {
let result = list_variables(file.clone(), specs).unwrap();
assert_eq!(result.select_result.get(spec), None);
assert_eq!(result.unsupported[0].code, expected_code);
assert_eq!(result.errs.len(), 0);
}

// test list variables from unsupported code
Expand Down Expand Up @@ -573,3 +576,52 @@ 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.select_result.get("a"), None);
assert_eq!(result.errs.len(), 1);
assert_eq!(result.errs[0].level, Level::Error);
assert_eq!(
result.errs[0].code,
Some(DiagnosticId::Error(ErrorKind::InvalidSyntax))
);
assert_eq!(result.errs[0].messages[0].message, "unexpected token ':'");
assert_eq!(result.errs[0].messages[0].range.0.filename, file);
assert_eq!(result.errs[0].messages[0].range.0.line, 1);
assert_eq!(result.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() {
Peefy marked this conversation as resolved.
Show resolved Hide resolved
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>-");
}
1 change: 1 addition & 0 deletions kclvm/spec/gpyrpc/gpyrpc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ message ListVariables_Args {
message ListVariables_Result {
map<string, Variable> variables = 1;
repeated string unsupported_codes = 2;
repeated Error errs = 3;
Peefy marked this conversation as resolved.
Show resolved Hide resolved
}

message Variable {
Expand Down
Loading