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

Add rule PLR5601 #10103

Closed
wants to merge 2 commits into from
Closed

Add rule PLR5601 #10103

wants to merge 2 commits into from

Conversation

tibor-reiss
Copy link
Contributor

@tibor-reiss tibor-reiss commented Feb 23, 2024

Add rule confusing-consecutive-elif (PLR5601)

See #970 for rules

Test plan: cargo test

Copy link
Contributor

github-actions bot commented Feb 23, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+161 -802 violations, +0 -0 fixes in 13 projects; 30 projects unchanged)

apache/airflow (+28 -585 violations, +0 -0 fixes)

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

+ airflow/cli/commands/db_command.py:121:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/cli/commands/db_command.py:164:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/cli/commands/task_command.py:176:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- airflow/cli/commands/webserver_command.py:48:7: PLR0902 Too many instance attributes (11/7)
+ airflow/configuration.py:1536:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- airflow/dag_processing/manager.py:330:7: PLR0902 Too many instance attributes (27/7)
- airflow/dag_processing/manager.py:99:7: PLR0902 Too many instance attributes (12/7)
- airflow/dag_processing/processor.py:69:7: PLR0902 Too many instance attributes (11/7)
+ airflow/decorators/setup_teardown.py:72:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- airflow/jobs/backfill_job_runner.py:65:7: PLR0902 Too many instance attributes (17/7)
- airflow/jobs/job.py:58:7: PLR0902 Too many instance attributes (12/7)
+ airflow/jobs/local_task_job_runner.py:284:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- airflow/jobs/local_task_job_runner.py:77:7: PLR0902 Too many instance attributes (13/7)
- airflow/jobs/scheduler_job_runner.py:129:7: PLR0902 Too many instance attributes (13/7)
- airflow/models/baseoperator.py:477:7: PLR0902 Too many instance attributes (53/7)
- airflow/models/connection.py:97:7: PLR0902 Too many instance attributes (8/7)
- airflow/models/dag.py:302:7: PLR0902 Too many instance attributes (42/7)
+ airflow/models/dag.py:3476:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- airflow/models/dagbag.py:79:7: PLR0902 Too many instance attributes (12/7)
- airflow/models/dagrun.py:110:7: PLR0902 Too many instance attributes (13/7)
- airflow/models/taskinstance.py:1211:7: PLR0902 Too many instance attributes (20/7)
- airflow/models/taskinstance.py:3518:7: PLR0902 Too many instance attributes (14/7)
- airflow/models/taskreschedule.py:44:7: PLR0902 Too many instance attributes (9/7)
... 570 additional changes omitted for rule PLR0902
+ airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py:480:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/amazon/aws/operators/eks.py:115:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/amazon/aws/sensors/s3.py:185:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/amazon/aws/transfers/gcs_to_s3.py:158:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/apache/spark/hooks/spark_submit.py:565:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/fab/auth_manager/security_manager/override.py:2241:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/google/cloud/operators/dataproc.py:500:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ airflow/providers/openlineage/utils/utils.py:91:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
... 582 additional changes omitted for project

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

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

+ samcli/commands/delete/delete_context.py:137:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- samcli/commands/delete/delete_context.py:30:7: PLR0902 Too many instance attributes (14/7)
- samcli/commands/deploy/deploy_context.py:43:7: PLR0902 Too many instance attributes (28/7)
- samcli/commands/deploy/guided_context.py:42:7: PLR0902 Too many instance attributes (32/7)
- samcli/commands/list/endpoints/endpoints_context.py:16:7: PLR0902 Too many instance attributes (10/7)
- samcli/commands/local/cli_common/invoke_context.py:62:7: PLR0902 Too many instance attributes (35/7)
- samcli/commands/local/lib/local_api_service.py:15:7: PLR0902 Too many instance attributes (9/7)
... 79 additional changes omitted for rule PLR0902
+ samcli/commands/pipeline/bootstrap/guided_context.py:231:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ samcli/lib/list/endpoints/endpoints_producer.py:440:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ samcli/lib/sync/infra_sync_executor.py:485:17: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
... 80 additional changes omitted for project

bokeh/bokeh (+10 -14 violations, +0 -0 fixes)

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

- release/config.py:27:7: PLR0902 Too many instance attributes (8/7)
- src/bokeh/application/handlers/code_runner.py:53:7: PLR0902 Too many instance attributes (11/7)
- src/bokeh/client/connection.py:76:7: PLR0902 Too many instance attributes (11/7)
+ src/bokeh/core/property/struct.py:92:17: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ src/bokeh/core/query.py:189:17: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ src/bokeh/core/serialization.py:400:13: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ src/bokeh/core/validation/check.py:215:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- src/bokeh/document/callbacks.py:86:7: PLR0902 Too many instance attributes (10/7)
- src/bokeh/document/document.py:111:7: PLR0902 Too many instance attributes (9/7)
- src/bokeh/models/util/structure.py:96:7: PLR0902 Too many instance attributes (10/7)
... 9 additional changes omitted for rule PLR0902
... 14 additional changes omitted for project

