ruff icon indicating copy to clipboard operation
ruff copied to clipboard

[`pylint`] - Implement `too-many-attributes rule` (`PLR0902`)

Open liushuyu opened this issue 1 year ago • 1 comments

Summary

Adds too-many-attributes rule (PLR0902) from Pylint. This implementation has a similar limitation as the Pylint one, where dynamically generated class attributes (e.g. through setattr or eval) are not considered.

Test Plan

A new test has been added and can be executed using cargo test.

liushuyu avatar Feb 05 '24 20:02 liushuyu

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

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

+ airflow/cli/commands/webserver_command.py:48:7: PLR0902 Too many instance attributes (11/7)
+ 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/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: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:478:7: PLR0902 Too many instance attributes (53/7)
+ airflow/models/connection.py:102:7: PLR0902 Too many instance attributes (8/7)
+ airflow/models/dag.py:302:7: PLR0902 Too many instance attributes (42/7)
+ 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:1212:7: PLR0902 Too many instance attributes (19/7)
+ airflow/models/taskinstance.py:3520:7: PLR0902 Too many instance attributes (14/7)
+ airflow/models/taskreschedule.py:44:7: PLR0902 Too many instance attributes (9/7)
+ airflow/operators/email.py:29:7: PLR0902 Too many instance attributes (10/7)
+ airflow/operators/trigger_dagrun.py:72:7: PLR0902 Too many instance attributes (8/7)
+ airflow/providers/airbyte/operators/airbyte.py:33:7: PLR0902 Too many instance attributes (8/7)
+ airflow/providers/amazon/aws/executors/ecs/ecs_executor.py:68:7: PLR0902 Too many instance attributes (9/7)
+ airflow/providers/amazon/aws/hooks/glue.py:33:7: PLR0902 Too many instance attributes (12/7)
+ airflow/providers/amazon/aws/operators/appflow.py:45:7: PLR0902 Too many instance attributes (9/7)
+ airflow/providers/amazon/aws/operators/athena.py:40:7: PLR0902 Too many instance attributes (13/7)
+ airflow/providers/amazon/aws/operators/batch.py:428:7: PLR0902 Too many instance attributes (12/7)
+ airflow/providers/amazon/aws/operators/batch.py:54:7: PLR0902 Too many instance attributes (22/7)
+ airflow/providers/amazon/aws/operators/datasync.py:35:7: PLR0902 Too many instance attributes (20/7)
+ airflow/providers/amazon/aws/operators/dms.py:30:7: PLR0902 Too many instance attributes (8/7)
+ airflow/providers/amazon/aws/operators/ec2.py:130:7: PLR0902 Too many instance attributes (10/7)
+ airflow/providers/amazon/aws/operators/ecs.py:355:7: PLR0902 Too many instance attributes (26/7)
+ airflow/providers/amazon/aws/operators/eks.py:140:7: PLR0902 Too many instance attributes (18/7)
+ airflow/providers/amazon/aws/operators/eks.py:436:7: PLR0902 Too many instance attributes (12/7)
+ airflow/providers/amazon/aws/operators/eks.py:561:7: PLR0902 Too many instance attributes (12/7)
+ airflow/providers/amazon/aws/operators/eks.py:675:7: PLR0902 Too many instance attributes (8/7)
+ airflow/providers/amazon/aws/operators/eks.py:807:7: PLR0902 Too many instance attributes (8/7)
+ airflow/providers/amazon/aws/operators/eks.py:898:7: PLR0902 Too many instance attributes (8/7)
+ airflow/providers/amazon/aws/operators/emr.py:1019:7: PLR0902 Too many instance attributes (10/7)
... 549 additional changes omitted for project
aws/aws-sam-cli (+87 -0 violations, +0 -0 fixes)

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

