Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions crates/ruff/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use ruff_db::diagnostic::{
use ruff_linter::fs::relativize_path;
use ruff_linter::logging::LogLevel;
use ruff_linter::message::{
Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter, JunitEmitter,
SarifEmitter, TextEmitter,
Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter, SarifEmitter,
TextEmitter,
};
use ruff_linter::notify_user;
use ruff_linter::settings::flags::{self};
Expand Down Expand Up @@ -252,7 +252,11 @@ impl Printer {
write!(writer, "{value}")?;
}
OutputFormat::Junit => {
JunitEmitter.emit(writer, &diagnostics.inner, &context)?;
let config = DisplayDiagnosticConfig::default()
.format(DiagnosticFormat::Junit)
.preview(preview);
let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner);
write!(writer, "{value}")?;
}
OutputFormat::Concise | OutputFormat::Full => {
TextEmitter::default()
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/tests/snapshots/lint__output_format_junit.snap
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ exit_code: 1
<testcase name="org.ruff.F821" classname="[TMP]/input" line="2" column="5">
<failure message="Undefined name `y`">line 2, col 5, Undefined name `y`</failure>
</testcase>
<testcase name="org.ruff" classname="[TMP]/input" line="3" column="1">
<testcase name="org.ruff.invalid-syntax" classname="[TMP]/input" line="3" column="1">
<failure message="SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)">line 3, col 1, SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10)</failure>
</testcase>
</testsuite>
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_db/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ glob = { workspace = true }
ignore = { workspace = true, optional = true }
matchit = { workspace = true }
path-slash = { workspace = true }
quick-junit = { workspace = true, optional = true }
rustc-hash = { workspace = true }
salsa = { workspace = true }
schemars = { workspace = true, optional = true }
Expand All @@ -56,6 +57,7 @@ tempfile = { workspace = true }

[features]
cache = ["ruff_cache"]
junit = ["dep:quick-junit"]
os = ["ignore", "dep:etcetera"]
serde = ["camino/serde1", "dep:serde", "dep:serde_json", "ruff_diagnostics/serde"]
# Exposes testing utilities.
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff_db/src/diagnostic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,9 @@ pub enum DiagnosticFormat {
Rdjson,
/// Print diagnostics in the format emitted by Pylint.
Pylint,
/// Print diagnostics in the format expected by JUnit.
#[cfg(feature = "junit")]
Junit,
}

/// A representation of the kinds of messages inside a diagnostic.
Expand Down
6 changes: 6 additions & 0 deletions crates/ruff_db/src/diagnostic/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ mod azure;
mod json;
#[cfg(feature = "serde")]
mod json_lines;
#[cfg(feature = "junit")]
mod junit;
mod pylint;
#[cfg(feature = "serde")]
mod rdjson;
Expand Down Expand Up @@ -196,6 +198,10 @@ impl std::fmt::Display for DisplayDiagnostics<'_> {
DiagnosticFormat::Pylint => {
PylintRenderer::new(self.resolver).render(f, self.diagnostics)?;
}
#[cfg(feature = "junit")]
DiagnosticFormat::Junit => {
junit::JunitRenderer::new(self.resolver).render(f, self.diagnostics)?;
}
}

Ok(())
Expand Down
161 changes: 161 additions & 0 deletions crates/ruff_db/src/diagnostic/render/junit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
use std::{collections::BTreeMap, ops::Deref, path::Path};

use quick_junit::{NonSuccessKind, Report, TestCase, TestCaseStatus, TestSuite, XmlString};

use ruff_source_file::LineColumn;

use crate::diagnostic::{Diagnostic, SecondaryCode, render::FileResolver};

pub struct JunitRenderer<'a> {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a link to the format specification?

resolver: &'a dyn FileResolver,
}

