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 <micha@reiser.io>

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()

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 -----

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.

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<T> 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.

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",
+        );
+    }
+}

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
+source: crates/ruff_db/src/diagnostic/render/pylint.rs
-expression: content
+expression: env.render_diagnostics(&diagnostics)
-snapshot_kind: text
 ---
 fib.py:1: [F401] `os` imported but unused
 fib.py:6: [F841] Local variable `x` is assigned to but never used

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

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> {

crates/ruff_linter/src/message/pylint.rs

@@ -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);
-    }
-}

crates/ruff_linter/src/message/snapshots/ruff_linter__message__pylint__tests__syntax_errors.snap

@@ -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

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)