[`pylint`] - Implement `too-few-public-methods` rule (`PLR0903`)

[diceroll123]

trafficstars

Summary

Adds too-few-public-methods rule (PLR0903)

Took some creative liberty with this by allowing the class to have any decorators or base classes. There are years-old issues on pylint that don't approve of the strictness when it comes to these things.

Especially considering that dataclass is a decorator and namedtuple can be a base class. I feel ignoring those explicitly is redundant all things considered, but it's not a hill I'm willing to die on!

See: #970

Test Plan

cargo test

[github-actions[bot]]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

ruff check --no-cache --exit-zero --preview --select ALL

+ airflow/api/auth/backend/kerberos_auth.py:70:1: PLR0903 Class could be dataclass or namedtuple
+ airflow/providers/amazon/aws/hooks/glue.py:59:5: PLR0903 Class could be dataclass or namedtuple
+ airflow/providers/amazon/aws/hooks/logs.py:53:5: PLR0903 Class could be dataclass or namedtuple
+ airflow/providers/celery/executors/celery_executor_utils.py:193:1: PLR0903 Class could be dataclass or namedtuple
+ airflow/providers/cncf/kubernetes/kube_config.py:24:1: PLR0903 Class could be dataclass or namedtuple
+ airflow/providers/elasticsearch/hooks/elasticsearch.py:46:1: PLR0903 Class could be dataclass or namedtuple
+ airflow/providers/google/cloud/operators/dataflow.py:62:1: PLR0903 Class could be dataclass or namedtuple
+ airflow/www/security_appless.py:27:1: PLR0903 Class could be dataclass or namedtuple
+ dev/breeze/src/airflow_breeze/prepare_providers/provider_documentation.py:123:1: PLR0903 Class could be dataclass or namedtuple
+ tests/always/test_secrets_backends.py:36:1: PLR0903 Class could be dataclass or namedtuple
+ tests/core/test_stats.py:46:1: PLR0903 Class could be dataclass or namedtuple
+ tests/providers/amazon/aws/hooks/test_eks.py:106:5: PLR0903 Class could be dataclass or namedtuple
+ tests/providers/amazon/aws/hooks/test_eks.py:144:5: PLR0903 Class could be dataclass or namedtuple
+ tests/providers/amazon/aws/hooks/test_eks.py:186:5: PLR0903 Class could be dataclass or namedtuple
+ tests/providers/apache/hive/__init__.py:168:1: PLR0903 Class could be dataclass or namedtuple
... 14 additional changes omitted for project

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

ruff check --no-cache --exit-zero --preview

+ samcli/commands/pipeline/init/pipeline_templates_manifest.py:31:1: PLR0903 Class could be dataclass or namedtuple
+ samcli/commands/pipeline/init/pipeline_templates_manifest.py:39:1: PLR0903 Class could be dataclass or namedtuple
+ samcli/commands/pipeline/init/pipeline_templates_manifest.py:48:1: PLR0903 Class could be dataclass or namedtuple
+ samcli/lib/observability/xray_traces/xray_events.py:119:1: PLR0903 Class could be dataclass or namedtuple
+ samcli/lib/utils/arn_utils.py:11:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/cli/test_cli_config_file.py:28:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/commands/_utils/custom_options/test_option_nargs.py:7:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/commands/buildcmd/test_build_context.py:32:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/commands/buildcmd/test_build_context.py:83:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/commands/init/test_cli.py:36:1: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview --select ALL

+ src/bokeh/core/property/wrappers.py:141:1: PLR0903 Class could be dataclass or namedtuple
+ src/typings/selenium/webdriver/chrome/service.pyi:1:1: PLR0903 Class could be dataclass or namedtuple
+ src/typings/selenium/webdriver/firefox/firefox_binary.pyi:3:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/bokeh/application/handlers/test_document_lifecycle.py:32:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/bokeh/client/test_states.py:32:1: PLR0903 Class could be dataclass or namedtuple
+ tests/unit/bokeh/document/test_events__document.py:41:1: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview

+ admin/securedrop_admin/__init__.py:628:9: PLR0903 Class could be dataclass or namedtuple
+ admin/tests/test_securedrop-admin.py:35:1: PLR0903 Class could be dataclass or namedtuple
+ securedrop/pretty_bad_protocol/_parsers.py:1872:1: PLR0903 Class could be dataclass or namedtuple
+ securedrop/pretty_bad_protocol/_parsers.py:918:1: PLR0903 Class could be dataclass or namedtuple
+ securedrop/pretty_bad_protocol/_parsers.py:950:1: PLR0903 Class could be dataclass or namedtuple
+ securedrop/tests/migrations/migration_c5a02eb52f2d.py:12:1: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview

+ blinkpy/helpers/util.py:137:1: PLR0903 Class could be dataclass or namedtuple
+ tests/test_auth.py:309:1: PLR0903 Class could be dataclass or namedtuple
+ tests/test_blinkpy.py:580:1: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview

+ ibis/backends/bigquery/tests/system/udf/test_udf_execute.py:59:9: PLR0903 Class could be dataclass or namedtuple
+ ibis/backends/impala/__init__.py:75:1: PLR0903 Class could be dataclass or namedtuple
+ ibis/common/tests/test_patterns.py:582:5: PLR0903 Class could be dataclass or namedtuple
+ ibis/expr/operations/tests/test_core.py:24:1: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview

