astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.57k stars 1.09k forks source link

How does ruff compare to pylint? #689

Closed olivier-lacroix closed 1 year ago

olivier-lacroix commented 1 year ago

Hello there,

I understand ruff’s primary point of comparison is flake8, but I am curious to know how it compares with pylint. This would likely help people considering migrating to ruff from pylint.

And maybe some inspiration could also be taken from pylint feature-wise? For instance, the use of “dev readable error codes” makes things a lot clearer when reading code.

LefterisJP commented 1 year ago

Pylint does a lot of AST analysis and has lots of modules and a modular system. It's pretty cool. I have written pylint modules in the past.

The problem is that it is excruciatingly slow.

If ruff could go towards that and stay as fast as it is ... oh boy I would be in love.

hmc-cs-mdrissi commented 1 year ago

Pylint has a very long list of rules. I currently use pylint but mainly stick to W/E/F rules as they catch most of interesting issues in my codebase. The more stylistic rules like formatting/variable naming/docstrings I value less.

I'd find most value in Error rules here. Almost all of them I enable, they've caught a number of bugs, and find false positive rate pretty low. One caveat is some rules I do rely on exception lists based on known problematic modules. As an example generated-members is useful way of dealing with modules/classes that have many dynamically defined attributes and static analysis would otherwise produce a lot of false positives.

charliermarsh commented 1 year ago

See: #970 for an up-to-date list of supported Pylint rules.

charliermarsh commented 1 year ago

Pylint supports 409 rules to Ruff's 224 rules. I'm gonna go through and see where we're lacking.

charliermarsh commented 1 year ago

For the sake of simplicity, I'm just gonna ignore the "Fatal" and "Information" categories.

That leaves 395 rules, of which Ruff implements 64 (this is a rough count, since the rules aren't always identical), with 331 unimplemented.

charliermarsh commented 1 year ago

We should add a bunch of these! I'd probably start with the E rules, though there's a lot of low-hanging fruit in the W category. I can do one or two to add least add Pylint as a category.

\cc @harupy if you're ever interested in some of these.

charliermarsh commented 1 year ago

Added to the README here: #969. For clarity, I'm going to close this out in favor of a new issue to track actual work towards improving parity: https://github.com/charliermarsh/ruff/issues/970.

harupy commented 1 year ago

@charliermarsh Awesome! I'll post a comment in #970 once I decide which rules I'd like to work on.

Pierre-Sassoulas commented 1 year ago

(I'm a pylint maintainer). One way to know if it's easy to implement a pylint rule is to check the confidence used when raising a message. If there's no inference (I.E. the check is static and could be done without astroid's inference capability) then the confidence is going to be HIGH in add_message. For example consider-using-any-or-all" does not use inference but some corner cases of use-implicit-booleaness-not-len do.

The confidence is not defined everywhere though, this is an ongoing effort.

nicois commented 1 year ago

Is there any intention of respecting pylint's annotations allowing errors to be disabled for a certain scope?

One impediment to switching over is the large number of pylint annotations which immediately throw errors. e.g. # pylint: disable=bare-except.

If ruff supported more pylint rules this would not be so important, as a single regex replacement over the codebase would allow migration. However in the interim we need to continue to use pylint in pre-commit hooks, but would like to enable ruff in developers' IDEs.

charliermarsh commented 1 year ago

Yeah this came up in #1203. I'll leave a comment there!

Lewiscowles1986 commented 1 year ago

Unsure if data or noise, but R1707 would have saved me a headache yesterday

A Change in Pydantic design might also have helped, but pylint catching these is a game-changer for large classes.

Screenshot: Visual Studio Code. Shows pylint integration highlighting trailing commas, in a way mypy, black, flake8 and ruff don't seem to

The other benefit of pylint for me here is the IDE integration with VSCode. I Still have not made time to check this in my PyCharm, but... yeah, I like pylint for at-least this reason.

Note: The above is an example minimal reproduction, not real code. It's running with the following editor plugins:

In addition to those plugins the official / recommended VSCode Python extension (which bundles other extensions) is also included.

charliermarsh commented 1 year ago

@Lewiscowles1986 - We do support that rule, but it's under a different code: trailing-comma-on-bare-tuple, known as COM818.

Lewiscowles1986 commented 1 year ago

@charliermarsh Thank you! Is it on-by-default, and what setup would need to be done to get it to show up via the linked VSCode extension?

Relevant pieces of my pyproject.toml (I'm using Poetry) ``` [tool.poetry.group.dev.dependencies] pydantic = "^2.3.0" mypy = "^1.5.1" ruff = "^0.0.290" black = "^23.9.1" pylint = "^2.17.5" [tool.ruff] select = ["E", "F", "W"] ignore = [] fixable = ["ALL"] unfixable = [] target-version = "py311" ```

Update: I got it working, although it was a lot more setup than I'd think was obvious.

Screenshot: VSCode IDE using Ruff integration, after adjusting pyproject.toml to 'select' ['ALL'] rules
Relevant updated pyproject.toml (I'm using Poetry) ``` [tool.ruff] select = ["ALL"] ignore = [] fixable = ["ALL"] unfixable = [] target-version = "py311" ```
charliermarsh commented 1 year ago

Yeah, you can add COM818 to your select to enable the rule.

ryangalamb commented 8 months ago

@charliermarsh If/when you get a chance, could you please update your comment here to include a link to #970 at the top?

This thread is my first search result for "ruff pylint parity", and I didn't see your comment about moving to #970 until I was a few hours into migrating. I imagine others might have the same issue (especially if they're doing a casual look to see if they can get rid of pylint yet.)

In the meantime, if a search brought you to this issue: #970 is the current issue with the actual checklist of which rules have been implemented. (There's been much more progress since this issue was closed. Big thanks to @charliermarsh and the rest of the ruff contributors for all the great work!)

mistercrunch commented 5 months ago

Hey, wondering if the checkbox list above is maintained/up-to-date (?) I'd love to have a full list of rules that I can disable in pylint cause it's very very slow. I'm working on a refactor that touches many python files (50+) and the pylint commit-hook is killing me, like it takes 5+ minutes to run :( . Now that I got a taste of that nice ruff speed I'm hooked.

I'm hoping if I can disable the redundant rules on the pylint side, the engine will run that much faster...

charliermarsh commented 5 months ago

@mistercrunch - we keep #970 up-to-date, in that we update it whenever we implement a new rule, and then whenever someone comments to point out that a rule is already covered or similar.