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

Warn about un-annotated attributes in dataclass definition #12877

Open
adigitoleo opened this issue Aug 14, 2024 · 1 comment · May be fixed by #14349
Open

Warn about un-annotated attributes in dataclass definition #12877

adigitoleo opened this issue Aug 14, 2024 · 1 comment · May be fixed by #14349
Labels
great writeup A wonderful example of a quality contribution rule Implementing or modifying a lint rule

Comments

@adigitoleo
Copy link

This is a feature request. The proposal is for a warning which would be triggered if a dataclass definition contains un-annotated attribute declarations. Attributes d and e in the following example would trigger the warning:

from dataclasses import dataclass

@dataclass
class Foo:  # Public API of a package.
    a: float = 4.2
    b: int = 42
    c: tuple = (4.2, 42)

@dataclass
class Bar(Foo):  # User-defined subclass.
    d = 999
    e = "foo"

Pitch

By default, all attributes defined in a dataclass definition are class attrtibutes. Those annotated with a type also become instance attributes, unless their annotation is typing.ClassVar. This leads to the potentially surprising behaviour that un-annotated attributes are automatically only class attributes. Doing Bar(d=1) or Bar(e="bar") using the above definition leads to an "unexpected keyword argument" error, which itself does not immediately indicate (to novice Python programmers) that d and e are class attributes. This is especially dangerous when considering to offer a dataclass as public API for users to subclass (comments in the example).

I propose that ruff could at least prompt developers to clarify that they intend for an attribute to be a class attribute by using typing.ClassVar. Type annotations are frequently described as "optional", however in this case, by leaving them out, the semantics of the attribute declaration changes. The decision on whether to prompt the addition of "optional" type annotations is also being discussed in #5243, which is about the related RUF012 rule. However, that rule only covers the case of mutable class attributes (if e were a list, in the above example), and its scope extends beyond dataclasses. Because the use of dataclasses arguably implies the use of annotations, I think it is reasonable to warn when they are missing.

I'm not sure if this would be best proposed as a Pylint warning, or as a Ruff-only rule, but considering the classification of RUF012 I chose to propose it here.

I also opened a possibly premature issue over at mypy and started a discussion on the forum.

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule needs-triage labels Aug 21, 2024
@dylwil3
Copy link
Collaborator

dylwil3 commented Nov 14, 2024

I'm in favor of adding this rule - It seems like a clear "gotcha" to me.

It should not be difficult to implement (there is already an is_dataclass helper method, and the logic for recommending a ClassVar annotation exists in RUF012). The documentation should include a reference to this part of the Python docs: https://docs.python.org/3/library/dataclasses.html#class-variables

The hardest part will be figuring out which number RUF rule it will be since there are a lot of open PRs out right now for those... 😅

@MichaReiser MichaReiser added the great writeup A wonderful example of a quality contribution label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
great writeup A wonderful example of a quality contribution rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants