mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 21:34:57 +00:00
Move GitHub rendering to ruff_db
(#20320)
## Summary This is the GitHub analog to #20117. This PR prepares to add a GitHub output format to ty by moving the implementation from `ruff_linter` to `ruff_db`. Hopefully this one is a bit easier to review commit-by-commit. Almost all of the refactoring this time is in the first commit, then the second commit adds the new `OutputFormat` variant and moves the file into `ruff_db`. The third commit is just a small touch up to use a private method that accommodates ty files so that we can run the tests and update/move the snapshots. I had to push a fourth commit to fix and test diagnostics without a span/file. ## Test Plan Existing tests
This commit is contained in:
parent
abb705aa4e
commit
5bf6977ded
8 changed files with 129 additions and 101 deletions
|
@ -14,9 +14,7 @@ use ruff_db::diagnostic::{
|
||||||
};
|
};
|
||||||
use ruff_linter::fs::relativize_path;
|
use ruff_linter::fs::relativize_path;
|
||||||
use ruff_linter::logging::LogLevel;
|
use ruff_linter::logging::LogLevel;
|
||||||
use ruff_linter::message::{
|
use ruff_linter::message::{Emitter, EmitterContext, GroupedEmitter, SarifEmitter, TextEmitter};
|
||||||
Emitter, EmitterContext, GithubEmitter, GroupedEmitter, SarifEmitter, TextEmitter,
|
|
||||||
};
|
|
||||||
use ruff_linter::notify_user;
|
use ruff_linter::notify_user;
|
||||||
use ruff_linter::settings::flags::{self};
|
use ruff_linter::settings::flags::{self};
|
||||||
use ruff_linter::settings::types::{OutputFormat, UnsafeFixes};
|
use ruff_linter::settings::types::{OutputFormat, UnsafeFixes};
|
||||||
|
@ -290,7 +288,11 @@ impl Printer {
|
||||||
self.write_summary_text(writer, diagnostics)?;
|
self.write_summary_text(writer, diagnostics)?;
|
||||||
}
|
}
|
||||||
OutputFormat::Github => {
|
OutputFormat::Github => {
|
||||||
GithubEmitter.emit(writer, &diagnostics.inner, &context)?;
|
let config = DisplayDiagnosticConfig::default()
|
||||||
|
.format(DiagnosticFormat::Github)
|
||||||
|
.preview(preview);
|
||||||
|
let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner);
|
||||||
|
write!(writer, "{value}")?;
|
||||||
}
|
}
|
||||||
OutputFormat::Gitlab => {
|
OutputFormat::Gitlab => {
|
||||||
let config = DisplayDiagnosticConfig::default()
|
let config = DisplayDiagnosticConfig::default()
|
||||||
|
|
|
@ -1447,6 +1447,11 @@ pub enum DiagnosticFormat {
|
||||||
/// [Code Quality]: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format
|
/// [Code Quality]: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format
|
||||||
#[cfg(feature = "serde")]
|
#[cfg(feature = "serde")]
|
||||||
Gitlab,
|
Gitlab,
|
||||||
|
|
||||||
|
/// Print diagnostics in the format used by [GitHub Actions] workflow error annotations.
|
||||||
|
///
|
||||||
|
/// [GitHub Actions]: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#setting-an-error-message
|
||||||
|
Github,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A representation of the kinds of messages inside a diagnostic.
|
/// A representation of the kinds of messages inside a diagnostic.
|
||||||
|
|
|
@ -25,11 +25,13 @@ use super::{
|
||||||
|
|
||||||
use azure::AzureRenderer;
|
use azure::AzureRenderer;
|
||||||
use concise::ConciseRenderer;
|
use concise::ConciseRenderer;
|
||||||
|
use github::GithubRenderer;
|
||||||
use pylint::PylintRenderer;
|
use pylint::PylintRenderer;
|
||||||
|
|
||||||
mod azure;
|
mod azure;
|
||||||
mod concise;
|
mod concise;
|
||||||
mod full;
|
mod full;
|
||||||
|
mod github;
|
||||||
#[cfg(feature = "serde")]
|
#[cfg(feature = "serde")]
|
||||||
mod gitlab;
|
mod gitlab;
|
||||||
#[cfg(feature = "serde")]
|
#[cfg(feature = "serde")]
|
||||||
|
@ -142,6 +144,9 @@ impl std::fmt::Display for DisplayDiagnostics<'_> {
|
||||||
DiagnosticFormat::Gitlab => {
|
DiagnosticFormat::Gitlab => {
|
||||||
gitlab::GitlabRenderer::new(self.resolver).render(f, self.diagnostics)?;
|
gitlab::GitlabRenderer::new(self.resolver).render(f, self.diagnostics)?;
|
||||||
}
|
}
|
||||||
|
DiagnosticFormat::Github => {
|
||||||
|
GithubRenderer::new(self.resolver).render(f, self.diagnostics)?;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|
109
crates/ruff_db/src/diagnostic/render/github.rs
Normal file
109
crates/ruff_db/src/diagnostic/render/github.rs
Normal file
|
@ -0,0 +1,109 @@
|
||||||
|
use crate::diagnostic::{Diagnostic, FileResolver};
|
||||||
|
|
||||||
|
pub(super) struct GithubRenderer<'a> {
|
||||||
|
resolver: &'a dyn FileResolver,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<'a> GithubRenderer<'a> {
|
||||||
|
pub(super) fn new(resolver: &'a dyn FileResolver) -> Self {
|
||||||
|
Self { resolver }
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(super) fn render(
|
||||||
|
&self,
|
||||||
|
f: &mut std::fmt::Formatter,
|
||||||
|
diagnostics: &[Diagnostic],
|
||||||
|
) -> std::fmt::Result {
|
||||||
|
for diagnostic in diagnostics {
|
||||||
|
write!(
|
||||||
|
f,
|
||||||
|
"::error title=Ruff ({code})",
|
||||||
|
code = diagnostic.secondary_code_or_id()
|
||||||
|
)?;
|
||||||
|
|
||||||
|
if let Some(span) = diagnostic.primary_span() {
|
||||||
|
let file = span.file();
|
||||||
|
write!(f, ",file={file}", file = file.path(self.resolver))?;
|
||||||
|
|
||||||
|
let (start_location, end_location) = if self.resolver.is_notebook(file) {
|
||||||
|
// We can't give a reasonable location for the structured formats,
|
||||||
|
// so we show one that's clearly a fallback
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
let diagnostic_source = file.diagnostic_source(self.resolver);
|
||||||
|
let source_code = diagnostic_source.as_source_code();
|
||||||
|
|
||||||
|
span.range().map(|range| {
|
||||||
|
(
|
||||||
|
source_code.line_column(range.start()),
|
||||||
|
source_code.line_column(range.end()),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
.unwrap_or_default();
|
||||||
|
|
||||||
|
write!(
|
||||||
|
f,
|
||||||
|
",line={row},col={column},endLine={end_row},endColumn={end_column}::",
|
||||||
|
row = start_location.line,
|
||||||
|
column = start_location.column,
|
||||||
|
end_row = end_location.line,
|
||||||
|
end_column = end_location.column,
|
||||||
|
)?;
|
||||||
|
|
||||||
|
write!(
|
||||||
|
f,
|
||||||
|
"{path}:{row}:{column}: ",
|
||||||
|
path = file.relative_path(self.resolver).display(),
|
||||||
|
row = start_location.line,
|
||||||
|
column = start_location.column,
|
||||||
|
)?;
|
||||||
|
} else {
|
||||||
|
write!(f, "::")?;
|
||||||
|
}
|
||||||
|
|
||||||
|
if let Some(code) = diagnostic.secondary_code() {
|
||||||
|
write!(f, "{code}")?;
|
||||||
|
} else {
|
||||||
|
write!(f, "{id}:", id = diagnostic.id())?;
|
||||||
|
}
|
||||||
|
|
||||||
|
writeln!(f, " {}", diagnostic.body())?;
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use crate::diagnostic::{
|
||||||
|
DiagnosticFormat,
|
||||||
|
render::tests::{TestEnvironment, create_diagnostics, create_syntax_error_diagnostics},
|
||||||
|
};
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn output() {
|
||||||
|
let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Github);
|
||||||
|
insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn syntax_errors() {
|
||||||
|
let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Github);
|
||||||
|
insta::assert_snapshot!(env.render_diagnostics(&diagnostics));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn missing_file() {
|
||||||
|
let mut env = TestEnvironment::new();
|
||||||
|
env.format(DiagnosticFormat::Github);
|
||||||
|
|
||||||
|
let diag = env.err().build();
|
||||||
|
|
||||||
|
insta::assert_snapshot!(
|
||||||
|
env.render(&diag),
|
||||||
|
@"::error title=Ruff (test-diagnostic)::test-diagnostic: main diagnostic message",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
|
@ -1,7 +1,6 @@
|
||||||
---
|
---
|
||||||
source: crates/ruff_linter/src/message/github.rs
|
source: crates/ruff_db/src/diagnostic/render/github.rs
|
||||||
expression: content
|
expression: env.render_diagnostics(&diagnostics)
|
||||||
snapshot_kind: text
|
|
||||||
---
|
---
|
||||||
::error title=Ruff (F401),file=fib.py,line=1,col=8,endLine=1,endColumn=10::fib.py:1:8: F401 `os` imported but unused
|
::error title=Ruff (F401),file=fib.py,line=1,col=8,endLine=1,endColumn=10::fib.py:1:8: F401 `os` imported but unused
|
||||||
::error title=Ruff (F841),file=fib.py,line=6,col=5,endLine=6,endColumn=6::fib.py:6:5: F841 Local variable `x` is assigned to but never used
|
::error title=Ruff (F841),file=fib.py,line=6,col=5,endLine=6,endColumn=6::fib.py:6:5: F841 Local variable `x` is assigned to but never used
|
|
@ -1,6 +1,6 @@
|
||||||
---
|
---
|
||||||
source: crates/ruff_linter/src/message/github.rs
|
source: crates/ruff_db/src/diagnostic/render/github.rs
|
||||||
expression: content
|
expression: env.render_diagnostics(&diagnostics)
|
||||||
---
|
---
|
||||||
::error title=Ruff (invalid-syntax),file=syntax_errors.py,line=1,col=15,endLine=2,endColumn=1::syntax_errors.py:1:15: invalid-syntax: Expected one or more symbol names after import
|
::error title=Ruff (invalid-syntax),file=syntax_errors.py,line=1,col=15,endLine=2,endColumn=1::syntax_errors.py:1:15: invalid-syntax: Expected one or more symbol names after import
|
||||||
::error title=Ruff (invalid-syntax),file=syntax_errors.py,line=3,col=12,endLine=4,endColumn=1::syntax_errors.py:3:12: invalid-syntax: Expected ')', found newline
|
::error title=Ruff (invalid-syntax),file=syntax_errors.py,line=3,col=12,endLine=4,endColumn=1::syntax_errors.py:3:12: invalid-syntax: Expected ')', found newline
|
|
@ -1,90 +0,0 @@
|
||||||
use std::io::Write;
|
|
||||||
|
|
||||||
use ruff_db::diagnostic::Diagnostic;
|
|
||||||
use ruff_source_file::LineColumn;
|
|
||||||
|
|
||||||
use crate::fs::relativize_path;
|
|
||||||
use crate::message::{Emitter, EmitterContext};
|
|
||||||
|
|
||||||
/// 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)
|
|
||||||
#[derive(Default)]
|
|
||||||
pub struct GithubEmitter;
|
|
||||||
|
|
||||||
impl Emitter for GithubEmitter {
|
|
||||||
fn emit(
|
|
||||||
&mut self,
|
|
||||||
writer: &mut dyn Write,
|
|
||||||
diagnostics: &[Diagnostic],
|
|
||||||
context: &EmitterContext,
|
|
||||||
) -> anyhow::Result<()> {
|
|
||||||
for diagnostic in diagnostics {
|
|
||||||
let source_location = diagnostic.ruff_start_location().unwrap_or_default();
|
|
||||||
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 {
|
|
||||||
source_location
|
|
||||||
};
|
|
||||||
|
|
||||||
let end_location = diagnostic.ruff_end_location().unwrap_or_default();
|
|
||||||
|
|
||||||
write!(
|
|
||||||
writer,
|
|
||||||
"::error title=Ruff ({code}),file={file},line={row},col={column},endLine={end_row},endColumn={end_column}::",
|
|
||||||
code = diagnostic.secondary_code_or_id(),
|
|
||||||
file = filename,
|
|
||||||
row = source_location.line,
|
|
||||||
column = source_location.column,
|
|
||||||
end_row = end_location.line,
|
|
||||||
end_column = end_location.column,
|
|
||||||
)?;
|
|
||||||
|
|
||||||
write!(
|
|
||||||
writer,
|
|
||||||
"{path}:{row}:{column}:",
|
|
||||||
path = relativize_path(&filename),
|
|
||||||
row = location.line,
|
|
||||||
column = location.column,
|
|
||||||
)?;
|
|
||||||
|
|
||||||
if let Some(code) = diagnostic.secondary_code() {
|
|
||||||
write!(writer, " {code}")?;
|
|
||||||
} else {
|
|
||||||
write!(writer, " {id}:", id = diagnostic.id())?;
|
|
||||||
}
|
|
||||||
|
|
||||||
writeln!(writer, " {}", diagnostic.body())?;
|
|
||||||
}
|
|
||||||
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(test)]
|
|
||||||
mod tests {
|
|
||||||
use insta::assert_snapshot;
|
|
||||||
|
|
||||||
use crate::message::GithubEmitter;
|
|
||||||
use crate::message::tests::{
|
|
||||||
capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics,
|
|
||||||
};
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn output() {
|
|
||||||
let mut emitter = GithubEmitter;
|
|
||||||
let content = capture_emitter_output(&mut emitter, &create_diagnostics());
|
|
||||||
|
|
||||||
assert_snapshot!(content);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn syntax_errors() {
|
|
||||||
let mut emitter = GithubEmitter;
|
|
||||||
let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics());
|
|
||||||
|
|
||||||
assert_snapshot!(content);
|
|
||||||
}
|
|
||||||
}
|
|
|
@ -9,7 +9,6 @@ use ruff_db::diagnostic::{
|
||||||
};
|
};
|
||||||
use ruff_db::files::File;
|
use ruff_db::files::File;
|
||||||
|
|
||||||
pub use github::GithubEmitter;
|
|
||||||
pub use grouped::GroupedEmitter;
|
pub use grouped::GroupedEmitter;
|
||||||
use ruff_notebook::NotebookIndex;
|
use ruff_notebook::NotebookIndex;
|
||||||
use ruff_source_file::SourceFile;
|
use ruff_source_file::SourceFile;
|
||||||
|
@ -20,7 +19,6 @@ pub use text::TextEmitter;
|
||||||
use crate::Fix;
|
use crate::Fix;
|
||||||
use crate::registry::Rule;
|
use crate::registry::Rule;
|
||||||
|
|
||||||
mod github;
|
|
||||||
mod grouped;
|
mod grouped;
|
||||||
mod sarif;
|
mod sarif;
|
||||||
mod text;
|
mod text;
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue