astral-sh / ruff

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

[Ruff v0.5] Stabilise 15 pylint rules #12051

Closed AlexWaygood closed 4 days ago

AlexWaygood commented 4 days ago

Summary

Stabilise the following rules:

These rules have all been in preview for over 3 months; there are no open issues about them, and there haven't been issues about them for months; and their recommendations seem pretty sane and uncontroversial. For some of them, the docs or messages to the user were slightly suboptimal; I've made a few touch-ups as part of this PR.

Many other pylint rules have somewhat opinionated or controversial changes; this PR deliberately leaves quite a few pylint rules still in preview.

Test Plan

cargo test, but the ecosystem check will be the real judge...

github-actions[bot] commented 4 days ago

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+66 -4 violations, +0 -0 fixes in 11 projects; 39 projects unchanged)

DisnakeDev/disnake (+2 -0 violations, +0 -0 fixes)

+ disnake/state.py:456:13: PLW0133 Missing `raise` statement on exception
+ disnake/state.py:476:13: PLW0133 Missing `raise` statement on exception

PlasmaPy/PlasmaPy (+12 -0 violations, +0 -0 fixes)

+ src/plasmapy/analysis/nullpoint.py:29:1: PLW0604 `global` at module level is redundant
+ src/plasmapy/dispersion/analytical/two_fluid_.py:316:9: PLC2401 Variable name `ω` contains a non-ASCII character
+ src/plasmapy/dispersion/numerical/kinetic_alfven_.py:234:9: PLC2401 Variable name `θ` contains a non-ASCII character
+ src/plasmapy/formulary/radiation.py:123:5: PLC2401 Variable name `ω` contains a non-ASCII character
+ src/plasmapy/formulary/radiation.py:124:5: PLC2401 Variable name `ω_pe` contains a non-ASCII character
+ src/plasmapy/formulary/relativity.py:187:5: PLC2401 Variable name `γ` contains a non-ASCII character
+ src/plasmapy/formulary/relativity.py:496:30: PLC2401 Argument name `γ` contains a non-ASCII character
... 4 additional changes omitted for rule PLC2401
+ tests/dispersion/test_dispersion_functions.py:10:25: PLC2403 Module alias `π` contains a non-ASCII character
+ tests/dispersion/test_dispersion_functions.py:11:36: PLC2403 Module alias `Γ` contains a non-ASCII character
... 3 additional changes omitted for project