impl<'a> JunitRenderer<'a> {
pub fn new(resolver: &'a dyn FileResolver) -> Self {
Self { resolver }
}

pub(super) fn render(
&self,
f: &mut std::fmt::Formatter,
diagnostics: &[Diagnostic],
) -> std::fmt::Result {
let mut report = Report::new("ruff");

if diagnostics.is_empty() {
let mut test_suite = TestSuite::new("ruff");
test_suite
.extra
.insert(XmlString::new("package"), XmlString::new("org.ruff"));
let mut case = TestCase::new("No errors found", TestCaseStatus::success());
case.set_classname("ruff");
test_suite.add_test_case(case);
report.add_test_suite(test_suite);
} else {
for (filename, diagnostics) in group_diagnostics_by_filename(diagnostics, self.resolver)
{
let mut test_suite = TestSuite::new(filename);
test_suite
.extra
.insert(XmlString::new("package"), XmlString::new("org.ruff"));

for diagnostic in diagnostics {
let DiagnosticWithLocation {
diagnostic,
start_location: location,
} = diagnostic;
let mut status = TestCaseStatus::non_success(NonSuccessKind::Failure);
status.set_message(diagnostic.body());

status.set_description(format!(
"line {row}, col {col}, {body}",
row = location.line,
col = location.column,
body = diagnostic.body()
));
Copy link
Member

Choose a reason for hiding this comment

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

Given that this is a free text field, I'd prefer if we omit row and column for diagnostics without a location rather than making up a location. For the future, I could see us use the full diagnostic renderer for the description similar to https://www.ibm.com/docs/en/developer-for-zos/16.0.x?topic=formats-junit-xml-format

let code = diagnostic
.secondary_code()
.map_or_else(|| diagnostic.name(), SecondaryCode::as_str);
let mut case = TestCase::new(format!("org.ruff.{code}"), status);
Copy link
Member

Choose a reason for hiding this comment

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

I know that's pre-existing but I wonder if the test case name is supposed to be unique (but it isn't if a file contains multiple diagnostics with the same code). But this isn't something that we need to solve in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, it does seem like it's probably supposed to be unique. This site says it's the test method name in the test's class specified by classname.

let file_path = Path::new(filename);
let file_stem = file_path.file_stem().unwrap().to_str().unwrap();
let classname = file_path.parent().unwrap().join(file_stem);
case.set_classname(classname.to_str().unwrap());
case.extra.insert(
XmlString::new("line"),
XmlString::new(location.line.to_string()),
);
case.extra.insert(
XmlString::new("column"),
XmlString::new(location.column.to_string()),
);

Copy link
Member

Choose a reason for hiding this comment

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

Same here, I think I'd prefer not to insert a made-up line and column information if they are missing

test_suite.add_test_case(case);
}
report.add_test_suite(test_suite);
}
}

// Safety: this should be infallible as long as the data we put in the report is valid
// UTF-8.
//
// It's a bit of a shame to call `to_string`, but `Report` otherwise only exposes a
// `serialize` method, which expects an `io::Write`, not a `fmt::Write`. `to_string`
// currently (2025-07-15) serializes into a `Vec<u8>` and converts to a `String` from there.
f.write_str(
&report
.to_string()
.expect("Failed to serialize JUnit report"),
)
Copy link
Member

Choose a reason for hiding this comment

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

This has now come up a couple of times (JSON reports too).

We could write a io::Write to fmt::Write adapter instead. It's slightly less efficient because it needs to validate that the string is valid UTF8

struct FmtAdapter<'a> {
    fmt: &'a mut dyn std::fmt::Write,
}

impl std::io::Write for FmtAdapter<'_> {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        self.fmt
            .write_str(str::from_utf8(buf).map_err(|_| {
                std::io::Error::new(
                    std::io::ErrorKind::InvalidData,
                    "Invalid UTF-8 in JUnit report",
                )
            })?)
            .map_err(io_error)?;

        Ok(buf.len())
    }

    fn flush(&mut self) -> std::io::Result<()> {
        Ok(())
    }

    fn write_fmt(&mut self, args: Arguments<'_>) -> std::io::Result<()> {
        self.fmt.write_fmt(args).map_err(io_error)
    }
}

fn io_error(error: std::fmt::Error) -> std::io::Error {
    std::io::Error::new(std::io::ErrorKind::Other, error)
}

