Implement Pylint #970

charliermarsh · 2022-11-30T04:52:53Z

This is the parent issue for tracking parity with Pylint. Below, we've enumerated all Pylint rules.

Rules that are checked off have been implemented in Ruff already (either as a Pylint rule, e.g., PLE0237, or by way of overlap with another linter like Pyflakes, as with missing-format-string-key).

Rules that are crossed out have been removed some consideration. (In most cases, crossed-out rules represent Pylint specific rules, e.g., rules related to Pylint configuration.)

At time of writing, many of the remaining rules require type inference and/or multi-file analysis, and aren't ready to be implemented in Ruff. (See: #970 (comment) for an enumeration.) If you're looking to start work on a specific rule, I'd suggest commenting in the issue, to get some input on whether Ruff is capable of supporting it at present.

For guidance on getting started, see the Contributing documentation.

Note: Don't implement rules that are part of Pylint extension until #1774 is completed. Don't implement rules that mainly target Python 2.

Error

Warning

Convention

Refactor

The text was updated successfully, but these errors were encountered:

charliermarsh · 2022-11-30T05:26:33Z

Need to figure out what the right "check code" approach is here. We could just start to put these under RUF. Or we could do PYE for "Pylint Error", and so on. Or we could use PYL for all of them. In either case, though, we have to allow four-digit codes, as opposed to our current three-digit codes.

JonathanPlasse · 2022-11-30T06:28:59Z

I vote for PYE as it would keep the same categories and the migration would be easier.

harupy · 2022-12-01T03:49:24Z

I'll work on unneeded-not this weekend.

relsunkaev · 2022-12-01T07:27:04Z

I would suggest PL as the pylint prefix (PLE -> PyLint Error). I think PYE might be a bit ambiguous with PYflakes, PYdocstyle, etc.

MartinBernstorff · 2022-12-01T09:33:12Z

Would absolutely love to see this happen. I know flake8 intentionally decided not to implement e.g. unexpected-keyword-arg because it required looking outside of the current file for imported functions.

I expect you've already considered this, and really hope this won't stop ruff from implementing it, but just wanted to raise it.

charliermarsh · 2022-12-01T15:22:58Z

@relsunkaev 👍 Went with PLE! The first code is checked in as PLE1142 (matching E1142 from Pylint).

charliermarsh · 2022-12-01T15:23:31Z

@MartinBernstorff - Definitely. The multi-file stuff will come, just not as quickly.

hmc-cs-mdrissi · 2022-12-08T02:41:23Z

Not sure if it should be separate issue, but looking at missing-function-docstring rule (C0116), pylint's version of it is different. It only requires that a function has docstring some in class hierarchy and does not require over-ridden methods to have a docstring. As an example,

class Foo:
  def validate(self):
    """Doc."""
    ...

class Bar(Foo):
  def validate(self):
    ...

gives no errors in pylint even though second validate is missing docstring being pylint allows inheriting one. This is very useful when implementing interface and the docstring would really be same for all things that subclass.

charliermarsh · 2023-01-05T18:53:47Z

I'd like to get pointless-statement, unnecessary-ellipsis, and unnecessary-pass in.

charliermarsh · 2023-01-05T19:20:12Z

Relevant source for defining a pointless-statement: https://github.com/PyCQA/pylint/blob/56121344f6c9c223d8e4477c0cdb58c46c840b4b/pylint/checkers/base/basic_checker.py#L447

thejcannon · 2023-01-12T16:08:49Z

A while ago I set out to find all the pylint codes make redundant by type checking.

Unfortunately that code got lost to the sands of time.

You might consider skipping those as type checking tools are very sophisticated, if you can do the effort of compiling that list

thejcannon · 2023-01-12T16:14:48Z

I've also thought very long and hard about the fact that pylint appears very sophisticated when it comes to types because it builds on astroid which works very hard to infer types based on code (pre-type-annotations)

Ideally ruff (or any modern linter) is type-aware to be as helpful as possible, but that requires either:

