Skip to content

Commit

Permalink
feat: change position of diag. Change the position of diagnostic from…
Browse files Browse the repository at this point in the history
… pos(start) to range(start, end)
  • Loading branch information
He1pa committed Aug 2, 2023
1 parent 82d149c commit 488d070
Show file tree
Hide file tree
Showing 22 changed files with 501 additions and 332 deletions.
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 @@ -1774,14 +1774,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
10 changes: 5 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: (Position, Position)) -> 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: (Position, Position),
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 @@ -127,7 +127,7 @@ impl Diagnostic {

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Message {
pub pos: Position,
pub range: (Position, Position),
pub style: Style,
pub message: String,
pub note: Option<String>,
Expand Down
64 changes: 31 additions & 33 deletions kclvm/error/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,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: (Position, Position)) -> &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 +106,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: (Position, Position)) -> &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 +120,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: (Position, Position)) -> &mut Self {
let diag = Diagnostic::new_with_code(
Level::Error,
msg,
None,
pos,
range,
Some(DiagnosticId::Error(E2L23.kind)),
);
self.add_diagnostic(diag);
Expand Down Expand Up @@ -229,17 +229,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 +249,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 +326,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 +397,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
6 changes: 3 additions & 3 deletions kclvm/parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ impl Loader {
self.sess.1.borrow_mut().add_error(
ErrorKind::CannotFindModule,
&[Message {
pos: Into::<(Position, Position)>::into(pos).0,
range: Into::<(Position, Position)>::into(pos),
style: Style::Line,
message: format!(
"the `{}` is found multiple times in the current package and vendor package",
Expand All @@ -374,7 +374,7 @@ impl Loader {
self.sess.1.borrow_mut().add_error(
ErrorKind::CannotFindModule,
&[Message {
pos: Into::<(Position, Position)>::into(pos).0,
range: Into::<(Position, Position)>::into(pos),
style: Style::Line,
message: format!("pkgpath {} not found in the program", pkg_path),
note: None,
Expand Down Expand Up @@ -544,7 +544,7 @@ impl Loader {
self.sess.1.borrow_mut().add_error(
ErrorKind::CannotFindModule,
&[Message {
pos: Into::<(Position, Position)>::into(pos).0,
range: Into::<(Position, Position)>::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
37 changes: 18 additions & 19 deletions kclvm/sema/src/lint/lints_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::resolver::scope::Scope;
use crate::{declare_lint_pass, resolver::scope::ScopeObjectKind};
use indexmap::IndexSet;
use kclvm_ast::ast;
use kclvm_error::{Handler, Level, Message, Position, Style, WarningKind};
use kclvm_ast::pos::GetPos;
use kclvm_error::{Handler, Level, Message, Style, WarningKind};

/// The 'import_position' lint detects import statements that are not declared at the top of file.
/// ### Example
Expand All @@ -30,7 +31,12 @@ pub static IMPORT_POSITION: &Lint = &Lint {
declare_lint_pass!(ImportPosition => [IMPORT_POSITION]);

impl LintPass for ImportPosition {
fn check_module(&mut self, handler: &mut Handler, ctx: &mut LintContext, module: &ast::Module) {
fn check_module(
&mut self,
handler: &mut Handler,
_ctx: &mut LintContext,
module: &ast::Module,
) {
let mut first_non_importstmt = std::u64::MAX;
for stmt in &module.body {
match &stmt.node {
Expand All @@ -48,11 +54,7 @@ impl LintPass for ImportPosition {
handler.add_warning(
WarningKind::ImportPositionWarning,
&[Message {
pos: Position {
filename: ctx.filename.clone(),
line: stmt.line,
column: None,
},
range: stmt.get_span_pos(),
style: Style::Line,
message: format!(
"Importstmt should be placed at the top of the module"
Expand Down Expand Up @@ -97,16 +99,12 @@ impl LintPass for UnusedImport {
let scope_objs = &scope.elems;
for (_, scope_obj) in scope_objs {
let scope_obj = scope_obj.borrow();
if let ScopeObjectKind::Module(_) = scope_obj.kind {
if let ScopeObjectKind::Module(_) = &scope_obj.kind {
if !scope_obj.used {
handler.add_warning(
WarningKind::UnusedImportWarning,
&[Message {
pos: Position {
filename: scope_obj.start.filename.clone(),
line: scope_obj.start.line,
column: None,
},
range: scope_obj.get_span_pos(),
style: Style::Line,
message: format!("Module '{}' imported but unused", scope_obj.name),
note: Some("Consider removing this statement".to_string()),
Expand Down Expand Up @@ -143,19 +141,20 @@ pub static REIMPORT: &Lint = &Lint {
declare_lint_pass!(ReImport => [REIMPORT]);

impl LintPass for ReImport {
fn check_module(&mut self, handler: &mut Handler, ctx: &mut LintContext, module: &ast::Module) {
fn check_module(
&mut self,
handler: &mut Handler,
_ctx: &mut LintContext,
module: &ast::Module,
) {
let mut import_names = IndexSet::<String>::new();
for stmt in &module.body {
if let ast::Stmt::Import(import_stmt) = &stmt.node {
if import_names.contains(&import_stmt.path) {
handler.add_warning(
WarningKind::ReimportWarning,
&[Message {
pos: Position {
filename: ctx.filename.clone(),
line: stmt.line,
column: None,
},
range: stmt.get_span_pos(),
style: Style::Line,
message: format!(
"Module '{}' is reimported multiple times",
Expand Down
12 changes: 6 additions & 6 deletions kclvm/sema/src/resolver/arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ impl<'ctx> Resolver<'ctx> {
if check_table.contains(arg_name) {
self.handler.add_compile_error(
&format!("{} has duplicated keyword argument {}", func_name, arg_name),
kw.get_pos(),
kw.get_span_pos(),
);
}
check_table.insert(arg_name.to_string());
let arg_value_type = self.expr_or_any_type(&kw.node.value);
kwarg_types.push((arg_name.to_string(), arg_value_type.clone()));
} else {
self.handler
.add_compile_error("missing argument", kw.get_pos());
.add_compile_error("missing argument", kw.get_span_pos());
}
}
if !params.is_empty() {
Expand All @@ -64,12 +64,12 @@ impl<'ctx> Resolver<'ctx> {
params.len(),
args.len(),
),
args[i].get_pos(),
args[i].get_span_pos(),
);
return;
}
};
self.must_assignable_to(ty.clone(), expected_ty, args[i].get_pos(), None)
self.must_assignable_to(ty.clone(), expected_ty, args[i].get_span_pos(), None)
}
for (i, (arg_name, kwarg_ty)) in kwarg_types.iter().enumerate() {
if !params
Expand All @@ -82,7 +82,7 @@ impl<'ctx> Resolver<'ctx> {
"{} got an unexpected keyword argument '{}'",
func_name, arg_name
),
kwargs[i].get_pos(),
kwargs[i].get_span_pos(),
);
}
let expected_types: Vec<Rc<Type>> = params
Expand All @@ -94,7 +94,7 @@ impl<'ctx> Resolver<'ctx> {
self.must_assignable_to(
kwarg_ty.clone(),
expected_types[0].clone(),
kwargs[i].get_pos(),
kwargs[i].get_span_pos(),
None,
);
};
Expand Down
Loading

0 comments on commit 488d070

Please sign in to comment.