freedomofpress/securedrop (+2 -10 violations, +0 -0 fixes)

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

- journalist_gui/journalist_gui/SecureDropUpdater.py:169:7: PLR0902 Too many instance attributes (8/7)
- journalist_gui/journalist_gui/updaterUI.py:10:7: PLR0902 Too many instance attributes (16/7)
+ securedrop/journalist_app/__init__.py:127:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- securedrop/journalist_app/sessions.py:18:7: PLR0902 Too many instance attributes (11/7)
- securedrop/journalist_app/sessions.py:83:7: PLR0902 Too many instance attributes (11/7)
- securedrop/pretty_bad_protocol/_meta.py:110:7: PLR0902 Too many instance attributes (14/7)
+ securedrop/pretty_bad_protocol/_meta.py:822:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- securedrop/pretty_bad_protocol/_parsers.py:1172:7: PLR0902 Too many instance attributes (8/7)
... 5 additional changes omitted for rule PLR0902
... 4 additional changes omitted for project

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

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

- blinkpy/auth.py:22:7: PLR0902 Too many instance attributes (12/7)
- blinkpy/blinkpy.py:41:7: PLR0902 Too many instance attributes (17/7)
- blinkpy/camera.py:19:7: PLR0902 Too many instance attributes (23/7)
- blinkpy/sync_module.py:23:7: PLR0902 Too many instance attributes (21/7)
- tests/mock_responses.py:5:7: PLR0902 Too many instance attributes (8/7)

ibis-project/ibis (+2 -3 violations, +0 -0 fixes)

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

+ ibis/backends/bigquery/compiler.py:332:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
- ibis/backends/bigquery/udf/core.py:105:7: PLR0902 Too many instance attributes (8/7)
- ibis/backends/flink/ddl.py:77:7: PLR0902 Too many instance attributes (10/7)
- ibis/backends/impala/ddl.py:73:7: PLR0902 Too many instance attributes (8/7)
+ ibis/backends/polars/compiler.py:149:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif

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

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

- pymilvus/bulk_writer/bulk_writer.py:36:7: PLR0902 Too many instance attributes (8/7)
- pymilvus/bulk_writer/remote_bulk_writer.py:38:11: PLR0902 Too many instance attributes (9/7)
- pymilvus/client/abstract.py:14:7: PLR0902 Too many instance attributes (12/7)
- pymilvus/client/abstract.py:186:7: PLR0902 Too many instance attributes (8/7)
- pymilvus/client/abstract.py:99:7: PLR0902 Too many instance attributes (14/7)
- pymilvus/client/asynch.py:56:7: PLR0902 Too many instance attributes (11/7)
... 5 additional changes omitted for rule PLR0902
+ pymilvus/client/entity_helper.py:335:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ pymilvus/client/prepare.py:408:9: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
+ pymilvus/orm/schema.py:59:5: PLR5601 Consecutive elif with differing indentation level, consider creating a function to separate the inner elif
... 4 additional changes omitted for project

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

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
PLR0902 802 0 802 0 0
PLR5601 161 161 0 0 0

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to read examples/How_to_handle_rate_limits.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: trailing comma at line 47 column 4

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to read examples/How_to_handle_rate_limits.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: trailing comma at line 47 column 4

@tibor-reiss tibor-reiss force-pushed the add-PLR5601 branch 2 times, most recently from 84ac1eb to 199379b Compare February 23, 2024 18:47
///
/// if old_conf:
/// extracted(old_conf, new_conf, machine)
/// elif new_conf:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original pylint docs mention an explicit else as alternative resolution. We should also provide that option I believe.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Feb 26, 2024
Copy link

codspeed-hq bot commented Feb 27, 2024

CodSpeed Performance Report

Merging #10103 will not alter performance

Comparing tibor-reiss:add-PLR5601 (5819d6b) with main (8dc22d5)

Summary

✅ 30 untouched benchmarks

@MichaReiser
Copy link
Member

Thank you, @tibor-reiss, for implementing this rule, and I am sorry that it took us so long to get back to you.

We're currently revising our rule acceptance criteria as part of the work we do around #1774, and I'm sorry to inform you that we can't accept this rule for now (we can reconsider once #1774 is completed). There are two reasons for it:

  1. This is a pylint extension rule. We've accepted pylint extension rules before but we have revised our standpoint because adding them to the pylint group gives users a different experience compared to pylint where they explicitly need to opt in to extension rules. The alternative is to have an explicit rule group for each extension rule but we rather not do that because we don't want 1:1 group to rule associations. We hope to have a better place for pylint extension rules once Rule categorization #1774 is complete.
  2. It's unclear if we'll accept this rule after Rule categorization #1774 is complete because the rule in my view falls into the "It's something someone doesn't like category" and the proposed fix has some clear downsides (performance, reduced readability in my view). Our acceptance guidelines aren't finalized yet, so our stand point on this might change, but I wanted to give you a fair warning ;)

Thank you again for working on the Rule and sorry that I can't give you a more positive feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants