Move RDJSON rendering to ruff_db (#19293)

## Summary

Another output format like #19133. This is the
[reviewdog](https://github.com/reviewdog/reviewdog) output format, which
is somewhat similar to regular JSON. Like #19270, in the first commit I
converted from using `json!` to `Serialize` structs, then in the second
commit I moved the module to `ruff_db`.

The reviewdog
[schema](320a8e73a9/proto/rdf/jsonschema/DiagnosticResult.json)
seems a bit more flexible than our JSON schema, so I'm not sure if we
need any preview checks here. I'll flag the places I wasn't sure about
as review comments.

## Test Plan

New tests in `rdjson.rs`, ported from the old `rjdson.rs` module, as
well as the new CLI output tests.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>

crates/ruff/src/printer.rs

@@ -16,7 +16,7 @@ use ruff_linter::fs::relativize_path;
 use ruff_linter::logging::LogLevel;
 use ruff_linter::message::{
     Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter, JunitEmitter,
-    PylintEmitter, RdjsonEmitter, SarifEmitter, TextEmitter,
+    PylintEmitter, SarifEmitter, TextEmitter,
 };
 use ruff_linter::notify_user;
 use ruff_linter::settings::flags::{self};
@@ -238,7 +238,11 @@ impl Printer {
                 write!(writer, "{value}")?;
             }
             OutputFormat::Rdjson => {
-                RdjsonEmitter.emit(writer, &diagnostics.inner, &context)?;
+                let config = DisplayDiagnosticConfig::default()
+                    .format(DiagnosticFormat::Rdjson)
+                    .preview(preview);
+                let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner);
+                write!(writer, "{value}")?;
             }
             OutputFormat::JsonLines => {
                 let config = DisplayDiagnosticConfig::default()

crates/ruff/tests/snapshots/lint__output_format_rdjson.snap

@@ -75,8 +75,7 @@ exit_code: 1
     },
     {
       "code": {
-        "url": null,
+        "value": "invalid-syntax"
-        "value": null
       },
       "location": {
         "path": "[TMP]/input.py",
@@ -94,7 +93,7 @@ exit_code: 1
       "message": "SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)"
     }
   ],
-  "severity": "warning",
+  "severity": "WARNING",
   "source": {
     "name": "ruff",
     "url": "https://docs.astral.sh/ruff"

crates/ruff_db/src/diagnostic/mod.rs

@@ -308,6 +308,10 @@ impl Diagnostic {
 
     /// Set the fix for this diagnostic.
     pub fn set_fix(&mut self, fix: Fix) {
+        debug_assert!(
+            self.primary_span().is_some(),
+            "Expected a source file for a diagnostic with a fix"
+        );
         Arc::make_mut(&mut self.inner).fix = Some(fix);
     }
 
@@ -1259,6 +1263,11 @@ pub enum DiagnosticFormat {
     /// format for an array of all diagnostics. See <https://jsonlines.org/> for more details.
     #[cfg(feature = "serde")]
     JsonLines,
+    /// Print diagnostics in the JSON format expected by [reviewdog].
+    ///
+    /// [reviewdog]: https://github.com/reviewdog/reviewdog
+    #[cfg(feature = "serde")]
+    Rdjson,
 }
 
 /// A representation of the kinds of messages inside a diagnostic.

crates/ruff_db/src/diagnostic/render.rs

@@ -28,6 +28,8 @@ mod azure;
 mod json;
 #[cfg(feature = "serde")]
 mod json_lines;
+#[cfg(feature = "serde")]
+mod rdjson;
 
 /// A type that implements `std::fmt::Display` for diagnostic rendering.
 ///
@@ -184,6 +186,10 @@ impl std::fmt::Display for DisplayDiagnostics<'_> {
                 json_lines::JsonLinesRenderer::new(self.resolver, self.config)
                     .render(f, self.diagnostics)?;
             }
+            #[cfg(feature = "serde")]
+            DiagnosticFormat::Rdjson => {
+                rdjson::RdjsonRenderer::new(self.resolver).render(f, self.diagnostics)?;
+            }
         }
 
         Ok(())

crates/ruff_db/src/diagnostic/render/json.rs

@@ -262,9 +262,6 @@ struct JsonEdit<'a> {
 
 #[cfg(test)]
 mod tests {
-    use ruff_diagnostics::{Edit, Fix};
-    use ruff_text_size::TextSize;
-
     use crate::diagnostic::{
         DiagnosticFormat,
         render::tests::{
@@ -297,13 +294,7 @@ mod tests {
         env.format(DiagnosticFormat::Json);
         env.preview(false);
 
-        let diag = env
+        let diag = env.err().build();
-            .err()
-            .fix(Fix::safe_edit(Edit::insertion(
-                "edit".to_string(),
-                TextSize::from(0),
-            )))
-            .build();
 
         insta::assert_snapshot!(
             env.render(&diag),
@@ -317,23 +308,7 @@ mod tests {
               "row": 1
             },
             "filename": "",
-            "fix": {
+            "fix": null,
-              "applicability": "safe",
-              "edits": [
-                {
-                  "content": "edit",
-                  "end_location": {
-                    "column": 1,
-                    "row": 1
-                  },
-                  "location": {
-                    "column": 1,
-                    "row": 1
-                  }
-                }
-              ],
-              "message": null
-            },
             "location": {
               "column": 1,
               "row": 1
@@ -353,13 +328,7 @@ mod tests {
         env.format(DiagnosticFormat::Json);
         env.preview(true);
 
-        let diag = env
+        let diag = env.err().build();
-            .err()
-            .fix(Fix::safe_edit(Edit::insertion(
-                "edit".to_string(),
-                TextSize::from(0),
-            )))
-            .build();
 
         insta::assert_snapshot!(
             env.render(&diag),
@@ -370,17 +339,7 @@ mod tests {
             "code": null,
             "end_location": null,
             "filename": null,
-            "fix": {
+            "fix": null,
-              "applicability": "safe",
-              "edits": [
-                {
-                  "content": "edit",
-                  "end_location": null,
-                  "location": null
-                }
-              ],
-              "message": null
-            },
             "location": null,
             "message": "main diagnostic message",
             "noqa_row": null,

crates/ruff_db/src/diagnostic/render/rdjson.rs

@@ -0,0 +1,235 @@
+use serde::ser::SerializeSeq;
+use serde::{Serialize, Serializer};
+
+use ruff_diagnostics::{Edit, Fix};
+use ruff_source_file::{LineColumn, SourceCode};
+use ruff_text_size::Ranged;
+
+use crate::diagnostic::Diagnostic;
+
+use super::FileResolver;
+
+pub struct RdjsonRenderer<'a> {
+    resolver: &'a dyn FileResolver,
+}
+
+impl<'a> RdjsonRenderer<'a> {
+    pub(super) fn new(resolver: &'a dyn FileResolver) -> Self {
+        Self { resolver }
+    }
+
+    pub(super) fn render(
+        &self,
+        f: &mut std::fmt::Formatter,
+        diagnostics: &[Diagnostic],
+    ) -> std::fmt::Result {
+        write!(
+            f,
+            "{:#}",
+            serde_json::json!(RdjsonDiagnostics::new(diagnostics, self.resolver))
+        )
+    }
+}
+
+struct ExpandedDiagnostics<'a> {
+    resolver: &'a dyn FileResolver,
+    diagnostics: &'a [Diagnostic],
+}
+
+impl Serialize for ExpandedDiagnostics<'_> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        let mut s = serializer.serialize_seq(Some(self.diagnostics.len()))?;
+
+        for diagnostic in self.diagnostics {
+            let value = diagnostic_to_rdjson(diagnostic, self.resolver);
+            s.serialize_element(&value)?;
+        }
+
+        s.end()
+    }
+}
+
+fn diagnostic_to_rdjson<'a>(
+    diagnostic: &'a Diagnostic,
+    resolver: &'a dyn FileResolver,
+) -> RdjsonDiagnostic<'a> {
+    let span = diagnostic.primary_span_ref();
+    let source_file = span.map(|span| {
+        let file = span.file();
+        (file.path(resolver), file.diagnostic_source(resolver))
+    });
+
+    let location = source_file.as_ref().map(|(path, source)| {
+        let range = diagnostic.range().map(|range| {
+            let source_code = source.as_source_code();
+            let start = source_code.line_column(range.start());
+            let end = source_code.line_column(range.end());
+            RdjsonRange::new(start, end)
+        });
+
+        RdjsonLocation { path, range }
+    });
+
+    let edits = diagnostic.fix().map(Fix::edits).unwrap_or_default();
+
+    RdjsonDiagnostic {
+        message: diagnostic.body(),
+        location,
+        code: RdjsonCode {
+            value: diagnostic
+                .secondary_code()
+                .map_or_else(|| diagnostic.name(), |code| code.as_str()),
+            url: diagnostic.to_ruff_url(),
+        },
+        suggestions: rdjson_suggestions(
+            edits,
+            source_file
+                .as_ref()
+                .map(|(_, source)| source.as_source_code()),
+        ),
+    }
+}
+
+fn rdjson_suggestions<'a>(
+    edits: &'a [Edit],
+    source_code: Option<SourceCode>,
+) -> Vec<RdjsonSuggestion<'a>> {
+    if edits.is_empty() {
+        return Vec::new();
+    }
+
+    let Some(source_code) = source_code else {
+        debug_assert!(false, "Expected a source file for a diagnostic with a fix");
+        return Vec::new();
+    };
+
+    edits
+        .iter()
+        .map(|edit| {
+            let start = source_code.line_column(edit.start());
+            let end = source_code.line_column(edit.end());
+            let range = RdjsonRange::new(start, end);
+
+            RdjsonSuggestion {
+                range,
+                text: edit.content().unwrap_or_default(),
+            }
+        })
+        .collect()
+}
+
+#[derive(Serialize)]
+struct RdjsonDiagnostics<'a> {
+    diagnostics: ExpandedDiagnostics<'a>,
+    severity: &'static str,
+    source: RdjsonSource,
+}
+
+impl<'a> RdjsonDiagnostics<'a> {
+    fn new(diagnostics: &'a [Diagnostic], resolver: &'a dyn FileResolver) -> Self {
+        Self {
+            source: RdjsonSource {
+                name: "ruff",
+                url: env!("CARGO_PKG_HOMEPAGE"),
+            },
+            severity: "WARNING",
+            diagnostics: ExpandedDiagnostics {
+                diagnostics,
+                resolver,
+            },
+        }
+    }
+}
+
+#[derive(Serialize)]
+struct RdjsonSource {
+    name: &'static str,
+    url: &'static str,
+}
+
+#[derive(Serialize)]
+struct RdjsonDiagnostic<'a> {
+    code: RdjsonCode<'a>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    location: Option<RdjsonLocation<'a>>,
+    message: &'a str,
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    suggestions: Vec<RdjsonSuggestion<'a>>,
+}
+
+#[derive(Serialize)]
+struct RdjsonLocation<'a> {
+    path: &'a str,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    range: Option<RdjsonRange>,
+}
+
+#[derive(Default, Serialize)]
+struct RdjsonRange {
+    end: LineColumn,
+    start: LineColumn,
+}
+
+impl RdjsonRange {
+    fn new(start: LineColumn, end: LineColumn) -> Self {
+        Self { start, end }
+    }
+}
+
+#[derive(Serialize)]
+struct RdjsonCode<'a> {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    url: Option<String>,
+    value: &'a str,
+}
+
+#[derive(Serialize)]
+struct RdjsonSuggestion<'a> {
+    range: RdjsonRange,
+    text: &'a str,
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostic::{
+        DiagnosticFormat,
+        render::tests::{TestEnvironment, create_diagnostics, create_syntax_error_diagnostics},
+    };
+
+    #[test]
+    fn output() {
+        let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Rdjson);
+        insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
+    }
+
+    #[test]
+    fn syntax_errors() {
+        let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Rdjson);
+        insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
+    }
+
+    #[test]
+    fn missing_file_stable() {
+        let mut env = TestEnvironment::new();
+        env.format(DiagnosticFormat::Rdjson);
+        env.preview(false);
+
+        let diag = env.err().build();
+
+        insta::assert_snapshot!(env.render(&diag));
+    }
+
+    #[test]
+    fn missing_file_preview() {
+        let mut env = TestEnvironment::new();
+        env.format(DiagnosticFormat::Rdjson);
+        env.preview(true);
+
+        let diag = env.err().build();
+
+        insta::assert_snapshot!(env.render(&diag));
+    }
+}

crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__rdjson__tests__missing_file_preview.snap

@@ -0,0 +1,20 @@
+---
+source: crates/ruff_db/src/diagnostic/render/rdjson.rs
+expression: env.render(&diag)
+---
+{
+  "diagnostics": [
+    {
+      "code": {
+        "url": "https://docs.astral.sh/ruff/rules/test-diagnostic",
+        "value": "test-diagnostic"
+      },
+      "message": "main diagnostic message"
+    }
+  ],
+  "severity": "WARNING",
+  "source": {
+    "name": "ruff",
+    "url": "https://docs.astral.sh/ruff"
+  }
+}

crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__rdjson__tests__missing_file_stable.snap

@@ -0,0 +1,20 @@
+---
+source: crates/ruff_db/src/diagnostic/render/rdjson.rs
+expression: env.render(&diag)
+---
+{
+  "diagnostics": [
+    {
+      "code": {
+        "url": "https://docs.astral.sh/ruff/rules/test-diagnostic",
+        "value": "test-diagnostic"
+      },
+      "message": "main diagnostic message"
+    }
+  ],
+  "severity": "WARNING",
+  "source": {
+    "name": "ruff",
+    "url": "https://docs.astral.sh/ruff"
+  }
+}

crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__rdjson__tests__output.snap

