Use structs for JSON serialization (#19270)

## Summary

See https://github.com/astral-sh/ruff/pull/19133#discussion_r2198413586
for recent discussion. This PR moves to using structs for the types in
our JSON output format instead of the `json!` macro.

I didn't rename any of the `message` references because that should be
handled when rebasing #19133 onto this.

My plan for handling the `preview` behavior with the new diagnostics is
to use a wrapper enum. Something like:

```rust
#[derive(Serialize)]
#[serde(untagged)]
pub(crate) enum JsonDiagnostic<'a> {
    Old(OldJsonDiagnostic<'a>),
}

#[derive(Serialize)]
pub(crate) struct OldJsonDiagnostic<'a> {
    // ...
}
```

Initially I thought I could use a `&dyn Serialize` for the affected
fields, but I see that `Serialize` isn't dyn-compatible in testing this
now.

## Test Plan

Existing tests. One quirk of the new types is that their fields are in
alphabetical order. I guess `json!` sorts the fields alphabetically? The
tests were failing before I sorted the struct fields.

## Other formats

It looks like the `rdjson`, `sarif`, and `gitlab` formats also use
`json!`, so if we decide to merge this, I can do something similar for
those before moving them to the new diagnostic format.
This commit is contained in:
Brent Westbrook 2025-07-11 09:37:44 -04:00 committed by GitHub
parent a67630f907
commit f14ee9edd5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 78 additions and 37 deletions

View file

@ -432,8 +432,9 @@ impl Diagnostic {
/// Returns the [`SourceFile`] which the message belongs to. /// Returns the [`SourceFile`] which the message belongs to.
/// ///
/// Panics if the diagnostic has no primary span, or if its file is not a `SourceFile`. /// Panics if the diagnostic has no primary span, or if its file is not a `SourceFile`.
pub fn expect_ruff_source_file(&self) -> SourceFile { pub fn expect_ruff_source_file(&self) -> &SourceFile {
self.expect_primary_span().expect_ruff_file().clone() self.ruff_source_file()
.expect("Expected a ruff source file")
} }
/// Returns the [`TextRange`] for the diagnostic. /// Returns the [`TextRange`] for the diagnostic.

View file

@ -21,7 +21,7 @@ use crate::{Applicability, Fix};
/// * Compute the diff from the [`Edit`] because diff calculation is expensive. /// * Compute the diff from the [`Edit`] because diff calculation is expensive.
pub(super) struct Diff<'a> { pub(super) struct Diff<'a> {
fix: &'a Fix, fix: &'a Fix,
source_code: SourceFile, source_code: &'a SourceFile,
} }
impl<'a> Diff<'a> { impl<'a> Diff<'a> {

View file

@ -1,10 +1,10 @@
use std::io::Write; use std::io::Write;
use ruff_diagnostics::Applicability;
use serde::ser::SerializeSeq; use serde::ser::SerializeSeq;
use serde::{Serialize, Serializer}; use serde::{Serialize, Serializer};
use serde_json::{Value, json};
use ruff_db::diagnostic::Diagnostic; use ruff_db::diagnostic::{Diagnostic, SecondaryCode};
use ruff_notebook::NotebookIndex; use ruff_notebook::NotebookIndex;
use ruff_source_file::{LineColumn, OneIndexed, SourceCode}; use ruff_source_file::{LineColumn, OneIndexed, SourceCode};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
@ -55,20 +55,15 @@ impl Serialize for ExpandedMessages<'_> {
} }
} }
pub(crate) fn message_to_json_value(message: &Diagnostic, context: &EmitterContext) -> Value { pub(crate) fn message_to_json_value<'a>(
message: &'a Diagnostic,
context: &'a EmitterContext<'a>,
) -> JsonDiagnostic<'a> {
let source_file = message.expect_ruff_source_file(); let source_file = message.expect_ruff_source_file();
let source_code = source_file.to_source_code(); let source_code = source_file.to_source_code();
let filename = message.expect_ruff_filename(); let filename = message.expect_ruff_filename();
let notebook_index = context.notebook_index(&filename); let notebook_index = context.notebook_index(&filename);
let fix = message.fix().map(|fix| {
json!({
"applicability": fix.applicability(),
"message": message.suggestion(),
"edits": &ExpandedEdits { edits: fix.edits(), source_code: &source_code, notebook_index },
})
});
let mut start_location = source_code.line_column(message.expect_range().start()); let mut start_location = source_code.line_column(message.expect_range().start());
let mut end_location = source_code.line_column(message.expect_range().end()); let mut end_location = source_code.line_column(message.expect_range().end());
let mut noqa_location = message let mut noqa_location = message
@ -88,29 +83,32 @@ pub(crate) fn message_to_json_value(message: &Diagnostic, context: &EmitterConte
noqa_location.map(|location| notebook_index.translate_line_column(&location)); noqa_location.map(|location| notebook_index.translate_line_column(&location));
} }
json!({ let fix = message.fix().map(|fix| JsonFix {
"code": message.secondary_code(), applicability: fix.applicability(),
"url": message.to_url(), message: message.suggestion(),
"message": message.body(), edits: ExpandedEdits {
"fix": fix, edits: fix.edits(),
"cell": notebook_cell_index, source_code,
"location": location_to_json(start_location), notebook_index,
"end_location": location_to_json(end_location), },
"filename": filename, });
"noqa_row": noqa_location.map(|location| location.line)
})
}
fn location_to_json(location: LineColumn) -> serde_json::Value { JsonDiagnostic {
json!({ code: message.secondary_code(),
"row": location.line, url: message.to_url(),
"column": location.column message: message.body(),
}) fix,
cell: notebook_cell_index,
location: start_location.into(),
end_location: end_location.into(),
filename,
noqa_row: noqa_location.map(|location| location.line),
}
} }
struct ExpandedEdits<'a> { struct ExpandedEdits<'a> {
edits: &'a [Edit], edits: &'a [Edit],
source_code: &'a SourceCode<'a, 'a>, source_code: SourceCode<'a, 'a>,
notebook_index: Option<&'a NotebookIndex>, notebook_index: Option<&'a NotebookIndex>,
} }
@ -169,11 +167,11 @@ impl Serialize for ExpandedEdits<'_> {
location = notebook_index.translate_line_column(&location); location = notebook_index.translate_line_column(&location);
} }
let value = json!({ let value = JsonEdit {
"content": edit.content().unwrap_or_default(), content: edit.content().unwrap_or_default(),
"location": location_to_json(location), location: location.into(),
"end_location": location_to_json(end_location) end_location: end_location.into(),
}); };
s.serialize_element(&value)?; s.serialize_element(&value)?;
} }
@ -182,6 +180,48 @@ impl Serialize for ExpandedEdits<'_> {
} }
} }
#[derive(Serialize)]
pub(crate) struct JsonDiagnostic<'a> {
cell: Option<OneIndexed>,
code: Option<&'a SecondaryCode>,
end_location: JsonLocation,
filename: String,
fix: Option<JsonFix<'a>>,
location: JsonLocation,
message: &'a str,
noqa_row: Option<OneIndexed>,
url: Option<String>,
}
#[derive(Serialize)]
struct JsonFix<'a> {
applicability: Applicability,
edits: ExpandedEdits<'a>,
message: Option<&'a str>,
}
#[derive(Serialize)]
struct JsonLocation {
column: OneIndexed,
row: OneIndexed,
}
impl From<LineColumn> for JsonLocation {
fn from(location: LineColumn) -> Self {
JsonLocation {
row: location.line,
column: location.column,
}
}
}
#[derive(Serialize)]
struct JsonEdit<'a> {
content: &'a str,
end_location: JsonLocation,
location: JsonLocation,
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use insta::assert_snapshot; use insta::assert_snapshot;