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

refactor(parser,diagnostic): one diagnostic struct to eliminate monomorphization of generic types #3214

Merged
merged 1 commit into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
106 changes: 103 additions & 3 deletions crates/oxc_diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ mod graphical_theme;
mod reporter;
mod service;

use std::path::PathBuf;
use std::{
fmt::{self, Display},
ops::Deref,
path::PathBuf,
};

pub use miette;
pub use thiserror;
Expand All @@ -21,9 +25,10 @@ pub type Error = miette::Error;
pub type Severity = miette::Severity;
pub type Report = miette::Report;

pub type Result<T> = std::result::Result<T, Error>;
pub type Result<T> = std::result::Result<T, OxcDiagnostic>;

use miette::Diagnostic;
pub use miette::LabeledSpan;
use miette::{Diagnostic, SourceCode};
use thiserror::Error;

#[derive(Debug, Error, Diagnostic)]
Expand All @@ -35,3 +40,98 @@ pub struct MinifiedFileError(pub PathBuf);
#[error("Failed to open file {0:?} with error \"{1}\"")]
#[diagnostic(help("Failed to open file {0:?} with error \"{1}\""))]
pub struct FailedToOpenFileError(pub PathBuf, pub std::io::Error);

#[derive(Debug, Clone)]
pub struct OxcDiagnostic {
// `Box` the data to make `OxcDiagnostic` 8 bytes so that `Result` is small.
// This is required because rust does not performance return value optimization.
inner: Box<OxcDiagnosticInner>,
}

impl Deref for OxcDiagnostic {
type Target = Box<OxcDiagnosticInner>;
fn deref(&self) -> &Self::Target {
&self.inner
}
}

#[derive(Debug, Clone)]
pub struct OxcDiagnosticInner {
pub message: String,
pub labels: Option<Vec<LabeledSpan>>,
pub help: Option<String>,
}

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

impl std::error::Error for OxcDiagnostic {}

impl Diagnostic for OxcDiagnostic {
fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
self.help.as_ref().map(Box::new).map(|c| c as Box<dyn Display>)
}

fn labels(&self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + '_>> {
self.labels
.as_ref()
.map(|ls| ls.iter().cloned())
.map(Box::new)
.map(|b| b as Box<dyn Iterator<Item = LabeledSpan>>)
}
}

impl OxcDiagnostic {
#[must_use]
pub fn new<T: Into<String>>(message: T) -> Self {
Self {
inner: Box::new(OxcDiagnosticInner {
message: message.into(),
labels: None,
help: None,
}),
}
}

#[must_use]
pub fn with_help<T: Into<String>>(mut self, help: T) -> Self {
self.inner.help = Some(help.into());
self
}

#[must_use]
pub fn with_label<T: Into<LabeledSpan>>(mut self, label: T) -> Self {
self.inner.labels = Some(vec![label.into()]);
self
}

#[must_use]
pub fn with_labels<T: IntoIterator<Item = LabeledSpan>>(mut self, labels: T) -> Self {
self.inner.labels = Some(labels.into_iter().collect());
self
}

#[must_use]
pub fn and_label<T: Into<LabeledSpan>>(mut self, label: T) -> Self {
let mut labels = self.inner.labels.unwrap_or_default();
labels.push(label.into());
self.inner.labels = Some(labels);
self
}

#[must_use]
pub fn and_labels<T: IntoIterator<Item = LabeledSpan>>(mut self, labels: T) -> Self {
let mut all_labels = self.inner.labels.unwrap_or_default();
all_labels.extend(labels);
self.inner.labels = Some(all_labels);
self
}

#[must_use]
pub fn with_source_code<T: SourceCode + Send + Sync + 'static>(self, code: T) -> Error {
Error::from(self).with_source_code(code)
}
}
5 changes: 4 additions & 1 deletion crates/oxc_language_server/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,10 @@ impl IsolatedLintHandler {
let reports = ret
.errors
.into_iter()
.map(|diagnostic| ErrorReport { error: diagnostic, fixed_content: None })
.map(|diagnostic| ErrorReport {
error: Error::from(diagnostic),
fixed_content: None,
})
.collect();
return Some(Self::wrap_diagnostics(path, &original_source_text, reports, start));
};
Expand Down
31 changes: 14 additions & 17 deletions crates/oxc_linter/examples/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ use std::{env, path::Path};

