Render Azure, JSON, and JSON lines output with the new diagnostics (#19133)

## Summary

This was originally stacked on #19129, but some of the changes I made
for JSON also impacted the Azure format, so I went ahead and combined
them. The main changes here are:

- Implementing `FileResolver` for Ruff's `EmitterContext`
- Adding `FileResolver::notebook_index` and `FileResolver::is_notebook`
methods
- Adding a `DisplayDiagnostics` (with an "s") type for rendering a group
of diagnostics at once
- Adding `Azure`, `Json`, and `JsonLines` as new `DiagnosticFormat`s

I tried a couple of alternatives to the `FileResolver::notebook` methods
like passing down the `NotebookIndex` separately and trying to reparse a
`Notebook` from Ruff's `SourceFile`. The latter seemed promising, but
the `SourceFile` only stores the concatenated plain text of the
notebook, not the re-parsable JSON. I guess the current version is just
a variation on passing the `NotebookIndex`, but at least we can reuse
the existing `resolver` argument. I think a lot of this can be cleaned
up once Ruff has its own actual file resolver.

As suggested, I also tried deleting the corresponding `Emitter` files in
`ruff_linter`, but it doesn't look like git was able to follow this as a
rename. It did, however, track that the tests were moved, so the
snapshots should be easy to review.

## Test Plan

Existing Ruff tests ported to tests in `ruff_db`. I think some other
existing ruff tests also cover parts of this refactor.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
Brent Westbrook 2025-07-11 15:04:46 -04:00 committed by GitHub
parent ee88abf77c
commit b5c5f710fc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
28 changed files with 1135 additions and 539 deletions

View file

@ -1,71 +0,0 @@
use std::io::Write;
use ruff_db::diagnostic::Diagnostic;
use ruff_source_file::LineColumn;
use crate::message::{Emitter, EmitterContext};
/// Generate error logging commands for Azure Pipelines format.
/// See [documentation](https://learn.microsoft.com/en-us/azure/devops/pipelines/scripts/logging-commands?view=azure-devops&tabs=bash#logissue-log-an-error-or-warning)
#[derive(Default)]
pub struct AzureEmitter;
impl Emitter for AzureEmitter {
fn emit(
&mut self,
writer: &mut dyn Write,
diagnostics: &[Diagnostic],
context: &EmitterContext,
) -> anyhow::Result<()> {
for diagnostic in diagnostics {
let filename = diagnostic.expect_ruff_filename();
let location = if context.is_notebook(&filename) {
// We can't give a reasonable location for the structured formats,
// so we show one that's clearly a fallback
LineColumn::default()
} else {
diagnostic.expect_ruff_start_location()
};
writeln!(
writer,
"##vso[task.logissue type=error\
;sourcepath={filename};linenumber={line};columnnumber={col};{code}]{body}",
line = location.line,
col = location.column,
code = diagnostic
.secondary_code()
.map_or_else(String::new, |code| format!("code={code};")),
body = diagnostic.body(),
)?;
}
Ok(())
}
}
#[cfg(test)]
mod tests {
use insta::assert_snapshot;
use crate::message::AzureEmitter;
use crate::message::tests::{
capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics,
};
#[test]
fn output() {
let mut emitter = AzureEmitter;
let content = capture_emitter_output(&mut emitter, &create_diagnostics());
assert_snapshot!(content);
}
#[test]
fn syntax_errors() {
let mut emitter = AzureEmitter;
let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics());
assert_snapshot!(content);
}
}

View file

