Skip to content

Commit

Permalink
Feat: Change diagnostic position from pos(start) to range(start, end) (…
Browse files Browse the repository at this point in the history 
…#638)

* feat: change position of diag. Change the position of diagnostic from pos(start) to range(start, end)

* define type alias `type Range = (Position, Position)`
  • Loading branch information
@He1pa
He1pa committed Aug 7, 2023
1 parent 3824aa0 commit 939c2ff
Show file tree
Hide file tree
Showing 26 changed files with 501 additions and 366 deletions.
6 changes: 3 additions & 3 deletions kclvm/ast/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use compiler_base_span::{Loc, Span};

use super::token;
use crate::{node_ref, pos::ContainsPos};
use kclvm_error::Position;
use kclvm_error::{diagnostic::Range, Position};

/// PosTuple denotes the tuple `(filename, line, column, end_line, end_column)`.
pub type PosTuple = (String, u64, u64, u64, u64);
Expand All @@ -62,8 +62,8 @@ impl Into<PosTuple> for Pos {
}
}

impl Into<(Position, Position)> for Pos {
fn into(self) -> (Position, Position) {
impl Into<Range> for Pos {
fn into(self) -> Range {
(
Position {
filename: self.0.clone(),
Expand Down
4 changes: 2 additions & 2 deletions kclvm/ast/src/pos.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use kclvm_error::Position;
use kclvm_error::{diagnostic::Range, Position};

use crate::ast;

Expand All @@ -9,7 +9,7 @@ pub trait ContainsPos {

pub trait GetPos {
/// Get start and end position from node.
fn get_span_pos(&self) -> (Position, Position) {
fn get_span_pos(&self) -> Range {
(self.get_pos(), self.get_end_pos())
}
/// Get start pos from node.
Expand Down
14 changes: 6 additions & 8 deletions kclvm/compiler/src/codegen/llvm/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1777,14 +1777,12 @@ impl<'ctx> LLVMCodeGenContext<'ctx> {
let is_in_schema = self.schema_stack.borrow().len() > 0;
if !is_in_schema {
let mut handler = self.handler.borrow_mut();
handler.add_compile_error(
&err.message,
Position {
filename: self.current_filename(),
line: *self.current_line.borrow(),
column: None,
},
);
let pos = Position {
filename: self.current_filename(),
line: *self.current_line.borrow(),
column: None,
};
handler.add_compile_error(&err.message, (pos.clone(), pos));
handler.abort_if_any_errors()
}
result
Expand Down
12 changes: 7 additions & 5 deletions kclvm/error/src/diagnostic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,22 @@ impl From<Loc> for Position {
}

impl Diagnostic {
pub fn new(level: Level, message: &str, pos: Position) -> Self {
Diagnostic::new_with_code(level, message, None, pos, None)
pub fn new(level: Level, message: &str, range: Range) -> Self {
Diagnostic::new_with_code(level, message, None, range, None)
}

/// New a diagnostic with error code.
pub fn new_with_code(
level: Level,
message: &str,
note: Option<&str>,
pos: Position,
range: Range,
code: Option<DiagnosticId>,
) -> Self {
Diagnostic {
level,
messages: vec![Message {
pos,
range,
style: Style::LineAndColumn,
message: message.to_string(),
note: note.map(|s| s.to_string()),
Expand All @@ -125,9 +125,11 @@ impl Diagnostic {
}
}

pub type Range = (Position, Position);

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Message {
pub pos: Position,
pub range: Range,
pub style: Style,
pub message: String,
pub note: Option<String>,
Expand Down
4 changes: 2 additions & 2 deletions kclvm/error/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub enum WarningKind {
/// let mut handler = Handler::default();
/// handler.add_warning(WarningKind::UnusedImportWarning, &[
/// Message {
/// pos: Position::dummy_pos(),
/// range: (Position::dummy_pos(), Position::dummy_pos()),
/// style: Style::LineAndColumn,
/// message: "Module 'a' imported but unused.".to_string(),
/// note: None,
Expand All @@ -168,7 +168,7 @@ impl std::fmt::Display for WarningKind {
/// let mut handler = Handler::default();
/// handler.add_warning(WarningKind::UnusedImportWarning, &[
/// Message {
/// pos: Position::dummy_pos(),
/// range: (Position::dummy_pos(), Position::dummy_pos()),
/// style: Style::LineAndColumn,
/// message: "Module 'a' imported but unused.".to_string(),
/// note: None,
Expand Down
71 changes: 35 additions & 36 deletions kclvm/error/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use compiler_base_error::{
};
use compiler_base_session::{Session, SessionDiagnostic};
use compiler_base_span::{span::new_byte_pos, Span};
use diagnostic::Range;
use indexmap::IndexSet;
use kclvm_runtime::PanicInfo;
use std::{any::Any, sync::Arc};
Expand Down Expand Up @@ -91,13 +92,13 @@ impl Handler {
}

/// 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 {
pub fn add_syntex_error(&mut self, msg: &str, range: Range) -> &mut Self {
let message = format!("Invalid syntax: {msg}");
let diag = Diagnostic::new_with_code(
Level::Error,
&message,
None,
pos,
range,
Some(DiagnosticId::Error(E1001.kind)),
);
self.add_diagnostic(diag);
Expand All @@ -106,12 +107,12 @@ impl Handler {
}

/// Construct a type error and put it into the handler diagnostic buffer
pub fn add_type_error(&mut self, msg: &str, pos: Position) -> &mut Self {
pub fn add_type_error(&mut self, msg: &str, range: Range) -> &mut Self {
let diag = Diagnostic::new_with_code(
Level::Error,
msg,
None,
pos,
range,
Some(DiagnosticId::Error(E2G22.kind)),
);
self.add_diagnostic(diag);
Expand All @@ -120,12 +121,12 @@ impl Handler {
}

/// Construct a type error and put it into the handler diagnostic buffer
pub fn add_compile_error(&mut self, msg: &str, pos: Position) -> &mut Self {
pub fn add_compile_error(&mut self, msg: &str, range: Range) -> &mut Self {
let diag = Diagnostic::new_with_code(
Level::Error,
msg,
None,
pos,
range,
Some(DiagnosticId::Error(E2L23.kind)),
);
self.add_diagnostic(diag);
Expand All @@ -146,7 +147,7 @@ impl Handler {
/// let mut handler = Handler::default();
/// handler.add_error(ErrorKind::InvalidSyntax, &[
/// Message {
/// pos: Position::dummy_pos(),
/// range: (Position::dummy_pos(), Position::dummy_pos()),
/// style: Style::LineAndColumn,
/// message: "Invalid syntax: expected '+', got '-'".to_string(),
/// note: None,
Expand All @@ -170,7 +171,7 @@ impl Handler {
/// let mut handler = Handler::default();
/// handler.add_warning(WarningKind::UnusedImportWarning, &[
/// Message {
/// pos: Position::dummy_pos(),
/// range: (Position::dummy_pos(), Position::dummy_pos()),
/// style: Style::LineAndColumn,
/// message: "Module 'a' imported but unused.".to_string(),
/// note: None,
Expand Down Expand Up @@ -210,7 +211,7 @@ impl Handler {
/// ```
/// use kclvm_error::*;
/// let mut handler = Handler::default();
/// handler.add_diagnostic(Diagnostic::new_with_code(Level::Error, "error message", None, Position::dummy_pos(), Some(DiagnosticId::Error(E1001.kind))));
/// handler.add_diagnostic(Diagnostic::new_with_code(Level::Error, "error message", None, (Position::dummy_pos(), Position::dummy_pos()), Some(DiagnosticId::Error(E1001.kind))));
/// ```
#[inline]
pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) -> &mut Self {
Expand All @@ -229,17 +230,12 @@ impl From<PanicInfo> for Diagnostic {
};

let mut diag = if panic_info.backtrace.is_empty() {
Diagnostic::new_with_code(
Level::Error,
&panic_msg,
None,
Position {
filename: panic_info.kcl_file.clone(),
line: panic_info.kcl_line as u64,
column: None,
},
None,
)
let pos = Position {
filename: panic_info.kcl_file.clone(),
line: panic_info.kcl_line as u64,
column: None,
};
Diagnostic::new_with_code(Level::Error, &panic_msg, None, (pos.clone(), pos), None)
} else {
let mut backtrace_msg = "backtrace:\n".to_string();
let mut backtrace = panic_info.backtrace.clone();
Expand All @@ -254,31 +250,33 @@ impl From<PanicInfo> for Diagnostic {
}
backtrace_msg.push_str("\n")
}
let pos = Position {
filename: panic_info.kcl_file.clone(),
line: panic_info.kcl_line as u64,
column: None,
};
Diagnostic::new_with_code(
Level::Error,
&panic_msg,
Some(&backtrace_msg),
Position {
filename: panic_info.kcl_file.clone(),
line: panic_info.kcl_line as u64,
column: None,
},
(pos.clone(), pos),
None,
)
};

if panic_info.kcl_config_meta_file.is_empty() {
return diag;
}
let pos = 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),
};
let mut config_meta_diag = Diagnostic::new_with_code(
Level::Error,
&panic_info.kcl_config_meta_arg_msg,
None,
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),
},
(pos.clone(), pos),
None,
);
config_meta_diag.messages.append(&mut diag.messages);
Expand Down Expand Up @@ -329,11 +327,12 @@ impl ParseError {
ParseError::Message { span, .. } => span,
};
let loc = sess.sm.lookup_char_pos(span.lo());
let pos: Position = loc.into();
Ok(Diagnostic::new_with_code(
Level::Error,
&self.to_string(),
None,
loc.into(),
(pos.clone(), pos),
Some(DiagnosticId::Error(ErrorKind::InvalidSyntax)),
))
}
Expand Down Expand Up @@ -399,21 +398,21 @@ impl SessionDiagnostic for Diagnostic {
},
}
for msg in &self.messages {
match Session::new_with_file_and_code(&msg.pos.filename, None) {
match Session::new_with_file_and_code(&msg.range.0.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);
let line = source.get_line((msg.range.0.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),
line_start: msg.range.0.line as usize,
origin: Some(&msg.range.0.filename),
annotations: vec![SourceAnnotation {
range: match msg.pos.column {
range: match msg.range.0.column {
Some(column) if content.len() >= 1 => {
let column = column as usize;
// If the position exceeds the length of the content,
Expand Down
9 changes: 5 additions & 4 deletions kclvm/parser/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use compiler_base_session::Session;
use compiler_base_span::span::new_byte_pos;
use kclvm_ast::ast;
use kclvm_config::modfile::{get_vendor_home, KCL_FILE_EXTENSION, KCL_FILE_SUFFIX, KCL_MOD_FILE};
use kclvm_error::{ErrorKind, Message, Position, Style};
use kclvm_error::diagnostic::Range;
use kclvm_error::{ErrorKind, Message, Style};
use kclvm_sema::plugin::PLUGIN_MODULE_PREFIX;
use kclvm_utils::path::PathPrefix;
use kclvm_utils::pkgpath::parse_external_pkg_name;
Expand Down Expand Up @@ -354,7 +355,7 @@ impl Loader {
self.sess.1.borrow_mut().add_error(
ErrorKind::CannotFindModule,
&[Message {
pos: Into::<(Position, Position)>::into(pos).0,
range: Into::<Range>::into(pos),
style: Style::Line,
message: format!(
"the `{}` is found multiple times in the current package and vendor package",
Expand All @@ -373,7 +374,7 @@ impl Loader {
self.sess.1.borrow_mut().add_error(
ErrorKind::CannotFindModule,
&[Message {
pos: Into::<(Position, Position)>::into(pos).0,
range: Into::<Range>::into(pos),
style: Style::Line,
message: format!("pkgpath {} not found in the program", pkg_path),
note: None,
Expand Down Expand Up @@ -469,7 +470,7 @@ impl Loader {
self.sess.1.borrow_mut().add_error(
ErrorKind::CannotFindModule,
&[Message {
pos: Into::<(Position, Position)>::into(pos).0,
range: Into::<Range>::into(pos),
style: Style::Line,
message: format!("the plugin package `{}` is not found, please confirm if plugin mode is enabled", pkgpath),
note: None,
Expand Down
Loading

0 comments on commit 939c2ff

Please sign in to comment.