@@ -1,7 +1,6 @@
 ---
-source: crates/ruff_linter/src/message/rdjson.rs
+source: crates/ruff_db/src/diagnostic/render/rdjson.rs
-expression: content
+expression: env.render_diagnostics(&diagnostics)
-snapshot_kind: text
 ---
 {
   "diagnostics": [
@@ -96,7 +95,7 @@ snapshot_kind: text
       "message": "Undefined name `a`"
     }
   ],
-  "severity": "warning",
+  "severity": "WARNING",
   "source": {
     "name": "ruff",
     "url": "https://docs.astral.sh/ruff"

crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__rdjson__tests__syntax_errors.snap

@@ -1,14 +1,12 @@
 ---
-source: crates/ruff_linter/src/message/rdjson.rs
+source: crates/ruff_db/src/diagnostic/render/rdjson.rs
-expression: content
+expression: env.render_diagnostics(&diagnostics)
-snapshot_kind: text
 ---
 {
   "diagnostics": [
     {
       "code": {
-        "url": null,
+        "value": "invalid-syntax"
-        "value": null
       },
       "location": {
         "path": "syntax_errors.py",
@@ -27,8 +25,7 @@ snapshot_kind: text
     },
     {
       "code": {
-        "url": null,
+        "value": "invalid-syntax"
-        "value": null
       },
       "location": {
         "path": "syntax_errors.py",
@@ -46,7 +43,7 @@ snapshot_kind: text
       "message": "SyntaxError: Expected ')', found newline"
     }
   ],
-  "severity": "warning",
+  "severity": "WARNING",
   "source": {
     "name": "ruff",
     "url": "https://docs.astral.sh/ruff"

crates/ruff_linter/src/message/mod.rs

@@ -16,7 +16,6 @@ pub use gitlab::GitlabEmitter;
 pub use grouped::GroupedEmitter;
 pub use junit::JunitEmitter;
 pub use pylint::PylintEmitter;
-pub use rdjson::RdjsonEmitter;
 use ruff_notebook::NotebookIndex;
 use ruff_source_file::{LineColumn, SourceFile};
 use ruff_text_size::{Ranged, TextRange, TextSize};
@@ -32,7 +31,6 @@ mod gitlab;
 mod grouped;
 mod junit;
 mod pylint;
-mod rdjson;
 mod sarif;
 mod text;
 
@@ -80,6 +78,13 @@ where
         body,
     );
 
+    let span = Span::from(file).with_range(range);
+    let mut annotation = Annotation::primary(span);
+    if let Some(suggestion) = suggestion {
+        annotation = annotation.message(suggestion);
+    }
+    diagnostic.annotate(annotation);
+
     if let Some(fix) = fix {
         diagnostic.set_fix(fix);
     }
@@ -92,13 +97,6 @@ where
         diagnostic.set_noqa_offset(noqa_offset);
     }
 
-    let span = Span::from(file).with_range(range);
-    let mut annotation = Annotation::primary(span);
-    if let Some(suggestion) = suggestion {
-        annotation = annotation.message(suggestion);
-    }
-    diagnostic.annotate(annotation);
-
     diagnostic.set_secondary_code(SecondaryCode::new(rule.noqa_code().to_string()));
 
     diagnostic

crates/ruff_linter/src/message/rdjson.rs

@@ -1,143 +0,0 @@
-use std::io::Write;
-
-use serde::ser::SerializeSeq;
-use serde::{Serialize, Serializer};
-use serde_json::{Value, json};
-
-use ruff_db::diagnostic::Diagnostic;
-use ruff_source_file::SourceCode;
-use ruff_text_size::Ranged;
-
-use crate::Edit;
-use crate::message::{Emitter, EmitterContext, LineColumn};
-
-#[derive(Default)]
-pub struct RdjsonEmitter;
-
-impl Emitter for RdjsonEmitter {
-    fn emit(
-        &mut self,
-        writer: &mut dyn Write,
-        diagnostics: &[Diagnostic],
-        _context: &EmitterContext,
-    ) -> anyhow::Result<()> {
-        serde_json::to_writer_pretty(
-            writer,
-            &json!({
-                "source": {
-                    "name": "ruff",
-                    "url": "https://docs.astral.sh/ruff",
-                },
-                "severity": "warning",
-                "diagnostics": &ExpandedMessages{ diagnostics }
-            }),
-        )?;
-
-        Ok(())
-    }
-}
-
-struct ExpandedMessages<'a> {
-    diagnostics: &'a [Diagnostic],
-}
-
-impl Serialize for ExpandedMessages<'_> {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        let mut s = serializer.serialize_seq(Some(self.diagnostics.len()))?;
-
-        for message in self.diagnostics {
-            let value = message_to_rdjson_value(message);
-            s.serialize_element(&value)?;
-        }
-
-        s.end()
-    }
-}
-
-fn message_to_rdjson_value(message: &Diagnostic) -> Value {
-    let source_file = message.expect_ruff_source_file();
-    let source_code = source_file.to_source_code();
-
-    let start_location = source_code.line_column(message.expect_range().start());
-    let end_location = source_code.line_column(message.expect_range().end());
-
-    if let Some(fix) = message.fix() {
-        json!({
-            "message": message.body(),
-            "location": {
-                "path": message.expect_ruff_filename(),
-                "range": rdjson_range(start_location, end_location),
-            },
-            "code": {
-                "value": message.secondary_code(),
-                "url": message.to_ruff_url(),
-            },
-            "suggestions": rdjson_suggestions(fix.edits(), &source_code),
-        })
-    } else {
-        json!({
-            "message": message.body(),
-            "location": {
-                "path": message.expect_ruff_filename(),
-                "range": rdjson_range(start_location, end_location),
-            },
-            "code": {
-                "value": message.secondary_code(),
-                "url": message.to_ruff_url(),
-            },
-        })
-    }
-}
-
-fn rdjson_suggestions(edits: &[Edit], source_code: &SourceCode) -> Value {
-    Value::Array(
-        edits
-            .iter()
-            .map(|edit| {
-                let location = source_code.line_column(edit.start());
-                let end_location = source_code.line_column(edit.end());
-
-                json!({
-                    "range": rdjson_range(location, end_location),
-                    "text": edit.content().unwrap_or_default(),
-                })
-            })
-            .collect(),
-    )
-}
-
-fn rdjson_range(start: LineColumn, end: LineColumn) -> Value {
-    json!({
-        "start": start,
-        "end": end,
-    })
-}
-
-#[cfg(test)]
-mod tests {
-    use insta::assert_snapshot;
-
-    use crate::message::RdjsonEmitter;
-    use crate::message::tests::{
-        capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics,
-    };
-
-    #[test]
-    fn output() {
-        let mut emitter = RdjsonEmitter;
-        let content = capture_emitter_output(&mut emitter, &create_diagnostics());
-
-        assert_snapshot!(content);
-    }
-
-    #[test]
-    fn syntax_errors() {
-        let mut emitter = RdjsonEmitter;
-        let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics());
-
-        assert_snapshot!(content);
-    }
-}