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

Introduce Diagnostic type and remove ariadne from public API #963

Merged
merged 18 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
69fdb85
refactor: Prefer explicit Clone over `From<&Borrowed> for Owned` impls
Xanewok May 15, 2024
27dc353
Introduce Diagnostic trait and remove error rendering in public API
Xanewok May 14, 2024
f2df66d
fixup: Don't depend on ariadne in the node addon runtime
Xanewok May 15, 2024
89bc471
refactor: Drop the Error bound from the Diagnostic trait
Xanewok May 15, 2024
7a618da
Document the new Diagnostic trait
Xanewok May 15, 2024
3c96df1
fix: Properly re-export diagnostic namespace in TS
Xanewok May 15, 2024
25327f1
Merge remote-tracking branch 'upstream/main' into diagnostic-api
Xanewok May 20, 2024
3dba9de
Drop non_exhaustive from Severity
Xanewok May 20, 2024
a4394fc
Don't impl Error for ParseError anymore
Xanewok May 21, 2024
27ab164
Add a changeset file
Xanewok May 21, 2024
6726a07
Merge remote-tracking branch 'upstream/main' into diagnostic-api
Xanewok May 22, 2024
716b7a1
Merge remote-tracking branch 'upstream/main' into diagnostic-api
Xanewok May 24, 2024
83f814e
fixup: Add missing line in TextIndex in errors.ts
Xanewok May 24, 2024
60f96ab
Rename Diagnostic::range to text_range
Xanewok May 27, 2024
793fd29
chore: Drop the line/column tracking comment about the Diagnostic tex…
Xanewok May 27, 2024
88092a8
feat: Use a TS-side interface to implement the Diagnostic trait methods
Xanewok May 27, 2024
49f3be0
Merge remote-tracking branch 'upstream/main' into diagnostic-api
Xanewok May 27, 2024
4e85c4d
feat: Emit correct label colors for diagnostics with different severity
Xanewok May 27, 2024
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
3 changes: 0 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion crates/codegen/runtime/cargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ anyhow = { workspace = true }
codegen_runtime_generator = { workspace = true }

[dependencies]
ariadne = { workspace = true }
ariadne = { workspace = true, optional = true }
napi = { workspace = true, optional = true }
napi-derive = { workspace = true, optional = true }
nom = { workspace = true }
Expand All @@ -28,6 +28,8 @@ thiserror = { workspace = true }
[features]
default = ["slang_napi_interfaces"]
slang_napi_interfaces = ["dep:napi", "dep:napi-derive", "dep:serde_json"]
# Only used by the `slang_solidity` CLI
__private_ariadne = ["dep:ariadne"]

[lints]
workspace = true
65 changes: 65 additions & 0 deletions crates/codegen/runtime/cargo/src/runtime/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use std::borrow::Cow;
use std::error::Error;

use crate::text_index::TextRange;

#[repr(u8)]
pub enum Severity {
Error = 1,
Warning = 2,
Information = 3,
Hint = 4,
}

pub trait Diagnostic: Error {
Xanewok marked this conversation as resolved.
Show resolved Hide resolved
fn range(&self) -> TextRange;
OmarTawfik marked this conversation as resolved.
Show resolved Hide resolved
fn code(&self) -> Option<Cow<'_, str>> {
OmarTawfik marked this conversation as resolved.
Show resolved Hide resolved
None
}
fn severity(&self) -> Severity;
fn message(&self) -> String;
}

#[cfg(feature = "__private_ariadne")]
pub fn render<D: Diagnostic>(error: &D, source_id: &str, source: &str, with_color: bool) -> String {
use ariadne::{Color, Config, Label, Report, ReportKind, Source};

use crate::text_index::TextRangeExtensions as _;

let kind = match error.severity() {
Severity::Error => ReportKind::Error,
Severity::Warning => ReportKind::Warning,
Severity::Information => ReportKind::Advice,
Severity::Hint => ReportKind::Advice,
};

let color = if with_color { Color::Red } else { Color::Unset };
OmarTawfik marked this conversation as resolved.
Show resolved Hide resolved

let message = error.message();

if source.is_empty() {
return format!("{kind}: {message}\n ─[{source_id}:0:0]");
}

let range = error.range().char();

let report = Report::build(kind, source_id, range.start)
.with_config(Config::default().with_color(with_color))
.with_message(message)
.with_label(
Label::new((source_id, range))
.with_color(color)
.with_message("Error occurred here."),
)
.finish();

let mut result = vec![];
report
.write((source_id, Source::from(&source)), &mut result)
.expect("Failed to write report");

return String::from_utf8(result)
.expect("Failed to convert report to utf8")
.trim()
.to_string();
}
1 change: 1 addition & 0 deletions crates/codegen/runtime/cargo/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub(crate) mod lexer;

pub mod cst;
pub mod cursor;
pub mod diagnostic;
pub mod parse_error;
pub mod parse_output;
pub mod query;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl RuleNode {
catch_unwind
)]
pub fn text_len(&self) -> TextIndex {
(&self.0.text_len).into()
self.0.text_len.into()
}

