Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[ruff] Also report problems for attrs dataclasses in preview mode (RUF008, RUF009) #14327

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

InSyncWithFoo
Copy link
Contributor

Summary

Resolves #9757.

Test Plan

cargo nextest run and cargo insta test.

Copy link
Contributor

github-actions bot commented Nov 14, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+8 -6 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)

apache/airflow (+8 -6 violations, +0 -0 fixes)

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

+ airflow/lineage/entities.py:64:23: RUF008 Do not use mutable default values for dataclass attributes
- airflow/lineage/entities.py:64:23: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
+ airflow/lineage/entities.py:86:23: RUF008 Do not use mutable default values for dataclass attributes
- airflow/lineage/entities.py:86:23: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
+ airflow/lineage/entities.py:88:29: RUF008 Do not use mutable default values for dataclass attributes
- airflow/lineage/entities.py:88:29: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
+ airflow/lineage/entities.py:89:26: RUF008 Do not use mutable default values for dataclass attributes
- airflow/lineage/entities.py:89:26: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
+ airflow/lineage/entities.py:90:29: RUF008 Do not use mutable default values for dataclass attributes
- airflow/lineage/entities.py:90:29: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
+ airflow/models/dag.py:433:25: RUF009 Do not perform function call in dataclass defaults
+ airflow/models/dag.py:434:24: RUF009 Do not perform function call `airflow_conf.get_mandatory_value` in dataclass defaults
+ providers/src/airflow/providers/papermill/operators/papermill.py:41:31: RUF008 Do not use mutable default values for dataclass attributes
- providers/src/airflow/providers/papermill/operators/papermill.py:41:31: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`

Changes by rule (3 rules affected)

code total + violation - violation + fix - fix
RUF008 6 6 0 0 0
RUF012 6 0 6 0 0
RUF009 2 2 0 0 0

Copy link

codspeed-hq bot commented Nov 14, 2024

CodSpeed Performance Report

Merging #14327 will not alter performance

Comparing InSyncWithFoo:RUF008 (a958a82) with main (9a3001b)

Summary

✅ 32 untouched benchmarks

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Nov 14, 2024
@MichaReiser
Copy link
Member

@AlexWaygood would you mind taking a look at this rule. I don't have the necessary context about what it changes to review it.

I do think that we should make this a preview-only change because it increases the rule's scope.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks @InSyncWithFoo, this is great! I pushed some changes:

  • Optimised it a bit by adding some tracking to the semantic model of whether attrs has been imported or not
  • Consolidated the logic in the rules::ruff::helpers module
  • Made the change preview-only for now, since it increases the scope of the rule.

@AlexWaygood AlexWaygood added the preview Related to preview mode features label Nov 14, 2024
@AlexWaygood AlexWaygood changed the title [ruff] Also report problems for attrs dataclasses (RUF008, RUF009) [ruff] Also report problems for attrs dataclasses in preview mode (RUF008, RUF009) Nov 14, 2024
@AlexWaygood AlexWaygood merged commit d8b1afb into astral-sh:main Nov 14, 2024
20 checks passed
@InSyncWithFoo InSyncWithFoo deleted the RUF008 branch November 14, 2024 18:46
@pwuertz
Copy link

pwuertz commented Nov 16, 2024

Note that RUF009 can be a bit misleading when auto_attribs=False (or with older attr.s codebases):

import attrs

@attrs.define(auto_attribs=False)
class X:
    a_field: int = attrs.field(default=42)
    not_a_field: list = (lambda: [])()
#                       ^^^^^^^^^^^^^^ RUF009 Do not perform function call in dataclass defaults

Without auto_attribs, not_a_field can't possibly be a dataclass field. In this example, it's an incorrectly typed class variable. Fixing the typing error also fixes the RUF009 warning:

@attrs.define(auto_attribs=False)
class X:
    a_field: int = attrs.field(default=42)
    not_a_field: typing.ClassVar[list] = (lambda: [])()

@pwuertz
Copy link

pwuertz commented Nov 16, 2024

This is an issue though when using attrs.field wrappers:

def my_field():
    return attrs.field(default=42, metadata={"my_metadata": 42})


@attrs.define(auto_attribs=False)
class X:
    a_field: int = my_field()
#                  ^^^^^^^^^^ RUF009 Do not perform function call `my_field` in dataclass defaults

@InSyncWithFoo
Copy link
Contributor Author

@pwuertz I'll work on a fix. Could you open a new issue with all the details, please?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Support attrs in RUF008 and RUF009
4 participants