Unify OldDiagnostic and Message (#18391)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions

Summary
--

This PR unifies the remaining differences between `OldDiagnostic` and
`Message` (`OldDiagnostic` was only missing an optional `noqa_offset`
field) and
replaces `Message` with `OldDiagnostic`.

The biggest functional difference is that the combined `OldDiagnostic`
kind no
longer implements `AsRule` for an infallible conversion to `Rule`. This
was
pretty easy to work around with `is_some_and` and `is_none_or` in the
few places
it was needed. In `LintContext::report_diagnostic_if_enabled` we can
just use
the new `Violation::rule` method, which takes care of most cases.

Most of the interesting changes are in [this
range](8156992540)
before I started renaming.

Test Plan
--

Existing tests

Future Work
--

I think it's time to start shifting some of these fields to the new
`Diagnostic`
kind. I believe we want `Fix` for sure, but I'm less sure about the
others. We
may want to keep a thin wrapper type here anyway to implement a `rule`
method,
so we could leave some of these fields on that too.
This commit is contained in:
Brent Westbrook 2025-06-19 09:37:58 -04:00 committed by GitHub
parent 4e83db4d40
commit 10a1d9f01e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
96 changed files with 902 additions and 914 deletions

View file

@ -3,7 +3,7 @@ use std::io::Write;
use ruff_source_file::LineColumn;
use crate::fs::relativize_path;
use crate::message::{Emitter, EmitterContext, Message};
use crate::message::{Emitter, EmitterContext, OldDiagnostic};
/// Generate error workflow command in GitHub Actions format.
/// See: [GitHub documentation](https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message)
@ -14,12 +14,12 @@ impl Emitter for GithubEmitter {
fn emit(
&mut self,
writer: &mut dyn Write,
messages: &[Message],
diagnostics: &[OldDiagnostic],
context: &EmitterContext,
) -> anyhow::Result<()> {
for message in messages {
let source_location = message.compute_start_location();
let location = if context.is_notebook(&message.filename()) {
for diagnostic in diagnostics {
let source_location = diagnostic.compute_start_location();
let location = if context.is_notebook(&diagnostic.filename()) {
// We can't give a reasonable location for the structured formats,
// so we show one that's clearly a fallback
LineColumn::default()
@ -27,15 +27,15 @@ impl Emitter for GithubEmitter {
source_location
};
let end_location = message.compute_end_location();
let end_location = diagnostic.compute_end_location();
write!(
writer,
"::error title=Ruff{code},file={file},line={row},col={column},endLine={end_row},endColumn={end_column}::",
code = message
code = diagnostic
.noqa_code()
.map_or_else(String::new, |code| format!(" ({code})")),
file = message.filename(),
file = diagnostic.filename(),
row = source_location.line,
column = source_location.column,
end_row = end_location.line,
@ -45,16 +45,16 @@ impl Emitter for GithubEmitter {
write!(
writer,
"{path}:{row}:{column}:",
path = relativize_path(&*message.filename()),
path = relativize_path(&*diagnostic.filename()),
row = location.line,
column = location.column,
)?;
if let Some(code) = message.noqa_code() {
if let Some(code) = diagnostic.noqa_code() {
write!(writer, " {code}")?;
}
writeln!(writer, " {}", message.body())?;
writeln!(writer, " {}", diagnostic.body())?;
}
Ok(())
@ -67,13 +67,13 @@ mod tests {
use crate::message::GithubEmitter;
use crate::message::tests::{
capture_emitter_output, create_messages, create_syntax_error_messages,
capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics,
};
#[test]
fn output() {
let mut emitter = GithubEmitter;
let content = capture_emitter_output(&mut emitter, &create_messages());
let content = capture_emitter_output(&mut emitter, &create_diagnostics());
assert_snapshot!(content);
}
@ -81,7 +81,7 @@ mod tests {
#[test]
fn syntax_errors() {
let mut emitter = GithubEmitter;
let content = capture_emitter_output(&mut emitter, &create_syntax_error_messages());
let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics());
assert_snapshot!(content);
}