ruff_db: switch diagnostic rendering over to std::fmt::Display

It was already using this approach internally, so this is "just" a
matter of rejiggering the public API of `Diagnostic`.

We were previously writing directly to a `std::io::Write` since it was
thought that this worked better with the linear typing fakery. Namely,
it increased confidence that the diagnostic rendering was actually
written somewhere useful, instead of just being converted to a string
that could potentially get lost.

For reasons discussed in #17130, the linear type fakery was removed.
And so there is less of a reason to require a `std::io::Write`
implementation for diagnostic rendering. Indeed, this would sometimes
result in `unwrap()` calls when one wants to convert to a `String`.

crates/red_knot/src/main.rs

@@ -288,7 +288,7 @@ impl MainLoop {
                             let diagnostics_count = result.len();
 
                             for diagnostic in result {
-                                diagnostic.print(db, &display_config, &mut stdout)?;
+                                write!(stdout, "{}", diagnostic.display(db, &display_config))?;
 
                                 failed |= diagnostic.severity() >= min_error_severity;
                             }

crates/red_knot_ide/src/goto.rs

@@ -241,6 +241,7 @@ pub(crate) fn find_goto_target(parsed: &ParsedModule, offset: TextSize) -> Optio
 
 #[cfg(test)]
 mod tests {
+    use std::fmt::Write;
 
     use crate::db::tests::TestDb;
     use crate::{goto_type_definition, NavigationTarget};
@@ -860,24 +861,19 @@ f(**kwargs<CURSOR>)
                 return "No type definitions found".to_string();
             }
 
-            let mut buf = vec![];
+            let mut buf = String::new();
 
             let source = targets.range;
 
-            for target in &*targets {
+            let config = DisplayDiagnosticConfig::default()
-                GotoTypeDefinitionDiagnostic::new(source, target)
-                    .into_diagnostic()
-                    .print(
-                        &self.db,
-                        &DisplayDiagnosticConfig::default()
                 .color(false)
-                            .format(DiagnosticFormat::Full),
+                .format(DiagnosticFormat::Full);
-                        &mut buf,
+            for target in &*targets {
-                    )
+                let diag = GotoTypeDefinitionDiagnostic::new(source, target).into_diagnostic();
-                    .unwrap();
+                write!(buf, "{}", diag.display(&self.db, &config)).unwrap();
             }
 
-            String::from_utf8(buf).unwrap()
+            buf
         }
     }
 

crates/red_knot_test/src/lib.rs

@@ -13,8 +13,7 @@ use ruff_db::parsed::parsed_module;
 use ruff_db::system::{DbWithWritableSystem as _, SystemPath, SystemPathBuf};
 use ruff_db::testing::{setup_logging, setup_logging_with_filter};
 use ruff_source_file::{LineIndex, OneIndexed};
-use std::fmt::Write as _;
+use std::fmt::Write;
-use std::io::Write as _;
 
 mod assertion;
 mod config;
@@ -324,7 +323,7 @@ fn create_diagnostic_snapshot(
 ) -> String {
     let display_config = DisplayDiagnosticConfig::default().color(false);
 
-    let mut snapshot = Vec::new();
+    let mut snapshot = String::new();
     writeln!(snapshot).unwrap();
     writeln!(snapshot, "---").unwrap();
     writeln!(snapshot, "mdtest name: {}", test.name()).unwrap();
@@ -360,8 +359,8 @@ fn create_diagnostic_snapshot(
             writeln!(snapshot).unwrap();
         }
         writeln!(snapshot, "```").unwrap();
-        diag.print(db, &display_config, &mut snapshot).unwrap();
+        write!(snapshot, "{}", diag.display(db, &display_config)).unwrap();
         writeln!(snapshot, "```").unwrap();
     }
-    String::from_utf8(snapshot).unwrap()
+    snapshot
 }

crates/red_knot_wasm/src/lib.rs

@@ -315,11 +315,10 @@ impl Diagnostic {
     #[wasm_bindgen]
     pub fn display(&self, workspace: &Workspace) -> JsString {
         let config = DisplayDiagnosticConfig::default().color(false);
-        let mut buf = vec![];
         self.inner
-            .print(workspace.db.upcast(), &config, &mut buf)
+            .display(workspace.db.upcast(), &config)
-            .unwrap();
+            .to_string()
-        String::from_utf8(buf).unwrap().into()
+            .into()
     }
 }
 

crates/ruff_db/src/diagnostic/mod.rs

@@ -5,11 +5,12 @@ use thiserror::Error;
 use ruff_annotate_snippets::Level as AnnotateLevel;
 use ruff_text_size::TextRange;
 
+pub use self::render::DisplayDiagnostic;
 pub use crate::diagnostic::old::OldSecondaryDiagnosticMessage;
 use crate::files::File;
 use crate::Db;
 
