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 all 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
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")]
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,
}
}
}

/// 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");
OmarTawfik marked this conversation as resolved.
Show resolved Hide resolved
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
Loading