Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/ruff_db/src/diagnostic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,9 @@ impl Diagnostic {
/// Returns the [`SourceFile`] which the message belongs to.
///
/// Panics if the diagnostic has no primary span, or if its file is not a `SourceFile`.
pub fn expect_ruff_source_file(&self) -> SourceFile {
self.expect_primary_span().expect_ruff_file().clone()
pub fn expect_ruff_source_file(&self) -> &SourceFile {
self.ruff_source_file()
.expect("Expected a ruff source file")
}

/// Returns the [`TextRange`] for the diagnostic.
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/message/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{Applicability, Fix};
/// * Compute the diff from the [`Edit`] because diff calculation is expensive.
pub(super) struct Diff<'a> {
fix: &'a Fix,
source_code: SourceFile,
source_code: &'a SourceFile,
}

impl<'a> Diff<'a> {
Expand Down
108 changes: 74 additions & 34 deletions crates/ruff_linter/src/message/json.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use std::io::Write;

use ruff_diagnostics::Applicability;
use serde::ser::SerializeSeq;
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_source_file::{LineColumn, OneIndexed, SourceCode};
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -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_code = source_file.to_source_code();
let filename = message.expect_ruff_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 end_location = source_code.line_column(message.expect_range().end());
let mut noqa_location = message
Expand All @@ -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));
}

json!({
"code": message.secondary_code(),
"url": message.to_url(),
"message": message.body(),
"fix": fix,
"cell": notebook_cell_index,
"location": location_to_json(start_location),
"end_location": location_to_json(end_location),
"filename": filename,
"noqa_row": noqa_location.map(|location| location.line)
})
}
let fix = message.fix().map(|fix| JsonFix {
applicability: fix.applicability(),
message: message.suggestion(),
edits: ExpandedEdits {
edits: fix.edits(),
source_code,
notebook_index,
},
});

fn location_to_json(location: LineColumn) -> serde_json::Value {
json!({
"row": location.line,
"column": location.column
})
JsonDiagnostic {
code: message.secondary_code(),
url: message.to_url(),
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> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaning towards reverting the last commit. The old implementation carefully avoided allocating a new vector for the edits. I also don't think that changing this is necessary for making this more typed. We can just replace the

           let value = json!({
                "content": edit.content().unwrap_or_default(),
                "location": location_to_json(location),
                "end_location": location_to_json(end_location)
            });

and initialize and then serialize said struct instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, reverted!

edits: &'a [Edit],
source_code: &'a SourceCode<'a, 'a>,
source_code: SourceCode<'a, 'a>,
notebook_index: Option<&'a NotebookIndex>,
}

Expand Down Expand Up @@ -169,11 +167,11 @@ impl Serialize for ExpandedEdits<'_> {
location = notebook_index.translate_line_column(&location);
}

let value = json!({
"content": edit.content().unwrap_or_default(),
"location": location_to_json(location),
"end_location": location_to_json(end_location)
});
let value = JsonEdit {
content: edit.content().unwrap_or_default(),
location: location.into(),
end_location: end_location.into(),
};

s.serialize_element(&value)?;
}
Expand All @@ -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)]
mod tests {
use insta::assert_snapshot;
Expand Down
Loading