@ -1,260 +0,0 @@
use std::io::Write;
use ruff_diagnostics::Applicability;
use serde::ser::SerializeSeq;
use serde::{Serialize, Serializer};
use ruff_db::diagnostic::{Diagnostic, SecondaryCode};
use ruff_notebook::NotebookIndex;
use ruff_source_file::{LineColumn, OneIndexed, SourceCode};
use ruff_text_size::Ranged;
use crate::Edit;
use crate::message::{Emitter, EmitterContext};
#[derive(Default)]
pub struct JsonEmitter;
impl Emitter for JsonEmitter {
fn emit(
&mut self,
writer: &mut dyn Write,
diagnostics: &[Diagnostic],
context: &EmitterContext,
) -> anyhow::Result<()> {
serde_json::to_writer_pretty(
writer,
&ExpandedMessages {
diagnostics,
context,
},
)?;
Ok(())
}
}
struct ExpandedMessages<'a> {
diagnostics: &'a [Diagnostic],
context: &'a EmitterContext<'a>,
}
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_json_value(message, self.context);
s.serialize_element(&value)?;
}
s.end()
}
}
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 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
.noqa_offset()
.map(|offset| source_code.line_column(offset));
let mut notebook_cell_index = None;
if let Some(notebook_index) = notebook_index {
notebook_cell_index = Some(
notebook_index
.cell(start_location.line)
.unwrap_or(OneIndexed::MIN),
);
start_location = notebook_index.translate_line_column(&start_location);
end_location = notebook_index.translate_line_column(&end_location);
noqa_location =
noqa_location.map(|location| notebook_index.translate_line_column(&location));
}
let fix = message.fix().map(|fix| JsonFix {
applicability: fix.applicability(),
message: message.suggestion(),
edits: ExpandedEdits {
edits: fix.edits(),
source_code,
notebook_index,
},
});
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> {
edits: &'a [Edit],
source_code: SourceCode<'a, 'a>,
notebook_index: Option<&'a NotebookIndex>,
}
impl Serialize for ExpandedEdits<'_> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let mut s = serializer.serialize_seq(Some(self.edits.len()))?;
for edit in self.edits {
let mut location = self.source_code.line_column(edit.start());
let mut end_location = self.source_code.line_column(edit.end());
if let Some(notebook_index) = self.notebook_index {
// There exists a newline between each cell's source code in the
// concatenated source code in Ruff. This newline doesn't actually
// exists in the JSON source field.
//
// Now, certain edits may try to remove this newline, which means
// the edit will spill over to the first character of the next cell.
// If it does, we need to translate the end location to the last
// character of the previous cell.
match (
notebook_index.cell(location.line),
notebook_index.cell(end_location.line),
) {
(Some(start_cell), Some(end_cell)) if start_cell != end_cell => {
debug_assert_eq!(end_location.column.get(), 1);
let prev_row = end_location.line.saturating_sub(1);
end_location = LineColumn {
line: notebook_index.cell_row(prev_row).unwrap_or(OneIndexed::MIN),
column: self
.source_code
.line_column(self.source_code.line_end_exclusive(prev_row))
.column,
};
}
(Some(_), None) => {
debug_assert_eq!(end_location.column.get(), 1);
let prev_row = end_location.line.saturating_sub(1);
end_location = LineColumn {
line: notebook_index.cell_row(prev_row).unwrap_or(OneIndexed::MIN),
column: self
.source_code
.line_column(self.source_code.line_end_exclusive(prev_row))
.column,
};
}
_ => {
end_location = notebook_index.translate_line_column(&end_location);
}
}
location = notebook_index.translate_line_column(&location);
}
let value = JsonEdit {
content: edit.content().unwrap_or_default(),
location: location.into(),
end_location: end_location.into(),
};
s.serialize_element(&value)?;
}
s.end()
}
}
#[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;
use crate::message::JsonEmitter;
use crate::message::tests::{
capture_emitter_notebook_output, capture_emitter_output, create_diagnostics,
create_notebook_diagnostics, create_syntax_error_diagnostics,
};
#[test]
fn output() {
let mut emitter = JsonEmitter;
let content = capture_emitter_output(&mut emitter, &create_diagnostics());
assert_snapshot!(content);
}
#[test]
fn syntax_errors() {
let mut emitter = JsonEmitter;
let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics());
assert_snapshot!(content);
}
#[test]
fn notebook_output() {
let mut emitter = JsonEmitter;
let (diagnostics, notebook_indexes) = create_notebook_diagnostics();
let content =
capture_emitter_notebook_output(&mut emitter, &diagnostics, &notebook_indexes);
assert_snapshot!(content);
}
}

View file

