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

refactor: enhance kcl and its tool cli error message and add more tests. #618

Merged
merged 1 commit into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 2 additions & 2 deletions kclvm/api/src/service/capi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ pub(crate) fn ping(serv: *mut kclvm_service, args: *const c_char) -> *const c_ch
///
///
/// `args`: [*const c_char]
/// the items and compile parameters selected by the user in the KCLVM CLI
/// the items and compile parameters selected by the user in the KCL CLI
/// serialized as protobuf byte sequence
///
/// # Returns
Expand All @@ -149,7 +149,7 @@ pub(crate) fn exec_program(serv: *mut kclvm_service, args: *const c_char) -> *co
///
///
/// `args`: [*const c_char]
/// kcl file , override specs and import paths selected by the user in the KCLVM CLI
/// kcl file , override specs and import paths selected by the user in the KCL CLI
/// serialized as protobuf byte sequence
///
/// # Returns
Expand Down
2 changes: 1 addition & 1 deletion kclvm/api/src/service/service_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl KclvmServiceImpl {
/// assert_eq!(result.formatted, source.as_bytes().to_vec());
/// ```
pub fn format_code(&self, args: &FormatCodeArgs) -> anyhow::Result<FormatCodeResult> {
let (formatted, _) = format_source(&args.source)?;
let (formatted, _) = format_source("", &args.source)?;
Ok(FormatCodeResult {
formatted: formatted.as_bytes().to_vec(),
})
Expand Down
12 changes: 11 additions & 1 deletion kclvm/cmd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,19 @@ pub fn app() -> Command {
Command::new("lint")
.about("lint")
.arg(arg!([input] ... "Sets the input file to use").num_args(0..))
.arg(arg!(output: -o --output <output> "Specify the YAML output file path"))
.arg(arg!(setting: -Y --setting <setting> ... "Sets the input file to use").num_args(1..))
.arg(arg!(verbose: -v --verbose "Print test information verbosely").action(ArgAction::Count))
.arg(arg!(emit_warning: --emit_warning "Emit warning message")),
.arg(arg!(emit_warning: --emit_warning "Emit warning message"))
.arg(arg!(disable_none: -n --disable_none "Disable dumping None values"))
.arg(arg!(strict_range_check: -r --strict_range_check "Do perform strict numeric range checks"))
.arg(arg!(debug: -d --debug "Run in debug mode (for developers only)"))
.arg(arg!(sort_keys: -k --sort_keys "Sort result keys"))
.arg(arg!(arguments: -D --argument <arguments> ... "Specify the top-level argument").num_args(1..))
.arg(arg!(path_selector: -S --path_selector <path_selector> ... "Specify the path selector").num_args(1..))
.arg(arg!(overrides: -O --overrides <overrides> ... "Specify the configuration override path and value").num_args(1..))
.arg(arg!(target: --target <target> "Specify the target type"))
.arg(arg!(package_map: -E --external <package_map> ... "Mapping of package name and path where the package is located").num_args(1..)),
)
.subcommand(
Command::new("fmt")
Expand Down
3 changes: 2 additions & 1 deletion kclvm/cmd/src/lint.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::util::*;
use anyhow::Result;
use clap::ArgMatches;
use kclvm_error::Handler;
Expand All @@ -24,7 +25,7 @@ pub fn lint_command(matches: &ArgMatches) -> Result<()> {
let (mut err_handler, mut warning_handler) = (Handler::default(), Handler::default());
(err_handler.diagnostics, warning_handler.diagnostics) =
lint_files(&files, Some(args.get_load_program_options()));
if matches.get_count("emit_warning") > 0 {
if bool_from_matches(matches, "emit_warning").unwrap_or_default() {
warning_handler.emit()?;
}
err_handler.abort_if_any_errors();
Expand Down
9 changes: 9 additions & 0 deletions kclvm/cmd/src/test_data/lint/test.k
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name = "kcl"
age = 1
schema Person:
name: str = "kcl"
age: int = 1

x0 = Person {}

x1 = Person {age = 101}
16 changes: 14 additions & 2 deletions kclvm/cmd/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use std::{
use kclvm_config::modfile::KCL_PKG_PATH;

use crate::{
app, fmt::fmt_command, run::run_command, settings::build_settings, util::hashmaps_from_matches,
vet::vet_command,
app, fmt::fmt_command, lint::lint_command, run::run_command, settings::build_settings,
util::hashmaps_from_matches, vet::vet_command,
};

#[cfg(unix)]
Expand Down Expand Up @@ -195,6 +195,18 @@ fn test_external_cmd_invalid() {
}
}

#[test]
fn test_lint_cmd() {
let input = std::path::Path::new(".")
.join("src")
.join("test_data")
.join("lint")
.join("test.k");
let matches = app().get_matches_from(&[ROOT_CMD, "lint", input.to_str().unwrap()]);
let matches = matches.subcommand_matches("lint").unwrap();
assert!(lint_command(&matches).is_ok())
}

#[test]
// All the unit test cases in [`test_run_command`] can not be executed concurrently.
fn test_run_command() {
Expand Down
2 changes: 1 addition & 1 deletion kclvm/driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use kclvm_parser::LoadProgramOptions;
use kclvm_utils::path::PathPrefix;
use kpm_metadata::fill_pkg_maps_for_k_file;
use std::{
fs::{self, read_dir},
fs::read_dir,
io::{self, ErrorKind},
path::{Path, PathBuf},
};
Expand Down
4 changes: 2 additions & 2 deletions kclvm/runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub mod tests;
/// It returns the KCL program executing result as Result<a_json_string, an_err_string>,
/// and mainly takes "program" (ast.Program returned by kclvm-parser) as input.
///
/// "args" is the items selected by the user in the KCLVM CLI.
/// "args" is the items selected by the user in the KCL CLI.
///
/// This method will first resolve “program” (ast.Program) and save the result to the "scope" (ProgramScope).
///
Expand Down Expand Up @@ -143,7 +143,7 @@ pub fn exec_program(
/// It returns the KCL program executing result as Result<a_json_string, an_err_string>,
/// and mainly takes "program" (ast.Program returned by kclvm-parser) as input.
///
/// "args" is the items selected by the user in the KCLVM CLI.
/// "args" is the items selected by the user in the KCL CLI.
///
/// This method will first resolve “program” (ast.Program) and save the result to the "scope" (ProgramScope).
///
Expand Down
20 changes: 10 additions & 10 deletions kclvm/runtime/src/_kcl_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,37 @@
use crate::*;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_buffer_t = Buffer;
type kclvm_buffer_t = Buffer;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_context_t = Context;
type kclvm_context_t = Context;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_kind_t = Kind;
type kclvm_kind_t = Kind;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_type_t = Type;
type kclvm_type_t = Type;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_value_ref_t = ValueRef;
type kclvm_value_ref_t = ValueRef;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_iterator_t = ValueIterator;
type kclvm_iterator_t = ValueIterator;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_char_t = i8;
type kclvm_char_t = i8;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_size_t = i32;
type kclvm_size_t = i32;

#[allow(dead_code, non_camel_case_types)]
type kclvm_bool_t = i8;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_int_t = i64;
type kclvm_int_t = i64;

#[allow(dead_code, non_camel_case_types)]
pub type kclvm_float_t = f64;
type kclvm_float_t = f64;

// const SHOULD_PROFILE: bool = false;

Expand Down
14 changes: 7 additions & 7 deletions kclvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::ffi::CStr;
use std::process::ExitCode;
use std::sync::Arc;

/// KCLVM CLI run function CAPI.
/// KCL CLI run function CAPI.
///
/// args is a ExecProgramArgs JSON string.
#[no_mangle]
Expand Down Expand Up @@ -51,7 +51,7 @@ pub unsafe extern "C" fn kclvm_cli_run(args: *const i8, plugin_agent: *const i8)
}
}

/// KCLVM CLI run function CAPI.
/// KCL CLI run function CAPI.
fn kclvm_cli_run_unsafe(args: *const i8, plugin_agent: *const i8) -> Result<String, String> {
let mut args = ExecProgramArgs::from_str(kclvm_runtime::c2str(args));
args.plugin_agent = plugin_agent as u64;
Expand All @@ -60,7 +60,7 @@ fn kclvm_cli_run_unsafe(args: *const i8, plugin_agent: *const i8) -> Result<Stri
.map(|r| r.json_result)
}

/// KCLVM CLI main function CAPI.
/// KCL CLI main function CAPI.
#[no_mangle]
pub unsafe extern "C" fn kclvm_cli_main(argc: c_int, argv: *const *const c_char) -> *mut ExitCode {
let prev_hook = std::panic::take_hook();
Expand All @@ -83,18 +83,18 @@ pub unsafe extern "C" fn kclvm_cli_main(argc: c_int, argv: *const *const c_char)
Ok(()) => Box::into_raw(Box::new(ExitCode::SUCCESS)),
Err(err) => {
let backtrace = format!("{}", err.backtrace());
if backtrace.is_empty() {
println!("Error: {}", err);
if backtrace.is_empty() || backtrace.contains("disabled backtrace") {
println!("Error: {err}");
} else {
println!("Error: {}\n\nStack backtrace:\n{}", err, backtrace);
println!("Error: {err}\nStack backtrace:\n{backtrace}");
}
Box::into_raw(Box::new(ExitCode::FAILURE))
}
},
Err(err) => {
let err_str = kclvm_error::err_to_str(err);
if !err_str.is_empty() {
println!("Error: {}", err_str);
println!("Error: {err_str}");
}
Box::into_raw(Box::new(ExitCode::FAILURE))
}
Expand Down
11 changes: 4 additions & 7 deletions kclvm/tools/src/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! The basic principle is to call the [kclvm_parser::parse_file] function to parse the
//! AST Module, and then use the AST printer [kclvm_tools::printer::print_ast_module]
//! to print it as source code string.
use anyhow::{anyhow, Result};
use anyhow::Result;
use kclvm_ast_pretty::print_ast_module;
use kclvm_driver::get_kcl_files;
use std::path::Path;
Expand Down Expand Up @@ -69,7 +69,7 @@ pub fn format<P: AsRef<Path>>(path: P, opts: &FormatOptions) -> Result<Vec<Strin
/// Formats a file and returns whether the file has been formatted and modified.
pub fn format_file(file: &str, opts: &FormatOptions) -> Result<bool> {
let src = std::fs::read_to_string(file)?;
let (source, is_formatted) = format_source(&src)?;
let (source, is_formatted) = format_source(file, &src)?;
if opts.is_stdout {
println!("{}", source);
} else {
Expand All @@ -80,11 +80,8 @@ pub fn format_file(file: &str, opts: &FormatOptions) -> Result<bool> {

/// Formats a code source and returns the formatted source and
/// whether the source is changed.
pub fn format_source(src: &str) -> Result<(String, bool)> {
let module = match parse_file("", Some(src.to_string())) {
Ok(module) => module,
Err(err) => return Err(anyhow!("{}", err)),
};
pub fn format_source(file: &str, src: &str) -> Result<(String, bool)> {
let module = parse_file(file, Some(src.to_string())).map_err(|err| anyhow::anyhow!(err))?;
let formatted_src = print_ast_module(&module);
let is_formatted = src != formatted_src;
Ok((formatted_src, is_formatted))
Expand Down
4 changes: 2 additions & 2 deletions kclvm/tools/src/format/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn read_data(data_name: &str) -> (String, String) {
.unwrap();

(
format_source(&src).unwrap().0,
format_source("", &src).unwrap().0,
std::fs::read_to_string(&format!(
"./src/format/test_data/format_data/{}{}",
data_name, FILE_OUTPUT_SUFFIX
Expand Down Expand Up @@ -111,7 +111,7 @@ fn test_format_integration_konfig() -> Result<()> {
file
);
let src = std::fs::read_to_string(file)?;
let (formatted_src, _) = format_source(&src)?;
let (formatted_src, _) = format_source("", &src)?;
let parse_result = parse_file("test.k", Some(formatted_src.clone() + "\n"));
assert!(
parse_result.is_ok(),
Expand Down
Loading