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

Commit

Permalink
chore: clippy and reduced size of diagnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Dec 30, 2022
1 parent ca248bc commit 77de9a4
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 49 deletions.
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 @@ -87,6 +87,6 @@ impl Report {

pub fn as_serialized_reports(&self) -> Result<String, WorkspaceError> {
serde_json::to_string(&self)
.map_err(|err| WorkspaceError::ReportNotSerializable(err.to_string()))
.map_err(|err| WorkspaceError::report_not_serializable(err.to_string()))
}
}
6 changes: 3 additions & 3 deletions crates/rome_lsp/src/handlers/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub(crate) fn format(

let printed = match result {
Ok(printed) => printed,
Err(WorkspaceError.format_with_errors_disabled()) | Err(WorkspaceError::FileIgnored(_)) => {
Err(WorkspaceError::FormatWithErrorsDisabled(_)) | Err(WorkspaceError::FileIgnored(_)) => {
return Ok(None)
}
Err(err) => return Err(Error::from(err)),
Expand Down Expand Up @@ -80,7 +80,7 @@ pub(crate) fn format_range(

let formatted = match result {
Ok(formatted) => formatted,
Err(WorkspaceError.format_with_errors_disabled()) | Err(WorkspaceError::FileIgnored(_)) => {
Err(WorkspaceError::FormatWithErrorsDisabled(_)) | Err(WorkspaceError::FileIgnored(_)) => {
return Ok(None)
}
Err(err) => return Err(Error::from(err)),
Expand Down Expand Up @@ -132,7 +132,7 @@ pub(crate) fn format_on_type(

let formatted = match result {
Ok(formatted) => formatted,
Err(WorkspaceError.format_with_errors_disabled()) | Err(WorkspaceError::FileIgnored(_)) => {
Err(WorkspaceError::FormatWithErrorsDisabled(_)) | Err(WorkspaceError::FileIgnored(_)) => {
return Ok(None)
}
Err(err) => return Err(Error::from(err)),
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_lsp/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl Session {
.unwrap()
.get(url)
.cloned()
.ok_or(WorkspaceError::not_found())
.ok_or_else(WorkspaceError::not_found)
}

/// Set the [`Document`] for the provided [`lsp_types::Url`]
Expand Down
60 changes: 27 additions & 33 deletions crates/rome_service/src/configuration/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use rome_console::fmt::Display;
use rome_console::{markup, MarkupBuf};
use rome_diagnostics::location::AsSpan;
use rome_diagnostics::{
Advices, Category, Diagnostic, DiagnosticTags, LineIndexBuf, Location, LogCategory,
MessageAndDescription, Severity, Visit,
Advices, Category, Diagnostic, DiagnosticTags, LineIndexBuf, Location, LogCategory, Severity,
Visit,
};
use rome_rowan::{SyntaxError, TextRange, TextSize};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -33,16 +33,16 @@ impl From<SyntaxError> for ConfigurationDiagnostic {
ConfigurationDiagnostic::Deserialization(Deserialization {
deserialization_advice: DeserializationAdvice::default(),
range: None,
reason: MessageAndDescription::from("Syntax Error".to_string()),
reason: markup! {"Syntax Error"}.to_owned(),
})
}
}

impl ConfigurationDiagnostic {
pub(crate) fn new_deserialization_error(reason: impl Into<String>, span: impl AsSpan) -> Self {
pub(crate) fn new_deserialization_error(reason: impl Display, span: impl AsSpan) -> Self {
Self::Deserialization(Deserialization {
range: span.as_span(),
reason: MessageAndDescription::from(reason.into()),
reason: markup! {{reason}}.to_owned(),
deserialization_advice: DeserializationAdvice::default(),
})
}
Expand All @@ -54,14 +54,14 @@ impl ConfigurationDiagnostic {
) -> Self {
Self::Deserialization(Deserialization {
range: range.as_span(),
reason: MessageAndDescription::from(
markup!("Found an extraneous key "<Emphasis>{{ key_name }}</Emphasis> ).to_owned(),
),
reason: markup!("Found an extraneous key "<Emphasis>{{ key_name }}</Emphasis> )
.to_owned(),

deserialization_advice: DeserializationAdvice {
known_keys: Some((
markup! { "Accepted keys" }.to_owned(),
known_members
.into_iter()
.iter()
.map(|message| markup! {{message}}.to_owned())
.collect::<Vec<_>>(),
)),
Expand All @@ -76,16 +76,14 @@ impl ConfigurationDiagnostic {
known_variants: &[&str],
) -> Self {
Self::Deserialization(Deserialization {
reason: MessageAndDescription::from(
markup!("Found an extraneous variant "<Emphasis>{{ variant_name }}</Emphasis> )
.to_owned(),
),
reason: markup!("Found an extraneous variant "<Emphasis>{{ variant_name }}</Emphasis> )
.to_owned(),
range: range.as_span(),
deserialization_advice: DeserializationAdvice {
known_keys: Some((
markup! { "Accepted values" }.to_owned(),
known_variants
.into_iter()
.iter()
.map(|message| markup! {{message}}.to_owned())
.collect::<Vec<_>>(),
)),
Expand All @@ -105,51 +103,47 @@ impl ConfigurationDiagnostic {
) -> Self {
Self::Deserialization(Deserialization {
range: range.as_span(),
reason: MessageAndDescription::from( markup! {
reason: markup! {
"The value of key "<Emphasis>{{key_name}}</Emphasis>" is incorrect. Expected "<Emphasis>{{expected_type}}</Emphasis>
}.to_owned()),
}.to_owned(),
deserialization_advice: DeserializationAdvice::default(),
})
}

pub(crate) fn new_incorrect_type(expected_type: impl Display, range: impl AsSpan) -> Self {
Self::Deserialization(Deserialization {
range: range.as_span(),
reason: MessageAndDescription::from(
markup! {
"Incorrect type, expected a "<Emphasis>{{expected_type}}</Emphasis>
}
.to_owned(),
),
reason: markup! {
"Incorrect type, expected a "<Emphasis>{{expected_type}}</Emphasis>
}
.to_owned(),

deserialization_advice: DeserializationAdvice::default(),
})
}

pub(crate) fn new_invalid_ignore_pattern(pattern: impl Display, reason: impl Display) -> Self {
Self::Deserialization(Deserialization {
reason: MessageAndDescription::from(
reason:
markup! { "Couldn't parse the pattern "<Emphasis>{{pattern}}</Emphasis>", reason: "<Emphasis>{{reason}}</Emphasis>"" }.to_owned()
),
,
range: None,
deserialization_advice: DeserializationAdvice::default()
})
}

pub(crate) fn new_already_exists() -> Self {
Self::Deserialization(Deserialization {
reason: MessageAndDescription::from(
"It seems that a configuration file already exists".to_string(),
),
reason: markup!("It seems that a configuration file already exists").to_owned(),
range: None,
deserialization_advice: DeserializationAdvice::default(),
})
}

pub(crate) fn unexpected(span: impl AsSpan) -> Self {
Self::Deserialization(Deserialization {
reason: MessageAndDescription::from(
"Unexpected content inside the configuration file".to_string(),
),
reason: markup!("Unexpected content inside the configuration file").to_owned(),

range: span.as_span(),
deserialization_advice: DeserializationAdvice::default(),
})
Expand Down Expand Up @@ -328,7 +322,7 @@ pub struct InvalidIgnorePattern {
)]
pub struct Deserialization {
#[message]
pub(crate) reason: MessageAndDescription,
pub(crate) reason: MarkupBuf,
#[location(span)]
pub(crate) range: Option<TextRange>,
#[advice]
Expand Down Expand Up @@ -381,8 +375,8 @@ mod test {

#[test]
fn diagnostic_size() {
assert_eq!(std::mem::size_of::<ConfigurationDiagnostic>(), 136);
assert_eq!(std::mem::size_of::<Deserialization>(), 136);
assert_eq!(std::mem::size_of::<ConfigurationDiagnostic>(), 112);
assert_eq!(std::mem::size_of::<Deserialization>(), 112);
assert_eq!(std::mem::size_of::<DeserializationAdvice>(), 72);
}

Expand Down
7 changes: 3 additions & 4 deletions crates/rome_service/src/configuration/parse/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::configuration::visitor::VisitConfigurationNode;
use crate::ConfigurationDiagnostic;
use indexmap::IndexSet;
use rome_console::markup;
use rome_diagnostics::MessageAndDescription;
use rome_json_syntax::{
AnyJsonValue, JsonArrayValue, JsonBooleanValue, JsonLanguage, JsonMemberName, JsonNumberValue,
JsonObjectValue, JsonRoot, JsonStringValue, JsonSyntaxNode,
Expand Down Expand Up @@ -335,10 +334,10 @@ fn emit_diagnostic_form_number(
value_range: TextRange,
maximum: impl rome_console::fmt::Display,
) -> ConfigurationDiagnostic {
if value_text.starts_with("-") {
if value_text.starts_with('-') {
ConfigurationDiagnostic::Deserialization(Deserialization {
range: Some(value_range),
reason: MessageAndDescription::from(parse_error.to_string()),
reason: markup! {{parse_error.to_string()}}.to_owned(),
deserialization_advice: DeserializationAdvice {
known_keys: None,
hint: Some(markup! {"Value can't be negative"}.to_owned()),
Expand All @@ -347,7 +346,7 @@ fn emit_diagnostic_form_number(
} else {
ConfigurationDiagnostic::Deserialization(Deserialization {
range: Some(value_range),
reason: MessageAndDescription::from(parse_error.to_string()),
reason: markup! {{parse_error.to_string()}}.to_owned(),
deserialization_advice: DeserializationAdvice {
known_keys: None,
hint: Some(markup! {"Maximum value accepted is "{{maximum}}}.to_owned()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::configuration::visitor::VisitConfigurationNode;
use crate::configuration::{FormatterConfiguration, PlainIndentStyle};
use crate::ConfigurationDiagnostic;
use rome_console::markup;
use rome_diagnostics::MessageAndDescription;
use rome_formatter::LineWidth;
use rome_json_syntax::{JsonLanguage, JsonSyntaxNode};
use rome_rowan::{AstNode, SyntaxNode};
Expand Down Expand Up @@ -47,7 +46,7 @@ impl VisitConfigurationNode<JsonLanguage> for FormatterConfiguration {
let line_width = self.map_to_u16(&value, name_text, LineWidth::MAX)?;
self.line_width = LineWidth::try_from(line_width).map_err(|err| {
ConfigurationDiagnostic::Deserialization(Deserialization {
reason: MessageAndDescription::from(err.to_string()),
reason: markup! {{err.to_string()}}.to_owned(),
range: Some(value.range()),
deserialization_advice: DeserializationAdvice {
hint: Some(
Expand Down
13 changes: 11 additions & 2 deletions crates/rome_service/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rome_formatter::FormatError;
use rome_js_analyze::utils::rename::RenameError;
use rome_js_analyze::RuleError;
use serde::{Deserialize, Serialize};
use std::error::Error;
use std::fmt;
use std::fmt::{Debug, Display, Formatter};
use std::process::{ExitCode, Termination};
Expand Down Expand Up @@ -77,8 +78,16 @@ impl WorkspaceError {
extension,
})
}

pub fn report_not_serializable(reason: impl Into<String>) -> Self {
Self::ReportNotSerializable(ReportNotSerializable {
reason: reason.into(),
})
}
}

impl Error for WorkspaceError {}

impl Debug for WorkspaceError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
std::fmt::Display::fmt(self, f)
Expand Down Expand Up @@ -120,8 +129,8 @@ impl Diagnostic for WorkspaceError {
fn description(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
match self {
WorkspaceError::FormatWithErrorsDisabled(error) => error.description(fmt),
WorkspaceError::FormatError(err) => err.description(fmt),
WorkspaceError::RuleError(error) => error.description(fmt),
WorkspaceError::FormatError(error) => Diagnostic::description(error, fmt),
WorkspaceError::RuleError(error) => Diagnostic::description(error, fmt),
WorkspaceError::Configuration(error) => error.description(fmt),
WorkspaceError::RenameError(error) => error.description(fmt),
WorkspaceError::TransportError(error) => error.description(fmt),
Expand Down
6 changes: 3 additions & 3 deletions crates/rome_service/src/workspace/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl WorkspaceServer {
let document = self
.documents
.get(rome_path)
.ok_or(WorkspaceError::not_found())?;
.ok_or_else(WorkspaceError::not_found)?;

let capabilities = self.get_capabilities(rome_path);
let parse = capabilities
Expand Down Expand Up @@ -318,7 +318,7 @@ impl Workspace for WorkspaceServer {
let mut document = self
.documents
.get_mut(&params.path)
.ok_or(WorkspaceError::not_found())?;
.ok_or_else(WorkspaceError::not_found)?;

debug_assert!(params.version > document.version);
document.version = params.version;
Expand All @@ -332,7 +332,7 @@ impl Workspace for WorkspaceServer {
fn close_file(&self, params: CloseFileParams) -> Result<(), WorkspaceError> {
self.documents
.remove(&params.path)
.ok_or(WorkspaceError::not_found())?;
.ok_or_else(WorkspaceError::not_found)?;

self.syntax.remove(&params.path);
Ok(())
Expand Down

0 comments on commit 77de9a4

Please sign in to comment.