@ -1,60 +0,0 @@
use std::io::Write;
use ruff_db::diagnostic::Diagnostic;
use crate::message::json::message_to_json_value;
use crate::message::{Emitter, EmitterContext};
#[derive(Default)]
pub struct JsonLinesEmitter;
impl Emitter for JsonLinesEmitter {
fn emit(
&mut self,
writer: &mut dyn Write,
diagnostics: &[Diagnostic],
context: &EmitterContext,
) -> anyhow::Result<()> {
for diagnostic in diagnostics {
serde_json::to_writer(&mut *writer, &message_to_json_value(diagnostic, context))?;
writer.write_all(b"\n")?;
}
Ok(())
}
}
#[cfg(test)]
mod tests {
use insta::assert_snapshot;
use crate::message::json_lines::JsonLinesEmitter;
use crate::message::tests::{
capture_emitter_notebook_output, capture_emitter_output, create_diagnostics,
create_notebook_diagnostics, create_syntax_error_diagnostics,
};
#[test]
fn output() {
let mut emitter = JsonLinesEmitter;
let content = capture_emitter_output(&mut emitter, &create_diagnostics());
assert_snapshot!(content);
}
#[test]
fn syntax_errors() {
let mut emitter = JsonLinesEmitter;
let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics());
assert_snapshot!(content);
}
#[test]
fn notebook_output() {
let mut emitter = JsonLinesEmitter;
let (messages, notebook_indexes) = create_notebook_diagnostics();
let content = capture_emitter_notebook_output(&mut emitter, &messages, &notebook_indexes);
assert_snapshot!(content);
}
}

View file

@ -3,17 +3,17 @@ use std::fmt::Display;
use std::io::Write;
use std::ops::Deref;
use ruff_db::diagnostic::{
Annotation, Diagnostic, DiagnosticId, LintName, SecondaryCode, Severity, Span,
};
use rustc_hash::FxHashMap;
pub use azure::AzureEmitter;
use ruff_db::diagnostic::{
Annotation, Diagnostic, DiagnosticId, FileResolver, Input, LintName, SecondaryCode, Severity,
Span, UnifiedFile,
};
use ruff_db::files::File;
pub use github::GithubEmitter;
pub use gitlab::GitlabEmitter;
pub use grouped::GroupedEmitter;
pub use json::JsonEmitter;
pub use json_lines::JsonLinesEmitter;
pub use junit::JunitEmitter;
pub use pylint::PylintEmitter;
pub use rdjson::RdjsonEmitter;
@ -26,13 +26,10 @@ pub use text::TextEmitter;
use crate::Fix;
use crate::registry::Rule;
mod azure;
mod diff;
mod github;
mod gitlab;
mod grouped;
mod json;
mod json_lines;
mod junit;
mod pylint;
mod rdjson;
@ -107,6 +104,34 @@ where
diagnostic
}
impl FileResolver for EmitterContext<'_> {
fn path(&self, _file: File) -> &str {
unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic");
}
fn input(&self, _file: File) -> Input {
unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic");
}
fn notebook_index(&self, file: &UnifiedFile) -> Option<NotebookIndex> {
match file {
UnifiedFile::Ty(_) => {
unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic")
}
UnifiedFile::Ruff(file) => self.notebook_indexes.get(file.name()).cloned(),
}
}
fn is_notebook(&self, file: &UnifiedFile) -> bool {
match file {
UnifiedFile::Ty(_) => {
unimplemented!("Expected a Ruff file for rendering a Ruff diagnostic")
}
UnifiedFile::Ruff(file) => self.notebook_indexes.get(file.name()).is_some(),
}
}
}
struct MessageWithLocation<'a> {
message: &'a Diagnostic,
start_location: LineColumn,

View file

@ -73,7 +73,7 @@ fn message_to_rdjson_value(message: &Diagnostic) -> Value {
},
"code": {
"value": message.secondary_code(),
"url": message.to_url(),
"url": message.to_ruff_url(),
},
"suggestions": rdjson_suggestions(fix.edits(), &source_code),
})
@ -86,7 +86,7 @@ fn message_to_rdjson_value(message: &Diagnostic) -> Value {
},
"code": {
"value": message.secondary_code(),
"url": message.to_url(),
"url": message.to_ruff_url(),
},
})
}

