From e9cac3684afbbb23b22f047e0545cee5cfc77a54 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Tue, 15 Jul 2025 10:14:49 -0400 Subject: [PATCH] Move Pylint rendering to `ruff_db` (#19340) Summary -- This is a very simple output format, the only decision is what to do if the file is missing from the diagnostic. For now, I opted to `unwrap_or_default` both the path and the `OneIndexed` row number, giving `:1: main diagnostic message` in the test without a file. Another quirk here is that the path is relativized. I just pasted in the `relativize_path` and `get_cwd` implementations from `ruff_linter::fs` for now, but maybe there's a better place for them. I didn't see any details about why this needs to be relativized in the original [issue](https://github.com/astral-sh/ruff/issues/1953), [PR](https://github.com/astral-sh/ruff/pull/1995), or in the pylint [docs](https://flake8.pycqa.org/en/latest/internal/formatters.html#pylint-formatter), but it did change the results of the CLI integration test when I tried deleting it. I haven't been able to reproduce that in the CLI, though, so it may only happen with `Command::current_dir`. Test Plan -- Tests ported from `ruff_linter` and a new test for the case with no file --------- Co-authored-by: Micha Reiser --- crates/ruff/src/printer.rs | 8 +- .../snapshots/lint__output_format_pylint.snap | 2 +- crates/ruff_db/src/diagnostic/mod.rs | 16 ++- crates/ruff_db/src/diagnostic/render.rs | 17 ++++ .../ruff_db/src/diagnostic/render/pylint.rs | 97 +++++++++++++++++++ ...ostic__render__pylint__tests__output.snap} | 5 +- ..._render__pylint__tests__syntax_errors.snap | 6 ++ crates/ruff_linter/src/message/mod.rs | 6 +- crates/ruff_linter/src/message/pylint.rs | 72 -------------- ...message__pylint__tests__syntax_errors.snap | 7 -- crates/ruff_source_file/src/line_index.rs | 6 ++ 11 files changed, 154 insertions(+), 88 deletions(-) create mode 100644 crates/ruff_db/src/diagnostic/render/pylint.rs rename crates/{ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__output.snap => ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__output.snap} (59%) create mode 100644 crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__syntax_errors.snap delete mode 100644 crates/ruff_linter/src/message/pylint.rs delete mode 100644 crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__syntax_errors.snap diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index f04f4a216c..2fa8e476cb 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -16,7 +16,7 @@ use ruff_linter::fs::relativize_path; use ruff_linter::logging::LogLevel; use ruff_linter::message::{ Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter, JunitEmitter, - PylintEmitter, SarifEmitter, TextEmitter, + SarifEmitter, TextEmitter, }; use ruff_linter::notify_user; use ruff_linter::settings::flags::{self}; @@ -294,7 +294,11 @@ impl Printer { GitlabEmitter::default().emit(writer, &diagnostics.inner, &context)?; } OutputFormat::Pylint => { - PylintEmitter.emit(writer, &diagnostics.inner, &context)?; + let config = DisplayDiagnosticConfig::default() + .format(DiagnosticFormat::Pylint) + .preview(preview); + let value = DisplayDiagnostics::new(&context, &config, &diagnostics.inner); + write!(writer, "{value}")?; } OutputFormat::Azure => { let config = DisplayDiagnosticConfig::default() diff --git a/crates/ruff/tests/snapshots/lint__output_format_pylint.snap b/crates/ruff/tests/snapshots/lint__output_format_pylint.snap index 8b30234a82..5e813a5c3a 100644 --- a/crates/ruff/tests/snapshots/lint__output_format_pylint.snap +++ b/crates/ruff/tests/snapshots/lint__output_format_pylint.snap @@ -18,6 +18,6 @@ exit_code: 1 ----- stdout ----- input.py:1: [F401] `os` imported but unused input.py:2: [F821] Undefined name `y` -input.py:3: SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10) +input.py:3: [invalid-syntax] SyntaxError: Cannot use `match` statement on Python 3.9 (syntax was added in Python 3.10) ----- stderr ----- diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 23412b8381..ade0a9402d 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1,4 +1,4 @@ -use std::{fmt::Formatter, sync::Arc}; +use std::{fmt::Formatter, path::Path, sync::Arc}; use ruff_diagnostics::Fix; use ruff_source_file::{LineColumn, SourceCode, SourceFile}; @@ -1012,6 +1012,18 @@ impl UnifiedFile { } } + /// Return the file's path relative to the current working directory. + pub fn relative_path<'a>(&'a self, resolver: &'a dyn FileResolver) -> &'a Path { + let cwd = resolver.current_directory(); + let path = Path::new(self.path(resolver)); + + if let Ok(path) = path.strip_prefix(cwd) { + return path; + } + + path + } + fn diagnostic_source(&self, resolver: &dyn FileResolver) -> DiagnosticSource { match self { UnifiedFile::Ty(file) => DiagnosticSource::Ty(resolver.input(*file)), @@ -1268,6 +1280,8 @@ pub enum DiagnosticFormat { /// [reviewdog]: https://github.com/reviewdog/reviewdog #[cfg(feature = "serde")] Rdjson, + /// Print diagnostics in the format emitted by Pylint. + Pylint, } /// A representation of the kinds of messages inside a diagnostic. diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index eac5858ed1..f189a757e6 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::path::Path; use ruff_annotate_snippets::{ Annotation as AnnotateAnnotation, Level as AnnotateLevel, Message as AnnotateMessage, @@ -22,12 +23,14 @@ use super::{ }; use azure::AzureRenderer; +use pylint::PylintRenderer; mod azure; #[cfg(feature = "serde")] mod json; #[cfg(feature = "serde")] mod json_lines; +mod pylint; #[cfg(feature = "serde")] mod rdjson; @@ -190,6 +193,9 @@ impl std::fmt::Display for DisplayDiagnostics<'_> { DiagnosticFormat::Rdjson => { rdjson::RdjsonRenderer::new(self.resolver).render(f, self.diagnostics)?; } + DiagnosticFormat::Pylint => { + PylintRenderer::new(self.resolver).render(f, self.diagnostics)?; + } } Ok(()) @@ -711,6 +717,9 @@ pub trait FileResolver { /// Returns whether the file given is a Jupyter notebook. fn is_notebook(&self, file: &UnifiedFile) -> bool; + + /// Returns the current working directory. + fn current_directory(&self) -> &Path; } impl FileResolver for T @@ -746,6 +755,10 @@ where UnifiedFile::Ruff(_) => unimplemented!("Expected an interned ty file"), } } + + fn current_directory(&self) -> &Path { + self.system().current_directory().as_std_path() + } } impl FileResolver for &dyn Db { @@ -778,6 +791,10 @@ impl FileResolver for &dyn Db { UnifiedFile::Ruff(_) => unimplemented!("Expected an interned ty file"), } } + + fn current_directory(&self) -> &Path { + self.system().current_directory().as_std_path() + } } /// An abstraction over a unit of user input. diff --git a/crates/ruff_db/src/diagnostic/render/pylint.rs b/crates/ruff_db/src/diagnostic/render/pylint.rs new file mode 100644 index 0000000000..7040c344af --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/pylint.rs @@ -0,0 +1,97 @@ +use crate::diagnostic::{Diagnostic, SecondaryCode, render::FileResolver}; + +/// Generate violations in Pylint format. +/// +/// The format is given by this string: +/// +/// ```python +/// "%(path)s:%(row)d: [%(code)s] %(text)s" +/// ``` +/// +/// See: [Flake8 documentation](https://flake8.pycqa.org/en/latest/internal/formatters.html#pylint-formatter) +pub(super) struct PylintRenderer<'a> { + resolver: &'a dyn FileResolver, +} + +impl<'a> PylintRenderer<'a> { + pub(super) fn new(resolver: &'a dyn FileResolver) -> Self { + Self { resolver } + } +} + +impl PylintRenderer<'_> { + pub(super) fn render( + &self, + f: &mut std::fmt::Formatter, + diagnostics: &[Diagnostic], + ) -> std::fmt::Result { + for diagnostic in diagnostics { + let (filename, row) = diagnostic + .primary_span_ref() + .map(|span| { + let file = span.file(); + + let row = span + .range() + .filter(|_| !self.resolver.is_notebook(file)) + .map(|range| { + file.diagnostic_source(self.resolver) + .as_source_code() + .line_column(range.start()) + .line + }); + + (file.relative_path(self.resolver).to_string_lossy(), row) + }) + .unwrap_or_default(); + + let code = diagnostic + .secondary_code() + .map_or_else(|| diagnostic.name(), SecondaryCode::as_str); + + let row = row.unwrap_or_default(); + + writeln!( + f, + "{path}:{row}: [{code}] {body}", + path = filename, + body = 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::Pylint); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn syntax_errors() { + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Pylint); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics)); + } + + #[test] + fn missing_file() { + let mut env = TestEnvironment::new(); + env.format(DiagnosticFormat::Pylint); + + let diag = env.err().build(); + + insta::assert_snapshot!( + env.render(&diag), + @":1: [test-diagnostic] main diagnostic message", + ); + } +} diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__output.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__output.snap similarity index 59% rename from crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__output.snap rename to crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__output.snap index 21ae8043c0..5d51a37c40 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__output.snap +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__output.snap @@ -1,7 +1,6 @@ --- -source: crates/ruff_linter/src/message/pylint.rs -expression: content -snapshot_kind: text +source: crates/ruff_db/src/diagnostic/render/pylint.rs +expression: env.render_diagnostics(&diagnostics) --- fib.py:1: [F401] `os` imported but unused fib.py:6: [F841] Local variable `x` is assigned to but never used diff --git a/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__syntax_errors.snap b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__syntax_errors.snap new file mode 100644 index 0000000000..179e762066 --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/snapshots/ruff_db__diagnostic__render__pylint__tests__syntax_errors.snap @@ -0,0 +1,6 @@ +--- +source: crates/ruff_db/src/diagnostic/render/pylint.rs +expression: env.render_diagnostics(&diagnostics) +--- +syntax_errors.py:1: [invalid-syntax] SyntaxError: Expected one or more symbol names after import +syntax_errors.py:3: [invalid-syntax] SyntaxError: Expected ')', found newline diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 3f6ebd24b7..2f0cb9414c 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -15,7 +15,6 @@ pub use github::GithubEmitter; pub use gitlab::GitlabEmitter; pub use grouped::GroupedEmitter; pub use junit::JunitEmitter; -pub use pylint::PylintEmitter; use ruff_notebook::NotebookIndex; use ruff_source_file::{LineColumn, SourceFile}; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -30,7 +29,6 @@ mod github; mod gitlab; mod grouped; mod junit; -mod pylint; mod sarif; mod text; @@ -128,6 +126,10 @@ impl FileResolver for EmitterContext<'_> { UnifiedFile::Ruff(file) => self.notebook_indexes.get(file.name()).is_some(), } } + + fn current_directory(&self) -> &std::path::Path { + crate::fs::get_cwd() + } } struct MessageWithLocation<'a> { diff --git a/crates/ruff_linter/src/message/pylint.rs b/crates/ruff_linter/src/message/pylint.rs deleted file mode 100644 index 5e6cbe5a6a..0000000000 --- a/crates/ruff_linter/src/message/pylint.rs +++ /dev/null @@ -1,72 +0,0 @@ -use std::io::Write; - -use ruff_db::diagnostic::Diagnostic; -use ruff_source_file::OneIndexed; - -use crate::fs::relativize_path; -use crate::message::{Emitter, EmitterContext}; - -/// Generate violations in Pylint format. -/// See: [Flake8 documentation](https://flake8.pycqa.org/en/latest/internal/formatters.html#pylint-formatter) -#[derive(Default)] -pub struct PylintEmitter; - -impl Emitter for PylintEmitter { - 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 row = 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 - OneIndexed::from_zero_indexed(0) - } else { - diagnostic.expect_ruff_start_location().line - }; - - let body = if let Some(code) = diagnostic.secondary_code() { - format!("[{code}] {body}", body = diagnostic.body()) - } else { - diagnostic.body().to_string() - }; - - writeln!( - writer, - "{path}:{row}: {body}", - path = relativize_path(&filename), - )?; - } - - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use insta::assert_snapshot; - - use crate::message::PylintEmitter; - use crate::message::tests::{ - capture_emitter_output, create_diagnostics, create_syntax_error_diagnostics, - }; - - #[test] - fn output() { - let mut emitter = PylintEmitter; - let content = capture_emitter_output(&mut emitter, &create_diagnostics()); - - assert_snapshot!(content); - } - - #[test] - fn syntax_errors() { - let mut emitter = PylintEmitter; - let content = capture_emitter_output(&mut emitter, &create_syntax_error_diagnostics()); - - assert_snapshot!(content); - } -} diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__syntax_errors.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__syntax_errors.snap deleted file mode 100644 index 2bfd7e3d17..0000000000 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__syntax_errors.snap +++ /dev/null @@ -1,7 +0,0 @@ ---- -source: crates/ruff_linter/src/message/pylint.rs -expression: content -snapshot_kind: text ---- -syntax_errors.py:1: SyntaxError: Expected one or more symbol names after import -syntax_errors.py:3: SyntaxError: Expected ')', found newline diff --git a/crates/ruff_source_file/src/line_index.rs b/crates/ruff_source_file/src/line_index.rs index 8da47e0dba..34631891bd 100644 --- a/crates/ruff_source_file/src/line_index.rs +++ b/crates/ruff_source_file/src/line_index.rs @@ -624,6 +624,12 @@ impl OneIndexed { } } +impl Default for OneIndexed { + fn default() -> Self { + Self::MIN + } +} + impl fmt::Display for OneIndexed { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { std::fmt::Debug::fmt(&self.0.get(), f)