Skip to content

Commit

Permalink
Introduce Diagnostic type and remove ariadne from public API (#963)
Browse files Browse the repository at this point in the history
Part of or might close #804 

Rather than coming up with perfect API that ties everything together,
let's start with the `Diagnostic` and iterate from there.
  • Loading branch information
Xanewok committed May 27, 2024
1 parent 46b1dde commit a5593f9
Show file tree
Hide file tree
Showing 47 changed files with 784 additions and 268 deletions.
5 changes: 5 additions & 0 deletions .changeset/two-colts-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@nomicfoundation/slang": minor
---

Introduce a `Diagnostic` API for compiler errors, warnings etc.
4 changes: 0 additions & 4 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 }
metaslang_cst = { workspace = true }
napi = { workspace = true, optional = true }
napi-derive = { workspace = true, optional = 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
69 changes: 69 additions & 0 deletions crates/codegen/runtime/cargo/src/runtime/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use crate::text_index::TextRange;

/// The severity of a diagnostic.
///
/// Explicitly compatible with the [LSP protocol](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticSeverity).
#[repr(u8)]
pub enum Severity {
Error = 1,
Warning = 2,
Information = 3,
Hint = 4,
}

/// A compiler diagnostic that can be rendered to a user.
pub trait Diagnostic {
/// The character range of the source that this diagnostic applies to.
/// at the moment.
fn text_range(&self) -> TextRange;
/// The severity of this diagnostic.
fn severity(&self) -> Severity;
/// The primary message associated with this diagnostic.
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};

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

let message = error.message();

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

let color = if with_color { color } else { Color::Unset };

let range = {
let start = source[..error.text_range().start.utf8].chars().count();
let end = source[..error.text_range().end.utf8].chars().count();
start..end
};

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 @@ -2,6 +2,7 @@
pub(crate) mod parser_support;
pub(crate) mod lexer;

pub mod diagnostic;
#[path = "generated/kinds.rs"]
pub mod kinds;
#[path = "generated/language.rs"]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
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")]
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,
}
}
}

/// A compiler diagnostic that can be rendered to a user.
#[napi(namespace = "diagnostic")]
pub struct Diagnostic(pub(crate) Box<dyn crate::diagnostic::Diagnostic>);

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

/// The character range of the source that this diagnostic applies to.
#[napi(ts_return_type = "text_index.TextRange")]
pub fn text_range(&self) -> TextRange {
self.0.text_range().into()
}

/// The primary message associated with this diagnostic.
#[napi]
pub fn message(&self) -> String {
self.0.message()
}
}

/// Exposes the [`Diagnostic`](crate::diagnostic::Diagnostic) methods implemented for a given type
/// as regular N-API functions, satisfying the custom `DiagnosticInterface` interface on the N-API side.
// NOTE(#987): This is required because we can't directly expose the trait using `#[napi]` or expose
// an interface that has only methods defined.
// To not require explicit conversions, we define the interface ourselves and expose the relevant methods to satisfy it.
#[macro_export]
macro_rules! expose_diagnostic_trait_interface {
($namespace:literal, $diagnostic:ty) => {
#[napi(namespace = $namespace)]
impl $diagnostic {
#[napi(ts_return_type = "diagnostic.Severity", catch_unwind)]
pub fn severity(&self) -> $crate::napi_interface::diagnostic::Severity {
$crate::diagnostic::Diagnostic::severity(&self.0).into()
}

#[napi(ts_return_type = "text_index.TextRange", catch_unwind)]
pub fn text_range(&self) -> $crate::napi_interface::text_index::TextRange {
$crate::diagnostic::Diagnostic::text_range(&self.0).into()
}

#[napi]
pub fn message(&self) -> String {
$crate::diagnostic::Diagnostic::message(&self.0)
}
}
};
}
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 @@ -2,9 +2,9 @@
#![allow(clippy::needless_pass_by_value)]

use napi_derive::napi;
use text_index::TextRange;

use crate::napi_interface::{text_index, RustParseError};
use crate::expose_diagnostic_trait_interface;
use crate::napi_interface::RustParseError;

#[napi(namespace = "parse_error")]
#[derive(PartialEq, Clone)]
Expand All @@ -16,15 +16,4 @@ impl From<RustParseError> for ParseError {
}
}

#[napi(namespace = "parse_error")]
impl ParseError {
#[napi(getter, ts_return_type = "text_index.TextRange", catch_unwind)]
pub fn text_range(&self) -> TextRange {
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)
}
}
expose_diagnostic_trait_interface!("parse_error", ParseError);
106 changes: 44 additions & 62 deletions crates/codegen/runtime/cargo/src/runtime/parse_error.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,28 @@
use std::collections::BTreeSet;
use std::fmt;

use crate::diagnostic::{self, Diagnostic};
use crate::kinds::TerminalKind;
use crate::text_index::TextRange;

/// Represents an error that occurred during parsing.
///
/// This could have been caused by a syntax error, or by reaching the end of the file when more tokens were expected.
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct ParseError {
pub(crate) text_range: TextRange,
pub(crate) terminals_that_would_have_allowed_more_progress: Vec<TerminalKind>,
}

impl ParseError {
/// The text range at which the error occurred.
pub fn text_range(&self) -> &TextRange {
&self.text_range
}

pub fn terminals_that_would_have_allowed_more_progress(&self) -> Vec<String> {
let terminals_that_would_have_allowed_more_progress = self
.terminals_that_would_have_allowed_more_progress
.iter()
.collect::<BTreeSet<_>>();

terminals_that_would_have_allowed_more_progress
.into_iter()
.map(TerminalKind::to_string)
.collect()
}

pub fn to_error_report(&self, source_id: &str, source: &str, with_color: bool) -> String {
render_error_report(self, source_id, source, with_color)
/// Renders the message for this error.
pub fn message(&self) -> String {
Diagnostic::message(self)
}
}

Expand All @@ -43,56 +38,43 @@ impl ParseError {
}
}

pub(crate) fn render_error_report(
error: &ParseError,
source_id: &str,
source: &str,
with_color: bool,
) -> String {
use ariadne::{Color, Config, Label, Report, ReportKind, Source};

let kind = ReportKind::Error;
let color = if with_color { Color::Red } else { Color::Unset };

let terminals_that_would_have_allowed_more_progress =
error.terminals_that_would_have_allowed_more_progress();
let message = if terminals_that_would_have_allowed_more_progress.is_empty() {
"Expected end of file.".to_string()
} else {
format!(
"Expected {expectations}.",
expectations = terminals_that_would_have_allowed_more_progress.join(" or ")
)
};

if source.is_empty() {
return format!("{kind}: {message}\n ─[{source_id}:0:0]");
impl fmt::Display for ParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self
.terminals_that_would_have_allowed_more_progress
.is_empty()
{
write!(f, "Expected end of file.")
} else {
let deduped = self
.terminals_that_would_have_allowed_more_progress
.iter()
.collect::<BTreeSet<_>>();

write!(f, "Expected ")?;

for kind in deduped.iter().take(deduped.len() - 1) {
write!(f, "{kind} or ")?;
}
let last = deduped.last().expect("we just checked that it's not empty");
write!(f, "{last}.")?;

Ok(())
}
}
}

let range = {
let start = source[..error.text_range.start.utf8].chars().count();
let end = source[..error.text_range.end.utf8].chars().count();
start..end
};

let mut builder = Report::build(kind, source_id, range.start)
.with_config(Config::default().with_color(with_color))
.with_message(message);

builder.add_label(
Label::new((source_id, range))
.with_color(color)
.with_message("Error occurred here.".to_string()),
);
impl Diagnostic for ParseError {
fn text_range(&self) -> TextRange {
self.text_range.clone()
}

let mut result = vec![];
builder
.finish()
.write((source_id, Source::from(&source)), &mut result)
.expect("Failed to write report");
fn severity(&self) -> diagnostic::Severity {
diagnostic::Severity::Error
}

return String::from_utf8(result)
.expect("Failed to convert report to utf8")
.trim()
.to_string();
fn message(&self) -> String {
// Uses the impl from `Display` above.
self.to_string()
}
}
1 change: 0 additions & 1 deletion crates/codegen/runtime/node_addon/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ slang_napi_interfaces = [
napi-build = { workspace = true }

[dependencies]
ariadne = { workspace = true }
metaslang_cst = { workspace = true }
napi = { workspace = true }
napi-derive = { workspace = true }
Expand Down
Loading

0 comments on commit a5593f9

Please sign in to comment.