fix(rome_diagnostics): allow diagnostic locations to be created witho…

…ut a resource (#3834)

crates/rome_analyze/src/diagnostics.rs

@@ -81,7 +81,7 @@ impl Diagnostic for AnalyzerDiagnostic {
         }
     }
 
-    fn location(&self) -> Option<Location<'_>> {
+    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,
         }
     }
 

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 {

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("<unknown>");
+                    let location = err.location();
+                    let path = match &location.resource {
+                        Some(Resource::File(file)) => file.path(),
+                        _ => None,
+                    };
 
+                    let file_name = path.unwrap_or("<unknown>");
                     let title = PrintDescription(&err).to_string();
                     let code = err.category().and_then(|code| code.name().parse().ok());
 

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,

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,

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(())
     }

crates/rome_diagnostics/src/context.rs

@@ -221,7 +221,7 @@ mod internal {
             self.source.as_diagnostic().verbose_advices(visitor)
         }
 
-        fn location(&self) -> Option<Location<'_>> {
+        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<Location<'_>> {
+        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<Location<'_>> {
-            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<Location<'_>> {
-            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<Location<'_>> {
+        fn location(&self) -> Location<'_> {
             self.source.as_diagnostic().location()
         }
 

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<Location<'_>> {
-        None
+    fn location(&self) -> Location<'_> {
+        Location::builder().build()
     }
 
     /// Tags convey additional boolean metadata about the nature of a diagnostic:

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<Location<'_>> {
+        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",

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<Location<'_>> {
+    pub fn location(&self) -> Location<'_> {
         self.as_diagnostic().location()
     }