View file

@ -1,8 +0,0 @@
---
source: crates/ruff_linter/src/message/azure.rs
expression: content
snapshot_kind: text
---
##vso[task.logissue type=error;sourcepath=fib.py;linenumber=1;columnnumber=8;code=F401;]`os` imported but unused
##vso[task.logissue type=error;sourcepath=fib.py;linenumber=6;columnnumber=5;code=F841;]Local variable `x` is assigned to but never used
##vso[task.logissue type=error;sourcepath=undef.py;linenumber=1;columnnumber=4;code=F821;]Undefined name `a`

View file

@ -1,7 +0,0 @@
---
source: crates/ruff_linter/src/message/azure.rs
expression: content
snapshot_kind: text
---
##vso[task.logissue type=error;sourcepath=syntax_errors.py;linenumber=1;columnnumber=15;]SyntaxError: Expected one or more symbol names after import
##vso[task.logissue type=error;sourcepath=syntax_errors.py;linenumber=3;columnnumber=12;]SyntaxError: Expected ')', found newline

View file

@ -1,106 +0,0 @@
---
source: crates/ruff_linter/src/message/json.rs
expression: content
snapshot_kind: text
---
[
{
"cell": 1,
"code": "F401",
"end_location": {
"column": 10,
"row": 2
},
"filename": "notebook.ipynb",
"fix": {
"applicability": "safe",
"edits": [
{
"content": "",
"end_location": {
"column": 10,
"row": 2
},
"location": {
"column": 1,
"row": 2
}
}
],
"message": "Remove unused import: `os`"
},
"location": {
"column": 8,
"row": 2
},
"message": "`os` imported but unused",
"noqa_row": 2,
"url": "https://docs.astral.sh/ruff/rules/unused-import"
},
{
"cell": 2,
"code": "F401",
"end_location": {
"column": 12,
"row": 2
},
"filename": "notebook.ipynb",
"fix": {
"applicability": "safe",
"edits": [
{
"content": "",
"end_location": {
"column": 1,
"row": 3
},
"location": {
"column": 1,
"row": 2
}
}
],
"message": "Remove unused import: `math`"
},
"location": {
"column": 8,
"row": 2
},
"message": "`math` imported but unused",
"noqa_row": 2,
"url": "https://docs.astral.sh/ruff/rules/unused-import"
},
{
"cell": 3,
"code": "F841",
"end_location": {
"column": 6,
"row": 4
},
"filename": "notebook.ipynb",
"fix": {
"applicability": "unsafe",
"edits": [
{
"content": "",
"end_location": {
"column": 10,
"row": 4
},
"location": {
"column": 1,
"row": 4
}
}
],
"message": "Remove assignment to unused variable `x`"
},
"location": {
"column": 5,
"row": 4
},
"message": "Local variable `x` is assigned to but never used",
"noqa_row": 4,
"url": "https://docs.astral.sh/ruff/rules/unused-variable"
}
]

View file

@ -1,90 +0,0 @@
---
source: crates/ruff_linter/src/message/json.rs
expression: content
snapshot_kind: text
---
[
{
"cell": null,
"code": "F401",
"end_location": {
"column": 10,
"row": 1
},
"filename": "fib.py",
"fix": {
"applicability": "unsafe",
"edits": [
{
"content": "",
"end_location": {
"column": 1,
"row": 2
},
"location": {
"column": 1,
"row": 1
}
}
],
"message": "Remove unused import: `os`"
},
"location": {
"column": 8,
"row": 1
},
"message": "`os` imported but unused",
"noqa_row": 1,
"url": "https://docs.astral.sh/ruff/rules/unused-import"
},
{
"cell": null,
"code": "F841",
"end_location": {
"column": 6,
"row": 6
},
"filename": "fib.py",
"fix": {
"applicability": "unsafe",
"edits": [
{
"content": "",
"end_location": {
"column": 10,
"row": 6
},
"location": {
"column": 5,
"row": 6
}
}
],
"message": "Remove assignment to unused variable `x`"
},
"location": {
"column": 5,
"row": 6
},
"message": "Local variable `x` is assigned to but never used",
"noqa_row": 6,
"url": "https://docs.astral.sh/ruff/rules/unused-variable"
},
{
"cell": null,
"code": "F821",
"end_location": {
"column": 5,
"row": 1
},
"filename": "undef.py",
"fix": null,
"location": {
"column": 4,
"row": 1
},
"message": "Undefined name `a`",
"noqa_row": 1,
"url": "https://docs.astral.sh/ruff/rules/undefined-name"
}
]

