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: Change diagnostic position from pos(start) to range(start, end) #638

Merged
merged 2 commits into from
Aug 7, 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
6 changes: 3 additions & 3 deletions kclvm/ast/src/ast.rs
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};
Peefy marked this conversation as resolved.
Show resolved Hide resolved

/// 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
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
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
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
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
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
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