Move Pylint rendering to `ruff_db` #19340

ntBre · 2025-07-14T21:24:51Z

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,
PR, or in the pylint
docs,
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

crates/ruff_db/src/diagnostic/render/pylint.rs

github-actions · 2025-07-14T21:28:18Z

`mypy_primer` results

No ecosystem changes detected ✅
No memory usage changes detected ✅

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](#1953), [PR](#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

github-actions · 2025-07-14T21:41:12Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

MichaReiser · 2025-07-15T08:02:42Z

crates/ruff_db/Cargo.toml

+path-absolutize = { workspace = true, features = [
+    "once_cell_cache",
+    "use_unix_paths_on_wasm",
+] }


The ruff_db crate shouldn't depend on any OS specific behavior except for code gated by the os feature. It otherwise makes compiling on other tagets (like you noticed with WASM) a fair bit more complicated.

For ty, making the path relative is implemented in FileResolver. Now, whether this is correct I'm not sure because it does mean that the paths are relative for all rendered output formats (including JSON where we might want to use absolute paths, although I'm not sure). But it is the right place to abstract away the integration logic between ty and Ruff.

ruff/crates/ruff_db/src/diagnostic/render.rs

Lines 714 to 716 in 988479d

fn path(&self, file: File) -> &str {

relativize_path(self.system().current_directory(), file.path(self).as_str())

}

One solution is to change UnifiedFile::path to also return a relative path for Ruff files. A better solution is probably to add a current_directory method to FileResolver which returns system.current_directory for ty and whatever path-absolutize uses for ruff. You can then add a relative_path method to UnifiedFile which makes ::path relative to file_resolver.current_directory

I was playing with both the ty and ruff CLIs yesterday, and I think they have similar behavior here. As I mentioned in the description, the only place I saw a difference was in the integration tests where the [TMP]/ prefix appeared in the output even though it's supposed to be the current_dir. So I was thinking that this relativize call might be unnecessary in general.

But I'll add a method to the FileResolver just in case, that seems like a nice approach.

crates/ruff_db/src/diagnostic/render/pylint.rs

MichaReiser · 2025-07-15T08:19:41Z

crates/ruff_db/src/diagnostic/render/pylint.rs

+            let filename = filename.unwrap_or_default();
+            let row = row.unwrap_or_default();
+
+            writeln!(f, "{path}:{row}: {body}", path = relativize_path(filename))?;


Interesting, it seems that this matches pylint's VS code reporter and not the text reporter:

https://github.com/pylint-dev/pylint/blob/a14b2c4e68d3585c74fb7a3bbd5636e807a3ce77/pylint/reporters/text.py#L204

I think it might be ParseableTextReporter actually: https://github.com/pylint-dev/pylint/blob/a14b2c4e68d3585c74fb7a3bbd5636e807a3ce77/pylint/reporters/text.py#L189

The VS Code one doesn't have a colon between path and row. This is the link in the renderer docstring that I copied over from ruff_linter: https://flake8.pycqa.org/en/latest/internal/formatters.html#pylint-formatter. I think it's supposed to be the Pylint output format from the flake8 tool.

Working on these definitely makes me curious if/how people are using the various output formats!

Co-authored-by: Micha Reiser <micha@reiser.io>

MichaReiser · 2025-07-15T12:59:55Z

I didn't see any details about why this needs to be relativized in the original
For the why: This is so that ruff only prints main.py if it is in the root of your project and not the full path

ntBre · 2025-07-15T13:16:33Z

Oh yeah, I understand why we relativize the path in general, I just meant that I couldn't find any cases in the CLI where this relativize call made a difference. However, I found one immediately today, so I guess I wasn't testing the right thing yesterday.

MichaReiser

Thank you

