Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_diagnostics): refactor some existing diagnostics to use the new Diagnostic trait #3315

Merged
merged 3 commits into from
Oct 4, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/rome_cli/src/reports/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ pub mod formatter;

use crate::reports::formatter::{FormatterReportFileDetail, FormatterReportSummary};
use formatter::FormatterReport;
use rome_diagnostics::{v2::Category, Severity};
use rome_diagnostics::v2::{Category, Severity};
use rome_service::RomeError;
use serde::Serialize;
use std::collections::HashMap;
Expand Down
236 changes: 127 additions & 109 deletions crates/rome_cli/src/traversal.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion crates/rome_diagnostics/src/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl<'a> Display for DiagnosticPrinter<'a> {
}

impl v2::Diagnostic for DiagnosticPrinter<'_> {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
self.d.code
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/src/v2/adapters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl From<io::Error> for IoError {
}

impl Diagnostic for IoError {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
Some(category!("internalError/io"))
}

Expand Down
8 changes: 4 additions & 4 deletions crates/rome_diagnostics/src/v2/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ mod internal {
}

impl<M: fmt::Display + 'static, E: AsDiagnostic> Diagnostic for ContextDiagnostic<M, E> {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
self.source.as_diagnostic().category()
}

Expand Down Expand Up @@ -280,7 +280,7 @@ mod internal {
}