apache/airflow (+30 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ airflow/cli/commands/task_command.py:702:38: PLR1704 Redefining argument with the local name `session`
+ airflow/decorators/base.py:157:13: PLR1704 Redefining argument with the local name `task_id`
+ dev/breeze/src/airflow_breeze/commands/ci_image_commands.py:444:17: PLR1704 Redefining argument with the local name `platform`
+ dev/breeze/src/airflow_breeze/commands/ci_image_commands.py:459:17: PLR1704 Redefining argument with the local name `python`
+ dev/breeze/src/airflow_breeze/commands/ci_image_commands.py:646:13: PLR1704 Redefining argument with the local name `python`
+ dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:689:51: PLR1704 Redefining argument with the local name `python`
... 13 additional changes omitted for rule PLR1704
+ dev/breeze/src/airflow_breeze/utils/parallel.py:297:57: PLR1736 [*] List index lookup in `enumerate()` loop
+ dev/breeze/src/airflow_breeze/utils/parallel.py:299:40: PLR1736 [*] List index lookup in `enumerate()` loop
+ docs/exts/docroles.py:21:1: PLR2044 [*] Line with empty comment
+ docs/exts/docroles.py:22:1: PLR2044 [*] Line with empty comment
+ scripts/ci/pre_commit/check_provider_airflow_compatibility.py:51:20: PLR1736 [*] List index lookup in `enumerate()` loop
+ tests/providers/common/sql/hooks/test_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/providers/common/sql/operators/test_sql.py:268:20: PLW0128 Redeclared variable `operator` in assignment
+ tests/providers/databricks/hooks/test_databricks_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/providers/exasol/hooks/test_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/providers/google/suite/transfers/test_gcs_to_gdrive.py:123:5: PLR2044 [*] Line with empty comment
... 3 additional changes omitted for rule PLR2044
... 14 additional changes omitted for project

aws/aws-sam-cli (+1 -0 violations, +0 -0 fixes)

+ samcli/local/docker/lambda_container.py:157:9: PLR2044 [*] Line with empty comment

bokeh/bokeh (+7 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ src/bokeh/document/callbacks.py:361:13: PLR1704 Redefining argument with the local name `callback_obj`
+ src/bokeh/plotting/_figure.py:387:13: PLR1704 Redefining argument with the local name `kw`
+ src/bokeh/plotting/_figure.py:428:13: PLR1704 Redefining argument with the local name `kw`
+ src/bokeh/plotting/_figure.py:478:17: PLR1704 Redefining argument with the local name `kw`
+ src/bokeh/plotting/_figure.py:567:13: PLR1704 Redefining argument with the local name `kw`
+ src/bokeh/plotting/_figure.py:609:13: PLR1704 Redefining argument with the local name `kw`
+ tests/support/util/examples.py:161:9: PLR1704 Redefining argument with the local name `path`

ibis-project/ibis (+7 -0 violations, +0 -0 fixes)

+ ibis/__init__.py:126:9: PLR1704 Redefining argument with the local name `name`
+ ibis/backends/dask/helpers.py:116:13: PLR1704 Redefining argument with the local name `name`
+ ibis/backends/sql/rewrites.py:258:9: PLR1704 Redefining argument with the local name `node`
+ ibis/expr/types/joins.py:372:13: PLR1704 Redefining argument with the local name `right`
+ ibis/expr/types/relations.py:1884:13: PLR1704 Redefining argument with the local name `table`
+ ibis/tests/expr/test_struct.py:75:32: PLR1704 Redefining argument with the local name `t`
+ ibis/tests/expr/test_struct.py:75:35: PLR1704 Redefining argument with the local name `s`

milvus-io/pymilvus (+1 -0 violations, +0 -0 fixes)

+ pymilvus/client/types.py:136:5: PLR2044 [*] Line with empty comment

pypa/pip (+1 -0 violations, +0 -0 fixes)

+ src/pip/_internal/utils/misc.py:152:5: PLE0704 Bare `raise` statement is not inside an exception handler

zulip/zulip (+4 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ zerver/lib/scim.py:232:13: PLR1704 Redefining argument with the local name `path`
+ zerver/webhooks/clubhouse/view.py:178:9: PLR1704 Redefining argument with the local name `action`
+ zerver/webhooks/clubhouse/view.py:467:13: PLR1704 Redefining argument with the local name `action`
+ zerver/webhooks/clubhouse/view.py:669:13: PLR1704 Redefining argument with the local name `action`

indico/indico (+0 -4 violations, +0 -0 fixes)

- indico/core/db/sqlalchemy/core.py:53:16: RUF100 [*] Unused `noqa` directive (non-enabled: `PLE0704`)
- indico/web/flask/errors.py:112:16: RUF100 [*] Unused `noqa` directive (non-enabled: `PLE0704`)
- indico/web/flask/errors.py:126:12: RUF100 [*] Unused `noqa` directive (non-enabled: `PLE0704`)
- indico/web/flask/errors.py:74:16: RUF100 [*] Unused `noqa` directive (non-enabled: `PLE0704`)

pytest-dev/pytest (+1 -0 violations, +0 -0 fixes)

+ src/_pytest/logging.py:402:13: PLE0704 Bare `raise` statement is not inside an exception handler

Changes by rule (10 rules affected)

| code | total | + violation | - violation | + fix | - fix | | ---- | ------- | --------- | -------- | ----- | ---- | | PLR1704 | 36 | 36 | 0 | 0 | 0 | | PLR2044 | 10 | 10 | 0 | 0 | 0 | | PLC2401 | 9 | 9 | 0 | 0 | 0 | | RUF100 | 4 | 0 | 4 | 0 | 0 | | PLR1736 | 3 | 3 | 0 | 0 | 0 | | PLW0133 | 2 | 2 | 0 | 0 | 0 | | PLC2403 | 2 | 2 | 0 | 0 | 0 | | PLE0704 | 2 | 2 | 0 | 0 | 0 | | PLW0604 | 1 | 1 | 0 | 0 | 0 | | PLW0128 | 1 | 1 | 0 | 0 | 0 |

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+852 -852 violations, +0 -0 fixes in 12 projects; 1 project error; 37 projects unchanged)

aiven/aiven-client (+1 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- aiven/client/argx.py:111:57: PLR6201 Use a `set` literal when testing for membership
+ aiven/client/argx.py:111:57: PLR6201 Use a set literal when testing for membership

PlasmaPy/PlasmaPy (+67 -67 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- src/plasmapy/dispersion/analytical/mhd_waves_.py:121:26: PLR6201 Use a `set` literal when testing for membership
+ src/plasmapy/dispersion/analytical/mhd_waves_.py:121:26: PLR6201 Use a set literal when testing for membership
- src/plasmapy/dispersion/analytical/mhd_waves_.py:131:30: PLR6201 Use a `set` literal when testing for membership
+ src/plasmapy/dispersion/analytical/mhd_waves_.py:131:30: PLR6201 Use a set literal when testing for membership
- src/plasmapy/dispersion/analytical/stix_.py:192:24: PLR6201 Use a `set` literal when testing for membership
+ src/plasmapy/dispersion/analytical/stix_.py:192:24: PLR6201 Use a set literal when testing for membership
... 107 additional changes omitted for rule PLR6201
+ src/plasmapy/dispersion/analytical/two_fluid_.py:316:9: PLC2401 Variable name `ω` contains a non-ASCII character
- src/plasmapy/dispersion/analytical/two_fluid_.py:316:9: PLC2401 Variable name `ω` contains a non-ASCII character, consider renaming it
+ src/plasmapy/dispersion/numerical/kinetic_alfven_.py:234:9: PLC2401 Variable name `θ` contains a non-ASCII character
- src/plasmapy/dispersion/numerical/kinetic_alfven_.py:234:9: PLC2401 Variable name `θ` contains a non-ASCII character, consider renaming it
... 124 additional changes omitted for project

apache/airflow (+327 -327 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- airflow/__init__.py:98:65: PLR6201 Use a `set` literal when testing for membership
+ airflow/__init__.py:98:65: PLR6201 Use a set literal when testing for membership
- airflow/__main__.py:49:31: PLR6201 Use a `set` literal when testing for membership
+ airflow/__main__.py:49:31: PLR6201 Use a set literal when testing for membership
- airflow/__main__.py:56:31: PLR6201 Use a `set` literal when testing for membership
+ airflow/__main__.py:56:31: PLR6201 Use a set literal when testing for membership
- airflow/api_connexion/endpoints/task_instance_endpoint.py:277:26: PLR6201 Use a `set` literal when testing for membership
+ airflow/api_connexion/endpoints/task_instance_endpoint.py:277:26: PLR6201 Use a set literal when testing for membership
- airflow/api_connexion/endpoints/task_instance_endpoint.py:729:20: PLR6201 Use a `set` literal when testing for membership
+ airflow/api_connexion/endpoints/task_instance_endpoint.py:729:20: PLR6201 Use a set literal when testing for membership
... 639 additional changes omitted for rule PLR6201
+ dev/breeze/src/airflow_breeze/utils/parallel.py:297:57: PLR1736 [*] List index lookup in `enumerate()` loop
- dev/breeze/src/airflow_breeze/utils/parallel.py:297:57: PLR1736 [*] Unnecessary lookup of list item by index
+ dev/breeze/src/airflow_breeze/utils/parallel.py:299:40: PLR1736 [*] List index lookup in `enumerate()` loop
- dev/breeze/src/airflow_breeze/utils/parallel.py:299:40: PLR1736 [*] Unnecessary lookup of list item by index
+ scripts/ci/pre_commit/check_provider_airflow_compatibility.py:51:20: PLR1736 [*] List index lookup in `enumerate()` loop
- scripts/ci/pre_commit/check_provider_airflow_compatibility.py:51:20: PLR1736 [*] Unnecessary lookup of list item by index
... 638 additional changes omitted for project

aws/aws-sam-cli (+42 -42 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- samcli/cli/main.py:89:27: PLR6201 Use a `set` literal when testing for membership
+ samcli/cli/main.py:89:27: PLR6201 Use a set literal when testing for membership
- samcli/cli/types.py:58:56: PLR6201 Use a `set` literal when testing for membership
+ samcli/cli/types.py:58:56: PLR6201 Use a set literal when testing for membership
- samcli/cli/types.py:58:84: PLR6201 Use a `set` literal when testing for membership
+ samcli/cli/types.py:58:84: PLR6201 Use a set literal when testing for membership
- samcli/commands/_utils/custom_options/hook_name_option.py:46:28: PLR6201 Use a `set` literal when testing for membership
+ samcli/commands/_utils/custom_options/hook_name_option.py:46:28: PLR6201 Use a set literal when testing for membership
- samcli/commands/_utils/template.py:162:34: PLR6201 Use a `set` literal when testing for membership
+ samcli/commands/_utils/template.py:162:34: PLR6201 Use a set literal when testing for membership
... 74 additional changes omitted for project

bokeh/bokeh (+28 -28 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- examples/output/apis/file_html.py:39:48: PLR6201 Use a `set` literal when testing for membership
+ examples/output/apis/file_html.py:39:48: PLR6201 Use a set literal when testing for membership
- examples/plotting/sprint.py:23:48: PLR6201 Use a `set` literal when testing for membership
+ examples/plotting/sprint.py:23:48: PLR6201 Use a set literal when testing for membership
- release/checks.py:69:30: PLR6201 Use a `set` literal when testing for membership
+ release/checks.py:69:30: PLR6201 Use a set literal when testing for membership
- src/bokeh/__init__.py:104:24: PLR6201 Use a `set` literal when testing for membership
+ src/bokeh/__init__.py:104:24: PLR6201 Use a set literal when testing for membership
- src/bokeh/command/subcommands/__init__.py:56:48: PLR6201 Use a `set` literal when testing for membership
+ src/bokeh/command/subcommands/__init__.py:56:48: PLR6201 Use a set literal when testing for membership
... 46 additional changes omitted for project

freedomofpress/securedrop (+40 -40 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- devops/scripts/verify-mo.py:140:35: PLR6201 Use a `set` literal when testing for membership
+ devops/scripts/verify-mo.py:140:35: PLR6201 Use a set literal when testing for membership
- molecule/ansible-config/tests/test_play_configuration.py:34:25: PLR6201 Use a `set` literal when testing for membership
+ molecule/ansible-config/tests/test_play_configuration.py:34:25: PLR6201 Use a set literal when testing for membership
- securedrop/manage.py:170:22: PLR6201 Use a `set` literal when testing for membership
+ securedrop/manage.py:170:22: PLR6201 Use a set literal when testing for membership
- securedrop/manage.py:172:24: PLR6201 Use a `set` literal when testing for membership
+ securedrop/manage.py:172:24: PLR6201 Use a set literal when testing for membership
- securedrop/pretty_bad_protocol/_parsers.py:1057:25: PLR6201 Use a `set` literal when testing for membership
+ securedrop/pretty_bad_protocol/_parsers.py:1057:25: PLR6201 Use a set literal when testing for membership
... 70 additional changes omitted for project

fronzbot/blinkpy (+3 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- blinkpy/auth.py:150:35: PLR6201 Use a `set` literal when testing for membership
+ blinkpy/auth.py:150:35: PLR6201 Use a set literal when testing for membership
- blinkpy/camera.py:144:41: PLR6201 Use a `set` literal when testing for membership
+ blinkpy/camera.py:144:41: PLR6201 Use a set literal when testing for membership
- blinkpy/camera.py:157:25: PLR6201 Use a `set` literal when testing for membership
+ blinkpy/camera.py:157:25: PLR6201 Use a set literal when testing for membership

ibis-project/ibis (+61 -61 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- ibis/backends/__init__.py:1378:18: PLR6201 Use a `set` literal when testing for membership
+ ibis/backends/__init__.py:1378:18: PLR6201 Use a set literal when testing for membership
- ibis/backends/bigquery/compiler.py:609:23: PLR6201 Use a `set` literal when testing for membership
+ ibis/backends/bigquery/compiler.py:609:23: PLR6201 Use a set literal when testing for membership
- ibis/backends/bigquery/compiler.py:635:23: PLR6201 Use a `set` literal when testing for membership
+ ibis/backends/bigquery/compiler.py:635:23: PLR6201 Use a set literal when testing for membership
- ibis/backends/bigquery/tests/system/test_client.py:308:22: PLR6201 Use a `set` literal when testing for membership
+ ibis/backends/bigquery/tests/system/test_client.py:308:22: PLR6201 Use a set literal when testing for membership
- ibis/backends/clickhouse/compiler.py:269:32: PLR6201 Use a `set` literal when testing for membership
+ ibis/backends/clickhouse/compiler.py:269:32: PLR6201 Use a set literal when testing for membership
... 112 additional changes omitted for project

milvus-io/pymilvus (+42 -42 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- pymilvus/bulk_writer/remote_bulk_writer.py:282:31: PLR6201 Use a `set` literal when testing for membership
+ pymilvus/bulk_writer/remote_bulk_writer.py:282:31: PLR6201 Use a set literal when testing for membership
- pymilvus/client/abstract.py:443:25: PLR6201 Use a `set` literal when testing for membership
+ pymilvus/client/abstract.py:443:25: PLR6201 Use a set literal when testing for membership
- pymilvus/client/abstract.py:553:39: PLR6201 Use a `set` literal when testing for membership
+ pymilvus/client/abstract.py:553:39: PLR6201 Use a set literal when testing for membership
- pymilvus/client/abstract.py:560:43: PLR6201 Use a `set` literal when testing for membership
+ pymilvus/client/abstract.py:560:43: PLR6201 Use a set literal when testing for membership
- pymilvus/client/abstract.py:562:45: PLR6201 Use a `set` literal when testing for membership
+ pymilvus/client/abstract.py:562:45: PLR6201 Use a set literal when testing for membership
... 74 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

demisto/content (error)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

``` warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`: - 'ignore' -> 'lint.ignore' - 'select' -> 'lint.select' - 'unfixable' -> 'lint.unfixable' - 'per-file-ignores' -> 'lint.per-file-ignores' warning: `PGH001` has been remapped to `S307`. warning: `PGH002` has been remapped to `G010`. warning: `PLR1701` has been remapped to `SIM101`. ruff failed Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled. ```

Changes by rule (4 rules affected)

| code | total | + violation | - violation | + fix | - fix | | ---- | ------- | --------- | -------- | ----- | ---- | | PLR6201 | 1676 | 838 | 838 | 0 | 0 | | PLC2401 | 18 | 9 | 9 | 0 | 0 | | PLR1736 | 6 | 3 | 3 | 0 | 0 | | PLC2403 | 4 | 2 | 2 | 0 | 0 |

airreality commented 4 days ago

Are you sure that PLR6201 is really useful rule? I believe initializing set for only one operation is too expensive and has no sense. I would prefer using tuple. And if the reason is to avoid duplicating items maybe it's better to have the rule which checks such items instead? I see there are a lot of violations in ecosystem. The same in my repoes. I guess most of users will add this rule in ignores.

AlexWaygood commented 4 days ago

Thanks @airreality. While I think it's a useful rule, and probably one that I would enable on my own projects, I agree that there's a lot of ecosystem hits for PLR6201. The performance pros and cons have already been discussed in some depth in https://github.com/astral-sh/ruff/issues/8758 and https://github.com/astral-sh/ruff/issues/9604; however, I missed that https://github.com/astral-sh/ruff/issues/8322 regarding this rule is still open. I think it'd be inappropriate to stabilise a rule while there's still open issues about its behaviour, so I'll axe that rule from the list.

I'm also slightly concerned about the number of hits for nan-comparison (PLW0177) on pandas, as that's a project I'd expect to follow best practices for this kind of thing. In the interests of getting the release out on schedule, I'll also axe that from the list of stabilisations for now, and I'll do an investigation later to see if those are false positives or true positives.