+ pymilvus/bulk_writer/remote_bulk_writer.py:35:5: PLR0903 Class could be dataclass or namedtuple

pandas-dev/pandas (+8 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ pandas/core/arrays/_arrow_string_mixins.py:20:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/core/arrays/sparse/accessor.py:31:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/core/internals/base.py:55:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/io/formats/excel.py:63:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/io/formats/style_render.py:1967:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/io/sas/sas7bdat.py:109:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/io/stata.py:957:1: PLR0903 Class could be dataclass or namedtuple
+ pandas/tests/dtypes/test_inference.py:355:5: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview

+ src/pip/_internal/build_env.py:35:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/cache.py:184:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/index/package_finder.py:326:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/models/direct_url.py:142:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/models/direct_url.py:67:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/models/index.py:4:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/models/scheme.py:12:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/models/selection_prefs.py:6:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/operations/prepare.py:83:1: PLR0903 Class could be dataclass or namedtuple
+ src/pip/_internal/req/constructors.py:194:1: PLR0903 Class could be dataclass or namedtuple
... 5 additional changes omitted for project

rotki/rotki (+6 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ rotkehlchen/accounting/cost_basis/base.py:445:1: PLR0903 Class could be dataclass or namedtuple
+ rotkehlchen/externalapis/interface.py:10:1: PLR0903 Class could be dataclass or namedtuple
+ rotkehlchen/tests/data_migrations/test_migrations.py:50:1: PLR0903 Class could be dataclass or namedtuple
+ rotkehlchen/tests/utils/makerdao.py:25:1: PLR0903 Class could be dataclass or namedtuple
+ rotkehlchen/tests/utils/makerdao.py:33:1: PLR0903 Class could be dataclass or namedtuple
+ rotkehlchen/utils/mixins/lockable.py:11:1: PLR0903 Class could be dataclass or namedtuple

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

ruff check --no-cache --exit-zero --preview --select ALL

+ confirmation/models.py:216:1: PLR0903 Class could be dataclass or namedtuple
+ tools/lib/template_parser.py:25:1: PLR0903 Class could be dataclass or namedtuple
+ tools/lib/template_parser.py:32:1: PLR0903 Class could be dataclass or namedtuple
+ tools/lib/template_parser.py:381:5: PLR0903 Class could be dataclass or namedtuple
+ zerver/lib/export.py:443:1: PLR0903 Class could be dataclass or namedtuple
+ zerver/lib/markdown/__init__.py:372:1: PLR0903 Class could be dataclass or namedtuple
+ zerver/lib/rate_limiter.py:501:1: PLR0903 Class could be dataclass or namedtuple
+ zerver/tests/test_outgoing_webhook_system.py:27:1: PLR0903 Class could be dataclass or namedtuple

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PLR0903	96	96	0	0	0

[zanieb]

Hi! Thanks for implementing this.

I'm hesitant to include this rule in Ruff. It looks like "too few methods" is used as a proxy for "this should be a dataclass" — it seems like a rule like "it only has an __init__, it should be a dataclass" would make more sense? Are there more use cases for this rule?

Separately, there's a false positive here for nested classes used for configuration — perhaps those should be excluded from the rule?

[diceroll123]

Hi! Thanks for implementing this.

I'm hesitant to include this rule in Ruff. It looks like "too few methods" is used as a proxy for "this should be a dataclass" — it seems like a rule like "it only has an __init__, it should be a dataclass" would make more sense? Are there more use cases for this rule?

Separately, there's a false positive here for nested classes used for configuration — perhaps those should be excluded from the rule?

Tweaked accordingly!

Changing up the name of the rule slightly may be in order, I figure? 🤔 Please advise!

[AlexWaygood]

@AlexWaygood do you have ideas/opinions on a name for this rule?

Possibly something like consider-using-dataclass could work well... but there might be other reasons you might want to consider using a dataclass, of course (such as code generation for several useful methods like __repr__, __eq__, etc.)!

I think the current name is okay, albeit a bit verbose, and there is some advantage to it having the same name as the original pylint rule

[MichaReiser]

Possibly something like consider-using-dataclass could work well... but there might be other reasons you might want to consider using a dataclass, of course (such as code generation for several useful methods like repr, eq, etc.)!

I like the name but it, unfortunately, doesn't fit our rule naming convention

I think the current name is okay, albeit a bit verbose, and there is some advantage to it having the same name as the original pylint rule

We can use the same rule code as pylint, but I would prefer another name that's more explicit about its motivation. Having a different name might also help to set expectations, considering that we're probably changing the semantics of the rule.

[AlexWaygood]

Maybe something like NonDataclassSimpleClass?

[MichaReiser]

This is interesting https://github.com/astral-sh/ruff/pull/10146 I haven't looked at how the two rules overlap but maybe there's an opportunity.

[AlexWaygood]

This is interesting #10146 I haven't looked at how the two rules overlap but maybe there's an opportunity.

That is interesting, indeed... it's flagging a different antipattern, but one that has the same solution

[diceroll123]

Let me know if there's any action items blocking this PR, not sure what to make of these comments tbh

[MichaReiser]

Note for reviewing: The rule itself is a style rule but we need to find a good name that expresses the motivation to use dataclasses/named tuples instead of enforcing a number of public methods. The name should read as allow(rule-name).

But the intent of the rule itself fits into ruff as a style rule.