Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect pylint ignore directives #1203

Open
LefterisJP opened this issue Dec 11, 2022 · 9 comments
Open

Respect pylint ignore directives #1203

LefterisJP opened this issue Dec 11, 2022 · 9 comments
Labels
core Related to core functionality suppression Related to supression of violations e.g. noqa

Comments

@LefterisJP
Copy link

It would be really cool for a codebase moving from pylint slowly to ruff to be able to understand and respect pylint directives.

For example ARG module has flake8-unused-arguments. That is very similar to something pylint does. And as usual if somewhere you want to disable the warning you can add the proper pylint directive. For this it would be # pylint: disable=unused-argument.

It would be cool if ruff understood those directives even for other modules like flake8-unused-arguments which do the same thing.

@charliermarsh charliermarsh added enhancement configuration Related to settings and configuration labels Dec 12, 2022
@charliermarsh charliermarsh added core Related to core functionality suppression Related to supression of violations e.g. noqa and removed enhancement configuration Related to settings and configuration labels Dec 31, 2022
@joshuachapnick-bc
Copy link

+1 for this feature

@charliermarsh
Copy link
Member

I think there are two pieces here:

  1. Supporting Pylint directives for rules that are mapped explicitly to existing Pylint rules (anything in PLR, PLC, etc.).
  2. Supporting Pylint directives for rules that aren't currently mapped, but have analogs.

I'm open to supporting (1), but it does require a decent amount of work, since Pylint provides a lot of flexibility in where enables and disables are placed (scopes, blocks, single lines, etc.).

(Unrelated to this issue, but I do eventually want to add Ruff's own system for ignores that's decoupled from Flake8 (while continuing to support Flake8's noqa), and that system would definitely support multi-line ignores.)

I'm hesitant to support (2). It feels hard to get right, since it requires that (e.g.) we're reporting errors on the exact same lines.

@LefterisJP
Copy link
Author

I guess (1) is already good enough.

For (2) there is some easy wins. For example at rotki we disable the ruff "BLE" check due to also running pylint with that check on and ruff not respecting/seeing the ignores.

@sbrugman
Copy link
Contributor

In the case of replacing pylint entirely with ruff, there could simply be a rule RUFF9XX that detects pylint directives, and as autofix removes them. This could be simple to implement and helpful to start. When explicit mapping is available, the fix could be changed to use flake8 or ruff directives.

@spaceone
Copy link
Contributor

A fixer which transforms the pylint-comments into ruff noqa-comments would be helpful.

-# pylint: disable-msg=R0133
+# noqa: PLR0133

@coby-soffer
Copy link

+1

@StefanKX

This comment has been minimized.

@kaddkaka
Copy link

kaddkaka commented Feb 27, 2024

A fixer which transforms the pylint-comments into ruff noqa-comments would be helpful.

-# pylint: disable-msg=R0133
+# noqa: PLR0133

Pylint disable support rule names. Without rule names, this lowers the value of the waiver comment.

-# pylint: disable=comparison-of-constants
+# noqa: PLR0133

After this transformation the next person arriving at the code will not easily understand why the disable is there.

After ruff adopts #1773 "human-friendly rules names", this transformation feels more attractive.

@kaddkaka
Copy link

I think there are two pieces here:

1. Supporting Pylint directives for rules that are mapped explicitly to existing Pylint rules (anything in `PLR`, `PLC`, etc.).

I'm open to supporting (1), but it does require a decent amount of work, since Pylint provides a lot of flexibility in where enables and disables are placed (scopes, blocks, single lines, etc.).

A possible way forward could be to only some kind of directive to start. Or is there a risk that would cause more confusion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core functionality suppression Related to supression of violations e.g. noqa
Projects
None yet
Development

No branches or pull requests

8 participants