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

Restrict builtin-attribute-shadowing to actual shadowed references #9462

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

charliermarsh
Copy link
Member

Summary

This PR attempts to improve builtin-attribute-shadowing (A003), a rule which has been repeatedly criticized, but does have value (just not in the current form).

Historically, this rule would flag cases like:

class Class:
    id: int

This led to an increasing number of exceptions and special-cases to the rule over time to try and improve it's specificity (e.g., ignore TypedDict, ignore @override).

The crux of the issue is that given the above, referencing id will never resolve to Class.id, so the shadowing is actually fine. There's one exception, however:

class Class:
    id: int

    def do_thing() -> id:
        pass

Here, id actually resolves to the id attribute on the class, not the id builtin.

So this PR completely reworks the rule around this much more targeted case, which will almost always be a mistake: when you reference a class member from within the class, and that member shadows a builtin.

Closes #6524.

Closes #7806.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jan 11, 2024
@charliermarsh charliermarsh marked this pull request as ready for review January 11, 2024 03:02
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+5 -225 violations, +0 -0 fixes in 6 projects; 37 projects unchanged)

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

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

- airflow/api_connexion/schemas/dataset_schema.py:123:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/dataset_schema.py:69:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/event_log_schema.py:35:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/job_schema.py:33:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/trigger_schema.py:33:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/auth/managers/models/resource_details.py:42:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/cli/cli_config.py:1011:5: A003 Class attribute `help` is shadowing a Python builtin
- airflow/cli/cli_config.py:1023:5: A003 Class attribute `help` is shadowing a Python builtin
- airflow/configuration.py:1296:9: A003 Class attribute `set` is shadowing a Python builtin
- airflow/jobs/job.py:69:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/models/connection.py:91:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/models/dagpickle.py:44:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/models/dagrun.py:120:5: A003 Class attribute `id` is shadowing a Python builtin
... 51 additional changes omitted for project

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

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

- examples/advanced/extensions/widget.py:44:5: A003 Class attribute `range` is shadowing a Python builtin
- src/bokeh/application/application.py:318:9: A003 Class attribute `id` is shadowing a Python builtin
- src/bokeh/client/session.py:365:9: A003 Class attribute `id` is shadowing a Python builtin
- src/bokeh/command/subcommand.py:129:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommand.py:69:5: A003 Class attribute `type` is shadowing a Python builtin
- src/bokeh/command/subcommand.py:72:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/build.py:53:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/info.py:107:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/init.py:53:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/json.py:82:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/sampledata.py:71:5: A003 Class attribute `help` is shadowing a Python builtin
... 44 additional changes omitted for project

commaai/openpilot (+3 -0 violations, +0 -0 fixes)

+ common/logging_extra.py:68:29: RUF100 [*] Unused `noqa` directive (unused: `A003`)
+ common/logging_extra.py:98:29: RUF100 [*] Unused `noqa` directive (unused: `A003`)
+ selfdrive/athena/athenad.py:82:21: RUF100 [*] Unused `noqa` directive (unused: `A003`)

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

+ rotkehlchen/api/v1/schemas.py:2183:42: RUF100 [*] Unused `noqa` directive (unused: `A003`)
+ rotkehlchen/utils/hexbytes.py:53:44: RUF100 [*] Unused `noqa` directive (unused: `A003`)

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

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

- analytics/management/commands/check_analytics_state.py:24:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/clear_analytics_tables.py:11:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/clear_single_stat.py:11:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/populate_analytics_db.py:42:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/update_analytics_counts.py:22:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/models.py:15:5: A003 Class attribute `property` is shadowing a Python builtin
- analytics/models.py:41:5: A003 Class attribute `property` is shadowing a Python builtin
- confirmation/models.py:206:5: A003 Class attribute `type` is shadowing a Python builtin
- corporate/models.py:119:5: A003 Class attribute `type` is shadowing a Python builtin
- corporate/models.py:77:5: A003 Class attribute `type` is shadowing a Python builtin
- tools/lib/gitlint_rules.py:82:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/actions/message_flags.py:28:5: A003 Class attribute `all` is shadowing a Python builtin
- zerver/actions/message_flags.py:29:5: A003 Class attribute `type` is shadowing a Python builtin
- zerver/lib/bot_lib.py:153:9: A003 Class attribute `quit` is shadowing a Python builtin
- zerver/lib/drafts.py:27:5: A003 Class attribute `type` is shadowing a Python builtin
- zerver/lib/logging_util.py:67:9: A003 Class attribute `filter` is shadowing a Python builtin
- zerver/lib/mention.py:30:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/mention.py:37:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/remote_server.py:47:5: A003 Class attribute `property` is shadowing a Python builtin
- zerver/lib/remote_server.py:49:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/remote_server.py:56:5: A003 Class attribute `property` is shadowing a Python builtin
- zerver/lib/remote_server.py:57:5: A003 Class attribute `id` is shadowing a Python builtin
... 83 additional changes omitted for project