use oxc_allocator::Allocator;
use oxc_ast::AstKind;
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_diagnostics::{LabeledSpan, OxcDiagnostic};
use oxc_parser::Parser;
use oxc_semantic::SemanticBuilder;
use oxc_span::{SourceType, Span};
Expand Down Expand Up @@ -35,18 +32,18 @@ fn main() -> std::io::Result<()> {
let semantic_ret =
SemanticBuilder::new(&source_text, source_type).with_trivias(ret.trivias).build(program);

let mut errors: Vec<oxc_diagnostics::Error> = vec![];
let mut errors: Vec<OxcDiagnostic> = vec![];

for node in semantic_ret.semantic.nodes().iter() {
match node.kind() {
AstKind::DebuggerStatement(stmt) => {
errors.push(NoDebugger(stmt.span).into());
errors.push(no_debugger(stmt.span));
}
AstKind::ArrayPattern(array) if array.elements.is_empty() => {
errors.push(NoEmptyPattern("array", array.span).into());
errors.push(no_empty_pattern("array", array.span));
}
AstKind::ObjectPattern(object) if object.properties.is_empty() => {
errors.push(NoEmptyPattern("object", object.span).into());
errors.push(no_empty_pattern("object", object.span));
}
_ => {}
}
Expand All @@ -61,7 +58,7 @@ fn main() -> std::io::Result<()> {
Ok(())
}

fn print_errors(source_text: &str, errors: Vec<oxc_diagnostics::Error>) {
fn print_errors(source_text: &str, errors: Vec<OxcDiagnostic>) {
for error in errors {
let error = error.with_source_code(source_text.to_string());
println!("{error:?}");
Expand All @@ -75,10 +72,9 @@ fn print_errors(source_text: &str, errors: Vec<oxc_diagnostics::Error>) {
// 1 │ debugger;
// · ─────────
// ╰────
#[derive(Debug, Error, Diagnostic)]
#[error("`debugger` statement is not allowed")]
#[diagnostic(severity(warning))]
struct NoDebugger(#[label] pub Span);
fn no_debugger(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::new("`debugger` statement is not allowed").with_labels([span0.into()])
}

// This prints:
//
Expand All @@ -88,7 +84,8 @@ struct NoDebugger(#[label] pub Span);
// · ─┬
// · ╰── Empty object binding pattern
// ╰────
#[derive(Debug, Error, Diagnostic)]
#[error("empty destructuring pattern is not allowed")]
#[diagnostic(severity(warning))]
struct NoEmptyPattern(&'static str, #[label("Empty {0} binding pattern")] pub Span);
fn no_empty_pattern(s0: &str, span1: Span) -> OxcDiagnostic {
OxcDiagnostic::new("empty destructuring pattern is not allowed").with_labels([
LabeledSpan::new_with_span(Some(format!("Empty {s0} binding pattern")), span1),
])
}
6 changes: 5 additions & 1 deletion crates/oxc_linter/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ impl Runtime {
.parse();

if !ret.errors.is_empty() {
return ret.errors.into_iter().map(|err| Message::new(err, None)).collect();
return ret
.errors
.into_iter()
.map(|err| Message::new(Error::from(err), None))
.collect();
};

let program = allocator.alloc(ret.program);
Expand Down
8 changes: 3 additions & 5 deletions crates/oxc_parser/src/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl<'a> ParserImpl<'a> {
fn test_escaped_keyword(&mut self, kind: Kind) {
if self.cur_token().escaped() && kind.is_all_keyword() {
let span = self.cur_token().span();
self.error(diagnostics::EscapedKeyword(span));
self.error(diagnostics::escaped_keyword(span));
}
}

Expand Down Expand Up @@ -158,7 +158,7 @@ impl<'a> ParserImpl<'a> {
pub(crate) fn asi(&mut self) -> Result<()> {
if !self.can_insert_semicolon() {
let span = Span::new(self.prev_token_end, self.cur_token().start);
return Err(diagnostics::AutoSemicolonInsertion(span).into());
return Err(diagnostics::auto_semicolon_insertion(span));
}
if self.at(Kind::Semicolon) {
self.advance(Kind::Semicolon);
Expand All @@ -178,9 +178,7 @@ impl<'a> ParserImpl<'a> {
pub(crate) fn expect_without_advance(&mut self, kind: Kind) -> Result<()> {
if !self.at(kind) {
let range = self.cur_token().span();
return Err(
diagnostics::ExpectToken(kind.to_str(), self.cur_kind().to_str(), range).into()
);
return Err(diagnostics::expect_token(kind.to_str(), self.cur_kind().to_str(), range));
}
Ok(())
}
Expand Down