Skip to content

Commit

Permalink
feat: introduce trait BuildErrorLike (#327)
Browse files Browse the repository at this point in the history
<!-- Thank you for contributing! -->

### Description

- Each error should defined as self contained. It includes it's error code and message.
- I used `enum` for exposing error kind to rust users of rolldown, and now I think it seems meaningless to care what the errors are.
  - However, I haven't think it through. Let's see when this need is really requires.

<!-- Please insert your description here and provide especially info about the "what" this PR is solving -->

### Test Plan

<!-- e.g. is there anything you'd like reviewers to focus on? -->

---
  • Loading branch information
hyf0 committed Nov 19, 2023
1 parent 475907e commit 158b11d
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 63 deletions.
11 changes: 3 additions & 8 deletions crates/rolldown/tests/common/case.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{borrow::Cow, path::Path};

use super::fixture::Fixture;
use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme};
use rolldown::Output;
use rolldown_error::BuildError;
use string_wizard::MagicString;
Expand Down Expand Up @@ -62,21 +61,17 @@ impl Case {

fn render_errors_to_snapshot(&mut self, mut errors: Vec<BuildError>) {
self.snapshot.append("# Errors\n\n");
errors.sort_by_key(|e| e.code().unwrap().to_string());
errors.sort_by_key(|e| e.code());

// Render with miette's diagnostic theme (without colors)
let reporter = GraphicalReportHandler::new_themed(GraphicalTheme::unicode_nocolor());

let rendered = errors
.iter()
.flat_map(|error| {
let mut out = String::new();
reporter.render_report(&mut out, error).unwrap();

[
Cow::Owned(format!("## {}\n", error.code().unwrap().to_string())),
Cow::Owned(format!("## {}\n", error.code())),
"```text".into(),
Cow::Owned(out),
Cow::Owned(error.to_string()),
"```".into(),
]
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,5 @@ input_file: crates/rolldown/tests/fixtures/errors/unresolved_entry
## UNRESOLVED_ENTRY

```text
UNRESOLVED_ENTRY
× Cannot resolve entry module ./main.js.
Cannot resolve entry module ./main.js.
```
51 changes: 30 additions & 21 deletions crates/rolldown_error/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,55 +1,64 @@
use std::{
borrow::Cow,
fmt::Display,
path::{Path, PathBuf},
};

use miette::Diagnostic;
use thiserror::Error;

use crate::error_kind::{
external_entry::ExternalEntry, unresolved_entry::UnresolvedEntry,
unresolved_import::UnresolvedImport, ErrorKind,
unresolved_import::UnresolvedImport, BuildErrorLike, NapiError,
};

type StaticStr = Cow<'static, str>;

#[derive(Error, Debug, Diagnostic)]
#[error(transparent)]
#[diagnostic(transparent)]
pub struct BuildError(ErrorKind);
#[derive(Debug)]
pub struct BuildError {
inner: Box<dyn BuildErrorLike>,
}

fn _assert_build_error_send_sync() {
fn _assert_send_sync<T: Send + Sync>() {}
_assert_send_sync::<BuildError>();
}

impl Display for BuildError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.inner.message().fmt(f)
}
}

impl BuildError {
pub fn new_with_kind(kind: ErrorKind) -> Self {
Self(kind)
pub fn code(&self) -> &'static str {
self.inner.code()
}

// --- private

fn new_inner(inner: impl Into<Box<dyn BuildErrorLike>>) -> Self {
Self { inner: inner.into() }
}

// --- Aligned with rollup
pub fn entry_cannot_be_external(unresolved_id: impl AsRef<Path>) -> Self {
Self::new_with_kind(ErrorKind::ExternalEntry(
ExternalEntry { id: unresolved_id.as_ref().to_path_buf() }.into(),
))
Self::new_inner(ExternalEntry { id: unresolved_id.as_ref().to_path_buf() })
}

pub fn unresolved_entry(unresolved_id: impl AsRef<Path>) -> Self {
Self::new_with_kind(ErrorKind::UnresolvedEntry(
UnresolvedEntry { unresolved_id: unresolved_id.as_ref().to_path_buf() }.into(),
))
Self::new_inner(UnresolvedEntry { unresolved_id: unresolved_id.as_ref().to_path_buf() })
}

pub fn unresolved_import(specifier: impl Into<StaticStr>, importer: impl Into<PathBuf>) -> Self {
Self::new_with_kind(ErrorKind::UnresolvedImport(
UnresolvedImport { specifier: specifier.into(), importer: importer.into() }.into(),
))
Self::new_inner(UnresolvedImport { specifier: specifier.into(), importer: importer.into() })
}

// --- rolldown specific
pub fn napi_error(status: String, reason: String) -> Self {
Self::new_with_kind(ErrorKind::Napi { status, reason })
Self::new_inner(NapiError { status, reason })
}
}

impl From<std::io::Error> for BuildError {
fn from(e: std::io::Error) -> Self {
Self::new_with_kind(ErrorKind::Io(Box::new(e)))
Self::new_inner(e)
}
}
12 changes: 12 additions & 0 deletions crates/rolldown_error/src/error_kind/external_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,21 @@ use miette::Diagnostic;
use std::path::PathBuf;
use thiserror::Error;

use super::BuildErrorLike;

#[derive(Error, Debug, Diagnostic)]
#[diagnostic(code = "UNRESOLVED_ENTRY")]
#[error("Entry module {} cannot be external.", id.relative_display())]
pub struct ExternalEntry {
pub(crate) id: PathBuf,
}

impl BuildErrorLike for ExternalEntry {
fn code(&self) -> &'static str {
"UNRESOLVED_ENTRY"
}

fn message(&self) -> String {
format!("Entry module {} cannot be external.", self.id.relative_display())
}
}
62 changes: 35 additions & 27 deletions crates/rolldown_error/src/error_kind/mod.rs
Original file line number Diff line number Diff line change
@@ -1,40 +1,48 @@
use std::borrow::Cow;
use std::fmt::Debug;

pub mod external_entry;
// pub mod impl_to_diagnostic;
pub mod unresolved_entry;
pub mod unresolved_import;

use miette::Diagnostic;
use thiserror::Error;
// TODO(hyf0): Not a good name, probably should rename to `BuildError`
pub trait BuildErrorLike: Debug + Sync + Send {
fn code(&self) -> &'static str;

use self::{
external_entry::ExternalEntry, unresolved_entry::UnresolvedEntry,
unresolved_import::UnresolvedImport,
};
fn message(&self) -> String;
}

impl<T: BuildErrorLike + 'static> From<T> for Box<dyn BuildErrorLike> {
fn from(e: T) -> Self {
Box::new(e)
}
}