+ samcli/commands/build/build_context.py:53:7: PLR0902 Too many instance attributes (32/7)
+ 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)
+ samcli/commands/local/lib/local_lambda.py:34:7: PLR0902 Too many instance attributes (12/7)
+ samcli/commands/package/package_context.py:44:7: PLR0902 Too many instance attributes (18/7)
+ samcli/commands/pipeline/bootstrap/guided_context.py:36:7: PLR0902 Too many instance attributes (16/7)
... 77 additional changes omitted for project
bokeh/bokeh (+14 -0 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/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)
+ src/bokeh/protocol/message.py:118:7: PLR0902 Too many instance attributes (10/7)
+ src/bokeh/resources.py:264:7: PLR0902 Too many instance attributes (12/7)
+ src/bokeh/server/contexts.py:144:7: PLR0902 Too many instance attributes (8/7)
+ src/bokeh/server/session.py:122:7: PLR0902 Too many instance attributes (13/7)
... 4 additional changes omitted for project
freedomofpress/securedrop (+10 -0 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/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/_parsers.py:1172:7: PLR0902 Too many instance attributes (8/7)
+ securedrop/pretty_bad_protocol/_parsers.py:1457:7: PLR0902 Too many instance attributes (17/7)
+ securedrop/pretty_bad_protocol/_parsers.py:978:7: PLR0902 Too many instance attributes (8/7)
+ securedrop/secure_tempfile.py:13:7: PLR0902 Too many instance attributes (9/7)
+ securedrop/upload-screenshots.py:72:7: PLR0902 Too many instance attributes (9/7)
fronzbot/blinkpy (+5 -0 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 (+3 -0 violations, +0 -0 fixes)

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

+ 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)
milvus-io/pymilvus (+10 -0 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:185: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)
+ pymilvus/client/grpc_handler.py:68:7: PLR0902 Too many instance attributes (17/7)
+ pymilvus/orm/iterator.py:280:7: PLR0902 Too many instance attributes (12/7)
+ pymilvus/orm/iterator.py:59:7: PLR0902 Too many instance attributes (11/7)
+ pymilvus/orm/schema.py:255:7: PLR0902 Too many instance attributes (10/7)
pandas-dev/pandas (+50 -0 violations, +0 -0 fixes)

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

+ asv_bench/benchmarks/categoricals.py:18:7: PLR0902 Too many instance attributes (12/7)
+ asv_bench/benchmarks/io/hdf.py:14:7: PLR0902 Too many instance attributes (12/7)
+ asv_bench/benchmarks/join_merge.py:367:7: PLR0902 Too many instance attributes (8/7)
+ asv_bench/benchmarks/join_merge.py:427:7: PLR0902 Too many instance attributes (12/7)
+ asv_bench/benchmarks/reindex.py:13:7: PLR0902 Too many instance attributes (8/7)
+ pandas/core/apply.py:115:7: PLR0902 Too many instance attributes (9/7)
+ pandas/core/groupby/groupby.py:1040:7: PLR0902 Too many instance attributes (11/7)
+ pandas/core/groupby/grouper.py:453:7: PLR0902 Too many instance attributes (12/7)
+ pandas/core/groupby/grouper.py:58:7: PLR0902 Too many instance attributes (12/7)
+ pandas/core/resample.py:122:7: PLR0902 Too many instance attributes (13/7)
... 40 additional changes omitted for project

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

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLR0902 806 806 0 0 0

github-actions[bot] avatar Feb 05 '24 20:02 github-actions[bot]

We hope to solve the ALL problem by recategorizing our rules so that more opinionated rules require explicit opt-in. But we may need to wait to add new rules till then.

This lint is current in the experimental state, so it won't be enabled by default ...? Although I agree enabling this lint by default without thinking is a bad idea (but could be helpful for those who are migrating from Pylint to reach 1:1 compatibility).

liushuyu avatar Feb 26 '24 18:02 liushuyu

This lint is current in the experimental state, so it won't be enabled by default ...?

That's correct. But the intent would be to stabilize the rule eventually (ideally one or two minor releases later).

MichaReiser avatar Feb 26 '24 19:02 MichaReiser

This lint is current in the experimental state, so it won't be enabled by default ...?

That's correct. But the intent would be to stabilize the rule eventually (ideally one or two minor releases later).

Understood. So this pull request would be put on the backburner and wait until a better grouping mechanism is added?

liushuyu avatar Feb 26 '24 19:02 liushuyu

I discussed this internally and @charliermarsh is fine with adding new pylint rules, even if they are very opinionated.

MichaReiser avatar Feb 27 '24 08:02 MichaReiser

Comments addressed and rebased against the latest main branch.

liushuyu avatar Mar 05 '24 18:03 liushuyu

I think this is the same as https://github.com/astral-sh/ruff/pull/10431. We need to decide which of the two PRs we want to move forward.

MichaReiser avatar Mar 20 '24 09:03 MichaReiser

I think this is the same as #10431. We need to decide which of the two PRs we want to move forward.

I hadn't noticed this PR existed before making my own, I concede to this one!

diceroll123 avatar Mar 23 '24 03:03 diceroll123

Thank you for contributing to this new rule, and I am sorry that it took so long to decide.

We agree that the rule itself is useful, and we're open to merging the rule once #1774 is complete, but we don't want to merge the rule today. The reasons are:

  • The rule is very opinionated. It intentionally restricts Python functionality by disallowing classes with more than X attributes. This is fine, but we currently have no good model to group such opinionated rules, and we don't want users using ALL or selecting pylint to enable this rule by default because we believe that would be misleading and not all users have the required knowledge to know that this rule might just not be for them.
  • The rule measures complexity similar to the mc-cabe rule. It is different in that it measures the complexity of a structure and not a control flow complexity. However, we must figure out how we want different metrics to measure complexity. Should these be separate rules or a single, configurable rule?

I'm sorry that it took us so long to come to that conclusion and that we haven't flagged this on the pylint issue. I plan to go through the Pylint rules and mark the rules that are unlikely to be accepted today, but I first have to catch up on open linter PRs.

Thank you again for contributing and I hope we can merge this rule in the future, once we figured out the right place for it.

MichaReiser avatar Mar 28 '24 16:03 MichaReiser