A Rust-implemented type checkers for Python. Short of a large company writing and maintaining this I doubt it'll keep up with the other big dogs
Having the type information be seeded to ruff. Aside from the overhead and protocol, this actually seems feasible.

dae · 2023-01-12T22:32:06Z

For what it's worth, the type awareness of pylint is the killer feature that keeps us using it in our project despite how painfully slow it is. Our first-party code is almost fully typed, and mypy is generally good at catching typing issues. We've found multiple instances where pylint caught an issue that mypy missed however - usually in third-party code with missing or incomplete type definitions, but occasionally also in our own code when mypy doesn't notice (eg due to the presence of __getattr__: python/mypy#6251).

thejcannon · 2023-01-12T22:41:02Z

Out of curiosity, have you tried pyright? It claims to have inference support for codebases that aren't fully typed.

dae · 2023-01-12T22:49:36Z

Mypy has a check_untyped_defs which sounds equivalent, and we use that. We currently rely on mypy's no_strict_optional which pyright has no equivalent to, so pyright is not currently usable on our codebase.

This PR adds [Pylint `R2004`](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/magic-value-comparison.html#magic-value-comparison-r2004) Feel free to suggest changes and additions, I have tried to maintain parity with the Pylint implementation [`magic_value.py`](https://github.com/PyCQA/pylint/blob/main/pylint/extensions/magic_value.py) See #970

We had already introduced ruff as a replacement for pydocstyle. This this commit, we also replace flake8 and black with it. It was decided not to remove bandit and pylint because they still have rules that are unsupported in ruff. Furthermore, all bandit and pylint rules will remain active because the performance improvement was tiny. The following script can theoretically extract all pylint messages that are unsupported in ruff: ```python from bs4 import BeautifulSoup import requests response = requests.get("astral-sh/ruff#970 (comment)") soup = BeautifulSoup(response.text, "html.parser") for li in soup.find_all("li", class_="task-list-item"): if "checked" in li.find("input").attrs: continue print(li.find_all("code")[0].get_text(), end="") print(",") ```

DetachHead · 2024-04-12T06:23:18Z

consider-using-augmented-assign / R6104 can be marked as done (#9932)

xrmx · 2024-04-12T13:16:08Z

FYI pylint 3.1.0 added more checks, there's at least: R1737: Use 'yield from' directly instead of yielding each element one by one (use-yield-from)

…sh#10781) ## Summary Implement `bad-staticmethod-argument` from pylint, part of astral-sh#970. ## Test Plan Text fixture added.

…-sh#10002) Add rule [consider-using-min-builtin (R1730)](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/consider-using-min-builtin.html) and [consider-using-max-builtin (R1731)](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/consider-using-max-builtin.html) See astral-sh#970 for rules Test Plan: `cargo test`

diceroll123 · 2024-04-13T05:44:21Z

@xrmx Pylint's R1737 is covered by PyUpgrade's UP028

## Summary This PR implements [`W0642`/`self-cls-assignment`](https://pylint.readthedocs.io/en/stable/user_guide/messages/warning/self-cls-assignment.html) See: #970 ## Test Plan Add test cases and verified the updated snapshots. --------- Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>

tibor-reiss · 2024-04-15T20:52:38Z

I had a go at the invalid dunder rules (PLE0303, PLE0305, PLE0308, PLE0309). Any feedback is appreciated on the PRs.

Note: I could not figure out why the cache::tests::same_results::ruff_linter_fixtures test (and others) break. All is fine locally until commit 670d66f, but then with the next commit, c221035 (which added a new rule), these tests start to fail. Any ideas please?
Edit: solved, thanks! The issue was that we crossed 832 rules and we are adding more :)

Ps.: E0304 and E0307 are both already implemented.

Add pylint rule invalid-bytes-returned (PLE0308) See #970 for rules Test Plan: `cargo test`

Add pylint rule invalid-length-returned (PLE0303) See #970 for rules Test Plan: `cargo test` TBD: from the description: "Strictly speaking `bool` is a subclass of `int`, thus returning `True`/`False` is valid. To be consistent with other rules (e.g. [PLE0305](#10962) invalid-index-returned), ruff will raise, compared to pylint which will not raise."

## What changes were proposed in this pull request? - Current pylint setup in Hue was linting staged changes only by checking for 3 rules. - Also pylint is comparatively slow and GPL licensed. - Switching to ruff has its own perks which are mentioned on: https://docs.astral.sh/ruff/ and it's MIT licensed. - For starters, it's waaayyy faster, can auto-fix errors and work as both linter and formatter replacing a lot of latest packages (like a all-in-one solution). - It is also supporting pylint rules which is WIP: astral-sh/ruff#970 - Currently this PR only targets the minimal transition from pylint to ruff. Iteratively, we will extend the rules to improve the Hue codebase, add its pre-commit and play around auto-fixing errors capability. This will also help in making our codebase more PEP8 compliant. #### Note - Ruff still does not support pylint's `W0311(bad-indentation)` rule which was present earlier with pylint in Hue. We will update it later when the support lands in a future release. ## How was this patch tested? - Manually in local setup.

Add pylint rule invalid-hash-returned (PLE0309) See #970 for rules Test Plan: `cargo test` TBD: from the description: "Strictly speaking `bool` is a subclass of `int`, thus returning `True`/`False` is valid. To be consistent with other rules (e.g. [PLE0305](#10962) invalid-index-returned), ruff will raise, compared to pylint which will not raise."

Add pylint rule invalid-index-returned (PLE0305) See #970 for rules Test Plan: `cargo test`

TomerBin · 2024-04-23T17:51:38Z

R1732 is covered by SIM115

com6056 · 2024-05-02T00:13:58Z

It would be awesome if ruff could support setting --allow-global-unused-variables=no 🎉 https://pylint.readthedocs.io/en/stable/user_guide/configuration/all-options.html#allow-global-unused-variables

Mellowdv · 2024-05-09T13:59:39Z

It would be awesome if ruff could support setting --allow-global-unused-variables=no 🎉 https://pylint.readthedocs.io/en/stable/user_guide/configuration/all-options.html#allow-global-unused-variables

I would like to work on this, if nobody is on it. We would like to use ruff at my workplace but we also require this rule.

charliermarsh · 2024-05-09T14:29:26Z

What rule does that setting impact?

Mellowdv · 2024-05-09T14:41:08Z

This is essentially Unused Variable (F841) except at Module scope instead of Function scope the way we understand it. My idea was to basically handle it in deferred_scopes when doing other ScopeKind::Module rules.

Edit: I misunderstood ScopeKind::Module. I understand now that unused variables local to functions will be caught in ScopeKind::Module as well. Then no new rule is probably necessary. F841 could probably be modified to run at Module scope instead of Function scope when the allow-global-unsued-variables=no flag is set.

DetachHead · 2024-05-15T13:38:02Z

new rule in pylint 3.2.0: contextmanager-generator-missing-cleanup

charliermarsh added the rule Implementing or modifying a lint rule label Nov 30, 2022

charliermarsh mentioned this issue Nov 30, 2022

How does ruff compare to pylint? #689

Closed

charliermarsh mentioned this issue Nov 30, 2022

Implement await-outside-async / E1142 #972

Merged

charliermarsh added plugin Implementing a known but unsupported plugin and removed rule Implementing or modifying a lint rule labels Dec 31, 2022

sbdchd mentioned this issue Jan 2, 2023

Implement flake8-pie #1543

Closed

7 tasks

max0x53 mentioned this issue Jan 12, 2023

Implement PLR2004 (MagicValueComparison) #1828

Merged

Glyphack pushed a commit to Glyphack/ruff that referenced this issue Apr 12, 2024

[pylint] Implement bad-staticmethod-argument (PLW0211) (astral-…

f7bfd17

…sh#10781) ## Summary Implement `bad-staticmethod-argument` from pylint, part of astral-sh#970. ## Test Plan Text fixture added.

diceroll123 mentioned this issue Apr 15, 2024

[pylint] - add class-variable-slots-conflict (PLR0242) #10944

Open

Harshg999 mentioned this issue Apr 17, 2024

[lint] Remove pylint and switch to ruff for Hue backend linting cloudera/hue#3703

Merged

charliermarsh pushed a commit that referenced this issue Apr 18, 2024

[pylint] Implement invalid-bytes-returned (E0308) (#10959)

1480d72

Add pylint rule invalid-bytes-returned (PLE0308) See #970 for rules Test Plan: `cargo test`

charliermarsh pushed a commit that referenced this issue Apr 19, 2024

[pylint] Implement invalid-index-returned (PLE0305) (#10962)

27902b7

Add pylint rule invalid-index-returned (PLE0305) See #970 for rules Test Plan: `cargo test`

BlindChickens mentioned this issue Apr 22, 2024

nan-comparison code mismatch with Pylint #10791

Closed

MichaReiser mentioned this issue Apr 29, 2024

When can ruff suppor checking the error of using not exsiting attribution? #11198

Closed

deen13 mentioned this issue May 8, 2024

Configure ruff import sorting and drop isort support digitalfabrik/integreat-cms#2758

Open

TheRealFalcon mentioned this issue May 9, 2024

refactor: migrate to ruff canonical/pycloudlib#374

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Pylint #970

Implement Pylint #970

charliermarsh commented Nov 30, 2022 •

edited

charliermarsh commented Nov 30, 2022 •

edited

JonathanPlasse commented Nov 30, 2022

harupy commented Dec 1, 2022

relsunkaev commented Dec 1, 2022 •

edited

MartinBernstorff commented Dec 1, 2022

charliermarsh commented Dec 1, 2022

charliermarsh commented Dec 1, 2022

hmc-cs-mdrissi commented Dec 8, 2022

charliermarsh commented Jan 5, 2023

charliermarsh commented Jan 5, 2023

thejcannon commented Jan 12, 2023

thejcannon commented Jan 12, 2023

dae commented Jan 12, 2023 •

edited

thejcannon commented Jan 12, 2023

dae commented Jan 12, 2023

DetachHead commented Apr 12, 2024

xrmx commented Apr 12, 2024

diceroll123 commented Apr 13, 2024

tibor-reiss commented Apr 15, 2024 •

edited

TomerBin commented Apr 23, 2024

com6056 commented May 2, 2024

Mellowdv commented May 9, 2024

charliermarsh commented May 9, 2024

Mellowdv commented May 9, 2024 •

edited

DetachHead commented May 15, 2024

Implement Pylint #970

Implement Pylint #970

Comments

charliermarsh commented Nov 30, 2022 • edited

Error

Warning

Convention

Refactor

charliermarsh commented Nov 30, 2022 • edited

JonathanPlasse commented Nov 30, 2022

harupy commented Dec 1, 2022

relsunkaev commented Dec 1, 2022 • edited

MartinBernstorff commented Dec 1, 2022

charliermarsh commented Dec 1, 2022

charliermarsh commented Dec 1, 2022

hmc-cs-mdrissi commented Dec 8, 2022

charliermarsh commented Jan 5, 2023

charliermarsh commented Jan 5, 2023

thejcannon commented Jan 12, 2023

thejcannon commented Jan 12, 2023

dae commented Jan 12, 2023 • edited

thejcannon commented Jan 12, 2023

dae commented Jan 12, 2023

DetachHead commented Apr 12, 2024

xrmx commented Apr 12, 2024

diceroll123 commented Apr 13, 2024

tibor-reiss commented Apr 15, 2024 • edited

TomerBin commented Apr 23, 2024

com6056 commented May 2, 2024

Mellowdv commented May 9, 2024

charliermarsh commented May 9, 2024

Mellowdv commented May 9, 2024 • edited

DetachHead commented May 15, 2024

charliermarsh commented Nov 30, 2022 •

edited

charliermarsh commented Nov 30, 2022 •

edited

relsunkaev commented Dec 1, 2022 •

edited

dae commented Jan 12, 2023 •

edited

tibor-reiss commented Apr 15, 2024 •

edited

Mellowdv commented May 9, 2024 •

edited