impl<E: AsDiagnostic> Diagnostic for CategoryDiagnostic<E> {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
Some(
self.source
.as_diagnostic()
Expand Down Expand Up @@ -335,7 +335,7 @@ mod internal {
}

impl<E: AsDiagnostic> Diagnostic for FilePathDiagnostic<E> {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
self.source.as_diagnostic().category()
}

Expand Down Expand Up @@ -410,7 +410,7 @@ mod internal {
}

impl<E: AsDiagnostic> Diagnostic for FileSourceCodeDiagnostic<E> {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
self.source.as_diagnostic().category()
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/src/v2/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub trait Diagnostic: Debug {
/// The category of a diagnostic uniquely identifying this
/// diagnostic type, such as `lint/correctness/noArguments`, `args/invalid`
/// or `format/disabled`.
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
None
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/src/v2/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ mod tests {
}

impl<A: Advices + std::fmt::Debug> Diagnostic for TestDiagnostic<A> {
fn category(&self) -> Option<&Category> {
fn category(&self) -> Option<&'static Category> {
Some(category!("internalError/io"))
}

Expand Down
15 changes: 4 additions & 11 deletions crates/rome_diagnostics/src/v2/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use super::{
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[cfg_attr(test, derive(Eq, PartialEq))]
pub struct Diagnostic {
category: Option<String>,
category: Option<&'static Category>,
severity: Severity,
description: String,
message: MarkupBuf,
Expand All @@ -35,7 +35,7 @@ impl Diagnostic {
}

fn new_impl<D: super::Diagnostic + ?Sized>(diag: &D) -> Self {
let category = diag.category().map(|category| category.name().to_string());
let category = diag.category();

let severity = diag.severity();

Expand Down Expand Up @@ -75,15 +75,8 @@ impl Diagnostic {
}

impl super::Diagnostic for Diagnostic {
fn category(&self) -> Option<&Category> {
self.category.as_deref().and_then(|name| {
let category: Option<&Category> = name.parse().ok();
debug_assert!(
category.is_some(),
"diagnostic category {name:?} does not exist in the static registry"
);
category
})
fn category(&self) -> Option<&'static Category> {
self.category
}

fn severity(&self) -> Severity {
Expand Down
46 changes: 41 additions & 5 deletions crates/rome_diagnostics_categories/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,53 @@ impl serde::Serialize for &'static Category {
}
}

#[cfg(feature = "serde")]
struct CategoryVisitor;

#[cfg(feature = "serde")]
fn deserialize_parse<E: serde::de::Error>(code: &str) -> Result<&'static Category, E> {
code.parse().map_err(|()| {
serde::de::Error::custom(format_args!("failed to deserialize category from {code}"))
})
}

#[cfg(feature = "serde")]
impl<'de> serde::de::Visitor<'de> for CategoryVisitor {
type Value = &'static Category;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str("a borrowed string")
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
deserialize_parse(v)
}

fn visit_borrowed_str<E>(self, v: &'de str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
deserialize_parse(v)
}

fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
deserialize_parse(&v)
}
}

#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for &'static Category {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
<&str>::deserialize(deserializer).and_then(|code| {
code.parse().map_err(|()| {
serde::de::Error::custom(format_args!("failed to deserialize category from {code}"))
})
})
deserializer.deserialize_str(CategoryVisitor)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/rome_diagnostics_macros/src/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn generate_category(input: &DeriveInput) -> TokenStream {
};

quote! {
fn category(&self) -> Option<&rome_diagnostics::v2::Category> {
fn category(&self) -> Option<&'static rome_diagnostics::v2::Category> {
Some(#category)
}
}
Expand Down
5 changes: 3 additions & 2 deletions crates/rome_fs/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ use std::{
};

use crate::{PathInterner, RomePath};
use rome_diagnostics::{file::FileId, v2::Category};
use rome_diagnostics::file::FileId;

mod memory;
mod os;

pub use memory::MemoryFileSystem;
pub use os::OsFileSystem;
use rome_diagnostics::v2::Error;
pub const CONFIG_NAME: &str = "rome.json";

pub trait FileSystem: Send + Sync + RefUnwindSafe {
Expand Down Expand Up @@ -127,7 +128,7 @@ pub trait TraversalContext: Sync {

/// Called by the traversal process to emit an error diagnostic associated
/// with a particular file ID when an IO error happens
fn push_diagnostic(&self, file_id: FileId, code: &'static Category, message: String);
fn push_diagnostic(&self, error: Error);

/// Checks if the traversal context can handle a particular path, used as
/// an optimization to bail out of scheduling a file handler if it wouldn't
Expand Down
10 changes: 4 additions & 6 deletions crates/rome_fs/src/fs/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,13 @@ mod tests {
};

use parking_lot::Mutex;
use rome_diagnostics::v2::Error;

use crate::fs::FileSystemExt;
use crate::{
AtomicInterner, FileSystem, MemoryFileSystem, PathInterner, RomePath, TraversalContext,
};
use rome_diagnostics::{file::FileId, v2::Category};
use rome_diagnostics::file::FileId;

#[test]
fn file_read_write() {
Expand Down Expand Up @@ -225,11 +226,8 @@ mod tests {
&self.interner
}

fn push_diagnostic(&self, file_id: FileId, code: &'static Category, message: String) {
panic!(
"unexpected error {:?} in file {file_id:?}: {message}",
code.name()
)
fn push_diagnostic(&self, err: Error) {
panic!("unexpected error {err:?}")
}

fn can_handle(&self, _: &RomePath) -> bool {
Expand Down
30 changes: 14 additions & 16 deletions crates/rome_fs/src/fs/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use crate::{
FileSystem, RomePath,
};
use rayon::{scope, Scope};
use rome_diagnostics::file::FileId;
use rome_diagnostics::v2::category;
use rome_diagnostics::v2::{adapters::IoError, Diagnostic, DiagnosticExt, Error, FileId};
use std::{
ffi::OsStr,
fs,
Expand Down Expand Up @@ -108,7 +107,7 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> {
let file_type = match path.metadata() {
Ok(meta) => meta.file_type(),
Err(err) => {
ctx.push_diagnostic(file_id, category!("internalError/fs"), err.to_string());
ctx.push_diagnostic(IoError::from(err).with_file_path(file_id));
return;
}
};
Expand All @@ -127,11 +126,7 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> {
return;
}

ctx.push_diagnostic(
file_id,
category!("internalError/fs"),
"unhandled file type".into(),
);
ctx.push_diagnostic(Error::from(UnhandledDiagnostic { file_id }));
}
}

Expand All @@ -157,7 +152,7 @@ fn handle_dir<'scope>(
let iter = match fs::read_dir(path) {
Ok(iter) => iter,
Err(err) => {
ctx.push_diagnostic(file_id, category!("internalError/fs"), err.to_string());
ctx.push_diagnostic(IoError::from(err).with_file_path(file_id));
return;
}
};
Expand All @@ -166,7 +161,7 @@ fn handle_dir<'scope>(
let entry = match entry {
Ok(entry) => entry,
Err(err) => {
ctx.push_diagnostic(file_id, category!("internalError/fs"), err.to_string());
ctx.push_diagnostic(IoError::from(err).with_file_path(file_id));
continue;
}
};
Expand All @@ -177,7 +172,7 @@ fn handle_dir<'scope>(
let file_type = match entry.file_type() {
Ok(file_type) => file_type,
Err(err) => {
ctx.push_diagnostic(file_id, category!("internalError/fs"), err.to_string());
ctx.push_diagnostic(IoError::from(err).with_file_path(file_id));
continue;
}
};
Expand Down Expand Up @@ -205,10 +200,13 @@ fn handle_dir<'scope>(
continue;
}

ctx.push_diagnostic(
file_id,
category!("internalError/fs"),
"unhandled file type".into(),
);
ctx.push_diagnostic(Error::from(UnhandledDiagnostic { file_id }));
}
}

#[derive(Debug, Diagnostic)]
#[diagnostic(category = "internalError/fs", message = "unhandled file type")]
struct UnhandledDiagnostic {
#[location(resource)]
file_id: FileId,
}
Loading