mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-03 05:13:00 +00:00
## Summary This PR is a collaboration with @AlexWaygood from our pairing session last Friday. The main goal here is removing `ruff_linter::message::OldDiagnostic` in favor of using `ruff_db::diagnostic::Diagnostic` directly. This involved a few major steps: - Transferring the fields - Transferring the methods and trait implementations, where possible - Converting some constructor methods to free functions - Moving the `SecondaryCode` struct - Updating the method names I'm hoping that some of the methods, especially those in the `expect_ruff_*` family, won't be necessary long-term, but I avoided trying to replace them entirely for now to keep the already-large diff a bit smaller. ### Related refactors Alex and I noticed a few refactoring opportunities while looking at the code, specifically the very similar implementations for `create_parse_diagnostic`, `create_unsupported_syntax_diagnostic`, and `create_semantic_syntax_diagnostic`. We combined these into a single generic function, which I then copied into `ruff_linter::message` with some small changes and a TODO to combine them in the future. I also deleted the `DisplayParseErrorType` and `TruncateAtNewline` types for reporting parse errors. These were added in #4124, I believe to work around the error messages from LALRPOP. Removing these didn't affect any tests, so I think they were unnecessary now that we fully control the error messages from the parser. On a more minor note, I factored out some calls to the `OldDiagnostic::filename` (now `Diagnostic::expect_ruff_filename`) function to avoid repeatedly allocating `String`s in some places. ### Snapshot changes The `show_statistics_syntax_errors` integration test changed because the `OldDiagnostic::name` method used `syntax-error` instead of `invalid-syntax` like in ty. I think this (`--statistics`) is one of the only places we actually use this name for syntax errors, so I hope this is okay. An alternative is to use `syntax-error` in ty too. The other snapshot changes are from removing this code, as discussed on [Discord](1388252408):34052a1185/crates/ruff_linter/src/message/mod.rs (L128-L135)I think both of these are technically breaking changes, but they only affect syntax errors and are very narrow in scope, while also pretty substantially simplifying the refactor, so I hope they're okay to include in a patch release. ## Test plan Existing tests, with the adjustments mentioned above --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
71 lines
2.2 KiB
Rust
71 lines
2.2 KiB
Rust
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);
|
|
}
|
|
}
|