The other alternative (but this might be worth a separate PR) is to explore if we can pass an std::io::Write instead of an std::fmt::Write

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I can add this for now. I poked a bit at passing a std::io::Write to all of these formats before, but I think it would justify its own PR like you said.

}
}

// TODO(brent) this and `group_diagnostics_by_filename` are also used by the `grouped` output
// format. I think they'd make more sense in that file, but I started here first. I'll move them to
// that module when adding the `grouped` output format.
struct DiagnosticWithLocation<'a> {
diagnostic: &'a Diagnostic,
start_location: LineColumn,
}

impl Deref for DiagnosticWithLocation<'_> {
type Target = Diagnostic;

fn deref(&self) -> &Self::Target {
self.diagnostic
}
}

fn group_diagnostics_by_filename<'a>(
diagnostics: &'a [Diagnostic],
resolver: &'a dyn FileResolver,
) -> BTreeMap<&'a str, Vec<DiagnosticWithLocation<'a>>> {
let mut grouped_diagnostics = BTreeMap::default();
for diagnostic in diagnostics {
let (filename, start_location) = diagnostic
.primary_span_ref()
.map(|span| {
let file = span.file();
let start_location =
span.range()
.filter(|_| !resolver.is_notebook(file))
.map(|range| {
file.diagnostic_source(resolver)
.as_source_code()
.line_column(range.start())
});

(span.file().path(resolver), start_location)
})
.unwrap_or_default();

grouped_diagnostics
.entry(filename)
.or_insert_with(Vec::new)
.push(DiagnosticWithLocation {
diagnostic,
start_location: start_location.unwrap_or_default(),
});
}
grouped_diagnostics
}

#[cfg(test)]
mod tests {
use crate::diagnostic::{
DiagnosticFormat,
render::tests::{create_diagnostics, create_syntax_error_diagnostics},
};

#[test]
fn output() {
let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Junit);
insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
}

#[test]
fn syntax_errors() {
let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Junit);
insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: crates/ruff_linter/src/message/junit.rs
expression: content
snapshot_kind: text
source: crates/ruff_db/src/diagnostic/render/junit.rs
expression: env.render_diagnostics(&diagnostics)
---
<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="ruff" tests="3" failures="3" errors="0">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
---
source: crates/ruff_linter/src/message/junit.rs
expression: content
snapshot_kind: text
source: crates/ruff_db/src/diagnostic/render/junit.rs
expression: env.render_diagnostics(&diagnostics)
---
<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="ruff" tests="2" failures="2" errors="0">
<testsuite name="syntax_errors.py" tests="2" disabled="0" errors="0" failures="2" package="org.ruff">
<testcase name="org.ruff" classname="syntax_errors" line="1" column="15">
<testcase name="org.ruff.invalid-syntax" classname="syntax_errors" line="1" column="15">
<failure message="SyntaxError: Expected one or more symbol names after import">line 1, col 15, SyntaxError: Expected one or more symbol names after import</failure>
</testcase>
<testcase name="org.ruff" classname="syntax_errors" line="3" column="12">
<testcase name="org.ruff.invalid-syntax" classname="syntax_errors" line="3" column="12">
<failure message="SyntaxError: Expected &apos;)&apos;, found newline">line 3, col 12, SyntaxError: Expected &apos;)&apos;, found newline</failure>
</testcase>
</testsuite>
Expand Down
3 changes: 1 addition & 2 deletions crates/ruff_linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ license = { workspace = true }
[dependencies]
ruff_annotate_snippets = { workspace = true }
ruff_cache = { workspace = true }
ruff_db = { workspace = true, features = ["serde"] }
ruff_db = { workspace = true, features = ["junit", "serde"] }
ruff_diagnostics = { workspace = true, features = ["serde"] }
ruff_notebook = { workspace = true }
ruff_macros = { workspace = true }
Expand Down Expand Up @@ -55,7 +55,6 @@ path-absolutize = { workspace = true, features = [
pathdiff = { workspace = true }
pep440_rs = { workspace = true }
pyproject-toml = { workspace = true }
quick-junit = { workspace = true }
regex = { workspace = true }
rustc-hash = { workspace = true }
schemars = { workspace = true, optional = true }
Expand Down
Loading
Loading