openai/openai-cookbook (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-preview --select A,E703,F704,B015,B018,D100

- examples/RAG_with_graph_db.ipynb:cell 45:58:9: A003 Class attribute `format` is shadowing a Python builtin

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
A003 225 0 225 0 0
RUF100 5 5 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+5 -216 violations, +0 -0 fixes in 6 projects; 37 projects unchanged)

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

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

- airflow/api_connexion/schemas/dataset_schema.py:123:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/dataset_schema.py:69:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/event_log_schema.py:35:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/job_schema.py:33:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/api_connexion/schemas/trigger_schema.py:33:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/auth/managers/models/resource_details.py:42:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/cli/cli_config.py:1011:5: A003 Class attribute `help` is shadowing a Python builtin
- airflow/cli/cli_config.py:1023:5: A003 Class attribute `help` is shadowing a Python builtin
- airflow/configuration.py:1296:9: A003 Class attribute `set` is shadowing a Python builtin
- airflow/jobs/job.py:69:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/models/connection.py:91:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/models/dagpickle.py:44:5: A003 Class attribute `id` is shadowing a Python builtin
- airflow/models/dagrun.py:120:5: A003 Class attribute `id` is shadowing a Python builtin
... 46 additional changes omitted for project

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

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

- examples/advanced/extensions/widget.py:44:5: A003 Class attribute `range` is shadowing a Python builtin
- src/bokeh/client/session.py:365:9: A003 Class attribute `id` is shadowing a Python builtin
- src/bokeh/command/subcommand.py:129:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommand.py:69:5: A003 Class attribute `type` is shadowing a Python builtin
- src/bokeh/command/subcommand.py:72:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/build.py:53:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/info.py:107:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/init.py:53:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/json.py:82:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/sampledata.py:71:5: A003 Class attribute `help` is shadowing a Python builtin
- src/bokeh/command/subcommands/secret.py:67:5: A003 Class attribute `help` is shadowing a Python builtin
... 41 additional changes omitted for project

commaai/openpilot (+3 -0 violations, +0 -0 fixes)

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

+ common/logging_extra.py:68:29: RUF100 [*] Unused `noqa` directive (unused: `A003`)
+ common/logging_extra.py:98:29: RUF100 [*] Unused `noqa` directive (unused: `A003`)
+ selfdrive/athena/athenad.py:82:21: RUF100 [*] Unused `noqa` directive (unused: `A003`)

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

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

+ rotkehlchen/api/v1/schemas.py:2183:42: RUF100 [*] Unused `noqa` directive (unused: `A003`)
+ rotkehlchen/utils/hexbytes.py:53:44: RUF100 [*] Unused `noqa` directive (unused: `A003`)

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

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

- analytics/management/commands/check_analytics_state.py:24:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/clear_analytics_tables.py:11:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/clear_single_stat.py:11:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/populate_analytics_db.py:42:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/management/commands/update_analytics_counts.py:22:5: A003 Class attribute `help` is shadowing a Python builtin
- analytics/models.py:15:5: A003 Class attribute `property` is shadowing a Python builtin
- analytics/models.py:41:5: A003 Class attribute `property` is shadowing a Python builtin
- confirmation/models.py:206:5: A003 Class attribute `type` is shadowing a Python builtin
- corporate/models.py:119:5: A003 Class attribute `type` is shadowing a Python builtin
- corporate/models.py:77:5: A003 Class attribute `type` is shadowing a Python builtin
- tools/lib/gitlint_rules.py:82:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/actions/message_flags.py:28:5: A003 Class attribute `all` is shadowing a Python builtin
- zerver/actions/message_flags.py:29:5: A003 Class attribute `type` is shadowing a Python builtin
- zerver/lib/bot_lib.py:153:9: A003 Class attribute `quit` is shadowing a Python builtin
- zerver/lib/drafts.py:27:5: A003 Class attribute `type` is shadowing a Python builtin
- zerver/lib/logging_util.py:67:9: A003 Class attribute `filter` is shadowing a Python builtin
- zerver/lib/mention.py:30:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/mention.py:37:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/remote_server.py:47:5: A003 Class attribute `property` is shadowing a Python builtin
- zerver/lib/remote_server.py:49:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/remote_server.py:56:5: A003 Class attribute `property` is shadowing a Python builtin
- zerver/lib/remote_server.py:57:5: A003 Class attribute `id` is shadowing a Python builtin
- zerver/lib/remote_server.py:64:5: A003 Class attribute `id` is shadowing a Python builtin
... 81 additional changes omitted for project

openai/openai-cookbook (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview --select A,E703,F704,B015,B018,D100

- examples/RAG_with_graph_db.ipynb:cell 45:58:9: A003 Class attribute `format` is shadowing a Python builtin

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
A003 216 0 216 0 0
RUF100 5 5 0 0 0

@charliermarsh charliermarsh merged commit 25bafd2 into main Jan 11, 2024
17 checks passed
@charliermarsh charliermarsh deleted the charlie/class-builtin branch January 11, 2024 17:59
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.

Remove rule A003 builtins-ignorelist for attributes only
2 participants