View file

@ -1,41 +0,0 @@
---
source: crates/ruff_linter/src/message/json.rs
expression: content
snapshot_kind: text
---
[
{
"cell": null,
"code": null,
"end_location": {
"column": 1,
"row": 2
},
"filename": "syntax_errors.py",
"fix": null,
"location": {
"column": 15,
"row": 1
},
"message": "SyntaxError: Expected one or more symbol names after import",
"noqa_row": null,
"url": null
},
{
"cell": null,
"code": null,
"end_location": {
"column": 1,
"row": 4
},
"filename": "syntax_errors.py",
"fix": null,
"location": {
"column": 12,
"row": 3
},
"message": "SyntaxError: Expected ')', found newline",
"noqa_row": null,
"url": null
}
]

View file

@ -1,8 +0,0 @@
---
source: crates/ruff_linter/src/message/json_lines.rs
expression: content
snapshot_kind: text
---
{"cell":1,"code":"F401","end_location":{"column":10,"row":2},"filename":"notebook.ipynb","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":10,"row":2},"location":{"column":1,"row":2}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":2},"message":"`os` imported but unused","noqa_row":2,"url":"https://docs.astral.sh/ruff/rules/unused-import"}
{"cell":2,"code":"F401","end_location":{"column":12,"row":2},"filename":"notebook.ipynb","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":1,"row":3},"location":{"column":1,"row":2}}],"message":"Remove unused import: `math`"},"location":{"column":8,"row":2},"message":"`math` imported but unused","noqa_row":2,"url":"https://docs.astral.sh/ruff/rules/unused-import"}
{"cell":3,"code":"F841","end_location":{"column":6,"row":4},"filename":"notebook.ipynb","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":10,"row":4},"location":{"column":1,"row":4}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":4},"message":"Local variable `x` is assigned to but never used","noqa_row":4,"url":"https://docs.astral.sh/ruff/rules/unused-variable"}

View file

@ -1,8 +0,0 @@
---
source: crates/ruff_linter/src/message/json_lines.rs
expression: content
snapshot_kind: text
---
{"cell":null,"code":"F401","end_location":{"column":10,"row":1},"filename":"fib.py","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":1,"row":2},"location":{"column":1,"row":1}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":1},"message":"`os` imported but unused","noqa_row":1,"url":"https://docs.astral.sh/ruff/rules/unused-import"}
{"cell":null,"code":"F841","end_location":{"column":6,"row":6},"filename":"fib.py","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":10,"row":6},"location":{"column":5,"row":6}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":6},"message":"Local variable `x` is assigned to but never used","noqa_row":6,"url":"https://docs.astral.sh/ruff/rules/unused-variable"}
{"cell":null,"code":"F821","end_location":{"column":5,"row":1},"filename":"undef.py","fix":null,"location":{"column":4,"row":1},"message":"Undefined name `a`","noqa_row":1,"url":"https://docs.astral.sh/ruff/rules/undefined-name"}

View file

@ -1,7 +0,0 @@
---
source: crates/ruff_linter/src/message/json_lines.rs
expression: content
snapshot_kind: text
---
{"cell":null,"code":null,"end_location":{"column":1,"row":2},"filename":"syntax_errors.py","fix":null,"location":{"column":15,"row":1},"message":"SyntaxError: Expected one or more symbol names after import","noqa_row":null,"url":null}
{"cell":null,"code":null,"end_location":{"column":1,"row":4},"filename":"syntax_errors.py","fix":null,"location":{"column":12,"row":3},"message":"SyntaxError: Expected ')', found newline","noqa_row":null,"url":null}