-use self::render::{DisplayDiagnostic, FileResolver};
+use self::render::FileResolver;
 
 // This module should not be exported. We are planning to migrate off
 // the APIs in this module.
@@ -100,22 +101,18 @@ impl Diagnostic {
         Arc::make_mut(&mut self.inner).subs.push(sub);
     }
 
-    /// Print this diagnostic to the given writer.
+    /// Return a `std::fmt::Display` implementation that renders this
+    /// diagnostic into a human readable format.
     ///
-    /// # Errors
+    /// Note that this `Display` impl includes a trailing line terminator, so
-    ///
+    /// callers should prefer using this with `write!` instead of `writeln!`.
-    /// This is guaranteed to only return an error if the underlying
+    pub fn display<'c, 'db, 'd>(
-    /// writer given returns an error. Otherwise, the formatting of
+        &'d self,
-    /// diagnostics themselves is infallible.
+        db: &'db dyn Db,
-    pub fn print(
+        config: &'c DisplayDiagnosticConfig,
-        &self,
+    ) -> DisplayDiagnostic<'c, 'db, 'd> {
-        db: &dyn Db,
-        config: &DisplayDiagnosticConfig,
-        mut wtr: impl std::io::Write,
-    ) -> std::io::Result<()> {
         let resolver = FileResolver::new(db);
-        let display = DisplayDiagnostic::new(&resolver, config, self);
+        DisplayDiagnostic::new(resolver, config, self)
-        writeln!(wtr, "{display}")
     }
 
     /// Returns the identifier for this diagnostic.

crates/ruff_db/src/diagnostic/render.rs

@@ -17,20 +17,31 @@ use super::{
     Annotation, Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, Severity, SubDiagnostic,
 };
 
+/// A type that implements `std::fmt::Display` for diagnostic rendering.
+///
+/// It is created via [`Diagnostic::display`].
+///
+/// The lifetime parameters are:
+///
+/// * `'c` is the lifetime of the rendering configuration.
+/// * `'r` is the lifetime of the resolver used to load the contents of `Span`
+///   values. When using Salsa, this most commonly corresponds to the lifetime
+///   of a Salsa `Db`.
+/// * `'d` is the lifetime of the diagnostic being rendered.
 #[derive(Debug)]
-pub(crate) struct DisplayDiagnostic<'a> {
+pub struct DisplayDiagnostic<'c, 'r, 'd> {
-    config: &'a DisplayDiagnosticConfig,
+    config: &'c DisplayDiagnosticConfig,
-    resolver: &'a FileResolver<'a>,
+    resolver: FileResolver<'r>,
     annotate_renderer: AnnotateRenderer,
-    diag: &'a Diagnostic,
+    diag: &'d Diagnostic,
 }
 
-impl<'a> DisplayDiagnostic<'a> {
+impl<'c, 'r, 'd> DisplayDiagnostic<'c, 'r, 'd> {
     pub(crate) fn new(
-        resolver: &'a FileResolver<'a>,
+        resolver: FileResolver<'r>,
-        config: &'a DisplayDiagnosticConfig,
+        config: &'c DisplayDiagnosticConfig,
-        diag: &'a Diagnostic,
+        diag: &'d Diagnostic,
-    ) -> DisplayDiagnostic<'a> {
+    ) -> DisplayDiagnostic<'c, 'r, 'd> {
         let annotate_renderer = if config.color {
             AnnotateRenderer::styled()
         } else {
@@ -45,7 +56,7 @@ impl<'a> DisplayDiagnostic<'a> {
     }
 }
 
-impl std::fmt::Display for DisplayDiagnostic<'_> {
+impl std::fmt::Display for DisplayDiagnostic<'_, '_, '_> {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         if matches!(self.config.format, DiagnosticFormat::Concise) {
             match self.diag.severity() {
@@ -65,15 +76,15 @@ impl std::fmt::Display for DisplayDiagnostic<'_> {
                 }
                 write!(f, ":")?;
             }
-            return write!(f, " {message}", message = self.diag.primary_message());
+            return writeln!(f, " {message}", message = self.diag.primary_message());
         }
 
-        let resolved = Resolved::new(self.resolver, self.diag);
+        let resolved = Resolved::new(&self.resolver, self.diag);
         let renderable = resolved.to_renderable(self.config.context);
         for diag in renderable.diagnostics.iter() {
             writeln!(f, "{}", self.annotate_renderer.render(diag.to_annotate()))?;
         }
-        Ok(())
+        writeln!(f)
     }
 }
 
@@ -2130,9 +2141,7 @@ watermelon
         ///
         /// (This will set the "printed" flag on `Diagnostic`.)
         fn render(&self, diag: &Diagnostic) -> String {
-            let mut buf = vec![];
+            diag.display(&self.db, &self.config).to_string()
-            diag.print(&self.db, &self.config, &mut buf).unwrap();
-            String::from_utf8(buf).unwrap()
         }
     }