From 99936be40ea649e671599bccbb5364eb8dcbfffb Mon Sep 17 00:00:00 2001 From: peefy Date: Wed, 8 Mar 2023 19:13:12 +0800 Subject: [PATCH 1/4] refactor: diagnostic 0-based column --- kclvm/error/src/diagnostic.rs | 4 ++-- kclvm/error/src/emitter.rs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/kclvm/error/src/diagnostic.rs b/kclvm/error/src/diagnostic.rs index 98a599c60..ca83a58f4 100644 --- a/kclvm/error/src/diagnostic.rs +++ b/kclvm/error/src/diagnostic.rs @@ -19,7 +19,7 @@ pub struct Diagnostic { /// line, and column location. /// /// A Position is valid if the line number is > 0. -/// The line and column are both 1 based. +/// The line is 1-based and the column is 0-based. #[derive(PartialEq, Clone, Eq, Hash, Debug, Default)] pub struct Position { pub filename: String, @@ -89,7 +89,7 @@ impl From for Position { line: loc.line as u64, column: if loc.col_display > 0 { // Loc col is the (0-based) column offset. - Some(loc.col.0 as u64 + 1) + Some(loc.col.0 as u64) } else { None }, diff --git a/kclvm/error/src/emitter.rs b/kclvm/error/src/emitter.rs index e96a8aa59..15d0b0041 100644 --- a/kclvm/error/src/emitter.rs +++ b/kclvm/error/src/emitter.rs @@ -188,6 +188,8 @@ impl Emitter for EmitterWriter { buffer.push(" ".repeat(i) + &line_source); if let Style::LineAndColumn = msg.style { if let Some(column) = msg.pos.column { + // The Position column is 0-based + let column = column + 1; let column_source = format!("{} ^", column); let prefix_space = line_hint_len + column as usize - column_source.len(); let column_source = " ".repeat(prefix_space) + &column_source; From bba1586a94431aa74b482a51e9037cd844591597 Mon Sep 17 00:00:00 2001 From: peefy Date: Thu, 9 Mar 2023 20:12:32 +0800 Subject: [PATCH 2/4] refactor: use compiler base session in resolver and remove un-used code in the kclvm error crate. --- kclvm/Cargo.lock | 23 +- kclvm/ast_pretty/Cargo.toml | 1 + kclvm/ast_pretty/src/node.rs | 2 +- kclvm/capi/src/service/service.rs | 4 +- kclvm/cmd/src/lint.rs | 2 +- kclvm/compiler/src/codegen/llvm/context.rs | 5 +- kclvm/error/Cargo.toml | 3 +- kclvm/error/src/bug.rs | 48 ---- kclvm/error/src/emitter.rs | 242 --------------------- kclvm/error/src/error.rs | 26 ++- kclvm/error/src/lib.rs | 194 +++++++++++------ kclvm/error/src/tests.rs | 17 -- kclvm/error/src/warning_codes/W1001.md | 1 + kclvm/parser/Cargo.toml | 1 + kclvm/parser/src/lexer/mod.rs | 2 +- kclvm/parser/src/lib.rs | 2 +- kclvm/query/Cargo.toml | 1 + kclvm/query/src/override.rs | 2 +- kclvm/runner/Cargo.toml | 1 + kclvm/runner/benches/bench_runner.rs | 4 +- kclvm/runner/src/assembler.rs | 2 +- kclvm/runner/src/lib.rs | 16 +- kclvm/runner/src/tests.rs | 4 +- kclvm/sema/Cargo.toml | 1 + kclvm/sema/src/lib.rs | 2 +- kclvm/sema/src/pre_process/identifier.rs | 5 +- kclvm/sema/src/resolver/global.rs | 24 +- kclvm/sema/src/resolver/mod.rs | 4 +- kclvm/sema/src/resolver/schema.rs | 32 ++- kclvm/sema/src/resolver/scope.rs | 20 +- kclvm/sema/src/resolver/tests.rs | 44 ++-- kclvm/tools/src/lint/mod.rs | 2 +- 32 files changed, 299 insertions(+), 438 deletions(-) delete mode 100644 kclvm/error/src/bug.rs delete mode 100644 kclvm/error/src/emitter.rs delete mode 100644 kclvm/error/src/tests.rs create mode 100644 kclvm/error/src/warning_codes/W1001.md diff --git a/kclvm/Cargo.lock b/kclvm/Cargo.lock index ecfa8d98f..125d1973f 100644 --- a/kclvm/Cargo.lock +++ b/kclvm/Cargo.lock @@ -33,9 +33,13 @@ dependencies = [ [[package]] name = "annotate-snippets" -version = "0.8.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d78ea013094e5ea606b1c05fe35f1dd7ea1eb1ea259908d040b25bd5ec677ee5" +checksum = "c3b9d411ecbaf79885c6df4d75fff75858d5995ff25385657a28af47e82f9c36" +dependencies = [ + "unicode-width", + "yansi-term", +] [[package]] name = "anyhow" @@ -1149,6 +1153,7 @@ dependencies = [ name = "kclvm-ast-pretty" version = "0.4.5" dependencies = [ + "compiler_base_macros", "compiler_base_session", "fancy-regex", "indexmap", @@ -1243,6 +1248,7 @@ dependencies = [ "anyhow", "atty", "compiler_base_error", + "compiler_base_macros", "compiler_base_session", "compiler_base_span 0.0.2", "indexmap", @@ -1279,6 +1285,7 @@ version = "0.4.5" dependencies = [ "bstr", "compiler_base_error", + "compiler_base_macros", "compiler_base_session", "compiler_base_span 0.0.2", "either", @@ -1306,6 +1313,7 @@ name = "kclvm-query" version = "0.4.5" dependencies = [ "anyhow", + "compiler_base_macros", "compiler_base_session", "kclvm-ast", "kclvm-ast-pretty", @@ -1322,6 +1330,7 @@ dependencies = [ "anyhow", "cc", "chrono", + "compiler_base_macros", "compiler_base_session", "criterion", "fslock", @@ -1381,6 +1390,7 @@ dependencies = [ "ahash", "bit-set", "bitflags", + "compiler_base_macros", "compiler_base_session", "compiler_base_span 0.0.2", "criterion", @@ -3452,3 +3462,12 @@ name = "yansi" version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" + +[[package]] +name = "yansi-term" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe5c30ade05e61656247b2e334a031dfd0cc466fadef865bdcdea8d537951bf1" +dependencies = [ + "winapi", +] diff --git a/kclvm/ast_pretty/Cargo.toml b/kclvm/ast_pretty/Cargo.toml index 597c39370..0c918e48c 100644 --- a/kclvm/ast_pretty/Cargo.toml +++ b/kclvm/ast_pretty/Cargo.toml @@ -14,3 +14,4 @@ indexmap = "1.0" fancy-regex = "0.7.1" pretty_assertions = "1.3.0" compiler_base_session = {path = "../../compiler_base/session", version = "0.0.12"} +compiler_base_macros = "0.0.1" diff --git a/kclvm/ast_pretty/src/node.rs b/kclvm/ast_pretty/src/node.rs index 744cfad65..99c3e21e2 100644 --- a/kclvm/ast_pretty/src/node.rs +++ b/kclvm/ast_pretty/src/node.rs @@ -1,11 +1,11 @@ use std::collections::HashSet; +use compiler_base_macros::bug; use kclvm_ast::{ ast::{self, CallExpr}, token::{DelimToken, TokenKind}, walker::MutSelfTypedResultWalker, }; -use kclvm_error::bug; use super::{Indentation, Printer}; diff --git a/kclvm/capi/src/service/service.rs b/kclvm/capi/src/service/service.rs index 9591ed427..98cbfe663 100644 --- a/kclvm/capi/src/service/service.rs +++ b/kclvm/capi/src/service/service.rs @@ -93,7 +93,7 @@ impl KclvmService { let kcl_paths_str = kcl_paths.iter().map(|s| s.as_str()).collect::>(); let mut result = ExecProgram_Result::default(); let sess = Arc::new(Session::default()); - let mut program = load_program(sess, &kcl_paths_str.as_slice(), Some(opts))?; + let mut program = load_program(sess.clone(), &kcl_paths_str.as_slice(), Some(opts))?; if let Err(err) = apply_overrides( &mut program, @@ -105,7 +105,7 @@ impl KclvmService { } let start_time = SystemTime::now(); - let exec_result = kclvm_runner::execute(program, self.plugin_agent, &native_args); + let exec_result = kclvm_runner::execute(sess, program, self.plugin_agent, &native_args); let escape_time = match SystemTime::now().duration_since(start_time) { Ok(dur) => dur.as_secs_f32(), Err(err) => return Err(err.to_string()), diff --git a/kclvm/cmd/src/lint.rs b/kclvm/cmd/src/lint.rs index 79740df87..7695de38a 100644 --- a/kclvm/cmd/src/lint.rs +++ b/kclvm/cmd/src/lint.rs @@ -25,7 +25,7 @@ pub fn lint_command(matches: &ArgMatches) -> Result<()> { (err_handler.diagnostics, warning_handler.diagnostics) = lint_files(&files, Some(args.get_load_program_options())); if matches.occurrences_of("emit_warning") > 0 { - warning_handler.emit(); + warning_handler.emit()?; } err_handler.abort_if_any_errors(); Ok(()) diff --git a/kclvm/compiler/src/codegen/llvm/context.rs b/kclvm/compiler/src/codegen/llvm/context.rs index 3358e5ffe..e08b1f860 100644 --- a/kclvm/compiler/src/codegen/llvm/context.rs +++ b/kclvm/compiler/src/codegen/llvm/context.rs @@ -1750,10 +1750,9 @@ impl<'ctx> LLVMCodeGenContext<'ctx> { column: None, }, ); - handler.abort_if_errors() - } else { - result + handler.abort_if_any_errors() } + result } } } diff --git a/kclvm/error/Cargo.toml b/kclvm/error/Cargo.toml index 19a0167dd..89c36be12 100644 --- a/kclvm/error/Cargo.toml +++ b/kclvm/error/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" compiler_base_span = {path = "../../compiler_base/span", version = "0.0.2"} compiler_base_session = {path = "../../compiler_base/session", version = "0.0.12"} compiler_base_error = "0.0.8" +compiler_base_macros = "0.0.1" kclvm-span = {path = "../span", version = "0.4.5"} kclvm-runtime = {path = "../runtime", version = "0.4.5"} @@ -16,6 +17,6 @@ anyhow = "1.0" tracing = "0.1" atty = "0.2" termcolor = "1.0" -annotate-snippets = "0.8.0" +annotate-snippets = { version = "0.9.0", default-features = false, features = ["color"] } termize = "0.1.1" indexmap = "1.0" diff --git a/kclvm/error/src/bug.rs b/kclvm/error/src/bug.rs deleted file mode 100644 index 2a0a027c6..000000000 --- a/kclvm/error/src/bug.rs +++ /dev/null @@ -1,48 +0,0 @@ -use std::{error, fmt, panic}; - -/// `bug!` macro is used to report compiler internal bug. -/// You can use bug! macros directly by adding `#[macro_use]extern crate kclvm_error;` -/// in the lib.rs, and then call as follows: -/// ```no_check -/// bug!(); -/// bug!("an error msg"); -/// bug!("an error msg with string format {}", "msg"); -/// ``` -#[macro_export] -macro_rules! bug { - () => ( $crate::bug::bug("impossible case reached") ); - ($msg:expr) => ({ $crate::bug::bug(&format!($msg)) }); - ($msg:expr,) => ({ $crate::bug::bug($msg) }); - ($fmt:expr, $($arg:tt)+) => ({ - $crate::bug::bug(&format!($fmt, $($arg)+)) - }); -} - -/// Signifies that the compiler died with an explicit call to `.bug` -/// rather than a failed assertion, etc. -#[derive(Clone, Debug)] -pub struct ExplicitBug { - msg: String, -} - -impl fmt::Display for ExplicitBug { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "Internal error, please report a bug to us. The error message is: {}", - self.msg - ) - } -} - -impl error::Error for ExplicitBug {} - -#[inline] -pub fn bug(msg: &str) -> ! { - panic!( - "{}", - ExplicitBug { - msg: msg.to_string() - } - ); -} diff --git a/kclvm/error/src/emitter.rs b/kclvm/error/src/emitter.rs deleted file mode 100644 index 15d0b0041..000000000 --- a/kclvm/error/src/emitter.rs +++ /dev/null @@ -1,242 +0,0 @@ -use crate::{ - diagnostic::{Diagnostic, Style}, - DiagnosticId, Level, -}; - -use kclvm_span::{FilePathMapping, SourceMap}; -use std::sync::Arc; -use std::{ - io::{self, Write}, - path::Path, -}; -use termcolor::{BufferWriter, Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; - -/// Emitter trait for emitting errors. -pub trait Emitter { - fn format_diagnostic(&mut self, diag: &Diagnostic) -> Vec; - /// Emit a structured diagnostic. - fn emit_diagnostic(&mut self, diag: &Diagnostic); - /// Checks if we can use colors in the current output stream. - fn supports_color(&self) -> bool { - false - } -} - -/// Emitter writer. -pub struct EmitterWriter { - dst: Destination, - short_message: bool, - source_map: Option>, -} - -impl Default for EmitterWriter { - fn default() -> Self { - Self { - dst: Destination::from_stderr(), - short_message: false, - source_map: None, - } - } -} - -impl EmitterWriter { - pub fn from_stderr(source_map: Arc) -> Self { - Self { - dst: Destination::from_stderr(), - short_message: false, - source_map: Some(source_map), - } - } -} - -/// Emit destinations -pub enum Destination { - Terminal(StandardStream), - Buffered(BufferWriter), - // The bool denotes whether we should be emitting ansi color codes or not - Raw(Box<(dyn Write + Send)>, bool), -} - -impl Destination { - #[allow(dead_code)] - pub fn from_raw(dst: Box, colored: bool) -> Self { - Destination::Raw(dst, colored) - } - - pub fn from_stderr() -> Self { - // On Windows we'll be performing global synchronization on the entire - // system for emitting rustc errors, so there's no need to buffer - // anything. - // - // On non-Windows we rely on the atomicity of `write` to ensure errors - // don't get all jumbled up. - if !cfg!(windows) { - Destination::Terminal(StandardStream::stderr(ColorChoice::Auto)) - } else { - Destination::Buffered(BufferWriter::stderr(ColorChoice::Auto)) - } - } - - fn supports_color(&self) -> bool { - match *self { - Self::Terminal(ref stream) => stream.supports_color(), - Self::Buffered(ref buffer) => buffer.buffer().supports_color(), - Self::Raw(_, supports_color) => supports_color, - } - } - - fn apply_style(&mut self, lvl: Level, style: Style) -> io::Result<()> { - let mut spec = ColorSpec::new(); - match style { - Style::Empty | Style::LineAndColumn => { - spec.set_bold(true); - spec = lvl.color(); - } - Style::Line => { - spec.set_bold(true); - spec.set_intense(true); - if cfg!(windows) { - spec.set_fg(Some(Color::Cyan)); - } else { - spec.set_fg(Some(Color::Blue)); - } - } - } - self.set_color(&spec) - } - - fn set_color(&mut self, color: &ColorSpec) -> io::Result<()> { - match *self { - Destination::Terminal(ref mut t) => t.set_color(color), - Destination::Buffered(ref mut t) => t.buffer().set_color(color), - Destination::Raw(_, _) => Ok(()), - } - } - - fn reset(&mut self) -> io::Result<()> { - match *self { - Destination::Terminal(ref mut t) => t.reset(), - Destination::Buffered(ref mut t) => t.buffer().reset(), - Destination::Raw(..) => Ok(()), - } - } -} - -impl<'a> Write for Destination { - fn write(&mut self, bytes: &[u8]) -> io::Result { - match *self { - Destination::Terminal(ref mut t) => t.write(bytes), - Destination::Buffered(ref mut t) => t.buffer().write(bytes), - Destination::Raw(ref mut t, _) => t.write(bytes), - } - } - - fn flush(&mut self) -> io::Result<()> { - match *self { - Destination::Terminal(ref mut t) => t.flush(), - Destination::Buffered(ref mut t) => t.buffer().flush(), - Destination::Raw(ref mut t, _) => t.flush(), - } - } -} - -impl Emitter for EmitterWriter { - fn supports_color(&self) -> bool { - self.dst.supports_color() - } - - fn emit_diagnostic(&mut self, diag: &Diagnostic) { - let buffer = self.format_diagnostic(diag); - if let Err(e) = emit_to_destination(&buffer, &diag.level, &mut self.dst, self.short_message) - { - panic!("failed to emit error: {}", e) - } - } - - fn format_diagnostic(&mut self, diag: &Diagnostic) -> Vec { - let mut buffer: Vec = vec![]; - let mut diag_str = "KCL ".to_string(); - diag_str += diag.level.to_str(); - if let Some(code) = &diag.code { - let code_str = match code { - DiagnosticId::Error(kind) => kind.name(), - DiagnosticId::Warning(warn_msg) => warn_msg.to_string(), - }; - diag_str += &format!(" [{}]", code_str); - } - buffer.push(diag_str); - for (i, msg) in diag.messages.iter().enumerate() { - buffer.push(" ".repeat(i) + &msg.pos.info()); - // To prevent read empty source content. - if msg.pos.line > 0 { - let mut line_source = format!("{} |", msg.pos.line); - let line_hint_len = line_source.len(); - if let Some(sm) = &self.source_map { - if let Some(source_file) = sm.source_file_by_filename(&msg.pos.filename) { - if let Some(line) = source_file.get_line(msg.pos.line as usize - 1) { - line_source += &line.to_string(); - } - } - } else { - let sm = SourceMap::new(FilePathMapping::empty()); - if let Ok(source_file) = sm.load_file(Path::new(&msg.pos.filename)) { - if let Some(line) = source_file.get_line(msg.pos.line as usize - 1) { - line_source += &line.to_string(); - } - } - } - buffer.push(" ".repeat(i) + &line_source); - if let Style::LineAndColumn = msg.style { - if let Some(column) = msg.pos.column { - // The Position column is 0-based - let column = column + 1; - let column_source = format!("{} ^", column); - let prefix_space = line_hint_len + column as usize - column_source.len(); - let column_source = " ".repeat(prefix_space) + &column_source; - buffer.push(" ".repeat(i) + &column_source); - } - } - } - buffer.push(" ".repeat(i) + &msg.message.clone()); - if !self.short_message { - if let Some(note) = &msg.note { - buffer.push(" ".repeat(i) + &format!("Note: {}", note)); - } - } - buffer.push("".to_string()); - } - buffer - } -} - -fn emit_to_destination( - rendered_buffer: &[String], - lvl: &Level, - dst: &mut Destination, - short_message: bool, -) -> io::Result<()> { - // In order to prevent error message interleaving, where multiple error lines get intermixed - // when multiple compiler processes error simultaneously, we emit errors with additional - // steps. - // - // On Unix systems, we write into a buffered terminal rather than directly to a terminal. When - // the .flush() is called we take the buffer created from the buffered writes and write it at - // one shot. Because the Unix systems use ANSI for the colors, which is a text-based styling - // scheme, this buffered approach works and maintains the styling. - // - // On Windows, styling happens through calls to a terminal API. This prevents us from using the - // same buffering approach. Instead, we use a global Windows mutex, which we acquire long - // enough to output the full error message, then we release. - for (pos, line) in rendered_buffer.iter().enumerate() { - if line.starts_with("KCL") { - dst.apply_style(*lvl, Style::LineAndColumn)?; - } - write!(dst, "{}", line)?; - dst.reset()?; - if !short_message || pos != rendered_buffer.len() - 1 { - writeln!(dst)?; - } - } - dst.flush()?; - Ok(()) -} diff --git a/kclvm/error/src/error.rs b/kclvm/error/src/error.rs index 7257ac5fb..46b3004dc 100644 --- a/kclvm/error/src/error.rs +++ b/kclvm/error/src/error.rs @@ -18,6 +18,23 @@ macro_rules! register_errors { ) } +macro_rules! register_warnings { + ($($ecode:ident: $kind:expr, $message:expr,)*) => ( + pub static WARNINGS: &[(&str, Warning)] = &[ + $( (stringify!($ecode), Warning { + code: stringify!($ecode), + kind: $kind, + message: Some($message), + }), )* + ]; + $(pub const $ecode: Warning = Warning { + code: stringify!($ecode), + kind: $kind, + message: Some($message), + };)* + ) +} + // Error messages for EXXXX errors. Each message should start and end with a // new line. register_errors! { @@ -31,6 +48,12 @@ register_errors! { E3M38: ErrorKind::EvaluationError, include_str!("./error_codes/E2D34.md"), } +// Error messages for WXXXX errors. Each message should start and end with a +// new line. +register_warnings! { + W1001: WarningKind::CompilerWarning, include_str!("./warning_codes/W1001.md"), +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Error { pub code: &'static str, @@ -92,13 +115,14 @@ impl ErrorKind { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Warning { pub code: &'static str, - pub kind: ErrorKind, + pub kind: WarningKind, pub message: Option<&'static str>, } // Kind of KCL warning. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum WarningKind { + CompilerWarning, UnusedImportWarning, ReimportWarning, ImportPositionWarning, diff --git a/kclvm/error/src/lib.rs b/kclvm/error/src/lib.rs index 6a6ab3110..d6f5acdcb 100644 --- a/kclvm/error/src/lib.rs +++ b/kclvm/error/src/lib.rs @@ -3,46 +3,39 @@ //! //! We can use `Handler` to create and emit diagnostics. -use compiler_base_span::Span; -use kclvm_runtime::{ErrType, PanicInfo}; - -#[macro_use] -pub mod bug; pub mod diagnostic; -mod emitter; mod error; -#[cfg(test)] -mod tests; +use annotate_snippets::{ + display_list::DisplayList, + display_list::FormatOptions, + snippet::{AnnotationType, Slice, Snippet, SourceAnnotation}, +}; use anyhow::Result; use compiler_base_error::{ components::{CodeSnippet, Label}, Diagnostic as DiagnosticTrait, DiagnosticStyle, }; use compiler_base_session::{Session, SessionDiagnostic}; -pub use diagnostic::{Diagnostic, DiagnosticId, Level, Message, Position, Style}; -pub use emitter::{Emitter, EmitterWriter}; -pub use error::*; +use compiler_base_span::{span::new_byte_pos, Span}; use indexmap::IndexSet; -use kclvm_span::SourceMap; +use kclvm_runtime::PanicInfo; use std::{any::Any, sync::Arc}; +pub use diagnostic::{Diagnostic, DiagnosticId, Level, Message, Position, Style}; +pub use error::*; + /// A handler deals with errors and other compiler output. /// Certain errors (error, bug) may cause immediate exit, /// others log errors for later reporting. +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Handler { - /// The number of errors that have been emitted, including duplicates. - /// - /// This is not necessarily the count that's reported to the user once - /// compilation ends. - emitter: Box, pub diagnostics: IndexSet, } impl Default for Handler { fn default() -> Self { Self { - emitter: Box::new(EmitterWriter::default()), diagnostics: Default::default(), } } @@ -50,16 +43,8 @@ impl Default for Handler { impl Handler { /// New a handler using a emitter - pub fn new(emitter: Box) -> Self { - Self { - emitter, - diagnostics: Default::default(), - } - } - - pub fn with_source_map(source_map: Arc) -> Self { + pub fn new() -> Self { Self { - emitter: Box::new(EmitterWriter::from_stderr(source_map)), diagnostics: Default::default(), } } @@ -67,7 +52,7 @@ impl Handler { /// Panic program and report a bug #[inline] pub fn bug(&self, msg: &str) -> ! { - bug!("{}", msg) + compiler_base_macros::bug!("{}", msg) } #[inline] @@ -78,61 +63,54 @@ impl Handler { } /// Emit all diagnostics and return whether has errors. - pub fn emit(&mut self) -> bool { - for diag in &self.diagnostics { - self.emitter.emit_diagnostic(diag); - } - self.has_errors() - } - /// Format and return all diagnostics msg. - pub fn format_diagnostic(&mut self) -> Vec { - let mut dia_msgs = Vec::new(); + pub fn emit(&mut self) -> Result { + let sess = Session::default(); for diag in &self.diagnostics { - dia_msgs.append(&mut self.emitter.format_diagnostic(diag)); - } - dia_msgs - } - - /// Emit all diagnostics and abort if has any errors. - pub fn abort_if_errors(&mut self) -> ! { - if self.emit() { - std::process::exit(1) - } else { - panic!("compiler internal error") - } - } - - /// Emit all diagnostics and abort if has any errors. - pub fn abort_if_any_errors(&mut self) { - if self.emit() { - std::process::exit(1) + sess.add_err(diag.clone()).unwrap(); } + sess.emit_stashed_diagnostics()?; + 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(&mut self) -> Result<(), String> { + pub fn alert_if_any_errors(&self) -> Result<(), String> { if self.has_errors() { for diag in &self.diagnostics { - let pos = diag.messages[0].pos.clone(); - let message = diag.messages[0].message.clone(); - - let mut panic_info = PanicInfo::default(); - - panic_info.__kcl_PanicInfo__ = true; - panic_info.message = message; - panic_info.err_type_code = ErrType::CompileError_TYPE as i32; - - 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; - - return Err(panic_info.to_json_string()); + 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()); + } } } Ok(()) } + /// Emit all diagnostics and abort if has any errors. + pub fn abort_if_any_errors(&mut self) { + match self.emit() { + Ok(has_error) => { + if has_error { + std::process::exit(1); + } + } + Err(err) => self.bug(&format!("{}", err.to_string())), + } + } + /// Construct a parse error and put it into the handler diagnostic buffer pub fn add_syntex_error(&mut self, msg: &str, pos: Position) -> &mut Self { let message = format!("Invalid syntax: {}", msg); @@ -261,6 +239,7 @@ pub enum ParseError { } impl ParseError { + /// New a unexpected token parse error with span and token information. pub fn unexpected_token(expected: &[&str], got: &str, span: Span) -> Self { ParseError::UnexpectedToken { expected: expected @@ -272,7 +251,7 @@ impl ParseError { } } - // New a message parse error with span + /// New a message parse error with span. pub fn message(message: String, span: Span) -> Self { ParseError::Message { message, span } } @@ -303,7 +282,7 @@ impl SessionDiagnostic for ParseError { let code_snippet = CodeSnippet::new(span, Arc::clone(&sess.sm)); diag.append_component(Box::new(code_snippet)); diag.append_component(Box::new(format!( - "unexpected one of {expected:?} got {got}\n" + " expected one of {expected:?} got {got}\n" ))); Ok(diag) } @@ -317,6 +296,77 @@ impl SessionDiagnostic for ParseError { } } +impl SessionDiagnostic for Diagnostic { + fn into_diagnostic(self, _: &Session) -> Result> { + let mut diag = DiagnosticTrait::::new(); + match self.code { + Some(id) => match id { + DiagnosticId::Error(error) => { + diag.append_component(Box::new(Label::Error(E2L23.code.to_string()))); + diag.append_component(Box::new(format!(": {}", error.name()))); + } + DiagnosticId::Warning(warning) => { + diag.append_component(Box::new(Label::Warning(W1001.code.to_string()))); + diag.append_component(Box::new(format!(": {}", warning.name()))); + } + }, + None => match self.level { + Level::Error => { + diag.append_component(Box::new(Label::Error(E2L23.code.to_string()))); + } + Level::Warning => { + diag.append_component(Box::new(Label::Warning(W1001.code.to_string()))); + } + Level::Note => { + diag.append_component(Box::new(Label::Note)); + } + }, + } + // Append a new line. + diag.append_component(Box::new(String::from("\n"))); + for msg in &self.messages { + let sess = Session::new_with_file_and_code(&msg.pos.filename, None)?; + let source = sess.sm.lookup_source_file(new_byte_pos(0)); + let line = source.get_line((msg.pos.line - 1) as usize); + let content = line + .as_ref() + .ok_or(anyhow::anyhow!("Failed to load source file"))?; + let snippet = Snippet { + title: None, + footer: vec![], + slices: vec![Slice { + source: content, + line_start: msg.pos.line as usize, + origin: Some(&msg.pos.filename), + annotations: vec![SourceAnnotation { + range: match msg.pos.column { + Some(column) => (column as usize, (column + 1) as usize), + None => (0, 0), + }, + label: &msg.message, + annotation_type: AnnotationType::Error, + }], + fold: true, + }], + opt: FormatOptions { + color: true, + anonymized_line_numbers: false, + margin: None, + }, + }; + let dl = DisplayList::from(snippet); + diag.append_component(Box::new(format!("{dl}\n"))); + if let Some(note) = &msg.note { + diag.append_component(Box::new(Label::Note)); + diag.append_component(Box::new(format!(": {}\n", note))); + } + // Append a new line. + diag.append_component(Box::new(String::from("\n"))); + } + Ok(diag) + } +} + /// Convert an error to string. /// /// ``` diff --git a/kclvm/error/src/tests.rs b/kclvm/error/src/tests.rs deleted file mode 100644 index 7fd5b76ec..000000000 --- a/kclvm/error/src/tests.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::*; - -#[test] -fn test_bug_macro() { - let result = std::panic::catch_unwind(|| { - bug!(); - }); - assert!(result.is_err()); - let result = std::panic::catch_unwind(|| { - bug!("an error msg"); - }); - assert!(result.is_err()); - let result = std::panic::catch_unwind(|| { - bug!("an error msg with string format {}", "msg"); - }); - assert!(result.is_err()); -} diff --git a/kclvm/error/src/warning_codes/W1001.md b/kclvm/error/src/warning_codes/W1001.md new file mode 100644 index 000000000..8f09bd85f --- /dev/null +++ b/kclvm/error/src/warning_codes/W1001.md @@ -0,0 +1 @@ +This warning indicates that the compiler warning has occurred. diff --git a/kclvm/parser/Cargo.toml b/kclvm/parser/Cargo.toml index 75b474e3b..86a9b00a1 100644 --- a/kclvm/parser/Cargo.toml +++ b/kclvm/parser/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" compiler_base_span = {path = "../../compiler_base/span", version = "0.0.2"} compiler_base_session = {path = "../../compiler_base/session", version = "0.0.12"} compiler_base_error = "0.0.8" +compiler_base_macros = "0.0.1" tracing = "0.1" serde = { version = "1", features = ["derive"] } serde_json = "1.0" diff --git a/kclvm/parser/src/lexer/mod.rs b/kclvm/parser/src/lexer/mod.rs index fa0e366f2..caebc2cd8 100644 --- a/kclvm/parser/src/lexer/mod.rs +++ b/kclvm/parser/src/lexer/mod.rs @@ -21,11 +21,11 @@ mod string; #[cfg(test)] mod tests; +use compiler_base_macros::bug; use compiler_base_span::{self, span::new_byte_pos, BytePos, Span}; use kclvm_ast::ast::NumberBinarySuffix; use kclvm_ast::token::{self, CommentKind, Token, TokenKind}; use kclvm_ast::token_stream::TokenStream; -use kclvm_error::bug; use kclvm_lexer::Base; use kclvm_span::symbol::Symbol; pub(crate) use string::str_content_eval; diff --git a/kclvm/parser/src/lib.rs b/kclvm/parser/src/lib.rs index 37a7dd9db..f8f08b6d2 100644 --- a/kclvm/parser/src/lib.rs +++ b/kclvm/parser/src/lib.rs @@ -10,11 +10,11 @@ mod tests; extern crate kclvm_error; use crate::session::ParseSession; +use compiler_base_macros::bug; use compiler_base_session::Session; use compiler_base_span::span::new_byte_pos; use kclvm_ast::ast; use kclvm_config::modfile::KCL_FILE_SUFFIX; -use kclvm_error::bug; use kclvm_runtime::PanicInfo; use kclvm_sema::plugin::PLUGIN_MODULE_PREFIX; use kclvm_utils::path::PathPrefix; diff --git a/kclvm/query/Cargo.toml b/kclvm/query/Cargo.toml index 939f185aa..887cf706d 100644 --- a/kclvm/query/Cargo.toml +++ b/kclvm/query/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] anyhow = "1.0" compiler_base_session = {path = "../../compiler_base/session", version = "0.0.12"} +compiler_base_macros = "0.0.1" kclvm-ast = {path = "../ast", version = "0.4.5"} kclvm-ast-pretty = {path = "../ast_pretty", version = "0.4.5"} diff --git a/kclvm/query/src/override.rs b/kclvm/query/src/override.rs index 216bf99b0..7adc3ca1c 100644 --- a/kclvm/query/src/override.rs +++ b/kclvm/query/src/override.rs @@ -2,12 +2,12 @@ use std::collections::HashSet; use anyhow::{anyhow, Result}; +use compiler_base_macros::bug; use kclvm_ast::config::try_get_config_expr_mut; use kclvm_ast::path::{get_attr_paths_from_config_expr, get_key_path}; use kclvm_ast::walker::MutSelfMutWalker; use kclvm_ast::{ast, walk_if_mut}; use kclvm_ast_pretty::print_ast_module; -use kclvm_error::bug; use kclvm_parser::parse_expr; use kclvm_sema::pre_process::{fix_config_expr_nest_attr, transform_multi_assign}; diff --git a/kclvm/runner/Cargo.toml b/kclvm/runner/Cargo.toml index 9512d2d13..362d4d278 100644 --- a/kclvm/runner/Cargo.toml +++ b/kclvm/runner/Cargo.toml @@ -24,6 +24,7 @@ anyhow = "1.0" once_cell = "1.10" cc = "1.0" compiler_base_session = {path = "../../compiler_base/session", version = "0.0.12"} +compiler_base_macros = "0.0.1" kclvm-ast = {path = "../ast", version = "0.4.5"} kclvm-parser = {path = "../parser", version = "0.4.5"} diff --git a/kclvm/runner/benches/bench_runner.rs b/kclvm/runner/benches/bench_runner.rs index 323cabae0..8b2d5ff74 100644 --- a/kclvm/runner/benches/bench_runner.rs +++ b/kclvm/runner/benches/bench_runner.rs @@ -1,8 +1,10 @@ use std::path::Path; +use std::sync::Arc; use criterion::{criterion_group, criterion_main, Criterion}; use walkdir::WalkDir; +use compiler_base_session::Session; use kclvm_parser::load_program; use kclvm_runner::{execute, runner::ExecProgramArgs}; @@ -34,7 +36,7 @@ fn exec(file: &str) -> Result { let plugin_agent = 0; let opts = args.get_load_program_options(); // Load AST program - let program = load_program(&[file], Some(opts)).unwrap(); + let program = load_program(Arc::new(Session::default()), &[file], Some(opts)).unwrap(); // Resolve ATS, generate libs, link libs and execute. execute(program, plugin_agent, &args) } diff --git a/kclvm/runner/src/assembler.rs b/kclvm/runner/src/assembler.rs index a8393003b..cdfae76b0 100644 --- a/kclvm/runner/src/assembler.rs +++ b/kclvm/runner/src/assembler.rs @@ -1,3 +1,4 @@ +use compiler_base_macros::bug; use indexmap::IndexMap; use kclvm_ast::ast::{self, Program}; use kclvm_compiler::codegen::{ @@ -5,7 +6,6 @@ use kclvm_compiler::codegen::{ EmitOptions, }; use kclvm_config::cache::{load_pkg_cache, save_pkg_cache, CacheOption}; -use kclvm_error::bug; use kclvm_sema::resolver::scope::ProgramScope; use std::{ collections::HashMap, diff --git a/kclvm/runner/src/lib.rs b/kclvm/runner/src/lib.rs index b63d6aa61..033674064 100644 --- a/kclvm/runner/src/lib.rs +++ b/kclvm/runner/src/lib.rs @@ -108,7 +108,7 @@ pub fn exec_program( } let start_time = SystemTime::now(); - let exec_result = execute(program, plugin_agent, args); + let exec_result = execute(sess, program, plugin_agent, args); let escape_time = match SystemTime::now().duration_since(start_time) { Ok(dur) => dur.as_secs_f32(), Err(err) => return Err(err.to_string()), @@ -183,20 +183,21 @@ pub fn exec_program( /// /// // Parse kcl file /// let kcl_path = "./src/test_datas/init_check_order_0/main.k"; -/// let prog = load_program(sess, &[kcl_path], Some(opts)).unwrap(); +/// let prog = load_program(sess.clone(), &[kcl_path], Some(opts)).unwrap(); /// /// // Resolve ast, generate libs, link libs and execute. /// // Result is the kcl in json format. -/// let result = execute(prog, plugin_agent, &args).unwrap(); +/// let result = execute(sess, prog, plugin_agent, &args).unwrap(); /// ``` pub fn execute( + sess: Arc, mut program: Program, plugin_agent: u64, args: &ExecProgramArgs, ) -> Result { // Resolve ast let scope = resolve_program(&mut program); - scope.alert_scope_diagnostics()?; + scope.alert_scope_diagnostics_with_session(sess)?; // Create a temp entry file and the temp dir will be delete automatically let temp_dir = tempdir().unwrap(); @@ -254,7 +255,12 @@ pub fn execute_module(mut m: Module) -> Result { cmd_overrides: vec![], }; - execute(prog, 0, &ExecProgramArgs::default()) + execute( + Arc::new(Session::default()), + prog, + 0, + &ExecProgramArgs::default(), + ) } /// Clean all the tmp files generated during lib generating and linking. diff --git a/kclvm/runner/src/tests.rs b/kclvm/runner/src/tests.rs index b47f9fac6..e1b475d28 100644 --- a/kclvm/runner/src/tests.rs +++ b/kclvm/runner/src/tests.rs @@ -215,7 +215,7 @@ fn execute_for_test(kcl_path: &String) -> String { // Parse kcl file let program = load_test_program(kcl_path.to_string()); // Generate libs, link libs and execute. - execute(program, plugin_agent, &args).unwrap() + execute(Arc::new(Session::default()), program, plugin_agent, &args).unwrap() } fn gen_assembler(entry_file: &str, test_kcl_case_path: &str) -> KclvmAssembler { @@ -565,7 +565,7 @@ fn exec(file: &str) -> Result { // Load AST program let program = load_program(sess.clone(), &[file], Some(opts)).unwrap(); // Resolve ATS, generate libs, link libs and execute. - execute(program, plugin_agent, &args) + execute(sess, program, plugin_agent, &args) } /// Run all kcl files at path and compare the exec result with the expect output. diff --git a/kclvm/sema/Cargo.toml b/kclvm/sema/Cargo.toml index 48d5478d5..b041560f1 100644 --- a/kclvm/sema/Cargo.toml +++ b/kclvm/sema/Cargo.toml @@ -21,6 +21,7 @@ kclvm-error = {path = "../error", version = "0.4.5"} kclvm-span = {path = "../span", version = "0.4.5"} compiler_base_span = {path = "../../compiler_base/span", version = "0.0.2"} compiler_base_session = {path = "../../compiler_base/session", version = "0.0.12"} +compiler_base_macros = "0.0.1" [dev-dependencies] kclvm-parser = {path = "../parser", version = "0.4.5"} diff --git a/kclvm/sema/src/lib.rs b/kclvm/sema/src/lib.rs index 6a7eb7ad8..9a90ffecf 100644 --- a/kclvm/sema/src/lib.rs +++ b/kclvm/sema/src/lib.rs @@ -8,4 +8,4 @@ pub mod resolver; pub mod ty; #[macro_use] -extern crate kclvm_error; +extern crate compiler_base_macros; diff --git a/kclvm/sema/src/pre_process/identifier.rs b/kclvm/sema/src/pre_process/identifier.rs index 2a6fc9d84..48e33d277 100644 --- a/kclvm/sema/src/pre_process/identifier.rs +++ b/kclvm/sema/src/pre_process/identifier.rs @@ -146,7 +146,10 @@ impl<'ctx> MutSelfMutWalker<'ctx> for QualifiedIdentifierTransformer { pos: self.global_names.get(name).unwrap().clone(), style: Style::LineAndColumn, message: format!("The variable '{}' is declared here firstly", name), - note: None, + note: Some(format!( + "change the variable name to '_{}' to make it mutable", + name + )), }, ], ); diff --git a/kclvm/sema/src/resolver/global.rs b/kclvm/sema/src/resolver/global.rs index 0eb28e732..dfc9c73ea 100644 --- a/kclvm/sema/src/resolver/global.rs +++ b/kclvm/sema/src/resolver/global.rs @@ -238,7 +238,10 @@ impl<'ctx> Resolver<'ctx> { .clone(), style: Style::LineAndColumn, message: format!("The variable '{}' is declared here firstly", name), - note: Some(format!("change the variable name to '_{}'", name)), + note: Some(format!( + "change the variable name to '_{}' to make it mutable", + name + )), }, ], ); @@ -254,16 +257,20 @@ impl<'ctx> Resolver<'ctx> { ErrorKind::TypeError, &[ Message { - pos: obj.start.clone(), + pos: start.clone(), style: Style::LineAndColumn, - message: format!("expect {}", obj.ty.ty_str()), + message: format!( + "can not change the type of '{}' to {}", + name, + obj.ty.ty_str() + ), note: None, }, Message { - pos: start.clone(), + pos: obj.start.clone(), style: Style::LineAndColumn, - message: format!("can not change the type of '{}'", name), - note: Some(format!("got {}", ty.ty_str())), + message: format!("expect {}", obj.ty.ty_str()), + note: None, }, ], ); @@ -322,7 +329,10 @@ impl<'ctx> Resolver<'ctx> { .clone(), style: Style::LineAndColumn, message: format!("The variable '{}' is declared here firstly", name), - note: Some(format!("Change the variable name to '_{}'", name)), + note: Some(format!( + "change the variable name to '_{}' to make it mutable", + name + )), }, ], ); diff --git a/kclvm/sema/src/resolver/mod.rs b/kclvm/sema/src/resolver/mod.rs index 8efcef916..ed101bbeb 100644 --- a/kclvm/sema/src/resolver/mod.rs +++ b/kclvm/sema/src/resolver/mod.rs @@ -84,7 +84,7 @@ impl<'ctx> Resolver<'ctx> { ProgramScope { scope_map: self.scope_map.clone(), import_names: self.ctx.import_names.clone(), - diagnostics: self.handler.diagnostics.clone(), + handler: self.handler.clone(), } } @@ -92,7 +92,7 @@ impl<'ctx> Resolver<'ctx> { let mut scope = self.check(pkgpath); self.lint_check_scope_map(); for diag in &self.linter.handler.diagnostics { - scope.diagnostics.insert(diag.clone()); + scope.handler.diagnostics.insert(diag.clone()); } scope } diff --git a/kclvm/sema/src/resolver/schema.rs b/kclvm/sema/src/resolver/schema.rs index 2b6ec3725..ffc61a295 100644 --- a/kclvm/sema/src/resolver/schema.rs +++ b/kclvm/sema/src/resolver/schema.rs @@ -7,7 +7,7 @@ use crate::resolver::Resolver; use crate::ty::{Decorator, DecoratorTarget, TypeKind}; use kclvm_ast::ast; use kclvm_ast::walker::MutSelfTypedResultWalker; -use kclvm_error::Position; +use kclvm_error::{ErrorKind, Message, Position, Style}; use super::node::ResolvedResult; use super::scope::{ScopeKind, ScopeObject, ScopeObjectKind}; @@ -18,7 +18,20 @@ impl<'ctx> Resolver<'ctx> { schema_stmt: &'ctx ast::SchemaStmt, ) -> ResolvedResult { let ty = self.lookup_type_from_scope(&schema_stmt.name.node, schema_stmt.name.get_pos()); - let scope_ty = ty.into_schema_type(); + let scope_ty = if ty.is_schema() { + ty.into_schema_type() + } else { + self.handler.add_error( + ErrorKind::TypeError, + &[Message { + pos: schema_stmt.get_pos(), + style: Style::LineAndColumn, + message: format!("expect schema type, got {}", ty.ty_str()), + note: None, + }], + ); + return ty; + }; self.ctx.schema = Some(Rc::new(RefCell::new(scope_ty.clone()))); let (start, end) = schema_stmt.get_span_pos(); self.do_parameters_check(&schema_stmt.args); @@ -92,7 +105,20 @@ impl<'ctx> Resolver<'ctx> { pub(crate) fn resolve_rule_stmt(&mut self, rule_stmt: &'ctx ast::RuleStmt) -> ResolvedResult { let ty = self.lookup_type_from_scope(&rule_stmt.name.node, rule_stmt.name.get_pos()); - let scope_ty = ty.into_schema_type(); + let scope_ty = if ty.is_schema() { + ty.into_schema_type() + } else { + self.handler.add_error( + ErrorKind::TypeError, + &[Message { + pos: rule_stmt.get_pos(), + style: Style::LineAndColumn, + message: format!("expect rule type, got {}", ty.ty_str()), + note: None, + }], + ); + return ty; + }; self.ctx.schema = Some(Rc::new(RefCell::new(scope_ty.clone()))); let (start, end) = rule_stmt.get_span_pos(); self.do_parameters_check(&rule_stmt.args); diff --git a/kclvm/sema/src/resolver/scope.rs b/kclvm/sema/src/resolver/scope.rs index 18e39d773..b513572e9 100644 --- a/kclvm/sema/src/resolver/scope.rs +++ b/kclvm/sema/src/resolver/scope.rs @@ -1,9 +1,9 @@ +use compiler_base_session::Session; use indexmap::IndexMap; -use indexmap::IndexSet; use kclvm_ast::{ast, MAIN_PKG}; -use kclvm_error::Diagnostic; use kclvm_error::Handler; +use std::sync::Arc; use std::{ cell::RefCell, rc::{Rc, Weak}, @@ -199,7 +199,7 @@ impl Scope { pub struct ProgramScope { pub scope_map: IndexMap>>, pub import_names: IndexMap>, - pub diagnostics: IndexSet, + pub handler: Handler, } impl ProgramScope { @@ -217,14 +217,20 @@ impl ProgramScope { /// Return diagnostic json string but do not abort if exist any diagnostic. pub fn alert_scope_diagnostics(&self) -> Result<(), String> { - if !self.diagnostics.is_empty() { - let mut err_handler = Handler::default(); - err_handler.diagnostics = self.diagnostics.clone(); - err_handler.alert_if_any_errors() + if !self.handler.diagnostics.is_empty() { + self.handler.alert_if_any_errors() } else { Ok(()) } } + + /// Return diagnostic json using session string but do not abort if exist any diagnostic. + pub fn alert_scope_diagnostics_with_session(&self, sess: Arc) -> Result<(), String> { + for diag in &self.handler.diagnostics { + sess.add_err(diag.clone()).unwrap(); + } + self.alert_scope_diagnostics() + } } /// Construct a builtin scope diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs index e810db6e0..b7144c450 100644 --- a/kclvm/sema/src/resolver/tests.rs +++ b/kclvm/sema/src/resolver/tests.rs @@ -82,10 +82,26 @@ fn test_pkg_init_in_schema_resolve() { #[test] fn test_resolve_program_fail() { + let cases = &[ + "./src/resolver/test_fail_data/attr.k", + "./src/resolver/test_fail_data/cannot_find_module.k", + "./src/resolver/test_fail_data/config_expr.k", + "./src/resolver/test_fail_data/module_optional_select.k", + "./src/resolver/test_fail_data/unmatched_args.k", + ]; + for case in cases { + let mut program = parse_program(case).unwrap(); + let scope = resolve_program(&mut program); + assert!(scope.handler.diagnostics.len() > 0); + } +} + +#[test] +fn test_resolve_program_mismatch_type_fail() { let mut program = parse_program("./src/resolver/test_fail_data/config_expr.k").unwrap(); let scope = resolve_program(&mut program); - assert_eq!(scope.diagnostics.len(), 1); - let diag = &scope.diagnostics[0]; + assert_eq!(scope.handler.diagnostics.len(), 1); + let diag = &scope.handler.diagnostics[0]; assert_eq!(diag.code, Some(DiagnosticId::Error(ErrorKind::TypeError))); assert_eq!(diag.messages.len(), 1); assert_eq!( @@ -113,8 +129,8 @@ fn test_resolve_program_cycle_reference_fail() { "Module 'file2' imported but unused", "Module 'file1' imported but unused", ]; - assert_eq!(scope.diagnostics.len(), err_messages.len()); - for (diag, msg) in scope.diagnostics.iter().zip(err_messages.iter()) { + assert_eq!(scope.handler.diagnostics.len(), err_messages.len()); + for (diag, msg) in scope.handler.diagnostics.iter().zip(err_messages.iter()) { assert_eq!(diag.messages[0].message, msg.to_string(),); } } @@ -157,16 +173,16 @@ fn test_cannot_find_module() { ) .unwrap(); let scope = resolve_program(&mut program); - assert_eq!(scope.diagnostics[0].messages[0].pos.column, None); + assert_eq!(scope.handler.diagnostics[0].messages[0].pos.column, None); } #[test] fn test_resolve_program_illegal_attr_fail() { let mut program = parse_program("./src/resolver/test_fail_data/attr.k").unwrap(); let scope = resolve_program(&mut program); - assert_eq!(scope.diagnostics.len(), 2); + assert_eq!(scope.handler.diagnostics.len(), 2); let expect_err_msg = "A attribute must be string type, got 'Data'"; - let diag = &scope.diagnostics[0]; + let diag = &scope.handler.diagnostics[0]; assert_eq!( diag.code, Some(DiagnosticId::Error(ErrorKind::IllegalAttributeError)) @@ -174,7 +190,7 @@ fn test_resolve_program_illegal_attr_fail() { assert_eq!(diag.messages.len(), 1); assert_eq!(diag.messages[0].pos.line, 4); assert_eq!(diag.messages[0].message, expect_err_msg,); - let diag = &scope.diagnostics[1]; + let diag = &scope.handler.diagnostics[1]; assert_eq!( diag.code, Some(DiagnosticId::Error(ErrorKind::IllegalAttributeError)) @@ -188,9 +204,9 @@ fn test_resolve_program_illegal_attr_fail() { fn test_resolve_program_unmatched_args_fail() { let mut program = parse_program("./src/resolver/test_fail_data/unmatched_args.k").unwrap(); let scope = resolve_program(&mut program); - assert_eq!(scope.diagnostics.len(), 2); + assert_eq!(scope.handler.diagnostics.len(), 2); let expect_err_msg = "\"Foo\" takes 1 positional argument but 3 were given"; - let diag = &scope.diagnostics[0]; + let diag = &scope.handler.diagnostics[0]; assert_eq!( diag.code, Some(DiagnosticId::Error(ErrorKind::CompileError)) @@ -200,7 +216,7 @@ fn test_resolve_program_unmatched_args_fail() { assert_eq!(diag.messages[0].message, expect_err_msg); let expect_err_msg = "\"f\" takes 1 positional argument but 2 were given"; - let diag = &scope.diagnostics[1]; + let diag = &scope.handler.diagnostics[1]; assert_eq!( diag.code, Some(DiagnosticId::Error(ErrorKind::CompileError)) @@ -215,10 +231,10 @@ fn test_resolve_program_module_optional_select_fail() { let mut program = parse_program("./src/resolver/test_fail_data/module_optional_select.k").unwrap(); let scope = resolve_program(&mut program); - assert_eq!(scope.diagnostics.len(), 2); + assert_eq!(scope.handler.diagnostics.len(), 2); let expect_err_msg = "For the module type, the use of '?.log' is unnecessary and it can be modified as '.log'"; - let diag = &scope.diagnostics[0]; + let diag = &scope.handler.diagnostics[0]; assert_eq!( diag.code, Some(DiagnosticId::Error(ErrorKind::CompileError)) @@ -228,7 +244,7 @@ fn test_resolve_program_module_optional_select_fail() { assert_eq!(diag.messages[0].message, expect_err_msg); let expect_err_msg = "Module 'math' imported but unused"; - let diag = &scope.diagnostics[1]; + let diag = &scope.handler.diagnostics[1]; assert_eq!( diag.code, Some(DiagnosticId::Warning(WarningKind::UnusedImportWarning)) diff --git a/kclvm/tools/src/lint/mod.rs b/kclvm/tools/src/lint/mod.rs index c9082eeca..808a5fc62 100644 --- a/kclvm/tools/src/lint/mod.rs +++ b/kclvm/tools/src/lint/mod.rs @@ -72,5 +72,5 @@ pub fn lint_files( ); } }; - classification(&resolve_program(&mut program).diagnostics) + classification(&resolve_program(&mut program).handler.diagnostics) } From 9c189c91918e54a0502186e451bf26318ce4afd2 Mon Sep 17 00:00:00 2001 From: peefy Date: Thu, 9 Mar 2023 21:31:52 +0800 Subject: [PATCH 3/4] feat: impl Into for PanicInfo and Diagnostic --- kclvm/Cargo.lock | 1 - kclvm/error/Cargo.toml | 1 - kclvm/error/src/diagnostic.rs | 38 +------------------- kclvm/error/src/lib.rs | 61 +++++++++++++++++++++++++++------ kclvm/parser/src/parser/expr.rs | 12 ++----- kclvm/tools/src/lint/mod.rs | 12 +++---- 6 files changed, 58 insertions(+), 67 deletions(-) diff --git a/kclvm/Cargo.lock b/kclvm/Cargo.lock index 125d1973f..9a9295f2f 100644 --- a/kclvm/Cargo.lock +++ b/kclvm/Cargo.lock @@ -1254,7 +1254,6 @@ dependencies = [ "indexmap", "kclvm-runtime", "kclvm-span", - "termcolor", "termize", "tracing", ] diff --git a/kclvm/error/Cargo.toml b/kclvm/error/Cargo.toml index 89c36be12..32689ea16 100644 --- a/kclvm/error/Cargo.toml +++ b/kclvm/error/Cargo.toml @@ -16,7 +16,6 @@ kclvm-runtime = {path = "../runtime", version = "0.4.5"} anyhow = "1.0" tracing = "0.1" atty = "0.2" -termcolor = "1.0" annotate-snippets = { version = "0.9.0", default-features = false, features = ["color"] } termize = "0.1.1" indexmap = "1.0" diff --git a/kclvm/error/src/diagnostic.rs b/kclvm/error/src/diagnostic.rs index ca83a58f4..444574d36 100644 --- a/kclvm/error/src/diagnostic.rs +++ b/kclvm/error/src/diagnostic.rs @@ -1,10 +1,7 @@ +use kclvm_span::Loc; use std::fmt; use std::hash::Hash; -use indexmap::IndexSet; -use kclvm_span::Loc; -use termcolor::{Color, ColorSpec}; - use crate::{ErrorKind, WarningKind}; /// Diagnostic structure. @@ -156,22 +153,6 @@ impl Level { Level::Note => "note", } } - - pub fn color(&self) -> ColorSpec { - let mut spec = ColorSpec::new(); - match self { - Level::Error => { - spec.set_fg(Some(Color::Red)).set_intense(true); - } - Level::Warning => { - spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows)); - } - Level::Note => { - spec.set_fg(Some(Color::Green)).set_intense(true); - } - } - spec - } } impl fmt::Display for Level { @@ -189,20 +170,3 @@ pub enum Style { LineAndColumn, Line, } - -/// Classify diagnostics into errors and warnings. -pub fn classification( - diagnostics: &IndexSet, -) -> (IndexSet, IndexSet) { - let (mut errs, mut warnings) = (IndexSet::new(), IndexSet::new()); - for diag in diagnostics { - if diag.level == Level::Error { - errs.insert(diag.clone()); - } else if diag.level == Level::Warning { - warnings.insert(diag.clone()); - } else { - continue; - } - } - (errs, warnings) -} diff --git a/kclvm/error/src/lib.rs b/kclvm/error/src/lib.rs index d6f5acdcb..983b3be3b 100644 --- a/kclvm/error/src/lib.rs +++ b/kclvm/error/src/lib.rs @@ -153,17 +153,7 @@ impl Handler { /// Put a runtime panic info the handler diagnostic buffer. pub fn add_panic_info(&mut self, panic_info: &PanicInfo) -> &mut Self { - let diag = Diagnostic::new_with_code( - Level::Error, - &panic_info.message, - Position { - filename: panic_info.kcl_file.clone(), - line: panic_info.kcl_line as u64, - column: Some(panic_info.kcl_col as u64), - }, - Some(DiagnosticId::Error(E2L23.kind)), - ); - self.add_diagnostic(diag); + self.add_diagnostic(panic_info.clone().into()); self } @@ -216,6 +206,21 @@ impl Handler { self } + /// Classify diagnostics into errors and warnings. + pub fn classification(&self) -> (IndexSet, IndexSet) { + let (mut errs, mut warnings) = (IndexSet::new(), IndexSet::new()); + for diag in &self.diagnostics { + if diag.level == Level::Error { + errs.insert(diag.clone()); + } else if diag.level == Level::Warning { + warnings.insert(diag.clone()); + } else { + continue; + } + } + (errs, warnings) + } + /// Store a diagnostics #[inline] fn add_diagnostic(&mut self, diagnostic: Diagnostic) -> &mut Self { @@ -225,6 +230,40 @@ impl Handler { } } +impl From for Diagnostic { + fn from(panic_info: PanicInfo) -> Self { + let mut diag = Diagnostic::new_with_code( + Level::Error, + if panic_info.kcl_arg_msg.is_empty() { + &panic_info.message + } else { + &panic_info.kcl_arg_msg + }, + Position { + filename: panic_info.kcl_file.clone(), + line: panic_info.kcl_line as u64, + column: None, + }, + Some(DiagnosticId::Error(E3M38.kind)), + ); + if panic_info.kcl_config_meta_file.is_empty() { + return diag; + } + let mut config_meta_diag = Diagnostic::new_with_code( + Level::Error, + &panic_info.kcl_config_meta_arg_msg, + Position { + filename: panic_info.kcl_config_meta_file.clone(), + line: panic_info.kcl_config_meta_line as u64, + column: Some(panic_info.kcl_config_meta_col as u64), + }, + Some(DiagnosticId::Error(E3M38.kind)), + ); + config_meta_diag.messages.append(&mut diag.messages); + config_meta_diag + } +} + #[derive(Debug, Clone)] pub enum ParseError { UnexpectedToken { diff --git a/kclvm/parser/src/parser/expr.rs b/kclvm/parser/src/parser/expr.rs index e13274248..17150527c 100644 --- a/kclvm/parser/src/parser/expr.rs +++ b/kclvm/parser/src/parser/expr.rs @@ -563,11 +563,7 @@ impl<'a> Parser<'a> { } } _ => self.sess.struct_token_error( - &[ - TokenKind::ident_value(), - TokenKind::literal_value(), - TokenKind::OpenDelim(DelimToken::NoDelim).into(), - ], + &[TokenKind::ident_value(), TokenKind::literal_value()], self.token, ), } @@ -769,11 +765,7 @@ impl<'a> Parser<'a> { } } _ => self.sess.struct_token_error( - &[ - TokenKind::ident_value(), - TokenKind::literal_value(), - TokenKind::OpenDelim(DelimToken::NoDelim).into(), - ], + &[TokenKind::ident_value(), TokenKind::literal_value()], self.token, ), } diff --git a/kclvm/tools/src/lint/mod.rs b/kclvm/tools/src/lint/mod.rs index 808a5fc62..eaa468838 100644 --- a/kclvm/tools/src/lint/mod.rs +++ b/kclvm/tools/src/lint/mod.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use compiler_base_session::Session; use indexmap::IndexSet; -use kclvm_error::{diagnostic::classification, Diagnostic, Handler}; +use kclvm_error::{Diagnostic, Handler}; use kclvm_parser::{load_program, LoadProgramOptions}; use kclvm_runtime::PanicInfo; use kclvm_sema::resolver::resolve_program; @@ -65,12 +65,10 @@ pub fn lint_files( let mut program = match load_program(Arc::new(Session::default()), files, opts) { Ok(p) => p, Err(err_str) => { - return classification( - &Handler::default() - .add_panic_info(&PanicInfo::from(err_str)) - .diagnostics, - ); + return Handler::default() + .add_panic_info(&PanicInfo::from(err_str)) + .classification(); } }; - classification(&resolve_program(&mut program).handler.diagnostics) + resolve_program(&mut program).handler.classification() } From 955b7652c40adee964650399bea109a6007f13dc Mon Sep 17 00:00:00 2001 From: peefy Date: Fri, 10 Mar 2023 12:00:44 +0800 Subject: [PATCH 4/4] refactor: enhance file not found error message showing. --- kclvm/error/src/lib.rs | 72 ++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/kclvm/error/src/lib.rs b/kclvm/error/src/lib.rs index 983b3be3b..268a2215b 100644 --- a/kclvm/error/src/lib.rs +++ b/kclvm/error/src/lib.rs @@ -66,7 +66,7 @@ impl Handler { pub fn emit(&mut self) -> Result { let sess = Session::default(); for diag in &self.diagnostics { - sess.add_err(diag.clone()).unwrap(); + sess.add_err(diag.clone())?; } sess.emit_stashed_diagnostics()?; Ok(self.has_errors()) @@ -364,37 +364,47 @@ impl SessionDiagnostic for Diagnostic { // Append a new line. diag.append_component(Box::new(String::from("\n"))); for msg in &self.messages { - let sess = Session::new_with_file_and_code(&msg.pos.filename, None)?; - let source = sess.sm.lookup_source_file(new_byte_pos(0)); - let line = source.get_line((msg.pos.line - 1) as usize); - let content = line - .as_ref() - .ok_or(anyhow::anyhow!("Failed to load source file"))?; - let snippet = Snippet { - title: None, - footer: vec![], - slices: vec![Slice { - source: content, - line_start: msg.pos.line as usize, - origin: Some(&msg.pos.filename), - annotations: vec![SourceAnnotation { - range: match msg.pos.column { - Some(column) => (column as usize, (column + 1) as usize), - None => (0, 0), - }, - label: &msg.message, - annotation_type: AnnotationType::Error, - }], - fold: true, - }], - opt: FormatOptions { - color: true, - anonymized_line_numbers: false, - margin: None, - }, + match Session::new_with_file_and_code(&msg.pos.filename, None) { + Ok(sess) => { + let source = sess.sm.lookup_source_file(new_byte_pos(0)); + let line = source.get_line((msg.pos.line - 1) as usize); + match line.as_ref() { + Some(content) => { + let snippet = Snippet { + title: None, + footer: vec![], + slices: vec![Slice { + source: content, + line_start: msg.pos.line as usize, + origin: Some(&msg.pos.filename), + annotations: vec![SourceAnnotation { + range: match msg.pos.column { + Some(column) => { + (column as usize, (column + 1) as usize) + } + None => (0, 0), + }, + label: &msg.message, + annotation_type: AnnotationType::Error, + }], + fold: true, + }], + opt: FormatOptions { + color: true, + anonymized_line_numbers: false, + margin: None, + }, + }; + let dl = DisplayList::from(snippet); + diag.append_component(Box::new(format!("{dl}\n"))); + } + None => { + diag.append_component(Box::new(format!("{}\n", msg.message))); + } + }; + } + Err(_) => diag.append_component(Box::new(format!("{}\n", msg.message))), }; - let dl = DisplayList::from(snippet); - diag.append_component(Box::new(format!("{dl}\n"))); if let Some(note) = &msg.note { diag.append_component(Box::new(Label::Note)); diag.append_component(Box::new(format!(": {}\n", note)));