* main: [ty] Combine CallArguments and CallArgumentTypes (#19337) Move Pylint rendering to `ruff_db` (#19340)

…finition * 'main' of https://github.com/astral-sh/ruff: (39 commits) [ty] Sync vendored typeshed stubs (astral-sh#19368) Fix typeshed-sync workflow (astral-sh#19367) Rework typeshed-sync workflow to also add docstrings for Windows- and MacOS-specific APIs (astral-sh#19360) [ty] Allow `-qq` for silent output mode (astral-sh#19366) [ty] Allow `-q` short alias for `--quiet` (astral-sh#19364) Add shellcheck to pre-commit (astral-sh#19361) distinguish references from definitions in `infer_nonlocal` [`pycodestyle`] Handle brace escapes for t-strings in logical lines (astral-sh#19358) [ty] Combine CallArguments and CallArgumentTypes (astral-sh#19337) Move Pylint rendering to `ruff_db` (astral-sh#19340) [`pylint`] Extend invalid string character rules to include t-strings (astral-sh#19355) Make TC010 docs example more realistic (astral-sh#19356) Move RDJSON rendering to `ruff_db` (astral-sh#19293) [`flake8-use-pathlib`] Skip single dots for `invalid-pathlib-with-suffix` (`PTH210`) on versions >= 3.14 (astral-sh#19331) [`ruff`] Allow `strict` kwarg when checking for `starmap-zip` (`RUF058`) in Python 3.14+ (astral-sh#19333) [ty] Reduce false positives for `TypedDict` types (astral-sh#19354) [ty] Remove `ConnectionInitializer` (astral-sh#19353) [ty] Use `Type::string_literal()` more (astral-sh#19352) [ty] Add ecosystem-report workflow (astral-sh#19349) [ty] Make use of salsa `Lookup` when interning values (astral-sh#19347) ... # Conflicts: # crates/ty_ide/src/goto.rs # crates/ty_server/src/server.rs

ntBre added internal An internal refactor or improvement diagnostics Related to reporting of diagnostics. labels Jul 14, 2025

ntBre commented Jul 14, 2025

View reviewed changes

crates/ruff_db/src/diagnostic/render/pylint.rs Outdated Show resolved Hide resolved

ntBre force-pushed the brent/pylint branch from 634b118 to 53a353a Compare July 14, 2025 21:29

ntBre marked this pull request as ready for review July 14, 2025 21:43

ntBre requested review from AlexWaygood, MichaReiser, carljm, dcreager and sharkdp as code owners July 14, 2025 21:43

ntBre removed request for AlexWaygood, carljm, dcreager and sharkdp July 14, 2025 21:43

MichaReiser reviewed Jul 15, 2025

View reviewed changes

ntBre mentioned this pull request Jul 15, 2025

Move RDJSON rendering to ruff_db #19293

Merged

ntBre and others added 3 commits July 15, 2025 08:40

use the diagnostic name if the secondary code is unavailable

fb0c10d

remove let-chain todo

444499a

map instead of mutable references

b8a3dac

Co-authored-by: Micha Reiser <micha@reiser.io>

ntBre added 3 commits July 15, 2025 09:10

add FileResolver::current_directory

1eb1d52

add UnifiedFile::relative_path

d86b323

revert relative_path copy-paste

ac6858a

Merge branch 'main' into brent/pylint

f200d5c

ntBre requested a review from MichaReiser July 15, 2025 14:00

MichaReiser approved these changes Jul 15, 2025

View reviewed changes

ntBre merged commit e9cac36 into main Jul 15, 2025
57 of 58 checks passed

ntBre deleted the brent/pylint branch July 15, 2025 14:14

dcreager added a commit that referenced this pull request Jul 15, 2025

Merge branch 'main' into dcreager/splat

67a5f66

* main: [ty] Combine CallArguments and CallArgumentTypes (#19337) Move Pylint rendering to `ruff_db` (#19340)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move Pylint rendering to `ruff_db` #19340

Move Pylint rendering to `ruff_db` #19340

Uh oh!

ntBre commented Jul 14, 2025

Uh oh!

Uh oh!

github-actions bot commented Jul 14, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jul 14, 2025 •

edited

Loading

Uh oh!

MichaReiser Jul 15, 2025

Uh oh!

ntBre Jul 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser Jul 15, 2025

Uh oh!

ntBre Jul 15, 2025

Uh oh!

MichaReiser commented Jul 15, 2025

Uh oh!

ntBre commented Jul 15, 2025

Uh oh!

MichaReiser left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	fn path(&self, file: File) -> &str {
	relativize_path(self.system().current_directory(), file.path(self).as_str())
	}

Move Pylint rendering to ruff_db #19340

Move Pylint rendering to ruff_db #19340

Uh oh!

Conversation

ntBre commented Jul 14, 2025

Summary

Test Plan

Uh oh!

Uh oh!

github-actions bot commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

github-actions bot commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

MichaReiser Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented Jul 15, 2025

Uh oh!

ntBre commented Jul 15, 2025

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Move Pylint rendering to `ruff_db` #19340

Move Pylint rendering to `ruff_db` #19340

github-actions bot commented Jul 14, 2025 •

edited

Loading

`mypy_primer` results

github-actions bot commented Jul 14, 2025 •

edited

Loading

`ruff-ecosystem` results