diff --git a/crates/rome_analyze/src/diagnostics.rs b/crates/rome_analyze/src/diagnostics.rs index 6c883db42cf..d49e7fdff77 100644 --- a/crates/rome_analyze/src/diagnostics.rs +++ b/crates/rome_analyze/src/diagnostics.rs @@ -81,7 +81,7 @@ impl Diagnostic for AnalyzerDiagnostic { } } - fn location(&self) -> Option> { + fn location(&self) -> Location<'_> { match &self.kind { DiagnosticKind::Rule { rule_diagnostic, @@ -111,13 +111,12 @@ impl Diagnostic for AnalyzerDiagnostic { detail.log_category, &markup! { {detail.message} }.to_owned(), )?; - if let Some(location) = Location::builder() - .span(&detail.range) - .resource(file_id) - .build() - { - visitor.record_frame(location)?; - } + visitor.record_frame( + Location::builder() + .span(&detail.range) + .resource(file_id) + .build(), + )?; } // we then print notes for (log_category, note) in &rule_advices.notes { @@ -169,7 +168,7 @@ impl AnalyzerDiagnostic { DiagnosticKind::Rule { rule_diagnostic, .. } => rule_diagnostic.span, - DiagnosticKind::Raw(error) => error.location().and_then(|location| location.span), + DiagnosticKind::Raw(error) => error.location().span, } } diff --git a/crates/rome_analyze/src/rule.rs b/crates/rome_analyze/src/rule.rs index dce3f2b1fcd..d90dee3ec94 100644 --- a/crates/rome_analyze/src/rule.rs +++ b/crates/rome_analyze/src/rule.rs @@ -397,9 +397,7 @@ impl Advices for RuleAdvice { detail.log_category, &markup! { {detail.message} }.to_owned(), )?; - if let Some(location) = Location::builder().span(&detail.range).build() { - visitor.record_frame(location)?; - } + visitor.record_frame(Location::builder().span(&detail.range).build())?; } // we then print notes for (log_category, note) in &self.notes { diff --git a/crates/rome_cli/src/traversal.rs b/crates/rome_cli/src/traversal.rs index 74806957221..fb89d8b4151 100644 --- a/crates/rome_cli/src/traversal.rs +++ b/crates/rome_cli/src/traversal.rs @@ -368,34 +368,33 @@ fn process_messages(options: ProcessMessagesOptions) { } Message::Error(mut err) => { - if let Some(location) = err.location() { - if let Resource::File(FilePath::FileId(file_id)) = location.resource { - // Retrieves the file name from the file ID cache, if it's a miss - // flush entries from the interner channel until it's found - let file_name = match paths.get(&file_id) { - Some(path) => Some(path), - None => loop { - match recv_files.recv() { - Ok((id, path)) => { - paths.insert(id, path.display().to_string()); - if id == file_id { - break Some(&paths[&file_id]); - } + let location = err.location(); + if let Some(Resource::File(FilePath::FileId(file_id))) = &location.resource { + // Retrieves the file name from the file ID cache, if it's a miss + // flush entries from the interner channel until it's found + let file_name = match paths.get(file_id) { + Some(path) => Some(path), + None => loop { + match recv_files.recv() { + Ok((id, path)) => { + paths.insert(id, path.display().to_string()); + if id == *file_id { + break Some(&paths[file_id]); } - // In case the channel disconnected without sending - // the path we need, print the error without a file - // name (normally this should never happen) - Err(_) => break None, } - }, - }; + // In case the channel disconnected without sending + // the path we need, print the error without a file + // name (normally this should never happen) + Err(_) => break None, + } + }, + }; - if let Some(path) = file_name { - err = err.with_file_path(FilePath::PathAndId { - path: path.as_str(), - file_id, - }); - } + if let Some(path) = file_name { + err = err.with_file_path(FilePath::PathAndId { + path: path.as_str(), + file_id: *file_id, + }); } } @@ -417,18 +416,13 @@ fn process_messages(options: ProcessMessagesOptions) { }); } } else { - let file_name = err - .location() - .and_then(|location| { - let path = match &location.resource { - Resource::File(file) => file, - _ => return None, - }; - - path.path() - }) - .unwrap_or(""); + let location = err.location(); + let path = match &location.resource { + Some(Resource::File(file)) => file.path(), + _ => None, + }; + let file_name = path.unwrap_or(""); let title = PrintDescription(&err).to_string(); let code = err.category().and_then(|code| code.name().parse().ok()); diff --git a/crates/rome_diagnostics/examples/fs.rs b/crates/rome_diagnostics/examples/fs.rs index 5ad3ec67fdf..5c8cf4d0209 100644 --- a/crates/rome_diagnostics/examples/fs.rs +++ b/crates/rome_diagnostics/examples/fs.rs @@ -38,7 +38,7 @@ impl Advices for NotFoundAdvices { visitor.record_log(LogCategory::Info, &"Ignore patterns were defined here")?; visitor.record_frame(Location { - resource: Resource::File(FilePath::Path(&self.configuration_path)), + resource: Some(Resource::File(FilePath::Path(&self.configuration_path))), span: Some(self.configuration_span), source_code: Some(SourceCode { text: &self.configuration_source_code, diff --git a/crates/rome_diagnostics/examples/lint.rs b/crates/rome_diagnostics/examples/lint.rs index 01d12d36a4d..6de97cc8a7e 100644 --- a/crates/rome_diagnostics/examples/lint.rs +++ b/crates/rome_diagnostics/examples/lint.rs @@ -44,7 +44,7 @@ impl Advices for LintAdvices { visitor.record_log(LogCategory::Info, &"This constant is declared here")?; visitor.record_frame(Location { - resource: Resource::File(FilePath::Path(&self.path)), + resource: Some(Resource::File(FilePath::Path(&self.path))), span: Some(self.declaration_span), source_code: Some(SourceCode { text: &self.source_code, diff --git a/crates/rome_diagnostics/src/advice.rs b/crates/rome_diagnostics/src/advice.rs index 4bd20e7ec0a..aeae608b886 100644 --- a/crates/rome_diagnostics/src/advice.rs +++ b/crates/rome_diagnostics/src/advice.rs @@ -120,9 +120,7 @@ where .source_code(&self.source_code) .build(); - if let Some(location) = location { - visitor.record_frame(location)?; - } + visitor.record_frame(location)?; Ok(()) } diff --git a/crates/rome_diagnostics/src/context.rs b/crates/rome_diagnostics/src/context.rs index 13f4f631c47..f51eb2382ec 100644 --- a/crates/rome_diagnostics/src/context.rs +++ b/crates/rome_diagnostics/src/context.rs @@ -221,7 +221,7 @@ mod internal { self.source.as_diagnostic().verbose_advices(visitor) } - fn location(&self) -> Option> { + fn location(&self) -> Location<'_> { self.source.as_diagnostic().location() } @@ -321,7 +321,7 @@ mod internal { self.source.as_diagnostic().verbose_advices(visitor) } - fn location(&self) -> Option> { + fn location(&self) -> Location<'_> { self.source.as_diagnostic().location() } @@ -371,32 +371,24 @@ mod internal { self.source.as_diagnostic().verbose_advices(visitor) } - fn location(&self) -> Option> { - self.source - .as_diagnostic() - .location() - .map(|loc| Location { - resource: match loc.resource { - Resource::Argv => Resource::Argv, - Resource::Memory => Resource::Memory, - Resource::File(file) => { - if let Some(Resource::File(path)) = &self.path { - Resource::File(file.or(path.as_deref())) - } else { - Resource::File(file) - } + fn location(&self) -> Location<'_> { + let loc = self.source.as_diagnostic().location(); + Location { + resource: match loc.resource { + Some(Resource::Argv) => Some(Resource::Argv), + Some(Resource::Memory) => Some(Resource::Memory), + Some(Resource::File(file)) => { + if let Some(Resource::File(path)) = &self.path { + Some(Resource::File(file.or(path.as_deref()))) + } else { + Some(Resource::File(file)) } - }, - span: loc.span, - source_code: loc.source_code, - }) - .or_else(|| { - Some(Location { - resource: self.path.as_ref()?.as_deref(), - span: None, - source_code: None, - }) - }) + } + None => self.path.as_ref().map(Resource::as_deref), + }, + span: loc.span, + source_code: loc.source_code, + } } fn tags(&self) -> DiagnosticTags { @@ -464,14 +456,14 @@ mod internal { } } - fn location(&self) -> Option> { - let location = self.source.as_diagnostic().location()?; - Some(Location { + fn location(&self) -> Location<'_> { + let location = self.source.as_diagnostic().location(); + Location { source_code: location .source_code .or_else(|| Some(self.source_code.as_ref()?.as_deref())), ..location - }) + } } fn tags(&self) -> DiagnosticTags { @@ -568,7 +560,7 @@ mod internal { self.source.as_diagnostic().verbose_advices(visitor) } - fn location(&self) -> Option> { + fn location(&self) -> Location<'_> { self.source.as_diagnostic().location() } diff --git a/crates/rome_diagnostics/src/diagnostic.rs b/crates/rome_diagnostics/src/diagnostic.rs index 74b8807eb69..f78469fef6c 100644 --- a/crates/rome_diagnostics/src/diagnostic.rs +++ b/crates/rome_diagnostics/src/diagnostic.rs @@ -82,8 +82,8 @@ pub trait Diagnostic: Debug { /// specific text range within the content of that location. Finally, it /// may also provide the source string for that location (this is required /// in order to display a code frame advice for the diagnostic). - fn location(&self) -> Option> { - None + fn location(&self) -> Location<'_> { + Location::builder().build() } /// Tags convey additional boolean metadata about the nature of a diagnostic: diff --git a/crates/rome_diagnostics/src/display.rs b/crates/rome_diagnostics/src/display.rs index 2e26d9b8f53..a6e6c030e52 100644 --- a/crates/rome_diagnostics/src/display.rs +++ b/crates/rome_diagnostics/src/display.rs @@ -64,30 +64,28 @@ impl<'fmt, D: Diagnostic + ?Sized> fmt::Display for PrintHeader<'fmt, D> { let mut slot = None; let mut fmt = CountWidth::wrap(f, &mut slot); - // Print the diagnostic location if it has one - if let Some(location) = diagnostic.location() { - // Print the path if it's a file - let file_name = match &location.resource { - Resource::File(file) => file.path(), - _ => None, - }; - - if let Some(name) = file_name { - fmt.write_str(name)?; - - // Print the line and column position if the location has a span and source code - // (the source code is necessary to convert a byte offset into a line + column) - if let (Some(span), Some(source_code)) = (location.span, location.source_code) { - let file = SourceFile::new(source_code); - if let Ok(location) = file.location(span.start()) { - fmt.write_markup(markup! { - ":"{location.line_number.get()}":"{location.column_number.get()} - })?; - } - } + // Print the diagnostic location if it has a file path + let location = diagnostic.location(); + let file_name = match &location.resource { + Some(Resource::File(file)) => file.path(), + _ => None, + }; - fmt.write_str(" ")?; + if let Some(name) = file_name { + fmt.write_str(name)?; + + // Print the line and column position if the location has a span and source code + // (the source code is necessary to convert a byte offset into a line + column) + if let (Some(span), Some(source_code)) = (location.span, location.source_code) { + let file = SourceFile::new(source_code); + if let Ok(location) = file.location(span.start()) { + fmt.write_markup(markup! { + ":"{location.line_number.get()}":"{location.column_number.get()} + })?; + } } + + fmt.write_str(" ")?; } // Print the category of the diagnostic, with a hyperlink if @@ -182,18 +180,14 @@ where { // Visit the advices of the diagnostic with a lightweight visitor that // detects if the diagnostic has any frame or backtrace advice - let skip_frame = if let Some(location) = diagnostic.location() { - let mut frame_visitor = FrameVisitor { - location, - skip_frame: false, - }; + let mut frame_visitor = FrameVisitor { + location: diagnostic.location(), + skip_frame: false, + }; - diagnostic.advices(&mut frame_visitor)?; + diagnostic.advices(&mut frame_visitor)?; - frame_visitor.skip_frame - } else { - false - }; + let skip_frame = frame_visitor.skip_frame; // Print the message for the diagnostic as a log advice print_message_advice(visitor, diagnostic, skip_frame)?; @@ -276,7 +270,8 @@ where // If the diagnostic has no explicit code frame or backtrace advice, print // a code frame advice with the location of the diagnostic if !skip_frame { - if let Some(location) = diagnostic.location().filter(|loc| loc.span.is_some()) { + let location = diagnostic.location(); + if location.span.is_some() { visitor.record_frame(location)?; } } @@ -624,7 +619,7 @@ mod tests { Ok(()) } - fn location(&self) -> Option> { + fn location(&self) -> Location<'_> { Location::builder() .resource(&self.path) .span(&self.span) @@ -668,7 +663,7 @@ mod tests { impl Advices for FrameAdvice { fn record(&self, visitor: &mut dyn Visit) -> io::Result<()> { visitor.record_frame(Location { - resource: Resource::File(FilePath::Path("other_path")), + resource: Some(Resource::File(FilePath::Path("other_path"))), span: Some(TextRange::new(TextSize::from(8), TextSize::from(16))), source_code: Some(SourceCode { text: "context location context", diff --git a/crates/rome_diagnostics/src/error.rs b/crates/rome_diagnostics/src/error.rs index f9b2eca2b88..151a43e6a8b 100644 --- a/crates/rome_diagnostics/src/error.rs +++ b/crates/rome_diagnostics/src/error.rs @@ -69,7 +69,7 @@ impl Error { } /// Calls [Diagnostic::location] on the [Diagnostic] wrapped by this [Error]. - pub fn location(&self) -> Option> { + pub fn location(&self) -> Location<'_> { self.as_diagnostic().location() } diff --git a/crates/rome_diagnostics/src/location.rs b/crates/rome_diagnostics/src/location.rs index 8c89b06026b..40d1371fb8b 100644 --- a/crates/rome_diagnostics/src/location.rs +++ b/crates/rome_diagnostics/src/location.rs @@ -8,7 +8,7 @@ use std::{borrow::Borrow, ops::Deref}; #[derive(Debug, Clone, Copy)] pub struct Location<'a> { /// The resource this diagnostic is associated with. - pub resource: Resource<&'a str>, + pub resource: Option>, /// An optional range of text within the resource associated with the /// diagnostic. pub span: Option, @@ -326,12 +326,12 @@ impl<'a> LocationBuilder<'a> { self } - pub fn build(self) -> Option> { - Some(Location { - resource: self.resource?, + pub fn build(self) -> Location<'a> { + Location { + resource: self.resource, span: self.span, source_code: self.source_code, - }) + } } } diff --git a/crates/rome_diagnostics/src/serde.rs b/crates/rome_diagnostics/src/serde.rs index e743b59f898..5d24bbba515 100644 --- a/crates/rome_diagnostics/src/serde.rs +++ b/crates/rome_diagnostics/src/serde.rs @@ -24,7 +24,7 @@ pub struct Diagnostic { message: MarkupBuf, advices: Advices, verbose_advices: Advices, - location: Option, + location: Location, tags: DiagnosticTags, source: Option>, } @@ -54,7 +54,7 @@ impl Diagnostic { // SAFETY: The Advices visitor never returns an error diag.verbose_advices(&mut verbose_advices).unwrap(); - let location = diag.location().map(Location::from); + let location = diag.location().into(); let tags = diag.tags(); @@ -99,14 +99,12 @@ impl super::Diagnostic for Diagnostic { self.verbose_advices.record(visitor) } - fn location(&self) -> Option> { - self.location.as_ref().and_then(|location| { - super::Location::builder() - .resource(&location.path) - .span(&location.span) - .source_code(&location.source_code) - .build() - }) + fn location(&self) -> super::Location<'_> { + super::Location::builder() + .resource(&self.location.path) + .span(&self.location.span) + .source_code(&self.location.source_code) + .build() } fn tags(&self) -> DiagnosticTags { @@ -134,7 +132,7 @@ impl<'fmt, D: super::Diagnostic + ?Sized> std::fmt::Display for PrintDescription #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[cfg_attr(test, derive(Eq, PartialEq))] struct Location { - path: Resource, + path: Option>, span: Option, source_code: Option, } @@ -142,7 +140,7 @@ struct Location { impl From> for Location { fn from(loc: super::Location<'_>) -> Self { Self { - path: loc.resource.to_owned(), + path: loc.resource.map(super::Resource::to_owned), span: loc.span, source_code: loc .source_code @@ -261,7 +259,7 @@ impl super::Advices for Advice { visitor.record_list(&as_display) } Advice::Frame(location) => visitor.record_frame(super::Location { - resource: location.path.as_deref(), + resource: location.path.as_ref().map(super::Resource::as_deref), span: location.span, source_code: location.source_code.as_deref().map(|text| SourceCode { text, diff --git a/crates/rome_diagnostics_macros/src/generate.rs b/crates/rome_diagnostics_macros/src/generate.rs index 76740e47ca2..1fe0b807d2e 100644 --- a/crates/rome_diagnostics_macros/src/generate.rs +++ b/crates/rome_diagnostics_macros/src/generate.rs @@ -212,7 +212,7 @@ fn generate_location(input: &DeriveInput) -> TokenStream { let method = input.location.iter().map(|(_, method)| method); quote! { - fn location(&self) -> Option> { + fn location(&self) -> rome_diagnostics::Location<'_> { rome_diagnostics::Location::builder() #( .#method(&self.#field) )* .build() diff --git a/crates/rome_js_analyze/src/lib.rs b/crates/rome_js_analyze/src/lib.rs index fd0f590c4ca..e4dd5954bb7 100644 --- a/crates/rome_js_analyze/src/lib.rs +++ b/crates/rome_js_analyze/src/lib.rs @@ -175,7 +175,7 @@ mod tests { dbg!("here"); if let Some(mut diag) = signal.diagnostic() { diag.set_severity(Severity::Warning); - error_ranges.push(diag.location().unwrap().span.unwrap()); + error_ranges.push(diag.location().span.unwrap()); let error = diag.with_file_path("ahahah").with_file_source_code(SOURCE); let text = markup_to_string(markup! { {PrintDiagnostic(&error)} diff --git a/crates/rome_js_parser/src/lib.rs b/crates/rome_js_parser/src/lib.rs index cd1ddc6f53f..c07da067118 100644 --- a/crates/rome_js_parser/src/lib.rs +++ b/crates/rome_js_parser/src/lib.rs @@ -464,10 +464,9 @@ impl Advices for ParserAdvice { file_id, } = detail; visitor.record_log(LogCategory::Info, &markup! { {message} }.to_owned())?; + let location = Location::builder().span(span).resource(file_id).build(); - if let Some(location) = location { - visitor.record_frame(location)?; - } + visitor.record_frame(location)?; } if let Some(hint) = &self.hint { visitor.record_log(LogCategory::Info, &markup! { {hint} }.to_owned())?; diff --git a/crates/rome_js_semantic/src/tests/assertions.rs b/crates/rome_js_semantic/src/tests/assertions.rs index a63360394b9..110f29a7be2 100644 --- a/crates/rome_js_semantic/src/tests/assertions.rs +++ b/crates/rome_js_semantic/src/tests/assertions.rs @@ -185,9 +185,7 @@ impl Advices for TestAdvice { for (span, message) in &self.advices { let location = Location::builder().span(&span).build(); visitor.record_log(LogCategory::Info, &message)?; - if let Some(location) = location { - visitor.record_frame(location)?; - } + visitor.record_frame(location)?; } Ok(()) } diff --git a/crates/rome_json_parser/src/lib.rs b/crates/rome_json_parser/src/lib.rs index a095b042a21..eaae2b5903a 100644 --- a/crates/rome_json_parser/src/lib.rs +++ b/crates/rome_json_parser/src/lib.rs @@ -73,9 +73,7 @@ impl Advices for ParserAdvice { } = detail; visitor.record_log(LogCategory::Info, &markup! { {message} }.to_owned())?; let location = Location::builder().span(span).resource(file_id).build(); - if let Some(location) = location { - visitor.record_frame(location)?; - } + visitor.record_frame(location)?; } if let Some(hint) = &self.hint { visitor.record_log(LogCategory::Info, &markup! { {hint} }.to_owned())?; diff --git a/crates/rome_lsp/src/utils.rs b/crates/rome_lsp/src/utils.rs index 0c713a3907f..468e6322426 100644 --- a/crates/rome_lsp/src/utils.rs +++ b/crates/rome_lsp/src/utils.rs @@ -200,9 +200,7 @@ pub(crate) fn diagnostic_to_lsp( url: &lsp::Url, line_index: &LineIndex, ) -> Result { - let location = diagnostic - .location() - .context("diagnostic has no location")?; + let location = diagnostic.location(); let span = location.span.context("diagnostic location has no span")?; let span = range(line_index, span).context("failed to convert diagnostic span to LSP range")?; diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 4a05e72a1d3..204ae738c50 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -578,7 +578,7 @@ export interface Diagnostic { advices: Advices; category?: Category; description: string; - location?: Location; + location: Location; message: MarkupBuf; severity: Severity; source?: Diagnostic; @@ -692,7 +692,7 @@ export type Category = | "flags/invalid" | "semanticTests"; export interface Location { - path: Resource_for_String; + path?: Resource_for_String; source_code?: string; span?: TextRange; }