type StaticStr = Cow<'static, str>;
// --- TODO(hyf0): These errors are only for compatibility with legacy code. They should be replaced with more specific errors.

#[derive(Error, Debug, Diagnostic)]
pub enum ErrorKind {
#[diagnostic(transparent)]
#[error(transparent)]
UnresolvedEntry(Box<UnresolvedEntry>),
#[derive(Debug)]
pub struct NapiError {
pub status: String,
pub reason: String,
}

#[diagnostic(transparent)]
#[error(transparent)]
ExternalEntry(Box<ExternalEntry>),
impl BuildErrorLike for NapiError {
fn code(&self) -> &'static str {
"NAPI_ERROR"
}

#[diagnostic(transparent)]
#[error(transparent)]
UnresolvedImport(Box<UnresolvedImport>),
fn message(&self) -> String {
format!("Napi error: {status}: {reason}", status = self.status, reason = self.reason)
}
}

#[diagnostic(code = "IO_ERROR")]
#[error(transparent)]
Io(Box<std::io::Error>),
impl BuildErrorLike for std::io::Error {
fn code(&self) -> &'static str {
"IO_ERROR"
}

// TODO: probably should remove this error
#[diagnostic(code = "NAPI_ERROR")]
#[error("Napi error: {status}: {reason}")]
Napi { status: String, reason: String },
fn message(&self) -> String {
format!("IO error: {self}")
}
}

// --- end
12 changes: 12 additions & 0 deletions crates/rolldown_error/src/error_kind/unresolved_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,21 @@ use miette::Diagnostic;
use std::path::PathBuf;
use thiserror::Error;

use super::BuildErrorLike;

#[derive(Error, Debug, Diagnostic)]
#[diagnostic(code = "UNRESOLVED_ENTRY")]
#[error("Cannot resolve entry module {}.", unresolved_id.relative_display())]
pub struct UnresolvedEntry {
pub(crate) unresolved_id: PathBuf,
}

impl BuildErrorLike for UnresolvedEntry {
fn code(&self) -> &'static str {
"UNRESOLVED_ENTRY"
}

fn message(&self) -> String {
format!("Cannot resolve entry module {}.", self.unresolved_id.relative_display())
}
}
14 changes: 12 additions & 2 deletions crates/rolldown_error/src/error_kind/unresolved_import.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::StaticStr;
use crate::PathExt;
use super::BuildErrorLike;
use crate::{PathExt, StaticStr};
use miette::Diagnostic;
use std::path::PathBuf;
use thiserror::Error;
Expand All @@ -11,3 +11,13 @@ pub struct UnresolvedImport {
pub(crate) specifier: StaticStr,
pub(crate) importer: PathBuf,
}

impl BuildErrorLike for UnresolvedImport {
fn code(&self) -> &'static str {
"UNRESOLVED_IMPORT"
}

fn message(&self) -> String {
format!("Could not resolve {} from {}.", self.specifier, self.importer.relative_display())
}
}
4 changes: 3 additions & 1 deletion crates/rolldown_error/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ mod error;
mod error_kind;
mod utils;

use std::path::Path;
use std::{borrow::Cow, path::Path};

use sugar_path::SugarPath;

pub(crate) type StaticStr = Cow<'static, str>;

// pub use crate::{diagnostic::Diagnostic, error::BuildError};
pub use crate::error::BuildError;

Expand Down

0 comments on commit 158b11d

Please sign in to comment.