#[napi(ts_return_type = "Array<cst.Node>", catch_unwind)]
Expand All @@ -81,7 +81,7 @@ impl RuleNode {
#[napi(ts_arg_type = "text_index.TextIndex")] text_offset: TextIndex,
) -> Cursor {
RustNode::Rule(Rc::clone(&self.0))
.cursor_with_offset((&text_offset).into())
.cursor_with_offset(text_offset.into())
.into()
}

Expand Down Expand Up @@ -151,7 +151,7 @@ impl TokenNode {
)]
pub fn text_len(&self) -> TextIndex {
let text_len: RustTextIndex = (&self.0.text).into();
(&text_len).into()
text_len.into()
}

#[napi(getter, catch_unwind)]
Expand All @@ -173,7 +173,7 @@ impl TokenNode {
#[napi(ts_arg_type = "text_index.TextIndex")] text_offset: TextIndex,
) -> Cursor {
RustNode::Token(Rc::clone(&self.0))
.cursor_with_offset((&text_offset).into())
.cursor_with_offset(text_offset.into())
.into()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ impl Cursor {

#[napi(getter, ts_return_type = "text_index.TextIndex", catch_unwind)]
pub fn text_offset(&self) -> TextIndex {
(&self.0.text_offset()).into()
self.0.text_offset().into()
}

#[napi(getter, ts_return_type = "text_index.TextRange", catch_unwind)]
pub fn text_range(&self) -> TextRange {
(&self.0.text_range()).into()
self.0.text_range().into()
}

#[allow(clippy::cast_possible_truncation)] // Cursor depth can't reasonably be larger than u32
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use napi_derive::napi;

use crate::napi_interface::text_index::TextRange;

/// Severity of the compiler diagnostic.
///
/// Explicitly compatible with the LSP protocol.
#[napi(namespace = "diagnostic")]
OmarTawfik marked this conversation as resolved.
Show resolved Hide resolved
pub enum Severity {
Error = 1,
Warning = 2,
Information = 3,
Hint = 4,
}

impl From<crate::diagnostic::Severity> for Severity {
fn from(value: crate::diagnostic::Severity) -> Severity {
match value {
crate::diagnostic::Severity::Error => Self::Error,
crate::diagnostic::Severity::Warning => Self::Warning,
crate::diagnostic::Severity::Information => Self::Information,
crate::diagnostic::Severity::Hint => Self::Hint,
}
}
}

#[napi(namespace = "diagnostic")]
pub struct Diagnostic(pub(crate) Box<dyn crate::diagnostic::Diagnostic>);

#[napi(namespace = "diagnostic")]
impl Diagnostic {
#[napi]
pub fn severity(&self) -> Severity {
self.0.severity().into()
}

#[napi(ts_return_type = "text_index.TextRange")]
pub fn text_range(&self) -> TextRange {
self.0.range().into()
}

#[napi]
pub fn message(&self) -> String {
self.0.message()
}

#[napi]
pub fn code(&self) -> String {
self.0.code().unwrap_or_default().into_owned()
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod cst;
pub mod cursor;
pub mod diagnostic;
pub mod parse_error;
pub mod parse_output;
pub mod query;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use napi_derive::napi;
use text_index::TextRange;

use crate::napi_interface::diagnostic::Diagnostic;
use crate::napi_interface::{text_index, RustParseError};

#[napi(namespace = "parse_error")]
Expand All @@ -20,11 +21,13 @@ impl From<RustParseError> for ParseError {
impl ParseError {
#[napi(getter, ts_return_type = "text_index.TextRange", catch_unwind)]
pub fn text_range(&self) -> TextRange {
self.0.text_range().into()
self.0.text_range().clone().into()
}

#[napi(catch_unwind)]
pub fn to_error_report(&self, source_id: String, source: String, with_color: bool) -> String {
self.0.to_error_report(&source_id, &source, with_color)
#[napi(ts_return_type = "diagnostic.Diagnostic", catch_unwind)]
pub fn to_diagnostic(&self) -> Diagnostic {
// TODO: Figure out if we can auto-gen Diagnostics methods
OmarTawfik marked this conversation as resolved.
Show resolved Hide resolved
// in TS for this implementor rather than having to clone here
Diagnostic(Box::new(self.0.clone()))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ pub struct TextIndex {
pub char: u32,
}

impl From<&RustTextIndex> for TextIndex {
fn from(value: &RustTextIndex) -> Self {
impl From<RustTextIndex> for TextIndex {
fn from(value: RustTextIndex) -> Self {
// We only support 32-byte indices on TS side.
#[allow(clippy::cast_possible_truncation)]
Self {
Expand All @@ -22,8 +22,8 @@ impl From<&RustTextIndex> for TextIndex {
}
}

impl From<&TextIndex> for RustTextIndex {
fn from(value: &TextIndex) -> Self {
impl From<TextIndex> for RustTextIndex {
fn from(value: TextIndex) -> Self {
Self {
utf8: value.utf8 as usize,
utf16: value.utf16 as usize,
Expand All @@ -39,11 +39,11 @@ pub struct TextRange {
pub end: TextIndex,
}

impl From<&RustTextRange> for TextRange {
fn from(value: &RustTextRange) -> Self {
impl From<RustTextRange> for TextRange {
fn from(value: RustTextRange) -> Self {
Self {
start: (&value.start).into(),
end: (&value.end).into(),
start: value.start.into(),
end: value.end.into(),
}
}
}
Loading
Loading